| Message ID | 20240817000027.654079-1-mmayer@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | package/dropbear: provide config option to turn off SHA1 for RSA | expand |
Hello, +Peter in Cc. On Fri, 16 Aug 2024 17:00:26 -0700 Markus Mayer via buildroot <buildroot@buildroot.org> wrote: > diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in > index 207c1f561700..099f61535aa2 100644 > --- a/package/dropbear/Config.in > +++ b/package/dropbear/Config.in > @@ -67,6 +67,12 @@ config BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO > DSA public keys > Diffie-Hellman Group1 key exchange > > +config BR2_PACKAGE_DROPBEAR_DISABLE_RSA_SHA1 > + bool "disable SHA1 hashing for RSA" > + help > + SHA1 is no longer considered secure. Users may want to disable > + it. However, this may preclude older clients from connecting. Inverted logic options are always a bit annoying. Wouldn't it be better to do: config BR2_PACKAGE_DROPBEAR_RSA_SHA1 bool "SHA1 hashing for RSA" default y help SHA1 is no longer considered secure, so users may want to disable it, but the lack of SHA1 support for RSA might preclude older clients from connecting This option defaults to enabled to preserve backward compatibility. Peter, what do you think? Or should we break backward compatibility for the sake of security, and leave SHA1 support disabled by default? Thomas
On Sat, 17 Aug 2024 at 03:10, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in > > index 207c1f561700..099f61535aa2 100644 > > --- a/package/dropbear/Config.in > > +++ b/package/dropbear/Config.in > > @@ -67,6 +67,12 @@ config BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO > > DSA public keys > > Diffie-Hellman Group1 key exchange > > > > +config BR2_PACKAGE_DROPBEAR_DISABLE_RSA_SHA1 > > + bool "disable SHA1 hashing for RSA" > > + help > > + SHA1 is no longer considered secure. Users may want to disable > > + it. However, this may preclude older clients from connecting. > > Inverted logic options are always a bit annoying. Wouldn't it be better > to do: > > config > bool "SHA1 hashing for RSA" > default y > help > SHA1 is no longer considered secure, so users may want to > disable it, but the lack of SHA1 support for RSA might > preclude older clients from connecting > > This option defaults to enabled to preserve backward > compatibility. > > Peter, what do you think? Or should we break backward compatibility for > the sake of security, and leave SHA1 support disabled by default? I don't mind either way. The reason I chose this approach was so that the default behaviour doesn't require a Buildroot configuration setting to be enabled. Also, because the Dropbear project is saying that SHA1 will be removed in the future, this config option will likely go away again when that happens. I thought it would be preferable to not introduce a config setting that is "y" by default when the setting will be removed again in the future. However, at the end of the day, I guess it doesn't really matter all that much if old config files contain BR2_PACKAGE_DROPBEAR_RSA_SHA1=y versus # BR2_PACKAGE_DROPBEAR_DISABLE_RSA_SHA1 is not set when the option does get removed. Kconfig will strip it out either way. So, I am okay either way. Just let me know which way you prefer. Regards, -Markus
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes: Hi, > Inverted logic options are always a bit annoying. Wouldn't it be better > to do: > config BR2_PACKAGE_DROPBEAR_RSA_SHA1 > bool "SHA1 hashing for RSA" > default y > help > SHA1 is no longer considered secure, so users may want to > disable it, but the lack of SHA1 support for RSA might > preclude older clients from connecting > This option defaults to enabled to preserve backward > compatibility. > Peter, what do you think? Or should we break backward compatibility for > the sake of security, and leave SHA1 support disabled by default? I think it makes most sense to do it like you suggest, but drop the default y so it behaves similar to BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO. Talking about _LEGACY_CRYPTO, I just noticed that dropbear 2022.83 has a bug, so it unconditionally enables support for the legacy DSS protocol (and 2024.84 fails to build without RSA SHA1). I'll bump 2024.02.x to dropbear 2024.85 to fix it.
Hi all, On Sun, 18 Aug 2024 at 13:48, Peter Korsgaard <peter@korsgaard.com> wrote: > > >>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes: > > > Inverted logic options are always a bit annoying. Wouldn't it be better > > to do: > > > config BR2_PACKAGE_DROPBEAR_RSA_SHA1 > > bool "SHA1 hashing for RSA" > > default y > > help > > SHA1 is no longer considered secure, so users may want to > > disable it, but the lack of SHA1 support for RSA might > > preclude older clients from connecting > > > This option defaults to enabled to preserve backward > > compatibility. > > > Peter, what do you think? Or should we break backward compatibility for > > the sake of security, and leave SHA1 support disabled by default? > > I think it makes most sense to do it like you suggest, but drop the > default y so it behaves similar to BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO. Hm. What about backward compatibility regarding _RSA_SHA1? Our default would change while dropbear's default hasn't changed (yet). Also consider the following discrepancy. The legacy crypto features are disabled by default in dropbear and a user has to turn them on manually. _LEGACY_CRYPTO matches this behaviour (off by default, needs to be enabled manually), so it all makes logical sense and works well. Whether someone builds dropbear with Buildroot or without, the legacy crypto functions have to be turned on manually. However, the situation with _RSA_SHA1 is reversed. The feature is enabled by default in dropbear and a user has to manually turn it off. If we model _RSA_SHA1 after _LEGACY_CRYPTO, on the one hand it is nice and consistent on the Buildroot side. On the other hand, it behaves opposite to what dropbear is doing. Buildroot would require the user to take action to receive the default dropbear behaviour (_RSA_SHA1 would need to be enabled explicitly because it would default to "unset"). This is the opposite experience than someone building dropbear without Buildroot. I am not saying we can't or shouldn't do it. However, I do want to point out the differences between _LEGACY_CRYPTO and _RSA_SHA1, and how those two options are handled differently on the dropbear side. With that said, if you do prefer to have an "enable" flag for RSA_SHA1 (rather than my "disable" flag) and it should be off by default, I will send a patch that does this. Thanks, -Markus
>>>>> "Markus" == Markus Mayer <mmayer@broadcom.com> writes: Hi, >> I think it makes most sense to do it like you suggest, but drop the >> default y so it behaves similar to BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO. > Hm. What about backward compatibility regarding _RSA_SHA1? Our default > would change while dropbear's default hasn't changed (yet). I don't think this is a big concern. Like you say, any update of Buildroot could bring different behaviour of a package (caused by an upstream change or by us), and as dropbear has stated that they will disable RSA_SHA1 by default in a future release it would be odd for us to have the opposite behaviour (on by default). > Also consider the following discrepancy. The legacy crypto features > are disabled by default in dropbear and a user has to turn them on > manually. _LEGACY_CRYPTO matches this behaviour (off by default, needs > to be enabled manually), so it all makes logical sense and works well. > Whether someone builds dropbear with Buildroot or without, the legacy > crypto functions have to be turned on manually. > However, the situation with _RSA_SHA1 is reversed. The feature is > enabled by default in dropbear and a user has to manually turn it off. > If we model _RSA_SHA1 after _LEGACY_CRYPTO, on the one hand it is nice > and consistent on the Buildroot side. On the other hand, it behaves > opposite to what dropbear is doing. Buildroot would require the user > to take action to receive the default dropbear behaviour (_RSA_SHA1 > would need to be enabled explicitly because it would default to > "unset"). This is the opposite experience than someone building > dropbear without Buildroot. True. It also somewhat depends on how fine grained configuration options we want, E.G. we could also simply handle the RSA_SHA1 option under the LEGACY_CRYPTO option WHEN upstream disables it by default, but then we have to wait for that. Looking elsewhere, I see that openwrt handles it with a "modern only" option to only enable modern/secure options, maybe that is a way to go? https://github.com/openwrt/openwrt/commit/bf900e02c7102601be2e9280231711e70f065877 Related to RSA_SHA1, I believe the original reason for you sending this patch was to disable everything related to SHA1, but SHA1 is also used for HMAC and key exchange, so we should consider disabling those as well: https://github.com/openwrt/openwrt/commit/2d9a0be307b534ceb717267c95402d1d707cd2c3 What do you say?
On Mon, 19 Aug 2024 at 00:11, Peter Korsgaard <peter@korsgaard.com> wrote: > > Hm. What about backward compatibility regarding _RSA_SHA1? Our default > > would change while dropbear's default hasn't changed (yet). > > I don't think this is a big concern. Like you say, any update of > Buildroot could bring different behaviour of a package (caused by an > upstream change or by us), and as dropbear has stated that they will > disable RSA_SHA1 by default in a future release it would be odd for us > to have the opposite behaviour (on by default). Sounds good to me. > > However, the situation with _RSA_SHA1 is reversed. The feature is > > enabled by default in dropbear and a user has to manually turn it off. > > If we model _RSA_SHA1 after _LEGACY_CRYPTO, on the one hand it is nice > > and consistent on the Buildroot side. On the other hand, it behaves > > opposite to what dropbear is doing. Buildroot would require the user > > to take action to receive the default dropbear behaviour (_RSA_SHA1 > > would need to be enabled explicitly because it would default to > > "unset"). This is the opposite experience than someone building > > dropbear without Buildroot. > > True. It also somewhat depends on how fine grained configuration options > we want, E.G. we could also simply handle the RSA_SHA1 option under the > LEGACY_CRYPTO option WHEN upstream disables it by default, but then we > have to wait for that. Do we really have to wait for upstream? We could do something like this: ifeq ($(BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO),y) define DROPBEAR_ENABLE_LEGACY_CRYPTO echo '#define DROPBEAR_3DES 1' >> $(@D)/localoptions.h echo '#define DROPBEAR_ENABLE_CBC_MODE 1' >> $(@D)/localoptions.h echo '#define DROPBEAR_SHA1_96_HMAC 1' >> $(@D)/localoptions.h echo '#define DROPBEAR_DH_GROUP1 1' >> $(@D)/localoptions.h echo '#define DROPBEAR_DSS 1' >> $(@D)/localoptions.h endef DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_ENABLE_LEGACY_CRYPTO else define DROPBEAR_DISABLE_RSA_SHA1 echo '#define DROPBEAR_RSA_SHA1 0' >> $(@D)/localoptions.h endef DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_DISABLE_RSA_SHA1 endif If _LEGACY_CRYPTO==y, it'll enable all the old algorithms and do nothing with SHA1 (which is enabled by default). If _LEGACY_CRYPTO is unset, it'll turn off SHA1 (and leave the legacy algorithms alone, so they are turned off). It's a little out there, but it should work... The localoptions.h file looked correct when I attempted this. > Looking elsewhere, I see that openwrt handles it with a "modern only" > option to only enable modern/secure options, maybe that is a way to go? > > https://github.com/openwrt/openwrt/commit/bf900e02c7102601be2e9280231711e70f065877 > > Related to RSA_SHA1, I believe the original reason for you sending this > patch was to disable everything related to SHA1, but SHA1 is also used > for HMAC and key exchange, so we should consider disabling those as > well: > > https://github.com/openwrt/openwrt/commit/2d9a0be307b534ceb717267c95402d1d707cd2c3 The initial motivation was that "ssh-audit" would flag the RSA algorithm using it. $ ssh-audit testbed | grep rsa (key) rsa-sha2-256 (4096-bit) -- [info] available since OpenSSH 7.2 (key) ssh-rsa (4096-bit) -- [fail] using weak hashing algorithm (fin) ssh-rsa: SHA256:a0+Ut09+aNXX7NeA6lcaJgvOAbeKcWUMMhFHuEJui6s (rec) -ssh-rsa -- key algorithm to remove This already goes away by disabling RSA_SHA1. $ ssh-audit -p 2222 testbed | grep rsa (key) rsa-sha2-256 (4096-bit) -- [info] available since OpenSSH 7.2 (fin) ssh-rsa: SHA256:a0+Ut09+aNXX7NeA6lcaJgvOAbeKcWUMMhFHuEJui6s I'll play around with this some more and see what we can do regarding HMAC and kex. Regards, -Markus
diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in index 207c1f561700..099f61535aa2 100644 --- a/package/dropbear/Config.in +++ b/package/dropbear/Config.in @@ -67,6 +67,12 @@ config BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO DSA public keys Diffie-Hellman Group1 key exchange +config BR2_PACKAGE_DROPBEAR_DISABLE_RSA_SHA1 + bool "disable SHA1 hashing for RSA" + help + SHA1 is no longer considered secure. Users may want to disable + it. However, this may preclude older clients from connecting. + config BR2_PACKAGE_DROPBEAR_LOCALOPTIONS_FILE string "path to custom localoptions.h definitions file" help diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk index 9423d891c8d3..56a0b9d910b4 100644 --- a/package/dropbear/dropbear.mk +++ b/package/dropbear/dropbear.mk @@ -77,6 +77,13 @@ endef DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_ENABLE_LEGACY_CRYPTO endif +ifeq ($(BR2_PACKAGE_DROPBEAR_DISABLE_RSA_SHA1),y) +define DROPBEAR_DISABLE_RSA_SHA1 + echo '#define DROPBEAR_RSA_SHA1 0' >> $(@D)/localoptions.h +endef +DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_DISABLE_RSA_SHA1 +endif + ifeq ($(BR2_PACKAGE_DROPBEAR_DISABLE_REVERSEDNS),) define DROPBEAR_ENABLE_REVERSE_DNS echo '#define DO_HOST_LOOKUP 1' >> $(@D)/localoptions.h
Since SHA1 hashing is considered insecure, users may wish to disable support for it. This will reduce compatibility with older systems but provide a more secure setup. SHA1 support for RSA is slated to be removed from dropbear at some point. This new option also gives users the ability to disable support early and evaluate what consequences this upcoming change might bring. Signed-off-by: Markus Mayer <mmayer@broadcom.com> --- package/dropbear/Config.in | 6 ++++++ package/dropbear/dropbear.mk | 7 +++++++ 2 files changed, 13 insertions(+)