Combining APPLE_COMMON_CRYPTO=1 and NO_OPENSSL=1 produces unexpected result

Combining APPLE_COMMON_CRYPTO=1 and NO_OPENSSL=1 produces unexpected result

[Jack Nagel]

When compiling git on OS X (where APPLE_COMMON_CRYPTO=1 is the
default) and specifying NO_OPENSSL=1, the resulting git uses the
BLK_SHA1 implementation rather than the functions available in
CommonCrypto.

$ uname -a
Darwin broadwell.local 15.0.0 Darwin Kernel Version 15.0.0: Sat Sep 19
15:53:46 PDT 2015; root:xnu-3247.10.11~1/RELEASE_X86_64 x86_64

$ make NO_OPENSSL=1
[...]
$ nm git | grep _SHA1_
0000000100173f00 t _blk_SHA1_Block
0000000100174e80 T _blk_SHA1_Final
000000010018fbb0 s _blk_SHA1_Final.pad
0000000100173de0 T _blk_SHA1_Init
0000000100173e10 T _blk_SHA1_Update

However, with OpenSSL available, it does use the CommonCrypto functions:

$ make
[...]
$ nm git | grep _SHA1_
                 U _CC_SHA1_Final
                 U _CC_SHA1_Init
                 U _CC_SHA1_Update

I would not expect the presence of NO_OPENSSL=1 to change the behavior
here, since neither case actually makes use of the OpenSSL SHA1
functions.

Re: Combining APPLE_COMMON_CRYPTO=1 and NO_OPENSSL=1 produces unexpected result

[Eric Sunshine]

On Wed, Nov 25, 2015 at 10:10 AM, Jack Nagel <jacknagel@gmail.com> wrote:
> When compiling git on OS X (where APPLE_COMMON_CRYPTO=1 is the
> default) and specifying NO_OPENSSL=1, the resulting git uses the
> BLK_SHA1 implementation rather than the functions available in
> CommonCrypto.
>
> $ make NO_OPENSSL=1
> [...]
> $ nm git | grep _SHA1_
> 0000000100173f00 t _blk_SHA1_Block
> 0000000100174e80 T _blk_SHA1_Final
> 000000010018fbb0 s _blk_SHA1_Final.pad
> 0000000100173de0 T _blk_SHA1_Init
> 0000000100173e10 T _blk_SHA1_Update

NO_OPENSSL disables all SSL-related functionality in Git, not just
SHA1 computation, however, Git still needs to compute SHA1 hashes for
other reasons, so it uses its own BLK_SHA1 versions when OpenSSL is
unavailable.

> However, with OpenSSL available, it does use the CommonCrypto functions:
>
> $ make
> [...]
> $ nm git | grep _SHA1_
>                  U _CC_SHA1_Final
>                  U _CC_SHA1_Init
>                  U _CC_SHA1_Update
>
> I would not expect the presence of NO_OPENSSL=1 to change the behavior
> here, since neither case actually makes use of the OpenSSL SHA1
> functions.

Apple's CommonCrypto headers present OpenSSL-compatible API. When
CommonCrypto is enabled in the Git makefile, aside from pulling in the
CommonCrypto header and linking against the CommonCrypto framework,
from Git's perspective, it still thinks it's using OpenSSL, and all
SSL-related functionality is enabled (including the SHA1 functions).

So, it might be easier to think of NO_OPENSSL as really meaning NO_SSL
(that is, "disable all SSL-related functionality"). Since the only SSL
implementation Git knows how to use is OpenSSL, perhaps one can
consider the name NO_OPENSSL a historic anomaly.

Re: Combining APPLE_COMMON_CRYPTO=1 and NO_OPENSSL=1 produces unexpected result

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> So, it might be easier to think of NO_OPENSSL as really meaning NO_SSL
> (that is, "disable all SSL-related functionality"). Since the only SSL
> implementation Git knows how to use is OpenSSL, perhaps one can
> consider the name NO_OPENSSL a historic anomaly.

That is a good explanation of what is observed.  I am not sure if it
is a good justification, though.  If you tell somebody who needs to
link an implementation of SHA-1 in that you (1) do not want to use
OpenSSL (or do not want to have SSL at all), and (2) do not mind
using Apple's CommonCrypto, and if you _know_ that CommonCrypto is
a possible source of the SHA-1 implementation, then I would think it
is reasonable to expect that CommonCrypto SHA-1 to be used.

	Note. To further explain the situation, the only reason we
	added CommonCrypto knob in the build system was to allow
	people to use OpenSSL as the SSL implementation.  Those who
	added the knob weren't making a conscious decision on which
	SHA-1 implementation to use in that scenario---they may not
	even have been aware of the fact that SHA-1 was offered by
	CommonCrypto for that matter.

A few questions we should be asking Apple users are:

 - Is there a strong-enough reason why those who do not want to use
   SSL should be able to choose the SHA-1 implementation available
   from CommonCrypto over block-sha1?

 - Is CommonCrypto SHA-1 a better implementation than block-sha1?

Depending on the answers to these questions, we might want to:

 - add a knob to allow choosing between two available
   implementations (i.e. when NO_APPLE_COMMON_CRYPTO is unset) of
   SHA-1, regardless of the setting of NO_OPENSSL.

 - decide which one between CommonCrypto and block-sha1 should be
   the default.

If we end up deciding that we use block-sha1 as the default, we
should do so even when both NO_OPENSSL and NO_APPLE_COMMON_CRYPTO
are left unset.  If we decide that block-sha1 should merely be a
fallback when no other SHA-1 implementation is availble, on the
other hand, we should be using CommonCrypto SHA-1 as long as the
user did not set NO_APPLE_COMMON_CRYPTO explicitly, even when we are
building with NO_OPENSSL.

If people do not care, we can leave things as they are.  It would
seem mysterious to use block-sha1 when we are not using CommonCrypto
for SSL (i.e. NO_OPENSSL), and otherwise CommonCrypto SHA-1, and
would invite a puzzlement we saw in this thread, though.

Re: Combining APPLE_COMMON_CRYPTO=1 and NO_OPENSSL=1 produces unexpected result

[David Aguilar]

On Sun, Dec 27, 2015 at 06:29:29PM -0800, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > So, it might be easier to think of NO_OPENSSL as really meaning NO_SSL
> > (that is, "disable all SSL-related functionality"). Since the only SSL
> > implementation Git knows how to use is OpenSSL, perhaps one can
> > consider the name NO_OPENSSL a historic anomaly.
> 
> That is a good explanation of what is observed.  I am not sure if it
> is a good justification, though.  If you tell somebody who needs to
> link an implementation of SHA-1 in that you (1) do not want to use
> OpenSSL (or do not want to have SSL at all), and (2) do not mind
> using Apple's CommonCrypto, and if you _know_ that CommonCrypto is
> a possible source of the SHA-1 implementation, then I would think it
> is reasonable to expect that CommonCrypto SHA-1 to be used.
> 
> 	Note. To further explain the situation, the only reason we
> 	added CommonCrypto knob in the build system was to allow
> 	people to use OpenSSL as the SSL implementation.  Those who
> 	added the knob weren't making a conscious decision on which
> 	SHA-1 implementation to use in that scenario---they may not
> 	even have been aware of the fact that SHA-1 was offered by
> 	CommonCrypto for that matter.

My original motivation for going down the CommonCrypto route was,
"bummer, git is not compiling.. let me try to fix that, somehow".

I think the best long-term solution would be to abandon the
CommonCrypto backend, if possible.  There's not a strong reason
for its existence.  It always seemed kinda hacky, and bolted-on.

> A few questions we should be asking Apple users are:
> 
>  - Is there a strong-enough reason why those who do not want to use
>    SSL should be able to choose the SHA-1 implementation available
>    from CommonCrypto over block-sha1?

IMO, no.

>  - Is CommonCrypto SHA-1 a better implementation than block-sha1?

I do not believe this to be true.

My gut feeling is that we cannot rely on the long-term stability
and availability of Apple's APIs.  Block-sha1 works fine on
the current Apple hardware and I suspect that it (or openssl)
will continue to work fine in the future.

> Depending on the answers to these questions, we might want to:
> 
>  - add a knob to allow choosing between two available
>    implementations (i.e. when NO_APPLE_COMMON_CRYPTO is unset) of
>    SHA-1, regardless of the setting of NO_OPENSSL.
> 
>  - decide which one between CommonCrypto and block-sha1 should be
>    the default.

How about, drop support for CommonCrypto so that there's no need
for the end-user to choose?  That means we would get block-sha1
by default.

> If we end up deciding that we use block-sha1 as the default, we
> should do so even when both NO_OPENSSL and NO_APPLE_COMMON_CRYPTO
> are left unset.  If we decide that block-sha1 should merely be a
> fallback when no other SHA-1 implementation is availble, on the
> other hand, we should be using CommonCrypto SHA-1 as long as the
> user did not set NO_APPLE_COMMON_CRYPTO explicitly, even when we are
> building with NO_OPENSSL.
> 
> If people do not care, we can leave things as they are.  It would
> seem mysterious to use block-sha1 when we are not using CommonCrypto
> for SSL (i.e. NO_OPENSSL), and otherwise CommonCrypto SHA-1, and
> would invite a puzzlement we saw in this thread, though.

I'm curious to see what others think about dropping CommonCrypto.
It seems like a good choice from a maintenance POV.
-- 
David

Re: Combining APPLE_COMMON_CRYPTO=1 and NO_OPENSSL=1 produces unexpected result

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> I think the best long-term solution would be to abandon the
> CommonCrypto backend, if possible.  There's not a strong reason
> for its existence.  It always seemed kinda hacky, and bolted-on.
> ...
>> A few questions we should be asking Apple users are:
>> 
>>  - Is there a strong-enough reason why those who do not want to use
>>    SSL should be able to choose the SHA-1 implementation available
>>    from CommonCrypto over block-sha1?
>
> IMO, no.
>
>>  - Is CommonCrypto SHA-1 a better implementation than block-sha1?
>
> I do not believe this to be true.
>
> My gut feeling is that we cannot rely on the long-term stability
> and availability of Apple's APIs.  Block-sha1 works fine on
> the current Apple hardware and I suspect that it (or openssl)
> will continue to work fine in the future.
> ...
>> If people do not care, we can leave things as they are.  It would
>> seem mysterious to use block-sha1 when we are not using CommonCrypto
>> for SSL (i.e. NO_OPENSSL), and otherwise CommonCrypto SHA-1, and
>> would invite a puzzlement we saw in this thread, though.
>
> I'm curious to see what others think about dropping CommonCrypto.
> It seems like a good choice from a maintenance POV.

Judging by a week-long silence, it seems nobody seems to have much
to say on this issue.  Let me summon the git_osx_installer
maintainer to hear from somebody who know a lot better than me about
things around OS X.

Thanks.

Re: Combining APPLE_COMMON_CRYPTO=1 and NO_OPENSSL=1 produces unexpected result

[Eric Sunshine]

On Fri, Jan 15, 2016 at 1:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>> I'm curious to see what others think about dropping CommonCrypto.
>> It seems like a good choice from a maintenance POV.
>
> Judging by a week-long silence, it seems nobody seems to have much
> to say on this issue.  Let me summon the git_osx_installer
> maintainer to hear from somebody who know a lot better than me about
> things around OS X.

Dropping CommonCrypto seems reasonable to me. Since Apple's
CommonCrypto provides only a subset of the OpenSSL functionality Git
uses, we still end up bringing in OpenSSL anyhow in addition to
CommonCrypto, so CommonCrypto isn't buying us much (if anything).
Unfortunately, we'd still want to keep the ugly hack to suppress the
unavoidable "noise" Apple thrust upon us by populating the OpenSSL
headers with #warnings about unstable ABI.