From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/1227 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: FreeSec crypt() Date: Sun, 24 Jun 2012 23:51:03 -0400 Message-ID: <20120625035103.GG544@brightrain.aerifal.cx> References: <20120612235113.GA21296@openwall.com> <20120613011842.GA163@brightrain.aerifal.cx> <20120613120754.GA21900@openwall.com> <20120613145318.GC163@brightrain.aerifal.cx> <20120624072112.GA3792@openwall.com> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: dough.gmane.org 1340596397 13757 80.91.229.3 (25 Jun 2012 03:53:17 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 25 Jun 2012 03:53:17 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-1228-gllmg-musl=m.gmane.org@lists.openwall.com Mon Jun 25 05:53:16 2012 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Sj0MV-0008JU-Lx for gllmg-musl@plane.gmane.org; Mon, 25 Jun 2012 05:53:15 +0200 Original-Received: (qmail 13795 invoked by uid 550); 25 Jun 2012 03:53:15 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 13787 invoked from network); 25 Jun 2012 03:53:15 -0000 Content-Disposition: inline In-Reply-To: <20120624072112.GA3792@openwall.com> User-Agent: Mutt/1.5.21 (2010-09-15) Xref: news.gmane.org gmane.linux.lib.musl.general:1227 Archived-At: On Sun, Jun 24, 2012 at 11:21:12AM +0400, Solar Designer wrote: > On Wed, Jun 13, 2012 at 10:53:18AM -0400, Rich Felker wrote: > > However, on the other hand I'm not sure I see the benefit. Why would > > you call crypt or crypt_r multiple times with the same key/salt except > > possibly for password cracking (in which case you'll want a highly > > optimized-for-speed implementation)? > > You're right. Additionally, preserving this requires that we keep > sensitive data around (from the previous password hashed). I've dropped > this stuff now. Ah, nice catch! > > > I agree that the vast majority of cases may involve UB, but so what - we > > > can't be 100% certain we don't have any cases of UB in our source code > > > even if we review it very carefully. > > > > For purely computational code that doesn't recurse or loop arbitrarily > > long based on the input or read and write all over memory, static > > analysis can determine the absence of UB. > > Are you performing such static analysis on musl? Not yet/only to a minimal extent so far. I haven't spent much time on it, but users/contributors have often posted clang's analysis outputs for discussion on IRC, and it looks like a promising direction. > > > value or you return an error indication (what to return from crypt() on > > > error is a separate non-trivial topic). > > > > I saw the notes on this. What real-world code breaks from the > > conformant behavior? > > I think the majority of daemons, etc. that do authentication with > crypt() don't handle possible NULL returns. This is just starting to > change now. They don't really break under normal conditions because > normally crypt() returns the hashed password and not NULL; but if it > does somehow return NULL (e.g., because the salt read from the shadow > file is invalid), then I'd expect most users of crypt() to crash. Indeed, the first program I checked, dropbear, uses the return value without checking it. It's rather disheartening that something as important as authentication code is not checking the return value of each function that could possibly fail. With TCB shadow especially, password hashes could be highly invalid... Anyway, nowhere does POSIX say crypt has to fail at all; as far as I can tell, it's completely possible and reasonable to make an implementation that always succeeds even on invalid salt. As long as you ensure that the output can never be matched, it's probably fine to do this. > ....Attached is my latest revision of crypt_freesec. I've reduced the > table sizes even further (7 KB, may be precomputed) and I made certain > other changes as discussed. I'd appreciate another review, and some > fuzzing against another implementation wouldn't hurt. I put this off until after the release so as not to break anything at the last minute, but I'll try to get it integrated soon. Rich