| Message ID | 1457638990-27544-1-git-send-email-gustavo@zacarias.com.ar |
|---|---|
| State | Accepted |
| Headers | show |
>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes: > DES is terribly outdated and a security vulnerability. > Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar> > --- > package/busybox/busybox.config | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/package/busybox/busybox.config b/package/busybox/busybox.config > index 2cf989d..693b9d2 100644 > --- a/package/busybox/busybox.config > +++ b/package/busybox/busybox.config > @@ -496,7 +496,7 @@ CONFIG_PASSWD=y > CONFIG_FEATURE_PASSWD_WEAK_CHECK=y > # CONFIG_CRYPTPW is not set > # CONFIG_CHPASSWD is not set > -CONFIG_FEATURE_DEFAULT_PASSWD_ALGO="des" > +CONFIG_FEATURE_DEFAULT_PASSWD_ALGO="md5" So this is what is used when you change password through the busybox passwd applet or create a new user, right? Shouldn't we instead tweak this based on BR2_TARGET_GENERIC_PASSWD_*?
On 10/03/16 17:01, Peter Korsgaard wrote: > So this is what is used when you change password through the busybox > passwd applet or create a new user, right? > > Shouldn't we instead tweak this based on BR2_TARGET_GENERIC_PASSWD_*? Yes. This is the lowest-common which is guaranteed to work on all scenarios, once we decide/switch to sha256 as default we can update it here, but i wouldn't defer this until then. Regards.
>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes: > On 10/03/16 17:01, Peter Korsgaard wrote: >> So this is what is used when you change password through the busybox >> passwd applet or create a new user, right? >> >> Shouldn't we instead tweak this based on BR2_TARGET_GENERIC_PASSWD_*? > Yes. > This is the lowest-common which is guaranteed to work on all > scenarios, once we decide/switch to sha256 as default we can update it > here, but i wouldn't defer this until then. Ehh, yes - But if you have enabled E.G BR2_TARGET_GENERIC_PASSWD_SHA512 then that is what the busybox passwd applet should use. Having it do md5 instead of des is still wrong.
On 03/10/16 23:00, Peter Korsgaard wrote: >>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes: > > > On 10/03/16 17:01, Peter Korsgaard wrote: > >> So this is what is used when you change password through the busybox > >> passwd applet or create a new user, right? > >> > >> Shouldn't we instead tweak this based on BR2_TARGET_GENERIC_PASSWD_*? > > > Yes. > > This is the lowest-common which is guaranteed to work on all > > scenarios, once we decide/switch to sha256 as default we can update it > > here, but i wouldn't defer this until then. > > Ehh, yes - But if you have enabled E.G BR2_TARGET_GENERIC_PASSWD_SHA512 > then that is what the busybox passwd applet should use. Having it do md5 > instead of des is still wrong. Peter, you yourself said that we should limit the amount of KCONFIG_* fixups we do. And there is no other way to make it depend on the BR2_TARGET_GENERIC_PASSWD_* choice. Regards, Arnout
Hello, On Thu, 10 Mar 2016 16:43:10 -0300, Gustavo Zacarias wrote: > DES is terribly outdated and a security vulnerability. > > Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar> > --- > package/busybox/busybox.config | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied to master, thanks. I've (obviously) read the discussion with Peter, but I believe the issue he is raising is different. We already have our Busybox configuration default to "des", which is not consistent with the algo choice made by the user in the Buildroot configuration. So switching to "md5" is not making things any worse from that perspective. Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: Hi, > I've (obviously) read the discussion with Peter, but I believe the > issue he is raising is different. We already have our Busybox > configuration default to "des", which is not consistent with the algo > choice made by the user in the Buildroot configuration. So switching > to "md5" is not making things any worse from that perspective. True. It would be good if we could make busybox default to the algorithm we've selected in buildroot, but that can be done later.
diff --git a/package/busybox/busybox.config b/package/busybox/busybox.config index 2cf989d..693b9d2 100644 --- a/package/busybox/busybox.config +++ b/package/busybox/busybox.config @@ -496,7 +496,7 @@ CONFIG_PASSWD=y CONFIG_FEATURE_PASSWD_WEAK_CHECK=y # CONFIG_CRYPTPW is not set # CONFIG_CHPASSWD is not set -CONFIG_FEATURE_DEFAULT_PASSWD_ALGO="des" +CONFIG_FEATURE_DEFAULT_PASSWD_ALGO="md5" CONFIG_SU=y CONFIG_FEATURE_SU_SYSLOG=y CONFIG_FEATURE_SU_CHECKS_SHELLS=y
DES is terribly outdated and a security vulnerability. Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar> --- package/busybox/busybox.config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)