1320093 - Segfault when server negotiates TLS 1.2 with a 0-rtt handshake

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

I don't think this is exploitable, so assigning sec-low. What happens is that we segfault when we have a 0-rtt ticket in the cache and connect to that server again, but this the server negotiates TLS 1.2.

../../lib/ssl/ssl3con.c:1582:26: runtime error: member access within null pointer of type 'const ssl3MACDef' (aka 'const struct ssl3MACDefStr')
    #0 0x597fc8 in ssl3_SetupPendingCipherSpec /home/worker/nss/out/Debug/../../lib/ssl/ssl3con.c:1582:26
    #1 0x5e68b2 in ssl3_HandleServerHelloPart2 /home/worker/nss/out/Debug/../../lib/ssl/ssl3con.c:6855:10
    #2 0x5cb1dc in ssl3_HandleServerHello /home/worker/nss/out/Debug/../../lib/ssl/ssl3con.c:6812:14
    #3 0x5c6a97 in ssl3_HandleHandshakeMessage /home/worker/nss/out/Debug/../../lib/ssl/ssl3con.c:11789:18
    #4 0x5d1a23 in ssl3_HandleHandshake /home/worker/nss/out/Debug/../../lib/ssl/ssl3con.c:11981:18
    #5 0x5cd757 in ssl3_HandleRecord /home/worker/nss/out/Debug/../../lib/ssl/ssl3con.c:12749:22
    #6 0x616cda in ssl3_GatherCompleteHandshake /home/worker/nss/out/Debug/../../lib/ssl/ssl3gthr.c:407:18
    #7 0x620f48 in ssl_GatherRecord1stHandshake /home/worker/nss/out/Debug/../../lib/ssl/sslcon.c:78:10
    #8 0x52a297 in ssl_Do1stHandshake /home/worker/nss/out/Debug/../../lib/ssl/sslsecur.c:65:14
    #9 0x52e862 in SSL_ForceHandshake /home/worker/nss/out/Debug/../../lib/ssl/sslsecur.c:414:14
    #10 0x4fd97e in client_fuzzing_target /home/worker/nss/out/Debug/../../fuzz/client_target.cc:348:7
    #11 0x6452db in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/worker/nss/out/Debug/../../fuzz/libFuzzer/FuzzerLoop.cpp:515:13
    #12 0x645487 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long) /home/worker/nss/out/Debug/../../fuzz/libFuzzer/FuzzerLoop.cpp:469:3
    #13 0x644f30 in fuzzer::Fuzzer::RunOne(std::vector<unsigned char, std::allocator<unsigned char> > const&) /home/worker/nss/out/Debug/../../fuzz/libFuzzer/FuzzerInternal.h:113:41
    #14 0x645ba8 in fuzzer::Fuzzer::FindExtraUnits(std::vector<std::vector<unsigned char, std::allocator<unsigned char> >, std::allocator<std::vector<unsigned char, std::allocator<unsigned char> > > > const&, std::vector<std::vector<unsigned char, std::allocator<unsigned char> >, std::allocator<std::vector<unsigned char, std::allocator<unsigned char> > > > const&) /home/worker/nss/out/Debug/../../fuzz/libFuzzer/FuzzerLoop.cpp:591:11
    #15 0x646009 in fuzzer::Fuzzer::Merge(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) /home/worker/nss/out/Debug/../../fuzz/libFuzzer/FuzzerLoop.cpp:626:15
    #16 0x63a1ab in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/worker/nss/out/Debug/../../fuzz/libFuzzer/FuzzerDriver.cpp:498:8
    #17 0x51c048 in main /home/worker/nss/out/Debug/../../fuzz/nssfuzz.cc:147:10
    #18 0x7f63b7f2682f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #19 0x42f3c8 in _start (/home/worker/dist/Debug/bin/nssfuzz+0x42f3c8)

SUMMARY: AddressSanitizer: undefined-behavior ../../lib/ssl/ssl3con.c:1582:26 in

Comment 1

[Eric Rescorla (:ekr)]

Oh, nice catch. I assume that the server is *accepting* the ticket but TLS 1.2?

That should eventually fail in the sid version match

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

The client segfaults because tls13_MaybeDo0RTTHandshake() calls tls13_SetCipherSpec(TrafficKeyEarlyApplicationData, CipherSpecWrite). That installs a cipher spec that the non-1.3 code can't handle (I think) when downgrading.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Here's a test that reproduces the issue:

> TEST_F(TlsConnectTest, FooBar) {
>   ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
>   ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
>   server_->Set0RttEnabled(true); // set ticket_allow_early_data
>   Connect();
>
>   SendReceive();  // Need to read so that we absorb the session tickets.
>   CheckKeys();
>
>   Reset();
>   ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
>   ExpectResumption(RESUME_TICKET);
>   client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, SSL_LIBRARY_VERSION_TLS_1_3);
>   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, SSL_LIBRARY_VERSION_TLS_1_2);
>   client_->Set0RttEnabled(true); // prepare for 0-rtt data
>   Connect();
> }

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Didn't mean to assign that to me right away, habit.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Eric Rescorla (:ekr) from comment #1)
> That should eventually fail in the sid version match

That's right, we segfault in ssl3_SetupPendingCipherSpec() just a few lines above the sid version check.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Eric Rescorla (:ekr) from comment #1)
> I assume that the server is *accepting* the ticket but TLS 1.2?

sid_match is actually zero, so we don't go into the branch that compares version numbers.

Comment 7

[Eric Rescorla (:ekr)]

Right. But the only reason we get into this function at all is because the server told us TLS 1.2, right? Otherwise we would be in tls13_HandleServerHelloPart2

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Eric Rescorla (:ekr) from comment #7)
> Right. But the only reason we get into this function at all is because the
> server told us TLS 1.2, right? Otherwise we would be in
> tls13_HandleServerHelloPart2

Yeah, that's correct. I posted a possible solution, although I'm not sure this is the best way to solve it. We need to install a 1.3 early app data cipher spec to send early app data, so reverting cipher specs when falling back seems to properly handle this.

https://nss-review.dev.mozaws.net/D94

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=f4aa62e5bbb2506a9c60e652498210cee85af8b4

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/72813bcb72e7