Bug 182518

Summary:

[login.conf] Better Password Hashes

Product:

Base System

Reporter:

A.J. Kehoe IV <n6gXhrf6>

Component:

conf

Assignee:

freebsd-bugs (Nobody) <bugs>

Status:

Open ---

Severity:

Affects Only Me

CC:

482254ac, Jungleboogie0, allanjude, emaste, feld, freebsd, ler, nukama, secteam, shawn.webb, tom, tommi.pernila

Priority:

Normal

Keywords:

feature, security

Version:

10.0-CURRENT

Hardware:

Any

OS:

Any

See Also:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=75934

Attachments:

Description	Flags
file.shar	none
Patch against -CURRENT 201502 with a ton of enhancements, see -security	none
Patch against -CURRENT revision 279936 (20150312) with changes discussed on -security.	none
Updated patch against -CURRENT revision 279936 (20150312) with changes discussed on -security.	none
Refreshed patch, against git/a8ff864 (20160730)	none
correct crypt_makesalt return code documentation	none

Description A.J. Kehoe IV 2013-09-30 18:50:00 UTC

My friend and fellow FreeBSD enthusiast Derek Marcotte recently pointed out that FreeBSD has no easy way to set the logarithmic rounds for bcrypt password hashes. Doing so is trivial in OpenBSD, and considering the capabilities of current GPU attacks, I want this functionality.

This issue was raised over eight years ago in kern/75934 by Steven Alexander Jr., who included a patch to add this feature. Unfortunately, this seems to have been completely overlooked, and there were no public responses to this PR.

I commissioned Derek to come up with a solution by either updating Steven's patch or by devising a new method. To paraphrase Derek's comments:

-----BEGIN PARAPHRASIS-----
I did some research into what other *BSDs are doing. OpenBSD and NetBSD use the algorithm name, a comma, and then the number of rounds:

http://www.openbsd.org/cgi-bin/man.cgi?query=login.conf&sektion=5

localcipher=blowfish,6

http://netbsd.gw.com/cgi-bin/man-cgi?passwd.conf+5+NetBSD-current

localcipher=blowfish,6

To me, this isn't a good way to do it because we'd need special rules to parse this extra field out of the previously unstructured data. This parsing would be algorithm dependent.

Everyone knows about modular crypt, so why not feed the modular crypt salt string as the parameter directly? Instead of messing with different names, give the power to the system admin to control this directly, so when crypt is updated, pam_unix can take advantage. Each implementation of crypt algorithms already includes parsing of the salt magic.

I found that patching pam_unix was the least invasive way to handle configurable hashes for login. I've added a passwd_modular parameter that will supersede passwd_format when defined. passwd_modular will feed directly into crypt, so any options that are passed to crypt via the salt are immediately available for use in the master.passwd file. For example:

:passwd_modular=$2a$11$:\

Now you can also set the rounds for sha512:

:passwd_modular=$6$rounds=1000000$:\

To disable passwd_modular and revert to passwd_format:

:passwd_modular=disabled:\

This also lets admins shoot themselves in the foot by supplying invalid or bad salts. For example:

:passwd_modular=$1$constantsalt:\

I had considered setting a second variable like ":passwd_param=8:\", but then you really have to mess with crypt to make it work. I think it would be a much more invasive change, and unnecessary, providing the documentation for login.conf is brought up to date.

FreeBSD 8.* doesn't have access to the SHA family of hashes. If this is merged back into 8, it will give much stronger password security when using $2a$08$ (or higher) than is currently available.

bcrypt is preferable to sha512 because of its resilience to current GPU attacks. This is expected to change. Hopefully, my patch will lay some groundwork to incorporate scrypt.
-----END PARAPHRASIS-----

Fix: I've attached a copy of Derek's patches for FreeBSD 10.0-CURRENT. I also have his patches for 9-STABLE, but have not included them here.

I really like Derek's solution. In my opinion, committing Derek's patches will allow kern/75934 to be closed.

Patch attached with submission follows:

Comment 1 Xin LI freebsd_committer

2013-09-30 18:52:37 UTC

Responsible Changed
From-To: freebsd-bugs->secteam

Take.

Comment 2 Mark Felder freebsd_committer

2014-10-10 22:51:06 UTC

Any update on this? The original issue is now nearing 9 years old :-)

Comment 3 Derek 2015-02-10 12:24:05 UTC

Created attachment 152839 [details]
Patch against -CURRENT 201502 with a ton of enhancements, see -security

does a few things with the base system:

1. allows modular crypt to be specified as passwd_format in /etc/login.conf
  - this allows setting the algorithm *and rounds*, i.e. $2b$10$ for users of varying classes.
  - this will allow any future algorithms and parameters supported by crypt(3) to be supported by the tools around login.conf

2. introduces a new api, crypt_makesalt which will generate an appropriate salt for any algorithm selected

3. updates userland to use this API, and removes totally the {crypt_set_format, login_setcryptfmt, login_getcryptfmt} APIs

4. switches crypt algorithms to use thread-local storage, so the good old global crypt buffer is thread-local

5. includes a bunch of new test vectors for libcrypt ATF tests

Comment 4 Derek 2015-03-12 20:58:02 UTC

Created attachment 154251 [details]
Patch against -CURRENT revision 279936 (20150312) with changes discussed on -security.

Comment 5 Derek 2015-03-13 10:56:14 UTC

Created attachment 154265 [details]
Updated patch against -CURRENT revision 279936 (20150312) with changes discussed on -security.

Forgot to add some documentation items.  They are included here.

The items specifically mentioned on -security were:

3. updates userland to use this API, and removes totally the {crypt_set_format, login_setcryptfmt, login_getcryptfmt} APIs

4. switches crypt algorithms to use thread-local storage, so the good old global crypt buffer is thread-local

#4 has be backed out.  #3 - we've kept the old APIs and updated their documentation.

Comment 6 Allan Jude freebsd_committer

2016-06-13 22:31:07 UTC

As discussed at BSDCan, submitted is going to refresh this patch

Comment 7 Derek 2016-08-02 11:02:55 UTC

Created attachment 173187 [details]
Refreshed patch, against git/a8ff864 (20160730)

To recap, this patch contains:

- updated libcrypt, which includes crypt_makesalt
- revised/rewritten crypt(3) manpage, detailing the uses of Modular Crypt Formats, and new crypt_makesalt api
- numerous test vectors for libcrypt 
- refactored pam_unix to use crypt_makesalt, instead of its own format
- refactored pw to support Modular Crypt Formats in login.conf

This will (hopefully!) lay the groundwork for a login.conf tunable to allow pam_unix to "upgrade" hashes on login to a suitable algorithm.

Again, the same discussion/review that took place on -security is still relevant, this simply brings the patch up to date.

Comment 8 Derek 2016-08-02 11:14:25 UTC

Created attachment 173188 [details]
correct crypt_makesalt return code documentation

Same as 173187: Refreshed patch, against git/a8ff864 (20160730), but corrected return code documentation for crypt_makesalt.

Comment 9 Ed Maste freebsd_committer

2017-07-27 18:50:10 UTC

I plan to have a look at this with folks attending BSDCam during the 1st week of August.

Comment 10 Derek 2017-07-28 10:26:12 UTC

That's great news.  Happy to iterate on this, if the feedback loop is shorter.

Comment 11 Mark Felder freebsd_committer

2018-05-23 20:42:26 UTC

It has been some years now, and it looks like we are still without a workable solution. Any update?

Comment 12 Kubilay Kocak freebsd_committer

2018-05-25 04:07:21 UTC

Reset status (In-Progress -> Open) on issues without a real assignee

Comment 13 Kubilay Kocak freebsd_committer

2018-05-25 04:14:23 UTC

Reset assignee (5 years since assignment). 

This issue doesn't strictly need to be lead/committed by secteam, and has likely gated progress due to it appearing 'taken'.

It will probably need to be at least reviewed if not approved by them however, so keep them CC'd.

It would be good to get the changes updated to patch/apply against CURRENT (12.0 today)

Comment 14 Derek 2018-05-25 20:33:14 UTC

I'm still here.  I'll be at BSDCan again this year.  Happy to iterate, but not in isolation.  Looking for someone in the project to help see this through so I'm not writing aimless patches.

Thanks

Comment 15 Mark Felder freebsd_committer

2018-05-25 20:48:54 UTC

(In reply to Derek from comment #14)

I will also be at BSDCan. Let's see if we can get this *committed* at BSDCan. I will help you find the right people to make this happen.

Comment 16 Derek 2018-06-09 21:17:16 UTC

FYI - this is in phabricator now:

https://reviews.freebsd.org/D15713

Comment 17 Mark Felder freebsd_committer

2018-07-06 19:55:25 UTC

Just checking in on this as the review went up just about a month ago.

Comment 18 Larry Rosenman freebsd_committer

2019-02-05 18:31:35 UTC

Any progress?

Comment 19 Thomas Hurst 2023-08-18 23:51:43 UTC

If this is going nowhere, at the very least the hardcoded iteration counts should be reviewed.  5000 iterations of SHA512 and 2^4 of blowfish are hardly reasonable.