1515385 - session resumption with invalid ClientHello is not rejected

Status: RESOLVED WONTFIX

(NSS :: Libraries, defect)

Description

[Hubert Kario]

When the client tries to resume a session and does not provide in new ClientHello the cipher that is associated with the session, the NSS server proceeds to a new handshake.

RFC 5246 states
   cipher_suites
      This is a list of the cryptographic options supported by the
      client, with the client's first preference first.  If the
      session_id field is not empty (implying a session resumption
      request), this vector MUST include at least the cipher_suite from
      that session.

In other words, sending a session ID without cipher_suite creates a self-inconsistent Client Hello, and as such it should not be accepted by server.

Test case for it is available in https://github.com/tomato42/tlsfuzzer/pull/487, test-resumption-with-wrong-ciphers.py

it needs to be run as
PYTHONPATH=. python scripts/test-resumption-with-wrong-ciphers.py --swap-ciphers

reproduced against hg default branch at b216206312d2

Comment 1

[Eric Rescorla (:ekr)]

This text seems to require the client to send a certain kind of CH. Why do you think that this text requires the server to generate an error rather than to reject resumption? Is there some other text to that effect?

Comment 2

[Hubert Kario]

> Why do you think that this text requires the server to generate an error rather than to reject resumption?

because such Client Hello is malformed, as the quoted text states

> Is there some other text to that effect?

the one right below, for compression_methods:

   compression_methods
      This is a list of the compression methods supported by the client,
      sorted by client preference.  If the session_id field is not empty
      (implying a session resumption request), it MUST include the
      compression_method from that session.  This vector MUST contain,
      and all implementations MUST support, CompressionMethod.null.
      Thus, a client and server will always be able to agree on a
      compression method.

where NSS already aborts with illegal_parameter if CompressionMethod.null is omitted.

and the RFC says that only the session ID and data in session itself should be used to decide if any given session should be resumed or not

Comment 3

[Eric Rescorla (:ekr)]

(In reply to Hubert Kario from comment #2)
> > Why do you think that this text requires the server to generate an error rather than to reject resumption?
> 
> because such Client Hello is malformed, as the quoted text states

Absent a requirement that the server reject, that's not dispositive.


> > Is there some other text to that effect?
> 
> the one right below, for compression_methods:
> 
>    compression_methods
>       This is a list of the compression methods supported by the client,
>       sorted by client preference.  If the session_id field is not empty
>       (implying a session resumption request), it MUST include the
>       compression_method from that session.  This vector MUST contain,
>       and all implementations MUST support, CompressionMethod.null.
>       Thus, a client and server will always be able to agree on a
>       compression method.
> 
> where NSS already aborts with illegal_parameter if CompressionMethod.null is
> omitted.

I'm not sure why you think this is dispositive either. NSS doesn't support compression and so failure to provide null represents a negotiation failure, so this is a different situation.


> and the RFC says that only the session ID and data in session itself should
> be used to decide if any given session should be resumed or not

I'm not sure what text you are referring to, but that's not my reading of S 7.3.

Comment 4

[Hubert Kario]

> Absent a requirement that the server reject, that's not dispositive.

which part of the RFC states that the server should accept malformed messages?!

> NSS doesn't support compression and so failure to provide null represents a negotiation failure, so this is a different situation.

so, when NSS doesn't find a ciphersuite in ClientHello that it supports, you're saying it should send an illegal_parameter to client?

> I'm not sure what text you are referring to, but that's not my reading of S 7.3.

the first listing in section 7.3 is ambiguous, but later text states:

   The client sends a ClientHello using the Session ID of the session to
   be resumed.  The server then checks its session cache for a match.
   If a match is found, and the server is willing to re-establish the
   connection under the specified session state,

   If a Session ID match is not
   found, the server generates a new session ID, and the TLS client and
   server perform a full handshake.

Later in Section 7.4.1.3:

      If the ClientHello.session_id was non-empty, the
      server will look in its session cache for a match.

so in all three cases, the match is based on session_id only, not on other parameters advertised by the client

Comment 5

[Eric Rescorla (:ekr)]

(In reply to Hubert Kario from comment #4)
> > Absent a requirement that the server reject, that's not dispositive.
> 
> which part of the RFC states that the server should accept malformed
> messages?!

That's not the way IETF specifications work. Absent some specific RFC 2119/8174 mandate, implementations are free to behave any way they please.



> > NSS doesn't support compression and so failure to provide null represents a negotiation failure, so this is a different situation.
> 
> so, when NSS doesn't find a ciphersuite in ClientHello that it supports,
> you're saying it should send an illegal_parameter to client?

No. It should send "handshake_failure". And it could do so for failure to negotiate compression as well, but because the ClientHello is nonconformant, it is also permitted to send "illegal_parameter".


> > I'm not sure what text you are referring to, but that's not my reading of S 7.3.
> 
> the first listing in section 7.3 is ambiguous, but later text states:
> 
>    The client sends a ClientHello using the Session ID of the session to
>    be resumed.  The server then checks its session cache for a match.
>    If a match is found, and the server is willing to re-establish the
>    connection under the specified session state,

The key text here is "willing to re-establish the connection". I think this sensibly covers this case.



>    If a Session ID match is not
>    found, the server generates a new session ID, and the TLS client and
>    server perform a full handshake.
> 
> Later in Section 7.4.1.3:
> 
>       If the ClientHello.session_id was non-empty, the
>       server will look in its session cache for a match.
> 
> so in all three cases, the match is based on session_id only, not on other
> parameters advertised by the client

That's not how I read this. Yes, you look-up by session_id, but then you decide if you are willing to resume, and that could not happen for any number of reasons.



MT, I think our current implementation is reasonable, and we should mark this WONTFIX/INVALID. Any disagreement?

Comment 6

[Martin Thomson [:mt:]]

Yes, I concur.  Rather than getting into lawyering of the text, I think that we probably want to examine reasons.  There was perhaps the notion that it was desirable to use the same cipher suite on resumption.  However, if you look at this from the lens of how this works in TLS 1.3, that's just a convenient fiction.  There, the handshake is completely new, just with certain properties that are now based on a pre-shared key.  Those properties don't need to include the cipher suite.  As long as the cipher suite is acceptable to both parties, resuming with any cipher suite is OK.

The one potentially valid argument in support of this is that resumption in TLS 1.3 does require that the hash/KDF remain the same.  TLS 1.3 is strict about this.  This is probably something we get wrong in TLS 1.2, but I think that we accept that shortcoming.  TLS 1.2 is not as idealized as 1.3 in many little ways like this.  As far as I know, there are no known problems here, just a complication in analysis that no one was willing to invest effort into.

Finally, I don't know if making the suggested change would be good for compatibility.  Especially for browsers.  If an NSS client decided to reject a handshake that both accepted resumption and changed the cipher suite, we would have to try again without the session.  That's certainly possible, but unpleasant, and not every user of NSS has an existing framework for retrying connections, so others would be affected more than us.  I'm sure we'd learn something in the process, but it's an expensive exercise.

I'm going with WONTFIX here.