443386 - Sync errors in FIPS mode in NSS (generateRandomKey or keyFromString, NS_ERROR_FAILURE)

Status: NEW

(Firefox :: Sync, defect)

Description

[Michel Meyers]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Weave 0.2

Signing in to Weave fails. The spinner in the status bar moves for a while, then the dialog prompting for login data reappears and when clicking OK, Weave just reverts the status bar display to 'Sign in'.

Here's the log:

2008-07-03 13:21:53	Chrome.Login	TRACE	Sync login window opened
2008-07-03 13:21:54	Chrome.Window	INFO	Logging in...
2008-07-03 13:21:54	Chrome.Window	INFO	User string: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
2008-07-03 13:21:54	Chrome.Window	INFO	Weave version: 0.2.0
2008-07-03 13:21:54	Service.Main	DEBUG	Logging in user Steltek
2008-07-03 13:21:54	Service.Main	INFO	Using server URL: https://services.mozilla.com/user/Steltek/
2008-07-03 13:21:54	Service.DAV	DEBUG	checkLogin called for user Steltek
2008-07-03 13:21:54	Service.DAV	DEBUG	GET request for root folder
2008-07-03 13:21:54	Chrome.Login	TRACE	Sync login window closed
2008-07-03 13:22:05	Service.DAV	DEBUG	checkLogin got response status 200
2008-07-03 13:22:05	Service.DAV	DEBUG	GET request for meta/version
2008-07-03 13:22:07	Service.Main	TRACE	Retrieving keypair from server
2008-07-03 13:22:07	Service.DAV	DEBUG	GET request for private/privkey
2008-07-03 13:22:11	Service.DAV	DEBUG	GET request for public/pubkey
2008-07-03 13:22:34	Chrome.Login	TRACE	Sync login window opened
2008-07-03 13:22:36	Async.Generator	ERROR	Exception: Could not acquire lock
2008-07-03 13:22:36	Async.Generator	DEBUG	Stack trace:
No traceback available.

2008-07-03 13:22:36	Chrome.Login	TRACE	Sync login window closed
2008-07-03 13:22:36	Async.Generator	ERROR	Exception: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [IWeaveCrypto.generateRandomKey] (JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/crypto.js :: Crypto_randomKeyGen :: line 176)
2008-07-03 13:22:36	Async.Generator	DEBUG	Stack trace:
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/crypto.js :: Crypto_randomKeyGen :: line 176
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: AsyncGen_run :: line 227
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: Async_run :: line 346
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: Async_sugar :: line 366
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/crypto.js :: Crypto_isPassphraseValid :: line 248
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: AsyncGen_run :: line 227
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: Async_run :: line 346
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: Async_sugar :: line 366
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/service.js :: WeaveSvc__getKeypair :: line 436
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: AsyncGen__cont :: line 240
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: anonymous :: line 140
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: AsyncGen__done :: line 293
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/async.js :: anonymous :: line 268
JS frame :: file:///C:/Documents%20and%20Settings/user/Application%20Data/Mozilla/Firefox/Profiles/ikby6kcp.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/util.js :: EL_notify :: line 439
This exception was raised by an asynchronous coroutine.
Initial async stack trace:
unknown (async) :: WeaveNotifyWrapper-85
module:wrap.js:170 :: WeaveLocalLockWrapper
module:service.js:581 :: WeaveSvc_login
chrome://weave/content/login.js:92 :: Login_doOK
chrome://global/content/bindings/dialog.xml:357 :: anonymous
chrome://global/content/bindings/dialog.xml:358 :: _fireButtonEvent
chrome://global/content/bindings/dialog.xml:332 :: _doButtonCommand
chrome://global/content/bindings/dialog.xml:321 :: _handleButtonCommand

I've tried the account on a different PC, it worked there so I assume one of the extensions on this system is conflicting with it (already checked the FAQ, I'm not using that 'undo close tab button' one). Short of disabling them all and reenabling them one by one, how could I find the culprit?

Reproducible: Always

Steps to Reproduce:
1. Attempt to sign into Weave

Actual Results:  
Login window pops up again after a while, then, after clicking OK, Weave is not signed in.

Expected Results:  
Weave should sign in.

Comment 1

[joe t.]

Having same problem, slightly different Verbose Log error:

2008-07-13 00:50:08	Async.Generator	ERROR	Exception: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [IWeaveCrypto.generateRandomKey] (JS frame :: file:///C:/Documents%20and%20Settings/joe/Application%20Data/Mozilla/Firefox/Profiles/rc86mmsv.Joe/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/crypto.js :: Crypto_randomKeyGen :: line 176)
2008-07-13 00:50:08	Async.Generator	DEBUG	Async stack trace:
unknown (async) :: WeaveNotifyWrapper-31 (last self.cb generated at module:wrap.js:92 :: WeaveNotifyWrapper)
module:wrap.js:170 :: WeaveLocalLockWrapper
module:async.js:232 :: AsyncGen_run
module:async.js:331 :: Async_run
module:async.js:351 :: Async_sugar
module:service.js:541 :: WeaveSvc_verifyPassphrase
chrome://weave/content/wizard.js:432 :: SyncWizard_verifyPassphrase
chrome://weave/content/wizard.xul:1 :: oncommand
<file:unknown>
chrome://global/content/bindings/textbox.xml:316 :: _fireCommand

Comment 2

[joe t.]

i don't know if it is the answer or not, but i've managed to correct my problem with this error.
See posts in the Weave Forum:
https://labs.mozilla.com/forum/index.php/topic,1239.msg5068.html#msg5068

Comment 3

[Dan Mills [:thunder]]

Thanks joe - Just so it's in the bug, the problem is that WeaveCrypto doesn't support FIPS mode.

Comment 4

[Michel Meyers]

I just wanted to confirm that disabling FIPS mode works around the issue here. Thanks Joe.

Comment 9

[Justin Dolske [:Dolske]]

Hmm, thought I had previously commented on this, guess not:

The basic failure here, IIRC, is that FIPS mode doesn't allow extracting unwrapped keys from the token. [It might even be failing in the keygen if we're trying to create an key with flags to allow extracting.]

The workaround, if we want to implement it, would be to generate an ephemeral wrapping key, and use it to wrap the key we generate. We'd then turn around and roll our own unwrap by decrypting it (as if it was just raw data -- same bits, different semantics). It's then probably going to fail later on, because I think it won't allow importing a key unless it's wrapped. So we'd have to do the reverse process to import the raw key data.

Lots of busy work for no gain. Better to just not support FIPS mode, IMO. It's been nothing but trouble.

Comment 10

[Dan Mills [:thunder]]

(In reply to comment #9)
> Hmm, thought I had previously commented on this, guess not:
> 
> The basic failure here, IIRC, is that FIPS mode doesn't allow extracting
> unwrapped keys from the token. [It might even be failing in the keygen if we're
> trying to create an key with flags to allow extracting.]
> 
> The workaround, if we want to implement it,...

That workaround would make it work when FIPS mode is enabled, but I don't think it would actually make it FIPS compliant anyway.  So-

> Lots of busy work for no gain. Better to just not support FIPS mode, IMO. It's
> been nothing but trouble.

Agreed, unless we actually change things to make it FIPS compliant (not just with NSS' FIPS mode, but something that could potentially be certified to some level of FIPS).  

I suspect we would need to make some larger changes for that, though I'm not sure what they are, or whether they would actually be worth it.

Note that we have always thought that it would be nice if the keys could be stored locally/in a flash drive, instead of on the server.  Making that work with NSS in a way that it supports might implicitly solve this bug.

Comment 11

[Ed Lee :Mardak]

With ctypes WeaveCrypto.js, this seems to manifest in a couple ways:

PK11_ExtractKeyValue failed
http://hg.mozilla.org/services/fx-sync/file/69bce31f39b2/services/crypto/WeaveCrypto.js#l682

and

(NS_ERROR_FAILURE) [nsIKeyObjectFactory.keyFromString]
http://hg.mozilla.org/services/fx-sync/file/69bce31f39b2/services/sync/modules/base_records/crypto.js#l157

Comment 13

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

When we generate the Sync Key (the user doesn't type in their own), we should not use the PBKDF2 function on the sync key. I am going to research this more thoroughly today, but it seems like the PBKDF2 function may actually weaken the key if they key is already strong.

If we do that, then AFAICT it will at least be possible to have a FIPS-compliant Sync. But, if we use PBKDF2 then we will never be FIPS compliant unless/until PBKDF2 becomes an approved algorithm and unless/until NSS's PBKDF2 implementation is validated.

My recommendation is to automatically disable Sync in FIPS mode until we have verified that it is FIPS compliant. Or, if we can't do that, then we should add instructions for disabling Sync to the documentation for Firefox FIPS mode.

Comment 14

[Philipp von Weitershausen [:philikon]]

If we can't use PBKDF2, what other algorithm do you suggest to derive a symmetric key from the passphrase? Note that we're considering simplifying the crypto setup greatly, leaving just the symkey-from-passphrase bit (which would be the one key that everything gets encrypted with.)

Comment 15

[Mike Connor [:mconnor]]

This is a long way down our priority list, but if there's a good link for how PBKDF2 would weaken the key, I'd love to read up on my flight.

Comment 16

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to comment #14)
> If we can't use PBKDF2, what other algorithm do you suggest to derive a
> symmetric key from the passphrase?

There is no FIPS-approved way of doing that. There are no provisions for weak keys in FIPS 140-2. In FIPS mode all the cryptography is strong and all the keys are strong from the start. There seems to be some effort by NIST to start the process of getting PBKDF2 approved but (a) there seems to be opposition to that from some cryptographers on the grounds that PKDBF2 is not very useful, and (b) that is at least a year or more away if it ever happens, (c) there would be a delay after that before NSS's PBKDF2 implementation gets validated.

(In reply to comment #15)
> This is a long way down our priority list, but if there's a good link for how
> PBKDF2 would weaken the key, I'd love to read up on my flight.

The one paper I remember reading is "Practical consequences of the aberration of narrow-pipe hash designs from ideal random functions" but for some reason it only mentions PBKDF1, not PBKDF2. I will contact the author to see why he didn't include an analysis for PBKDF2.

Comment 17

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

A FIPS-140-allowed use of PBKDF2 would have to adhere to the final version of NIST Draft SP800-132 [1], which currently requires at least 100,000 iterations *minimum* and recommends 1,000,000 iterations. Currently Sync is using 4,096 iterations. (Apple iOS 4.0 uses 10,000 and 3.0 used 2,000.)

[1] http://csrc.nist.gov/publications/drafts/800-132/draft-sp800-132_june2010.pdf

Comment 18

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

The iOS does not have a PBKDF2 function in its crypto library. We have a simple implementation in Home. So we are in control of the number of iterations.

Currently Home runs PBKDF2() every time it uses the private key. This can obviously be cached once during startup and put into the keychain.

Comment 19

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

A clarification on my earlier comments:

The FIPS mode is a necessary conditional for FIPS compliance but it is not sufficient. It is possible (and, IMO, desirable) for Sync to work in FIPS mode even if such a configuration is not good enough for the actual government use. For FIPS compliance, the system administrator will have to disable or remove Sync in addition to enabling FIPS mode.

So, this bug should really just be about getting Sync to work in FIPS mode, not about making Sync FIPS compliant. I am not sure that Sync in FIPS mode is possible but it is much more likely to be possible than FIPS compliance is.

Comment 21

[Adrian Kalla [:adriank]]

It took me weeks to figure out why Sync didn't work for me. Couldn't there just be a simple error message shown to the user: 
"Sync currently doesn't work with FIPS enabled. Please turn off FIPS in order to use Sync."
until this bug isn't fixed? It would save some people a lot of time...

Comment 22

[Mike Connor [:mconnor]]

That'd be bug 578136, which is a dep here.

Comment 24

[Richard Newman [:rnewman]]

Improving the subject line for searchability.

We really should throw a nice error here, if we can detect FIPS mode.

Comment 25

[Mike Connor [:mconnor]]

bug 578136 is the bug you're looking for for that.

Comment 27

[Christian Kujau]

FWIW, this has been explained quite well here:
https://groups.google.com/group/mozilla-labs-weave/msg/920ce857e55e914d

Q: Can't we have some kind of a toggle when enabling FIPS mode, still allowing
   "Sync"? Like, "Yes, please activate FIPS and only allow FIPS-compliant
   operations, EXCEPT for Sync - I'm going to use that."?

Of course, we might need such an exception for any other future crypto-related feature, which may not be able to be FIPS certified.

Comment 28

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

The Softoken FIPS mode doesn't prevent the use of non-certified algorithms. For the most part, all it does is prevent the extraction of key material from Softoken. Sync extracts the key material from Softoken because we were in a hurry when implementing this part and didn't want to change the existing JS code, which was dependent on having access to the key material. AFAICT, Sync could easily be changed to work even when Softoken is in FIPS mode. I specifically designed the key confirmation protocol with this change in mind. If/when it becomes a priority, I would love to help make such a change.

> "Yes, please activate FIPS and only allow FIPS-compliant
> operations, EXCEPT for Sync - I'm going to use that."?

The non-FIPS-mode-capable operation is "give me the key material for a key" and Softoken doesn't have any context to know if it is going to be used by Sync or something else. It would be easier to make the above change than it would be to change Softoken to carry this contextual information.

Comment 32

[Allison Naaktgeboren :ally]

sync triage: this wont happen until the re-write, correct?

Comment 34

[Mark Hammond [:markh] [:mhammond]]

FWIW, FxA telemetry tells us this is still a problem in the wild for both Ubuntu and Win7 (and possibly others - the raw volume is low)

Comment 36

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 13 duplicates.
:skhamis, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 37

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.