Re: [TLS] (strict) decoding of legacy_record_version?

[Eric Rescorla <[e-mail]>]

On Mon, Nov 7, 2016 at 4:04 PM, David Benjamin <[e-mail]>
wrote:

> On Mon, Nov 7, 2016 at 6:50 PM Benjamin Kaduk <[e-mail]> wrote:
>
> (was Re: [TLS] PR#625: Change alert requirements)
>
> Digging up an old sub-thread...
>
> On 09/20/2016 08:03 AM, Eric Rescorla wrote:
>
> in Record Layer there's the following text:
>
>     legacy_record_version : This value MUST be set to { 3, 1 } for all
>     records. This field is deprecated and MUST be ignored for all purposes.
>
> in Record Layer Protection there's the following text:
>
>     legacy_record_version : The legacy_record_version field is identical to
>     TLSPlaintext.legacy_record_version and is always { 3, 1 }. Note that
> the
>     handshake protocol including the ClientHello and ServerHello messages
>     authenticates the protocol version, so this value is redundant.
>
> which doesn't say if the version can be ignored completely (skipped while
> parsing) or if it should be verified.
>
>
> These are different fields.
>
>
> There's still the question of whether the receiver should enforce 0x0301
> in either/both cases.
> OpenSSL is implementing and seems to be reading the spec that it MUST be
> ignored (even though I guess strictly speaking that MUST only applies
> before record protection is engaged); if I'm doing my code survey
> correctly, Mint and NSS always enforce, and Boring only checks the first
> octet.
>
> Is there a reason to not do strict enforcement?
>
>
> Before you know the version, it is not safe to enforce. Prior versions of
> TLS did not pin down the value. (BoringSSL checks the first octet because
> OpenSSL did so, and it's vaguely nice to sanity-check on the first few
> bytes of data in the protocol.)
>
> Immediately after you know the version, enforcement is also problematic.
> Alerts sent in response to a ClientHello or ServerHello have somewhat
> ambiguous versions. One side may have decided the version is set while the
> other has not. Prior to 1.3, the record version needs to be set to the
> protocol version (or you break pre-1.3 enforcing implementations). But this
> means a receiver which enforces will fail to parse the alert. Not the end
> of the world, but weird.
>
> Once you've gotten as far as to switch to TLSCiphertext, I don't see a
> reason not to enforce. Keying on versions is problematic (which is why we
> avoided a transition to enforcement), but keying on whether the record is
> encrypted seems fine. I think it just didn't occur to us to base it on
> that. :-)
>

As you say, this is what NSS does. There's no legit reason for a TLS 1.3
encrypted record to have the wrong version number here, so I suggest
changing the text.

-Ekr


> David
>