Re: [hackers] [slock] Unify how we check passwords between different OSes

From: Markus Teich <markus.teich_AT_stusta.mhn.de>
Date: Wed, 7 Sep 2016 12:26:02 +0200

Heyho Quentin,

Quentin Rameau wrote:
> These variables seems to be about the auth_userokay()

So the removal is fine. Thanks for the clarification.

> > > diff --git a/slock.c b/slock.c
> > > …
> > > +#else
> > > + if (rval[0] == '*' && rval[1] == '\0') {
> > > +#ifdef __OpenBSD__
> > > + if (!(pw = getpwnam_shadow(getenv("USER"))))
> > > + die("cannot retrieve shadow entry (make sure to suid or sgid slock)\n");
> > > + rval = pw->pw_passwd;
> > > +#else
> > > + die("cannot retrieve shadow entry (make sure to suid or sgid slock)\n");
> > > +#endif /* __OpenBSD__ */
> > > + }
> > > +#endif /* HAVE_SHADOW_H */
> >
> > We should probably use another error message in the not __OpenBSD__ case,
> > maybe something like "slock: unknown shadow passwd system\n". Also please
> > add the "slock: " prefix to the other die() call.
> The second message is right here: if we got a pw->pw_passwd being "*", it
> means that we didn't have access to the shadow password in the first pass,
> which is how it works on other systems (those being the one stated in the
> previous mail).

I see that obviously only one of those die() calls will be compiled into the
binary, but keeping error messages unique helps finding the actual place where
it was called. In this case having different messages would mean we don't have
to ask someone reporting this error to us if he had __OpenBSD__ defined or not
during the build.

I'd even go as far as adding "Keep error messages unique" to the style guide.

--Markus
Received on Wed Sep 07 2016 - 12:26:02 CEST