On Tue, 9 Apr 2024 at 19:56, Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
>
> On Mon, Apr 8, 2024 at 11:31 AM Christian Göttsche
> <cgoettsche@xxxxxxxxxxxxx> wrote:
> >
> > From: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> >
> > Perform the password hash comparison in a time constant way to avoid
> > leaking the length of the identical prefix via a side-channel.
> > Since only hashes are compared leaking the total length is non critical.
> >
> > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
>
> Should we just require PAM for newrole and be done with it?
Since the shadow backend still works and seems to not to have a high
maintenance cost, I would argue to keep it.
Maybe one could stronger emphasize to use the PAM backend, via a note
in the Makefile or a warning during compilation.
>
> > ---
> > policycoreutils/newrole/newrole.c | 20 +++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/policycoreutils/newrole/newrole.c b/policycoreutils/newrole/newrole.c
> > index 5a1a1129..1e01d2ef 100644
> > --- a/policycoreutils/newrole/newrole.c
> > +++ b/policycoreutils/newrole/newrole.c
> > @@ -338,6 +338,24 @@ static void memzero(void *ptr, size_t size)
> > }
> > }
> >
> > +static int streq_constant(const char *userinput, const char *secret)
> > +{
> > + const volatile char *x = userinput, *y = secret;
> > + size_t i, u_len, s_len;
> > + int ret = 0;
> > +
> > + u_len = strlen(userinput);
> > + s_len = strlen(secret);
> > +
> > + if (u_len != s_len)
> > + return 0;
> > +
> > + for (i = 0; i < u_len; i++)
> > + ret |= x[i] ^ y[i];
> > +
> > + return ret == 0;
> > +}
> > +
> > /* authenticate_via_shadow_passwd()
> > *
> > * in: uname - the calling user's user name
> > @@ -383,7 +401,7 @@ static int authenticate_via_shadow_passwd(const char *uname)
> > return 0;
> > }
> >
> > - ret = !strcmp(encrypted_password_s, p_shadow_line->sp_pwdp);
> > + ret = streq_constant(encrypted_password_s, p_shadow_line->sp_pwdp);
> > memzero(encrypted_password_s, strlen(encrypted_password_s));
> > return ret;
> > }
> > --
> > 2.43.0
> >
> >
