[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20261106231834.GB23438@brightrain.aerifal.cx>
Date: Fri, 6 Nov 2026 18:18:34 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] [RFC] Harden against misuse of crypt
On Sat, Nov 07, 2026 at 12:02:59AM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@...c.org> [2026-11-06 16:54:05 -0500]:
> > This patch changes crypt to return "*0" instead of "*" as the "failure
> > token" that will not match any password.
> >
> > Likewise, "*1" is used in place of "x" if the failure token would be
> > identical to the input salt, but this case is presently unreachable.
> > Since the existing DES path interprets characters outside the portable
> > range as valid DES salt, passing a setting of "*0" will not produce
> > "*1" but instead a DES-hashed password with "*0" as the salt. I don't
> > see any problem with this -- "*0" should never appear in the
> > passwd/shadow db anyway, and if it somehow did, crypt(pass,"*0") would
> > still return something not-equal to "*0": "*0" followed by 11
> > characters of DES hash.
> >
> > In any case, I put the "*1" code path in there in case anything ever
> > changes with how we handle DES, to be sure crypt(pass,"*0") can never
> > return "*0".
> >
> > Rich
>
> looks good,
> but this commit has no reference to the analysis in
> https://www.openwall.com/lists/musl/2026/11/06/7
> i would add a link. or a note that the misuse was observed in
> the wild motivating the change. and that "*0" has historical
> precendent so broken callers are less likely.
Yes, I just haven't written a commit message yet. This was from
git-diff not git-format-patch.
> as a friend put it, if the goal is to minimize the likelihood
> of the failure token matching a passwd entry then it is
> possible to do better than "*0": e.g. 100 non-printable bytes
> (in case we really want to be defensive about misuses and typos
> in passwd files)
Well there are multiple goals. We want a passwd utility to be able to
conclude that setting the password failed; I assume they do that now
by something like strlen(hash)<13.
Tthe smart way would be checking whether the application of
crypt(pass,ยท) is idempotent - IOW, if f(h)=crypt(pass,h), does
f(f(h))==f(h)? This is directly testing "will I be able to get back in
with this password and hash?" But I doubt that's what anything does.
In any case, even if the output is "100 non-printable bytes", there is
still the risk of a bad passwd-like utility putting it in the
shadow/passwd file. That's no different from "*0". The only way that
could be avoided is by making the output entirely nondeterministic,
e.g. 256 bites of getentropy output encoded base64. But I don't think
that's a reasonable behavior.
> but if the plan is to move to return NULL then i guess "*0" is
> the least risky solution for now.
I still think that's the nicer behavior to move towards, but if there
are still programs that will break, I'm rather hesitant to do it
without a good transition period to get them fixed.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
