1513586 - ServerHello.random does not include downgrade sentinel when negotiating TLS 1.1 or earlier

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Alicja Kario]

Using 66cae6a8f4b6

The new test script from tlsfuzzer to verify if server sets the downgrade sentinels in ServerHello fails for cases with TLS 1.0 and TLS 1.1.


$ PYTHONPATH=. python scripts/test-downgrade-protection.py --server-max-protocol=TLSv1.3
sanity ...
OK

TLS 1.3 downgrade check for Protocol (3, 1) ...
Error encountered while processing node <tlsfuzzer.expect.ExpectServerHello object at 0x7f78588b1290> (child: <tlsfuzzer.expect.ExpectCertificate object at 0x7f78588b12d0>) with last message being: <tlslite.messages.Message object at 0x7f78588bab50>
Error while processing
Traceback (most recent call last):
  File "scripts/test-downgrade-protection.py", line 212, in main
    runner.run()
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/runner.py", line 219, in run
    node.process(self.state, msg)
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/expect.py", line 508, in process
    self._check_downgrade_protection(srv_hello)
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/expect.py", line 599, in _check_downgrade_protection
    "Server failed to set downgrade protection sentinel in "
AssertionError: Server failed to set downgrade protection sentinel in ServerHello.random value

TLS 1.3 downgrade check for Protocol (3, 3) ...
OK

TLS 1.3 downgrade check for Protocol (3, 2) ...
Error encountered while processing node <tlsfuzzer.expect.ExpectServerHello object at 0x7f78588b1690> (child: <tlsfuzzer.expect.ExpectCertificate object at 0x7f78588b16d0>) with last message being: <tlslite.messages.Message object at 0x7f7858882c10>
Error while processing
Traceback (most recent call last):
  File "scripts/test-downgrade-protection.py", line 212, in main
    runner.run()
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/runner.py", line 219, in run
    node.process(self.state, msg)
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/expect.py", line 508, in process
    self._check_downgrade_protection(srv_hello)
  File "/home/hkario/dev/tlsfuzzer/tlsfuzzer/expect.py", line 599, in _check_downgrade_protection
    "Server failed to set downgrade protection sentinel in "
AssertionError: Server failed to set downgrade protection sentinel in ServerHello.random value

sanity ...
OK

Check if server correctly return ServerHello Random 
with downgrade protection values to TLS1.2 and below 
clients

version: 1

Test end
successful: 3
failed: 2
  'TLS 1.3 downgrade check for Protocol (3, 1)'
  'TLS 1.3 downgrade check for Protocol (3, 2)'

this is contrary to requirement from https://tools.ietf.org/html/rfc8446#section-4.1.3:

   If negotiating TLS 1.1 or below, TLS 1.3 servers MUST, and TLS 1.2
   servers SHOULD, set the last 8 bytes of their ServerHello.Random
   value to the bytes:

Comment 1

[Alicja Kario]

one more point: it looks like when the server is set to TLS 1.2 as the highest version supported, it does set the downgrade sentinels for TLS 1.1 and TLS 1.0 connections

Comment 2

[Martin Thomson [:mt:]]

Comment 1 makes this sound like a non-issue.

Help me understand what is going on here. Under which conditions does the server fail to produce the downgrade sentinel?

Comment 3

[Alicja Kario]

Situation A:

• Server: TLS 1.0, 1.1, 1.2 and 1.3 are enabled. (-V tls1.0:tls1.3)
• Client is advertising 1.2 as the highest supported version, result: TLS 1.2 is negotiated and the ServerHello.random includes downgrade sentinel (expected)
• Client is advertising 1.1 as the highest supported version, result: TLS 1.1 is negotiated and the ServerHello.random DOES NOT include downgrade sentinel (unexpected)
• Client is advertising 1.0 as the highest supported version, result: TLS 1.0 is negotiated and the ServerHello.random DOES NOT include downgrade sentinel (unexpected)

Situation B:

• Server: TLS 1.0, 1.1 and 1.2 are enabled (-V tls1.0:tls1.2)
• Client is advertising 1.2 as the highest supported version, result: TLS 1.2 is negotiated and the ServerHello.random does not include downgrade sentinel (expected)
• Client is advertising 1.1 as the highest supported version, result: TLS 1.1 is negotiated and the ServerHello.random does include downgrade sentinel (expected)
• same as above for 1.0

Comment 4

[Eric Rescorla (:ekr)]

OK, I see what's going on.

    switch (ss->vrange.max) {
        case SSL_LIBRARY_VERSION_TLS_1_3:
            PORT_Memcpy(downgradeSentinel,
                        tls13_downgrade_random, sizeof(tls13_downgrade_random));
            break;
        case SSL_LIBRARY_VERSION_TLS_1_2:
            PORT_Memcpy(downgradeSentinel,
                        tls12_downgrade_random, sizeof(tls12_downgrade_random));
            break;
        default:
            /* Do not change random. */
            break;
    }

We're setting the sentinel based on the max version, not on the version we negotiate, because the algorithm in the spec is kind of counterintuitive. So we are setting the sentinel

Setting this to security because it actually would stop downgrade from the client from 1.2 -> 1.1 if the server was 1.2. Note that TLS 1.3 clients would in fact do downgrade protection correctly because they are required to check for both values.

Comment 5

[Alicja Kario]

Martin, I did link this bug (along with the description of issue) from https://github.com/tomato42/tlsfuzzer/wiki/Found-bugs

would you prefer for me to remove it for now, and keep it secret until this bug is private, or should we not put too much of a spotlight on the issue by removing it from the listing?

Comment 6

[Martin Thomson [:mt:]]

I will request that you remove this from the bugs list there until we fix this. Obviously, the information is out there now, but we can keep from broadcasting it for the moment.

Comment 7

[Alicja Kario]

no problem, done

Comment 8

[Alicja Kario]

ping? 3.44 shipped, but this is still unfixed

Comment 9

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

Sorry, this hasn't been on my radar. I'm setting this so we re-triage it at the start of the next cycle.

Comment 10

[Kevin Jacobs [:kjacobs]]

Attachment: Bug 1513586 - Set downgrade sentinel for client TLS versions lower than 1.2.

Per-[[ https://tools.ietf.org/html/rfc8446#section-4.1.3 | RFC 8446 ]], the downgrade sentinel must be set by a TLS 1.3 server (and should be set by a TLS 1.2 server) that negotiates TLS 1.0 or 1.1. This patch corrects the behavior and adds a test.

Comment 11

[Kevin Jacobs [:kjacobs]]

https://hg.mozilla.org/projects/nss/rev/993717228da05c6bf468ac861f01b6849a94eae3