Open Bug 1021184 Opened 10 years ago Updated 2 years ago

Add safety net, make sure we'll never derive from any empty key material

Categories

(NSS :: Libraries, defect, P2)

3.16.2

Tracking

(Not tracked)

People

(Reporter: KaiE, Unassigned)

Details

This bug is inspired by the recent issue identified in OpenSSL (we believe NSS is safe). In OpenSSL, an early, untimely ChangeCipherSpec (CCS) message could trick OpenSSL into using an empty pre-master-secret (PMS). We believe that NSS isn't vulnerable to that attack, because of the careful checks of the SSL/TLS states being implemented, which should cause a failure on an untimely CCS message. Furthermore, because NSS uses structures, not buffers, for the PMS, we should detect that it's uninitialized and shouldn't arrive in the PKCS#11 layer (from Bob). Nevertheless, it might be good to have safety checks at all layers. In function NSC_DeriveKey (pkcs11c.c), there aren't size checks if the mechanism is related to DH (see variable isDH). This is a suggestion to check for reasonable PMS sizes in all scenarios.
Target Milestone: --- → 3.16.2
Target Milestone: 3.16.2 → 3.16.3
There is a key difference between the ways NSS and OpenSSL handle ChangeCipherSpec: NSS has a wait state for ChangeCipherSpec (wait_change_cipher of the SSL3WaitState enum type), whereas OpenSSL doesn't. The reason OpenSSL doesn't have a wait state for ChangeCipherSpec is that OpenSSL only has wait states for handshake messages but ChangeCipherSpec is not a handshake message. (In the ssl3_HandleChangeCipherSpecs function, if the wait state is not wait_change_cipher, NSS won't process the ChangeCipherSpec.) Because of this difference, NSS is unlikely to suffer from the same bug. (In fact, the OpenSSL fix is to add an approximation of a wait state for ChangeCipherSpec, called SSL3_FLAGS_CCS_OK.) Kai, it seems that you and Bob have already inspected the libSSL code and are satisfied with it, so we just need to add key size checks to NSC_DeriveKey? Would you like to take a stab at it, or should I do this?
Assignee: nobody → wtc
Status: NEW → ASSIGNED
Priority: -- → P2
We do have a check for normal derive (if you are using RSA), but if you are using a DH algorithm, we don't know what the actually keys size should be. We could force a minimum key size for DH algorithms, though. That's probably the best way to handle it. I was worried that for DHE ciphers, someone could change the public key to be p or p*k, which would yield a zero key for Diffie Helman. It's not a big worry because The client will choke at the finish step since the attacker can only force the server keys and not the client keys (the attacker would have to replace the server's public key to the client, which it can't because the server signed it). This is really a special case of the small subgroup attack, so it's not a serious threat, but it would be a simple fix to prevent. bob
Target Milestone: 3.16.3 → 3.17

The bug assignee didn't login in Bugzilla in the last months and this bug has priority 'P2'.
:beurdouche, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: wtc → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(bbeurdouche)
Severity: normal → S3

We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.

Flags: needinfo?(bbeurdouche)
You need to log in before you can comment on or make changes to this bug.