Closed Bug 524403 Opened 11 years ago Closed 8 months ago
Summary: softtoken's master password KDF process should be stronger (currently easily brute forced due to low iteration count)
I recently ran across "Firemaster", a master password cracking tool (http://www.securityxploded.com/firemaster.php). The keys-per-second claimed seemed higher than I was expecting, so I went looking at exactly how the master password was handled by softtoken. Skimming over a few details, sftkdb_passwordToKey() first computes an intermediate key with just SHA1(salt,passphrase). This is then used in sftkdb_DecryptAttribute() as input to a PKCS#5 PBKDF1 routine. The default iteration count is just 1, though [set by sftkdb_EncryptAttribute() passing 1 to nsspkcs5_NewParam()]. A higher iteration count would make this more resistant to brute forcing (by increasing the cost of testing password), the PKCS#5 spec suggests a "modest value" of 1000 iterations. And that was 10 years ago. :) As an implementation detail, it might be better to change sftkdb_passwordToKey() to actually use PKCS#5 with a high iteration count (instead of just a single simple hash), and leave sftkdb_EncryptAttribute() as-is. The intermediate key is only computed once, so attribute crypting would remain fast. [And, of course, there's still the whole issue of making this compatible with existing DBs.]
Bob, could you take a look at this bug?
Need an owner for this. /be
(In reply to Brendan Eich [:brendan] from comment #3) > Need an owner for this. What is driving this?
Some inbound media mention. needinfo?rrelyea is unanswered, looks like a bug that could use some guidance *in the bug* for anyone interested -- especially potential volunteers. /be
I did some work on this previously. We also had some discussions about what to do and how to prioritize this on a few different occasions. Most of those were face-to-face meetings but I took some notes that I need to dig up. A short summary of the previous conclusions, that I remember off the top of my head are quite specific to Firefox and other Gecko-based products. Since this is an NSS bug, and NSS is a shared component amongst many products, including Google Chrome and many Oracle products in particular, I cloned this bug into a Gecko-specific bug so we can discuss the Firefox-specific (Gecko-specific) aspects without dragging the other NSS organizations into it. I trimmed the CC list of that bug; please add yourself back to the CC list if you are interested in the Firefox/Gecko-specific issues. Please don't discuss Firefox-/Gecko-specific issues in this bug. The bug to talk about Firefox's Master Password crypto strength is the new bug, which is bug 973759.
(Tweaking summary since I always have trouble finding this)
Summary: softtoken's master password KDF process should be stronger → softtoken's master password KDF process should be stronger (currently easily brute forced due to low iteration count)
9 years later I looked the same topic without being aware of this discussion and was shocked to see a single SHA1 iteration being used to hash passwords. This has been highly discouraged for a while already, see http://cynosureprime.blogspot.de/2017/08/320-million-hashes-exposed.html for example. A single GTX 1080 GPU can calculate 8.5 billion SHA1 hashes per second. With humans choosing passwords that on average have merely 40 bits of entropy (see http://research.microsoft.com/pubs/74164/www2007.pdf) the average password can be guessed in a minute. I'm sure that Mozilla can do better. Any plans to address this? At the very least, PBKDF2 should be used with an appropriate number of iterations (at least 100k, more is better). Algorithms like bcrypt or scrypt are more resistant to bruteforcing but these aren't implemented by NSS yet.
(In reply to Wladimir Palant (for Adblock Plus info Cc email@example.com) from comment #8) > I'm sure that Mozilla can do better. Any plans to address this? Maybe https://mozilla-lockbox.github.io/ will at some point replace the current password manager?
I don't think that waiting for some potential password manager rewrite is the solution here. Also, keep in mind that Lockbox also expects a key as input (they call it "app prekey"). With the current design, this key comes from Firefox Accounts. But relying on Firefox Accounts isn't an option if Lockbox is to replace a fundamental feature such as the password manager. That means that it will again be up to NSS to generate this key.
Hmm, I thought we had fixed this a while back, but I bet it was for PKCS #12, not for the database. We definately should change this, the current iteration count for the database is 1. There are 2 places in sftkpwd.c that need to change: sftkdb_EncryptAttribute() and sftkdb_SignAttribute. Both call nsspkcs5_NewParam with a hard coded interation count of 1. The only affect of changing the interaction count is the performance of login and private key operations. Old versions of NSS can still read the database (the iteration count for decrypt is read from the database record). Old databases will still be able to be read, though they will still have an iteration count of 1. Changing the password (even to the same password) on those database will update the iteration count. The only other concession is what we need to change that value to. pkcs12 is set to 10000 (10K) in debug builds and 1000000 (1M) in optimized builds.
Some kind of automated batch upgrade mechanism would also be very desirable. I have over 300 passwords that would need to have their iteration counts bumped.
We are talking about the NSS database password (Firefox Master Password), not your saved, encrypted passwords. Those passwords are encrypted with a fixed key, stored in the NSS database (encrypted with the Master Password). Changing your Firefox Master Password will change the iteration count on all the records in the NSS database (including the the record on the fixed key used to encrypt all your passwords), so you only need to change your Firefox Master Password (once) and all your keys will get the new iteration count. If you have lots of databases, then you could use certutil to build a script (but you'd have to supply the password to each database to make this work).
One other consideration. NSS treats the empty password "" as "doesn't require a password", but that is at the higher level (check to see if I can decrypt the password record with "" and if I can just pass "" as the password and don't prompt the user). This means if we just change the iteration count, even users with no passwords will see the slower response when accessing certain operations. It would require some surgery if we think we need to special case the NULL password because by the time it hits the PBE, its already hash and salted into a random password, so it's no longer distinguished as "". I'm I just noticed this was marked fix and duplicate, so someone else also thought it was fixed when we fixed the PKCS #12 issues, but I'm not seeing the change for the databases so I'm reopening this.
OK, well I wasn't able to reopen this. Hmm...
(In reply to Robert Relyea from comment #13) > We are talking about the NSS database password (Firefox Master Password), > not your saved, encrypted passwords. Those passwords are encrypted with a > fixed key, stored in the NSS database (encrypted with the Master Password). > Changing your Firefox Master Password will change the iteration count on all > the records in the NSS database (including the the record on the fixed key > used to encrypt all your passwords), so you only need to change your Firefox > Master Password (once) and all your keys will get the new iteration count. Ahh. I misunderstood how the pieces fit together. Sorry about that.
(In reply to Robert Relyea from comment #15) > OK, well I wasn't able to reopen this. Hmm... The bug is still open.
FYI. There's a new blog post which discusses this bug. It's currently the second most popular link in /r/programming. https://www.bleepingcomputer.com/news/security/firefox-master-password-system-has-been-poorly-secured-for-the-past-9-years/
ouch seeing this really hurts. why didnt this get any real action? it wouldnt have hurt to bump the iteration count a bit every few years to count in the advance of computer power, but well... I personally would go even further and run a piece of argon2id as the KDF, this thing after all doesnt just eat computation cycles but also RAM for breakfast and even relatively low values as 16MB are helping because trying many attacks in parallel like with gpus or so can end pretty quickly because the RAM is getting an issue. one thing that would be even more awesome would be obviously about:config settings for how crazy it should go so that users who know their stuff can even increase the RAM usage or iteration count and therefore the security of the thing.
Two things: First, any way to address this needs to have a way to completely rekey the password database, because if the passphrase has already been hacked the database key is already lost -- thus any changing of the passphrase would be closing the barn door after all the cows are gone, especially for future entries. Second, :openjck's link suggests that Mozilla's official response is to push people toward using something like the Lockbox extension (currently in fairly early alpha). Please ensure that any replacement doesn't use the OS clipboard. I use Mozilla's password manager specifically because it doesn't, in case undetected malware is monitoring the clipboard. (Since there's no way to apply an ACL to the clipboard for particular applications to be able to read/paste, anything using it is less secure than the in-app channels that already exist between the password manager and the login forms.)
I don't think that Lockbox could be a solution - mainly because it is Firefox only. However the weak master password problem affects Firefox and Thunderbird!
@comment #22 I think rekeying is needed AS WELL AS changing the passwords in the database. because if the key is already known, re-encypting it with a new scheme wont help. regarding argon2, full ack, as I already noted in comment #20.
I think we should not lose sight of what's going on here, and what's relevant. If the key is already compromised it means an attacker already has your data, period, so that's not a situation you should consider here because in that case you should change all your passwords anyway. In case an attacker does NOT have your data (and you make sure they don't get a hold of a backup of your profile) simply letting the master key be re-keyed with the new value would certainly be sufficient, since the actual encryption used otherwise is sound (although a bit dated, des-ede3 is certainly good enough for this purpose) and the only weak link here is being able to brute-force guess the master password. A fast and simple solution would definitely be to not use 1 round. However, it needs a few adjustments because nsspkcs5_NewParam takes a raw int (=short in most implementations) for iterations, which won't accept a suggested high value; it'll have to be at least a long/int32_t.
@comment #24 I personally think security should always assume the worsz. Murphy's Law FTW. of course if an attacher has the key it also has your PWs meaning you would need to change them, true, which is why it would be best in my opinion to obviously both rekey the DB and change the PWs. and even if it hasnt been stolen, a rekey still isnt gonna do something bad nor is it particularly hard.
> a rekey still isnt gonna do something bad nor is it particularly hard. It isn't, but it's also not necessary as stated in comment 23. As for using a different encryption/keying scheme altogether: anything like that, although a nice new and shiny and super-secure way of doing things, will require (probably complex) one-way migration code which is going to be a total PITA with a lot of people potentially losing their stored passwords if anything goes wrong. It might be good for a follow-up bug, but making shorter master passwords significantly more difficult to guess should be an immediate goal, so K.I.S.S. this for now? The encryption itself isn't weak, IIUC the only risk is, as the bug title says, a possibility to correctly brute-force guess the master password in a feasible amount of computing time. If that brute forcing is mitigated by making every guess take a million times as long, your average 40-bit password will still take probably a few years to brute-force guess. Keeping the method and changing the rounds, as stated, would be pretty much fully transparent I've done some quick tests by changing the rounds indicated in comment 11 (to something within int range), but it breaks the use of the master password as in making it error with "unable to change" and having an unrecoverably locked store afterwards, so it must have been missing something, elsewhere. Unfortunately I'm not at all familiar with this piece of code so probably better if the NSS folks look at this directly.
@comment #26 certainly it does need migration but is it that hard? I dont precisely know the format of the encrypted database, but if it has something to give away whether it's the old scheme or the new one it's gonna get even easier. at least in theory the flow should be as follows: find out whether the old scheme is used (either by checking the data or by trying) if it's in the old format: -> decrypt everything and put it in RAM -> create a new key -> encrypt the database with the key -> run the password through whatever new KDF we want -> encrypt the key using the KDF'd password -> store both the encrypted database and the encrypted key. ELSE: -> just use the database also I implemented migration for password hashes in websites before, so it's not like I am completely oblivious to this whole thing, and there such a thing should always exist for the sole reason to easily change the parameters or the hash functions used. this kind of migration (even if you dont need to rekey, you do need to re-encrypt the database key with the newly hashed password) would be needed in all cases no matter whether you just bump the number of iterations or whether you straight up go to make it use an entirely new hash function. regarding the necissity of a rekey, the problem is that a database stolen once would have it's key revealed to to the easy bruteforce, and the problem is that if it continues to use the same key later future passwords would also be compromised, which wouldnt be nice.
@comment #23 @comment #26 I don't fully agree. Security considerations and practical solutions seem to being talked down in this bug forum in favour of implementation of new features, application re-designs and individual software architecture preferences of sorts, at times. Where the mitigating solution you suggest would certainly qualify as a practial quick-fix for now, please keep the following in mind: Given the horrendously long times, (e. g. like with Bug 278689) until certain security-related issues are being properly adressed, it seems to be a good idea to take at least one step ahead, if finally taking care of an issue. Fixing it with general progressions in the field of security counter-measures to come, will probably help preventing the next several years of non-maintenance in this area becoming a more serious problem, than it needs to. Specifically, NSS seems not getting the kind of attention it would require, given its delicate nature, compared to, say, the UI or gecko itself. If a quick-fix will be applied to this problem, I'm afraid, a real (and necessary) solution will be put into backlog for an indefinite time.
Sorry but I disagree. I've seen enough bugs never get any progress because people want to get a "perfect solution" and nothing else in. Considering the sensitivity of this and the risk involved for the security of passwords of browser users, I think that putting in a quick, low-risk fix is exactly what this needs right now, and that changing the security altogether (either by switching to a different key system or an external service or whatever else has been discussed) should become a follow-up bug after the major risk has been mitigated in this one. Let's move this forward instead of running aground in exchanges of opinion, shall we?
Meant to refer to @comment #24, not #23 above, sorry. I see your point, and it was exactly, why things got out of range in bug 278689 mentioned above. Basically, I agree with not waiting for the perfect solution for major or critical bugs and blockers. It just should be really ensured, that this issue will be prioritized and adressed with a real solution according to its nature in a timely manner. NSS in general seems to be dire need for some major overhauling. Mark Mayo should definitely provide more resources for these kind of problems and re-designs soon. With "taking a step ahead" above, I mean applying a better hashing algorithm, a more modern and secure combination of encryption/cipher mode, like sha-256, maybe even stronger, which offers a decent bf protection layer against physical access attacks, because "des-ede3" does obviously not - see also Bug 973759 and: https://security.stackexchange.com/questions/92593/why-is-there-a-des-ede3-cbc-in-my-rsa-private-key https://www.openssl.org/blog/blog/2016/08/24/sweet32/ Since the Mozilla password safe holds numerous other, potentially very sensitive passwords, e. g. to banking accounts, there should definitely be no false compromises in terms of security anymore. The performance costs for higher encryption standards (if any) would be well-justified, and should not be a real issue anyway, given the low number of manual accesses to the pw safe. When combined with securely tunneled access per smartcard and U2F, along with a re-designed browser architecture, a modern means of pw safe encrption should provide much better security for bf and other kinds of attacks, helping to maintain privacy, to some extent for malware attacks, and even if the mail data base was fetched.
A few weeks of silence have passed. Robert, what do you think? I clearly like the simple short-term solution to secure the pw store without a complicated migration path for a different crypto approach, with further improvements down the line.
QA Contact: franziskuskiefer
Assignee: nobody → kaie
Flags: needinfo?(rjesup) → needinfo?(mconley)
Whiteboard: [sg:want P2] → [sg:want P2][qf]
Flags: needinfo?(mconley) → needinfo?(sfoster)
Status: REOPENED → ASSIGNED
Comment on attachment 9074180 [details] [diff] [review] 524403-bob-v3.patch Moving patch to bug 1562671.
Attachment #9074180 - Attachment is obsolete: true
Attachment #9074181 - Attachment is obsolete: true
Attachment #9074182 - Attachment is obsolete: true
Whiteboard: [sg:want P2][qf] → [sg:want P2]
Flags: needinfo?(rjesup) → needinfo?(mstange)
Summary: softtoken's master password KDF process should be stronger (currently easily brute forced due to low iteration count) → softtoken's master password KDF process should be stronger
Status: ASSIGNED → RESOLVED
Closed: 1 year ago → 8 months ago
No longer depends on: 1562671
Resolution: --- → DUPLICATE
Summary: softtoken's master password KDF process should be stronger → Summary: softtoken's master password KDF process should be stronger (currently easily brute forced due to low iteration count)
Duplicate of bug: 1562671
You need to log in before you can comment on or make changes to this bug.