From deb6b3311db473ef6d368c8534a8f92d0ef796f7 Mon Sep 17 00:00:00 2001 From: Franziskus Kiefer Date: Fri, 17 Mar 2017 06:01:56 +0100 Subject: [PATCH] Bug 1345368 - land NSS 37ccb22f8e51, r=me --HG-- extra : rebase_source : 9e311a3410733d0db12818c57542c8321b8fddad --- security/nss/TAG-INFO | 2 +- .../taskcluster/docker-aarch64/Dockerfile | 2 +- security/nss/coreconf/config.gypi | 17 +- security/nss/coreconf/coreconf.dep | 1 - .../nss/coreconf/precommit.clang-format.sh | 2 +- security/nss/gtests/nss_bogo_shim/config.json | 2 +- .../ssl_gtest/ssl_extension_unittest.cc | 9 + .../nss/gtests/ssl_gtest/ssl_fuzz_unittest.cc | 97 ++++++++++ security/nss/gtests/ssl_gtest/tls_agent.cc | 4 + security/nss/gtests/ssl_gtest/tls_agent.h | 1 + security/nss/gtests/ssl_gtest/tls_parser.h | 1 + security/nss/lib/freebl/freebl_base.gypi | 5 + security/nss/lib/freebl/sysrand.c | 4 +- security/nss/lib/freebl/unix_rand.c | 171 ------------------ security/nss/lib/freebl/unix_urandom.c | 50 +++++ security/nss/lib/ssl/ssl3exthandle.c | 55 +++++- security/nss/lib/ssl/sslimpl.h | 2 + security/nss/lib/ssl/tls13con.c | 23 +-- security/nss/tests/ssl_gtests/ssl_gtests.sh | 2 +- 19 files changed, 241 insertions(+), 209 deletions(-) create mode 100644 security/nss/lib/freebl/unix_urandom.c diff --git a/security/nss/TAG-INFO b/security/nss/TAG-INFO index 539e6fba78cd..f90e75a86e99 100644 --- a/security/nss/TAG-INFO +++ b/security/nss/TAG-INFO @@ -1 +1 @@ -cf81ccc154dd +37ccb22f8e51 diff --git a/security/nss/automation/taskcluster/docker-aarch64/Dockerfile b/security/nss/automation/taskcluster/docker-aarch64/Dockerfile index cd58278f5972..2d7ade357868 100644 --- a/security/nss/automation/taskcluster/docker-aarch64/Dockerfile +++ b/security/nss/automation/taskcluster/docker-aarch64/Dockerfile @@ -13,7 +13,7 @@ ADD setup.sh /tmp/setup.sh RUN bash /tmp/setup.sh # Change user. -USER worker +# USER worker # See bug 1347473. # Env variables. ENV HOME /home/worker diff --git a/security/nss/coreconf/config.gypi b/security/nss/coreconf/config.gypi index f338dd2a0b1e..55a730e36b86 100644 --- a/security/nss/coreconf/config.gypi +++ b/security/nss/coreconf/config.gypi @@ -108,6 +108,7 @@ 'ct_verif%': 0, 'nss_public_dist_dir%': '<(nss_dist_dir)/public', 'nss_private_dist_dir%': '<(nss_dist_dir)/private', + 'only_dev_random%': 1, }, 'target_defaults': { # Settings specific to targets should go here. @@ -218,16 +219,18 @@ '-Wl,--gc-sections', ], 'conditions': [ - ['OS=="dragonfly" or OS=="freebsd" or OS=="netbsd" or OS=="openbsd"', { - # Bug 1321317 - unix_rand.c:880: undefined reference to `environ' - 'ldflags': [ - '-Wl,--warn-unresolved-symbols', - ], - }], ['no_zdefs==0', { 'ldflags': [ '-Wl,-z,defs', ], + 'conditions': [ + ['OS=="dragonfly" or OS=="freebsd" or OS=="netbsd" or OS=="openbsd"', { + # Bug 1321317 - unix_rand.c:880: undefined reference to `environ' + 'ldflags': [ + '-Wl,--warn-unresolved-symbols', + ], + }], + ], }], ], }], @@ -360,10 +363,12 @@ [ 'fuzz_tls==1', { 'cflags': [ '-Wno-unused-function', + '-Wno-unused-variable', ], 'xcode_settings': { 'OTHER_CFLAGS': [ '-Wno-unused-function', + '-Wno-unused-variable', ], }, }], diff --git a/security/nss/coreconf/coreconf.dep b/security/nss/coreconf/coreconf.dep index 590d1bfaeee3..5182f75552c8 100644 --- a/security/nss/coreconf/coreconf.dep +++ b/security/nss/coreconf/coreconf.dep @@ -10,4 +10,3 @@ */ #error "Do not include this header file." - diff --git a/security/nss/coreconf/precommit.clang-format.sh b/security/nss/coreconf/precommit.clang-format.sh index 11382c39bfdd..b638b298e5e1 100755 --- a/security/nss/coreconf/precommit.clang-format.sh +++ b/security/nss/coreconf/precommit.clang-format.sh @@ -19,7 +19,7 @@ if [ "$1" = "install" ]; then if [ "$hg" -eq 1 ]; then hgrc="$(hg root)"/.hg/hgrc if ! grep -q '^pretxncommit.clang-format' "$hgrc"; then - echo '[hook]' >> "$hgrc" + echo '[hooks]' >> "$hgrc" echo 'pretxncommit.clang-format = [ ! -x ./coreconf/precommit.clang-format.sh ] || ./coreconf/precommit.clang-format.sh' >> "$hgrc" echo "Installed mercurial pretxncommit hook" exit diff --git a/security/nss/gtests/nss_bogo_shim/config.json b/security/nss/gtests/nss_bogo_shim/config.json index 7a52888189bb..f6743dde6431 100644 --- a/security/nss/gtests/nss_bogo_shim/config.json +++ b/security/nss/gtests/nss_bogo_shim/config.json @@ -55,7 +55,7 @@ "UnsolicitedServerNameAck-TLS1*":"Boring wants us to fail with an unexpected_extension alert, we simply ignore ssl_server_name_xtn.", "RequireAnyClientCertificate-TLS1*":"Bug 1339387", "SendExtensionOnClientCertificate-TLS13":"Bug 1339392", - "ALPNClient-Mismatch-TLS1*":"Bug 1339418" + "ALPNClient-Mismatch-TLS13": "NSS sends alerts in response to errors in protected handshake messages in the clear" }, "ErrorMap" : { ":HANDSHAKE_FAILURE_ON_CLIENT_HELLO:":"SSL_ERROR_NO_CYPHER_OVERLAP", diff --git a/security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc b/security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc index 293878d76dd0..db161a2bacb6 100644 --- a/security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc +++ b/security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc @@ -476,6 +476,15 @@ TEST_P(TlsExtensionTestPre13, AlpnReturnedBadNameLength) { ssl_app_layer_protocol_xtn, extension)); } +TEST_P(TlsExtensionTestPre13, AlpnReturnedUnknownName) { + EnableAlpn(); + const uint8_t val[] = {0x00, 0x02, 0x01, 0x67}; + DataBuffer extension(val, sizeof(val)); + ServerHelloErrorTest(std::make_shared( + ssl_app_layer_protocol_xtn, extension), + kTlsAlertIllegalParameter); +} + TEST_P(TlsExtensionTestDtls, SrtpShort) { EnableSrtp(); ClientHelloErrorTest( diff --git a/security/nss/gtests/ssl_gtest/ssl_fuzz_unittest.cc b/security/nss/gtests/ssl_gtest/ssl_fuzz_unittest.cc index aa6a9cfca04b..b2abfa9a8ee1 100644 --- a/security/nss/gtests/ssl_gtest/ssl_fuzz_unittest.cc +++ b/security/nss/gtests/ssl_gtest/ssl_fuzz_unittest.cc @@ -226,4 +226,101 @@ FUZZ_P(TlsConnectGeneric, BogusClientAuthSignature) { std::make_shared(kTlsHandshakeCertificateVerify)); Connect(); } + +// Check that session ticket resumption works. +FUZZ_P(TlsConnectGeneric, SessionTicketResumption) { + ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET); + Connect(); + SendReceive(); + + Reset(); + ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET); + ExpectResumption(RESUME_TICKET); + Connect(); + SendReceive(); +} + +class TlsSessionTicketMacDamager : public TlsExtensionFilter { + public: + TlsSessionTicketMacDamager() {} + virtual PacketFilter::Action FilterExtension(uint16_t extension_type, + const DataBuffer& input, + DataBuffer* output) { + if (extension_type != ssl_session_ticket_xtn && + extension_type != ssl_tls13_pre_shared_key_xtn) { + return KEEP; + } + + *output = input; + + // Handle everything before TLS 1.3. + if (extension_type == ssl_session_ticket_xtn) { + // Modify the last byte of the MAC. + output->data()[output->len() - 1] ^= 0xff; + } + + // Handle TLS 1.3. + if (extension_type == ssl_tls13_pre_shared_key_xtn) { + TlsParser parser(input); + + uint32_t ids_len; + EXPECT_TRUE(parser.Read(&ids_len, 2) && ids_len > 0); + + uint32_t ticket_len; + EXPECT_TRUE(parser.Read(&ticket_len, 2) && ticket_len > 0); + + // Modify the last byte of the MAC. + output->data()[2 + 2 + ticket_len - 1] ^= 0xff; + } + + return CHANGE; + } +}; + +// Check that session ticket resumption works with a bad MAC. +FUZZ_P(TlsConnectGeneric, SessionTicketResumptionBadMac) { + ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET); + Connect(); + SendReceive(); + + Reset(); + ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET); + ExpectResumption(RESUME_TICKET); + + client_->SetPacketFilter(std::make_shared()); + Connect(); + SendReceive(); +} + +// Check that session tickets are not encrypted. +FUZZ_P(TlsConnectGeneric, UnencryptedSessionTickets) { + ConfigureSessionCache(RESUME_TICKET, RESUME_TICKET); + + auto i1 = std::make_shared( + kTlsHandshakeNewSessionTicket); + server_->SetPacketFilter(i1); + Connect(); + + size_t offset = 4; /* lifetime */ + if (version_ == SSL_LIBRARY_VERSION_TLS_1_3) { + offset += 1 + 1 + /* ke_modes */ + 1 + 1; /* auth_modes */ + } + + offset += 2 + /* ticket length */ + 16 + /* SESS_TICKET_KEY_NAME_LEN */ + 16 + /* AES-128 IV */ + 2 + /* ciphertext length */ + 2; /* TLS_EX_SESS_TICKET_VERSION */ + + // Check the protocol version number. + uint32_t tls_version = 0; + EXPECT_TRUE(i1->buffer().Read(offset, sizeof(version_), &tls_version)); + EXPECT_EQ(version_, static_cast(tls_version)); + + // Check the cipher suite. + uint32_t suite = 0; + EXPECT_TRUE(i1->buffer().Read(offset + sizeof(version_), 2, &suite)); + client_->CheckCipherSuite(static_cast(suite)); +} } diff --git a/security/nss/gtests/ssl_gtest/tls_agent.cc b/security/nss/gtests/ssl_gtest/tls_agent.cc index 9c528224030a..97aae79f4304 100644 --- a/security/nss/gtests/ssl_gtest/tls_agent.cc +++ b/security/nss/gtests/ssl_gtest/tls_agent.cc @@ -241,6 +241,10 @@ bool TlsAgent::GetPeerChainLength(size_t* count) { return true; } +void TlsAgent::CheckCipherSuite(uint16_t cipher_suite) { + EXPECT_EQ(csinfo_.cipherSuite, cipher_suite); +} + void TlsAgent::RequestClientAuth(bool requireAuth) { EXPECT_TRUE(EnsureTlsSetup()); ASSERT_EQ(SERVER, role_); diff --git a/security/nss/gtests/ssl_gtest/tls_agent.h b/security/nss/gtests/ssl_gtest/tls_agent.h index 0cbfe6b566d1..60fad1aa9834 100644 --- a/security/nss/gtests/ssl_gtest/tls_agent.h +++ b/security/nss/gtests/ssl_gtest/tls_agent.h @@ -163,6 +163,7 @@ class TlsAgent : public PollTarget { void ConfigNamedGroups(const std::vector& groups); void DisableECDHEServerKeyReuse(); bool GetPeerChainLength(size_t* count); + void CheckCipherSuite(uint16_t cipher_suite); const std::string& name() const { return name_; } diff --git a/security/nss/gtests/ssl_gtest/tls_parser.h b/security/nss/gtests/ssl_gtest/tls_parser.h index 8b6e5b9c051e..d0de21bc5213 100644 --- a/security/nss/gtests/ssl_gtest/tls_parser.h +++ b/security/nss/gtests/ssl_gtest/tls_parser.h @@ -26,6 +26,7 @@ const uint8_t kTlsApplicationDataType = 23; const uint8_t kTlsHandshakeClientHello = 1; const uint8_t kTlsHandshakeServerHello = 2; +const uint8_t kTlsHandshakeNewSessionTicket = 4; const uint8_t kTlsHandshakeHelloRetryRequest = 6; const uint8_t kTlsHandshakeEncryptedExtensions = 8; const uint8_t kTlsHandshakeCertificate = 11; diff --git a/security/nss/lib/freebl/freebl_base.gypi b/security/nss/lib/freebl/freebl_base.gypi index 7570eec88633..d154e11e13b8 100644 --- a/security/nss/lib/freebl/freebl_base.gypi +++ b/security/nss/lib/freebl/freebl_base.gypi @@ -177,6 +177,11 @@ 'CT_VERIF', ], }], + [ 'only_dev_random==1', { + 'defines': [ + 'SEED_ONLY_DEV_URANDOM', + ] + }], [ 'OS=="mac"', { 'conditions': [ [ 'target_arch=="ia32"', { diff --git a/security/nss/lib/freebl/sysrand.c b/security/nss/lib/freebl/sysrand.c index 1b6cbdfcd567..763f6af11957 100644 --- a/security/nss/lib/freebl/sysrand.c +++ b/security/nss/lib/freebl/sysrand.c @@ -8,7 +8,9 @@ #include "seccomon.h" -#if defined(XP_UNIX) || defined(XP_BEOS) +#if (defined(XP_UNIX) || defined(XP_BEOS)) && defined(SEED_ONLY_DEV_URANDOM) +#include "unix_urandom.c" +#elif defined(XP_UNIX) || defined(XP_BEOS) #include "unix_rand.c" #endif #ifdef XP_WIN diff --git a/security/nss/lib/freebl/unix_rand.c b/security/nss/lib/freebl/unix_rand.c index 775e117acca2..20c76ec661ce 100644 --- a/security/nss/lib/freebl/unix_rand.c +++ b/security/nss/lib/freebl/unix_rand.c @@ -682,134 +682,6 @@ RNG_GetNoise(void *buf, size_t maxbytes) return n; } -#define SAFE_POPEN_MAXARGS 10 /* must be at least 2 */ - -/* - * safe_popen is static to this module and we know what arguments it is - * called with. Note that this version only supports a single open child - * process at any time. - */ -static pid_t safe_popen_pid; -static struct sigaction oldact; - -static FILE * -safe_popen(char *cmd) -{ - int p[2], fd, argc; - pid_t pid; - char *argv[SAFE_POPEN_MAXARGS + 1]; - FILE *fp; - static char blank[] = " \t"; - static struct sigaction newact; - - if (pipe(p) < 0) - return 0; - - fp = fdopen(p[0], "r"); - if (fp == 0) { - close(p[0]); - close(p[1]); - return 0; - } - - /* Setup signals so that SIGCHLD is ignored as we want to do waitpid */ - newact.sa_handler = SIG_DFL; - newact.sa_flags = 0; - sigfillset(&newact.sa_mask); - sigaction(SIGCHLD, &newact, &oldact); - - pid = fork(); - switch (pid) { - int ndesc; - - case -1: - fclose(fp); /* this closes p[0], the fd associated with fp */ - close(p[1]); - sigaction(SIGCHLD, &oldact, NULL); - return 0; - - case 0: - /* dup write-side of pipe to stderr and stdout */ - if (p[1] != 1) - dup2(p[1], 1); - if (p[1] != 2) - dup2(p[1], 2); - - /* - * close the other file descriptors, except stdin which we - * try reassociating with /dev/null, first (bug 174993) - */ - if (!freopen("/dev/null", "r", stdin)) - close(0); - ndesc = getdtablesize(); - for (fd = PR_MIN(65536, ndesc); --fd > 2; close(fd)) - ; - - /* clean up environment in the child process */ - putenv("PATH=/bin:/usr/bin:/sbin:/usr/sbin:/etc:/usr/etc"); - putenv("SHELL=/bin/sh"); - putenv("IFS= \t"); - - /* - * The caller may have passed us a string that is in text - * space. It may be illegal to modify the string - */ - cmd = strdup(cmd); - /* format argv */ - argv[0] = strtok(cmd, blank); - argc = 1; - while ((argv[argc] = strtok(0, blank)) != 0) { - if (++argc == SAFE_POPEN_MAXARGS) { - argv[argc] = 0; - break; - } - } - - /* and away we go */ - execvp(argv[0], argv); - exit(127); - break; - - default: - close(p[1]); - break; - } - - /* non-zero means there's a cmd running */ - safe_popen_pid = pid; - return fp; -} - -static int -safe_pclose(FILE *fp) -{ - pid_t pid; - int status = -1, rv; - - if ((pid = safe_popen_pid) == 0) - return -1; - safe_popen_pid = 0; - - fclose(fp); - - /* yield the processor so the child gets some time to exit normally */ - PR_Sleep(PR_INTERVAL_NO_WAIT); - - /* if the child hasn't exited, kill it -- we're done with its output */ - while ((rv = waitpid(pid, &status, WNOHANG)) == -1 && errno == EINTR) - ; - if (rv == 0) { - kill(pid, SIGKILL); - while ((rv = waitpid(pid, &status, 0)) == -1 && errno == EINTR) - ; - } - - /* Reset SIGCHLD signal hander before returning */ - sigaction(SIGCHLD, &oldact, NULL); - - return status; -} - #ifdef DARWIN #include #if !TARGET_OS_IPHONE @@ -817,15 +689,9 @@ safe_pclose(FILE *fp) #endif #endif -/* Fork netstat to collect its output by default. Do not unset this unless - * another source of entropy is available - */ -#define DO_NETSTAT 1 - void RNG_SystemInfoForRNG(void) { - FILE *fp; char buf[BUFSIZ]; size_t bytes; const char *const *cp; @@ -860,12 +726,6 @@ RNG_SystemInfoForRNG(void) }; #endif -#if defined(BSDI) - static char netstat_ni_cmd[] = "netstat -nis"; -#else - static char netstat_ni_cmd[] = "netstat -ni"; -#endif - GiveSystemInfo(); bytes = RNG_GetNoise(buf, sizeof(buf)); @@ -890,7 +750,6 @@ RNG_SystemInfoForRNG(void) if (gethostname(buf, sizeof(buf)) == 0) { RNG_RandomUpdate(buf, strlen(buf)); } - GiveSystemInfo(); /* grab some data from system's PRNG before any other files. */ bytes = RNG_FileUpdate("/dev/urandom", SYSTEM_RNG_SEED_COUNT); @@ -914,33 +773,12 @@ RNG_SystemInfoForRNG(void) for (cp = files; *cp; cp++) RNG_FileForRNG(*cp); -/* - * Bug 100447: On BSD/OS 4.2 and 4.3, we have problem calling safe_popen - * in a pthreads environment. Therefore, we call safe_popen last and on - * BSD/OS we do not call safe_popen when we succeeded in getting data - * from /dev/urandom. - * - * Bug 174993: On platforms providing /dev/urandom, don't fork netstat - * either, if data has been gathered successfully. - */ - #if defined(BSDI) || defined(FREEBSD) || defined(NETBSD) || defined(OPENBSD) || defined(DARWIN) || defined(LINUX) || defined(HPUX) if (bytes) return; #endif #ifdef SOLARIS - -/* - * On Solaris, NSS may be initialized automatically from libldap in - * applications that are unaware of the use of NSS. safe_popen forks, and - * sometimes creates issues with some applications' pthread_atfork handlers. - * We always have /dev/urandom on Solaris 9 and above as an entropy source, - * and for Solaris 8 we have the libkstat interface, so we don't need to - * fork netstat. - */ - -#undef DO_NETSTAT if (!bytes) { /* On Solaris 8, /dev/urandom isn't available, so we use libkstat. */ PRUint32 kstat_bytes = 0; @@ -951,15 +789,6 @@ RNG_SystemInfoForRNG(void) PORT_Assert(bytes); } #endif - -#ifdef DO_NETSTAT - fp = safe_popen(netstat_ni_cmd); - if (fp != NULL) { - while ((bytes = fread(buf, 1, sizeof(buf), fp)) > 0) - RNG_RandomUpdate(buf, bytes); - safe_pclose(fp); - } -#endif } #define TOTAL_FILE_LIMIT 1000000 /* one million */ diff --git a/security/nss/lib/freebl/unix_urandom.c b/security/nss/lib/freebl/unix_urandom.c new file mode 100644 index 000000000000..25e6ad91cf58 --- /dev/null +++ b/security/nss/lib/freebl/unix_urandom.c @@ -0,0 +1,50 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include +#include +#include "secerr.h" +#include "secrng.h" +#include "prprf.h" + +void +RNG_SystemInfoForRNG(void) +{ + PRUint8 bytes[SYSTEM_RNG_SEED_COUNT]; + size_t numBytes = RNG_SystemRNG(bytes, SYSTEM_RNG_SEED_COUNT); + if (!numBytes) { + /* error is set */ + return; + } + RNG_RandomUpdate(bytes, numBytes); +} + +size_t +RNG_SystemRNG(void *dest, size_t maxLen) +{ + int fd; + int bytes; + size_t fileBytes = 0; + unsigned char *buffer = dest; + + fd = open("/dev/urandom", O_RDONLY); + if (fd < 0) { + PORT_SetError(SEC_ERROR_NEED_RANDOM); + return 0; + } + while (fileBytes < maxLen) { + bytes = read(fd, buffer, maxLen - fileBytes); + if (bytes <= 0) { + break; + } + fileBytes += bytes; + buffer += bytes; + } + (void)close(fd); + if (fileBytes != maxLen) { + PORT_SetError(SEC_ERROR_NEED_RANDOM); + return 0; + } + return fileBytes; +} diff --git a/security/nss/lib/ssl/ssl3exthandle.c b/security/nss/lib/ssl/ssl3exthandle.c index 04c8182cc299..1a194fac8525 100644 --- a/security/nss/lib/ssl/ssl3exthandle.c +++ b/security/nss/lib/ssl/ssl3exthandle.c @@ -354,6 +354,27 @@ ssl3_ParseEncryptedSessionTicket(sslSocket *ss, SECItem *data, return SECSuccess; } +PRBool +ssl_AlpnTagAllowed(const sslSocket *ss, const SECItem *tag) +{ + const unsigned char *data = ss->opt.nextProtoNego.data; + unsigned int length = ss->opt.nextProtoNego.len; + unsigned int offset = 0; + + if (!tag->len) + return PR_TRUE; + + while (offset < length) { + unsigned int taglen = (unsigned int)data[offset]; + if ((taglen == tag->len) && + !PORT_Memcmp(data + offset + 1, tag->data, tag->len)) + return PR_TRUE; + offset += 1 + taglen; + } + + return PR_FALSE; +} + /* handle an incoming Next Protocol Negotiation extension. */ SECStatus ssl3_ServerHandleNextProtoNegoXtn(const sslSocket *ss, TLSExtensionData *xtnData, PRUint16 ex_type, @@ -568,6 +589,12 @@ ssl3_ClientHandleAppProtoXtn(const sslSocket *ss, TLSExtensionData *xtnData, PRU return SECFailure; } + if (!ssl_AlpnTagAllowed(ss, &protocol_name)) { + ssl3_ExtSendAlert(ss, alert_fatal, illegal_parameter); + PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + return SECFailure; + } + SECITEM_FreeItem(&xtnData->nextProto, PR_FALSE); xtnData->nextProtoState = SSL_NEXT_PROTO_SELECTED; xtnData->negotiated[xtnData->numNegotiated++] = ex_type; @@ -977,9 +1004,13 @@ ssl3_EncodeSessionTicket(sslSocket *ss, + sizeof(ticket->ticket_lifetime_hint) /* ticket lifetime hint */ + sizeof(ticket->flags) /* ticket flags */ + 1 + alpnSelection.len; /* npn value + length field. */ +#ifdef UNSAFE_FUZZER_MODE + padding_length = 0; +#else padding_length = AES_BLOCK_SIZE - (ciphertext_length % AES_BLOCK_SIZE); +#endif ciphertext_length += padding_length; if (SECITEM_AllocItem(NULL, &plaintext_item, ciphertext_length) == NULL) @@ -1132,6 +1163,10 @@ ssl3_EncodeSessionTicket(sslSocket *ss, /* Generate encrypted portion of ticket. */ PORT_Assert(aes_key); +#ifdef UNSAFE_FUZZER_MODE + ciphertext.len = plaintext_item.len; + PORT_Memcpy(ciphertext.data, plaintext_item.data, plaintext_item.len); +#else aes_ctx = PK11_CreateContextBySymKey(cipherMech, CKA_ENCRYPT, aes_key, &ivItem); if (!aes_ctx) goto loser; @@ -1143,10 +1178,10 @@ ssl3_EncodeSessionTicket(sslSocket *ss, PK11_DestroyContext(aes_ctx, PR_TRUE); if (rv != SECSuccess) goto loser; +#endif /* Convert ciphertext length to network order. */ - length_buf[0] = (ciphertext.len >> 8) & 0xff; - length_buf[1] = (ciphertext.len) & 0xff; + (void)ssl_EncodeUintX(ciphertext.len, 2, length_buf); /* Compute MAC. */ PORT_Assert(mac_key); @@ -1310,9 +1345,11 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data) */ if (PORT_Memcmp(enc_session_ticket.key_name, key_name, SESS_TICKET_KEY_NAME_LEN) != 0) { +#ifndef UNSAFE_FUZZER_MODE SSL_DBG(("%d: SSL[%d]: Session ticket key_name sent mismatch.", SSL_GETPID(), ss->fd)); goto no_ticket; +#endif } /* Verify the MAC on the ticket. MAC verification may also @@ -1349,9 +1386,11 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data) if (NSS_SecureMemcmp(computed_mac, enc_session_ticket.mac, computed_mac_length) != 0) { +#ifndef UNSAFE_FUZZER_MODE SSL_DBG(("%d: SSL[%d]: Session ticket MAC mismatch.", SSL_GETPID(), ss->fd)); goto no_ticket; +#endif } /* We ignore key_name for now. @@ -1365,6 +1404,12 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data) enc_session_ticket.encrypted_state.len); PORT_Assert(aes_key); +#ifdef UNSAFE_FUZZER_MODE + decrypted_state->len = enc_session_ticket.encrypted_state.len; + PORT_Memcpy(decrypted_state->data, + enc_session_ticket.encrypted_state.data, + enc_session_ticket.encrypted_state.len); +#else ivItem.data = enc_session_ticket.iv; ivItem.len = AES_BLOCK_SIZE; aes_ctx = PK11_CreateContextBySymKey(cipherMech, CKA_DECRYPT, @@ -1395,6 +1440,7 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data) if (padding_length != (PRUint32)*padding) goto no_ticket; } +#endif /* Deserialize session state. */ buffer = decrypted_state->data; @@ -1562,9 +1608,12 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data) goto no_ticket; } +#ifndef UNSAFE_FUZZER_MODE /* Done parsing. Check that all bytes have been consumed. */ - if (buffer_len != padding_length) + if (buffer_len != padding_length) { goto no_ticket; + } +#endif /* Use the ticket if it has not expired, otherwise free the allocated * memory since the ticket is of no use. diff --git a/security/nss/lib/ssl/sslimpl.h b/security/nss/lib/ssl/sslimpl.h index f4d285dce6be..50eb53c0203d 100644 --- a/security/nss/lib/ssl/sslimpl.h +++ b/security/nss/lib/ssl/sslimpl.h @@ -1856,6 +1856,8 @@ ssl3_TLSPRFWithMasterSecret(sslSocket *ss, ssl3CipherSpec *spec, const unsigned char *val, unsigned int valLen, unsigned char *out, unsigned int outLen); +PRBool ssl_AlpnTagAllowed(const sslSocket *ss, const SECItem *tag); + #ifdef TRACE #define SSL_TRACE(msg) ssl_Trace msg #else diff --git a/security/nss/lib/ssl/tls13con.c b/security/nss/lib/ssl/tls13con.c index 25ece5446744..0a5e0bb90d05 100644 --- a/security/nss/lib/ssl/tls13con.c +++ b/security/nss/lib/ssl/tls13con.c @@ -941,27 +941,6 @@ tls13_CanResume(sslSocket *ss, const sslSessionID *sid) return PR_TRUE; } -static PRBool -tls13_AlpnTagAllowed(const sslSocket *ss, const SECItem *tag) -{ - const unsigned char *data = ss->opt.nextProtoNego.data; - unsigned int length = ss->opt.nextProtoNego.len; - unsigned int offset = 0; - - if (!tag->len) - return PR_TRUE; - - while (offset < length) { - unsigned int taglen = (unsigned int)data[offset]; - if ((taglen == tag->len) && - !PORT_Memcmp(data + offset + 1, tag->data, tag->len)) - return PR_TRUE; - offset += 1 + taglen; - } - - return PR_FALSE; -} - static PRBool tls13_CanNegotiateZeroRtt(sslSocket *ss, const sslSessionID *sid) { @@ -4296,7 +4275,7 @@ tls13_ClientAllow0Rtt(const sslSocket *ss, const sslSessionID *sid) return PR_FALSE; if ((sid->u.ssl3.locked.sessionTicket.flags & ticket_allow_early_data) == 0) return PR_FALSE; - return tls13_AlpnTagAllowed(ss, &sid->u.ssl3.alpnSelection); + return ssl_AlpnTagAllowed(ss, &sid->u.ssl3.alpnSelection); } SECStatus diff --git a/security/nss/tests/ssl_gtests/ssl_gtests.sh b/security/nss/tests/ssl_gtests/ssl_gtests.sh index 7aaa09ef0f7f..ac39f212ceb6 100755 --- a/security/nss/tests/ssl_gtests/ssl_gtests.sh +++ b/security/nss/tests/ssl_gtests/ssl_gtests.sh @@ -174,7 +174,7 @@ ssl_gtest_start() # Helper function used when 'parallel' isn't available. parallel_fallback() { - eval ${*//\{\}/0} + eval "${@//\{\}/0}" } parse_report()