diff mbox series

package/git: prefer OpenSSL SHA-1 and SHA-256 implementation

Message ID 20230119095842.445227-1-bagasdotme@gmail.com
State Rejected
Headers show
Series package/git: prefer OpenSSL SHA-1 and SHA-256 implementation | expand

Commit Message

Bagas Sanjaya Jan. 19, 2023, 9:58 a.m. UTC
Git is shipped with its own implementation of SHA-1 and SHA-256.
However, when OpenSSL is available (BR2_PACKAGE_OPENSSL=y), it is
preferable to leverage the OpenSSL version of these hash algorithm
instead. --with-openssl configure flag doesn't enable that, though.

Set OPENSSL_{SHA1,SHA256} make variable when OpenSSL package is
selected.

Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 package/git/git.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


base-commit: c77c66b7df478f508b8f2affa7a64795504db699

Comments

Peter Korsgaard Jan. 19, 2023, 10:04 a.m. UTC | #1
>>>>> "Bagas" == Bagas Sanjaya <bagasdotme@gmail.com> writes:

 > Git is shipped with its own implementation of SHA-1 and SHA-256.
 > However, when OpenSSL is available (BR2_PACKAGE_OPENSSL=y), it is
 > preferable to leverage the OpenSSL version of these hash algorithm
 > instead. --with-openssl configure flag doesn't enable that, though.

 > Set OPENSSL_{SHA1,SHA256} make variable when OpenSSL package is
 > selected.

 > Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>

What is the added value of doing this? Size? Speed? Why is it not the
upstream default?

A quick search for git + OPENSSL_SHA1 lead me to:

https://www.spinics.net/lists/git/msg125401.html

Which is old, but argued for not using the openssl implementation
because of performance concerns.
Peter Seiderer Jan. 19, 2023, 8:35 p.m. UTC | #2
Hello *,

On Thu, 19 Jan 2023 11:04:13 +0100, Peter Korsgaard <peter@korsgaard.com> wrote:

> >>>>> "Bagas" == Bagas Sanjaya <bagasdotme@gmail.com> writes:
>
>  > Git is shipped with its own implementation of SHA-1 and SHA-256.
>  > However, when OpenSSL is available (BR2_PACKAGE_OPENSSL=y), it is
>  > preferable to leverage the OpenSSL version of these hash algorithm
>  > instead. --with-openssl configure flag doesn't enable that, though.
>
>  > Set OPENSSL_{SHA1,SHA256} make variable when OpenSSL package is
>  > selected.
>
>  > Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
>
> What is the added value of doing this? Size? Speed? Why is it not the
> upstream default?
>
> A quick search for git + OPENSSL_SHA1 lead me to:
>
> https://www.spinics.net/lists/git/msg125401.html
>
> Which is old, but argued for not using the openssl implementation
> because of performance concerns.
>

Or 'Makefile: make DC_SHA1 the default' ([1]):

	Makefile: make DC_SHA1 the default

	We used to use the SHA1 implementation from the OpenSSL library by
	default.  As we are trying to be careful against collision attacks
	after the recent "shattered" announcement, switch the default to
	encourage people to use DC_SHA1 implementation instead.  Those who
	want to use the implementation from OpenSSL can explicitly ask for
	it by OPENSSL_SHA1=YesPlease when running "make".

And 'INSTALL: remove discussion of SHA-1 backends' ([2]):

	INSTALL: remove discussion of SHA-1 backends

	The claim that OpenSSL is the default SHA-1 backend hasn't been true
	since e6b07da (Makefile: make DC_SHA1 the default, 2017-03-17),
	but more importantly tweaking the SHA-1 backend isn't something that's
	common enough to warrant discussing in the INSTALL document, so let's
	remove this paragraph.

	This discussion was originally added in c538d2d (Add some
	installation notes in INSTALL, 2005-06-17) when tweaking the default
	backend was more common. The current wording was added in
	5beb577 (INSTALL: Describe dependency knobs from Makefile,
	2009-09-10).

Regards,
Peter


[1] https://github.com/git/git/commit/e6b07da2780f349c29809bd75d3eca6ad3c35d19
[2] https://github.com/git/git/commit/0ced11d32fea5caec3f014b4e60dd197b9c8aa0f
Bagas Sanjaya Jan. 20, 2023, 2:35 a.m. UTC | #3
On 1/20/23 03:35, Peter Seiderer wrote:
> Or 'Makefile: make DC_SHA1 the default' ([1]):
> 
> 	Makefile: make DC_SHA1 the default
> 
> 	We used to use the SHA1 implementation from the OpenSSL library by
> 	default.  As we are trying to be careful against collision attacks
> 	after the recent "shattered" announcement, switch the default to
> 	encourage people to use DC_SHA1 implementation instead.  Those who
> 	want to use the implementation from OpenSSL can explicitly ask for
> 	it by OPENSSL_SHA1=YesPlease when running "make".
> 
> And 'INSTALL: remove discussion of SHA-1 backends' ([2]):
> 
> 	INSTALL: remove discussion of SHA-1 backends
> 
> 	The claim that OpenSSL is the default SHA-1 backend hasn't been true
> 	since e6b07da (Makefile: make DC_SHA1 the default, 2017-03-17),
> 	but more importantly tweaking the SHA-1 backend isn't something that's
> 	common enough to warrant discussing in the INSTALL document, so let's
> 	remove this paragraph.
> 
> 	This discussion was originally added in c538d2d (Add some
> 	installation notes in INSTALL, 2005-06-17) when tweaking the default
> 	backend was more common. The current wording was added in
> 	5beb577 (INSTALL: Describe dependency knobs from Makefile,
> 	2009-09-10).
> 

Ah, I didn't see the discussions above before I submitted this patch.

Withdrawn, thanks.
diff mbox series

Patch

diff --git a/package/git/git.mk b/package/git/git.mk
index 9918d4c1ef..916ccc0cf3 100644
--- a/package/git/git.mk
+++ b/package/git/git.mk
@@ -16,7 +16,9 @@  GIT_DEPENDENCIES = zlib $(TARGET_NLS_DEPENDENCIES)
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 GIT_DEPENDENCIES += host-pkgconf openssl
 GIT_CONF_OPTS += --with-openssl
-GIT_MAKE_OPTS += LIB_4_CRYPTO="`$(PKG_CONFIG_HOST_BINARY) --libs libssl libcrypto`"
+GIT_MAKE_OPTS += \
+	LIB_4_CRYPTO="`$(PKG_CONFIG_HOST_BINARY) --libs libssl libcrypto`" \
+	OPENSSL_SHA1=YesPlease OPENSSL_SHA256=YesPlease
 else
 GIT_CONF_OPTS += --without-openssl
 endif