1516593 - Client doesn't generate new Random value when renegotiating

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Eric Rescorla (:ekr)]

See: https://searchfox.org/nss/source/lib/ssl/ssl3con.c#4951

    /* Generate a new random if this is the first attempt. */
    if (type == client_hello_initial) {
        rv = ssl3_GetNewRandom(ss->ssl3.hs.client_random);
        if (rv != SECSuccess) {
            goto loser; /* err set by GetNewRandom. */
        }
    }

However, if you do renegotiation, then:
https://searchfox.org/nss/source/lib/ssl/ssl3con.c#5222
https://searchfox.org/nss/source/lib/ssl/ssl3con.c#13027

Then you pass in |client_hello_renegotiation|

Introduced in https://hg.mozilla.org/projects/nss/rev/d08b88ced5a9

I haven't given this a complete analysis, but I don't think it's a major problem:

1. It only happens on renegotiation, which means you have authenticated the server already and we do insist that the server remain the same.
2. If the server doesn't replay its random, you will get fresh keys.
3. If you do client auth, then you do a fresh handshake and so you get freshness from the CKE, and the CV covers this, so you don't get a replayable signature (and anyway, it's the server's random that does most of the anti-replay).
4. This does allow replay of the SKE, but it can't be moved across origins.

I would be interested in other people's thoughts, especially MT.

The fix is to just add |client_hello_renegotiation| to the test here.

Comment 1

[Eric Rescorla (:ekr)]

JC, do you feel comfortable taking a crack at fixing this and writing a test for it?

Comment 2

[Martin Thomson [:mt:]]

The analysis here looks right. In a way, this is like post-handshake authentication in 1.3. If we're confident that the original handshake was ok then this reduces to two things: fresh key exchange, which should be ok because the server provides fresh entropy; and, fresh authentication, which should be fine on the basis of it being like 1.3.

Comment 3

[J.C. Jones [:jcj] (he/him)]

I can take it. I won't be able to seriously pick it up until I'm back on 4 January though.

Comment 4

[Andrew McCreight [:mccr8]]

J.C., what sort of security rating should this get? Thanks.

Comment 5

[J.C. Jones [:jcj] (he/him)]

The way I read it, this is a sec-low. Exploiting it requires the server we are using to make a reciprocal mistake, and shouldn't be triggerable except by said connected server.

Comment 6

[Kevin Jacobs [:kjacobs]]

Attachment: Bug 1516593 - Client to generate new random during renegotiation. r?mt

This patch adds client_hello_renegotiation to the list of //sslClientHelloTypes// for which a new client_random is generated.

It also adds renegotiation tests to confirm that new client and server (for completeness) randoms are generated during renegotiation.

Comment 7

[J.C. Jones [:jcj] (he/him)]

Per team meeting, removing from upcoming point releases to lower the point-releases' risk.

Comment 8

[J.C. Jones [:jcj] (he/him)]

https://hg.mozilla.org/projects/nss/rev/54d7aee231cc83431a415666c75bd16d54a74c34