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

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Kai Engert (:KaiE:)]

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.

Comment 1

[Wan-Teh Chang]

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?

Comment 2

[Robert Relyea]

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

Comment 3

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

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.

Comment 4

[Marco Castelluccio [:marco]]

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