Subject: Re: [PATCH] Limit the size of the data block passed to SHA1_Update()

From: Junio C Hamano <[email-masked]>
Date: Wed, 04 Nov 2015 09:09:03 -0800

Junio C Hamano <[email-masked]> writes:

>> ifdef BLK_SHA1
>>         SHA1_HEADER = "block-sha1/sha1.h"
>>         LIB_OBJS += block-sha1/sha1.o
>> else
>> ifdef PPC_SHA1
>>         SHA1_HEADER = "ppc/sha1.h"
>>         LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
>> else
>> ifdef APPLE_COMMON_CRYPTO
>>         COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>>         SHA1_HEADER = <CommonCrypto/CommonDigest.h>
>>         SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
>> else
>>         SHA1_HEADER = <openssl/sha.h>
>>         EXTLIBS += $(LIB_4_CRYPTO)
>> endif
>>
>> which seems to imply that BLK_SHA1 and APPLE_COMMON_CRYPTO are
>> mutually exclusive?
>
> Yes, you are correct that these two cannot be used at the same time.
> In general (not limited to BLK_SHA1 and APPLE_COMMON_CRYPTO) you can
> pick only _one_ underlying SHA-1 implementation to use with the
> system.

Our "seems to imply" above is a faulty reading.  The four lines I
wrote are not incorrect per-se, but this exchange was misleading.

When BLK_SHA1 is defined, the above fragment from the Makefile is
only saying "CommonCrypto may or may not be used for any other
purposes, but for SHA-1 hashing, I'll use our own block-sha1/
implementation."  It does not say anything about how the system
favours CommonCrypto over OpenSSL with APPLE_COMMON_CRYPTO.

And it is legit to define both APPLE_COMMON_CRYPTO and BLK_SHA1; the
resulting build would still use SSL-related functions what other
people may use from OpenSSL from CommonCrypto.  Filipe Cabecinhas
pointed out that such a configuration works (and solves the issue
that triggered this thread) very early in the thread.

I haven't looked carefully at the latest version of your patch, but
I just wanted to make sure that I didn't mislead you to add an
unnecessary "we check if both APPLE_COMMON_CRYPTO and BLK_SHA1 are
defined and error out because they are incompatible" check.  Sorry
for an earlier message that may have been confusing.

Thanks.