Record layer version number is ignored.

[richmoore]

s2n appears to be perfectly willing to accept version numbers such as 0x0000 in the record layer, and also artificially high ones such as 0x400 or 0xffff. This will lead to interoperability problems with future TLS versions.

[colmmacc]

Thanks for the note - can you provide some more detail? I think the principle we are following here is "be liberal in what you accept" ... which should promote interoperability. I believe in general the version number in the record layer isn't particularly meaningful and I've seen proposals to "fix" it to 0x03 0x01 .. for the benefit of some implementations that were overly restrictive.

What behavior would you prefer to see?

[raycoll]

Looks like we're honoring the version number now. https://github.com/awslabs/s2n/blob/master/tls/s2n_record_read.c#L75 ?

[alexw91]

Looks like we also had a bug in the Record Layer Version number we were writing in the Client Hello: #418.

This issue is the reason that the client side bug went undetected for so long, our server code was liberal in what it accepted, so we didn't notice that we introduced a bug in the client side.

[alexw91]

Specifically, the RFC states:

Earlier versions of the TLS specification were not fully clear on what the record layer version number (TLSPlaintext.version) should contain when sending ClientHello (i.e., before it is known which version of the protocol will be employed). Thus, TLS servers compliant with this specification MUST accept any value {03,XX} as the record layer version number for ClientHello.

TLS clients that wish to negotiate with older servers MAY send any value {03,XX} as the record layer version number. Typical values would be {03,00}, the lowest version number supported by the client, and the value of ClientHello.client_version. No single value will guarantee interoperability with all old servers, but this is a complex topic beyond the scope of this document.

So this looks like s2n is following the RFC by accepting any Record Layer version up until conn->actual_protocol_version_established is set. Once it is set, we enforce that the Record Layer Protocol Version matches the Actual Version Established.