Bug 1501587 - land NSS f5ef34273311 UPGRADE_NSS_RELEASE, r=me
authorJ.C. Jones <jc@mozilla.com>
Fri, 30 Nov 2018 18:00:06 +0000
changeset 508258 6ed6878e9cea191089acd4237756d7488e40edad
parent 508257 3124848b93c461baa76da73df141e392cc0eea39
child 508259 6f52d12b5a1e60b8ebb6c9042b51c89a50aafeb2
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1501587
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1501587 - land NSS f5ef34273311 UPGRADE_NSS_RELEASE, r=me
security/nss/TAG-INFO
security/nss/coreconf/coreconf.dep
security/nss/gtests/freebl_gtest/mpi_unittest.cc
security/nss/gtests/freebl_gtest/rsa_unittest.cc
security/nss/gtests/ssl_gtest/ssl_custext_unittest.cc
security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc
security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc
security/nss/gtests/ssl_gtest/ssl_loopback_unittest.cc
security/nss/gtests/ssl_gtest/ssl_resumption_unittest.cc
security/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
security/nss/lib/freebl/mpi/mpi.c
security/nss/lib/freebl/mpi/mpi.h
security/nss/lib/freebl/rsapkcs.c
security/nss/lib/ssl/SSLerrs.h
security/nss/lib/ssl/ssl3con.c
security/nss/lib/ssl/sslerr.h
security/nss/lib/ssl/tls13con.c
--- a/security/nss/TAG-INFO
+++ b/security/nss/TAG-INFO
@@ -1,1 +1,1 @@
-4b9cf6e61a48
+f5ef34273311
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -5,9 +5,8 @@
 
 /*
  * A dummy header file that is a dependency for all the object files.
  * Used to force a full recompilation of NSS in Mozilla's Tinderbox
  * depend builds.  See comments in rules.mk.
  */
 
 #error "Do not include this header file."
-
--- a/security/nss/gtests/freebl_gtest/mpi_unittest.cc
+++ b/security/nss/gtests/freebl_gtest/mpi_unittest.cc
@@ -10,17 +10,17 @@
 #ifdef __MACH__
 #include <mach/clock.h>
 #include <mach/mach.h>
 #endif
 
 #include "mpi.h"
 namespace nss_test {
 
-void gettime(struct timespec *tp) {
+void gettime(struct timespec* tp) {
 #ifdef __MACH__
   clock_serv_t cclock;
   mach_timespec_t mts;
 
   host_get_clock_service(mach_host_self(), SYSTEM_CLOCK, &cclock);
   clock_get_time(cclock, &mts);
   mach_port_deallocate(mach_task_self(), cclock);
 
@@ -64,16 +64,49 @@ class MPITest : public ::testing::Test {
     mp_read_radix(&c, result.c_str(), 16);
     EXPECT_EQ(MP_OKAY, mp_div(&a, &b, &a, &b));
     EXPECT_EQ(0, mp_cmp(&a, &c));
 
     mp_clear(&a);
     mp_clear(&b);
     mp_clear(&c);
   }
+
+  void dump(const std::string& prefix, const uint8_t* buf, size_t len) {
+    auto flags = std::cerr.flags();
+    std::cerr << prefix << ": [" << std::dec << len << "] ";
+    for (size_t i = 0; i < len; ++i) {
+      std::cerr << std::hex << std::setw(2) << std::setfill('0')
+                << static_cast<int>(buf[i]);
+    }
+    std::cerr << std::endl << std::resetiosflags(flags);
+  }
+
+  void TestToFixedOctets(const std::vector<uint8_t>& ref, size_t len) {
+    mp_int a;
+    ASSERT_EQ(MP_OKAY, mp_init(&a));
+    ASSERT_EQ(MP_OKAY, mp_read_unsigned_octets(&a, ref.data(), ref.size()));
+    uint8_t buf[len];
+    ASSERT_EQ(MP_OKAY, mp_to_fixlen_octets(&a, buf, len));
+    size_t compare;
+    if (len > ref.size()) {
+      for (size_t i = 0; i < len - ref.size(); ++i) {
+        ASSERT_EQ(0U, buf[i]) << "index " << i << " should be zero";
+      }
+      compare = ref.size();
+    } else {
+      compare = len;
+    }
+    dump("value", ref.data(), ref.size());
+    dump("output", buf, len);
+    ASSERT_EQ(0, memcmp(buf + len - compare, ref.data() + ref.size() - compare,
+                        compare))
+        << "comparing " << compare << " octets";
+    mp_clear(&a);
+  }
 };
 
 TEST_F(MPITest, MpiCmp01Test) { TestCmp("0", "1", -1); }
 TEST_F(MPITest, MpiCmp10Test) { TestCmp("1", "0", 1); }
 TEST_F(MPITest, MpiCmp00Test) { TestCmp("0", "0", 0); }
 TEST_F(MPITest, MpiCmp11Test) { TestCmp("1", "1", 0); }
 TEST_F(MPITest, MpiDiv32ErrorTest) {
   TestDiv("FFFF00FFFFFFFF000000000000", "FFFF00FFFFFFFFFF", "FFFFFFFFFF");
@@ -108,41 +141,82 @@ TEST_F(MPITest, MpiCmpUnalignedTest) {
   ASSERT_TRUE(strncmp(c_tmp, "feffffffffffffff100000000000000", 31));
 
   mp_clear(&a);
   mp_clear(&b);
   mp_clear(&c);
 }
 #endif
 
+TEST_F(MPITest, MpiFixlenOctetsZero) {
+  std::vector<uint8_t> zero = {0};
+  TestToFixedOctets(zero, 1);
+  TestToFixedOctets(zero, 2);
+  TestToFixedOctets(zero, sizeof(mp_digit));
+  TestToFixedOctets(zero, sizeof(mp_digit) + 1);
+}
+
+TEST_F(MPITest, MpiFixlenOctetsVarlen) {
+  std::vector<uint8_t> packed;
+  for (size_t i = 0; i < sizeof(mp_digit) * 2; ++i) {
+    packed.push_back(0xa4);  // Any non-zero value will do.
+    TestToFixedOctets(packed, packed.size());
+    TestToFixedOctets(packed, packed.size() + 1);
+    TestToFixedOctets(packed, packed.size() + sizeof(mp_digit));
+  }
+}
+
+TEST_F(MPITest, MpiFixlenOctetsTooSmall) {
+  uint8_t buf[sizeof(mp_digit) * 3];
+  std::vector<uint8_t> ref;
+  for (size_t i = 0; i < sizeof(mp_digit) * 2; i++) {
+    ref.push_back(3);  // Any non-zero value will do.
+    dump("ref", ref.data(), ref.size());
+
+    mp_int a;
+    ASSERT_EQ(MP_OKAY, mp_init(&a));
+    ASSERT_EQ(MP_OKAY, mp_read_unsigned_octets(&a, ref.data(), ref.size()));
+#ifdef DEBUG
+    // ARGCHK maps to assert() in a debug build.
+    EXPECT_DEATH(mp_to_fixlen_octets(&a, buf, ref.size() - 1), "");
+#else
+    EXPECT_EQ(MP_BADARG, mp_to_fixlen_octets(&a, buf, ref.size() - 1));
+#endif
+    ASSERT_EQ(MP_OKAY, mp_to_fixlen_octets(&a, buf, ref.size()));
+    ASSERT_EQ(0, memcmp(buf, ref.data(), ref.size()));
+
+    mp_clear(&a);
+  }
+}
+
 // This test is slow. Disable it by default so we can run these tests on CI.
 class DISABLED_MPITest : public ::testing::Test {};
 
 TEST_F(DISABLED_MPITest, MpiCmpConstTest) {
   mp_int a, b, c;
   MP_DIGITS(&a) = 0;
   MP_DIGITS(&b) = 0;
   MP_DIGITS(&c) = 0;
   ASSERT_EQ(MP_OKAY, mp_init(&a));
   ASSERT_EQ(MP_OKAY, mp_init(&b));
   ASSERT_EQ(MP_OKAY, mp_init(&c));
 
   mp_read_radix(
       &a,
-      const_cast<char *>(
+      const_cast<char*>(
           "FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551"),
       16);
   mp_read_radix(
       &b,
-      const_cast<char *>(
+      const_cast<char*>(
           "FF0FFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551"),
       16);
   mp_read_radix(
       &c,
-      const_cast<char *>(
+      const_cast<char*>(
           "FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632550"),
       16);
 
 #ifdef CT_VERIF
   mp_taint(&b);
   mp_taint(&c);
 #endif
 
--- a/security/nss/gtests/freebl_gtest/rsa_unittest.cc
+++ b/security/nss/gtests/freebl_gtest/rsa_unittest.cc
@@ -16,46 +16,82 @@ struct ScopedDelete {
       PORT_FreeArena(ptr->arena, PR_TRUE);
     }
   }
 };
 
 typedef std::unique_ptr<RSAPrivateKey, ScopedDelete<RSAPrivateKey>>
     ScopedRSAPrivateKey;
 
-class RSANewKeyTest : public ::testing::Test {
+class RSATest : public ::testing::Test {
  protected:
   RSAPrivateKey* CreateKeyWithExponent(int keySizeInBits,
                                        unsigned char publicExponent) {
     SECItem exp = {siBuffer, 0, 0};
     unsigned char pubExp[1] = {publicExponent};
     exp.data = pubExp;
     exp.len = 1;
 
     return RSA_NewKey(keySizeInBits, &exp);
   }
 };
 
-TEST_F(RSANewKeyTest, expOneTest) {
+TEST_F(RSATest, expOneTest) {
   ScopedRSAPrivateKey key(CreateKeyWithExponent(2048, 0x01));
   ASSERT_TRUE(key == nullptr);
 }
-TEST_F(RSANewKeyTest, expTwoTest) {
+TEST_F(RSATest, expTwoTest) {
   ScopedRSAPrivateKey key(CreateKeyWithExponent(2048, 0x02));
   ASSERT_TRUE(key == nullptr);
 }
-TEST_F(RSANewKeyTest, expFourTest) {
+TEST_F(RSATest, expFourTest) {
   ScopedRSAPrivateKey key(CreateKeyWithExponent(2048, 0x04));
   ASSERT_TRUE(key == nullptr);
 }
-TEST_F(RSANewKeyTest, WrongKeysizeTest) {
+TEST_F(RSATest, WrongKeysizeTest) {
   ScopedRSAPrivateKey key(CreateKeyWithExponent(2047, 0x03));
   ASSERT_TRUE(key == nullptr);
 }
 
-TEST_F(RSANewKeyTest, expThreeTest) {
+TEST_F(RSATest, expThreeTest) {
   ScopedRSAPrivateKey key(CreateKeyWithExponent(2048, 0x03));
 #ifdef NSS_FIPS_DISABLED
   ASSERT_TRUE(key != nullptr);
 #else
   ASSERT_TRUE(key == nullptr);
 #endif
 }
+
+TEST_F(RSATest, DecryptBlockTestErrors) {
+  unsigned char pubExp[3] = {0x01, 0x00, 0x01};
+  SECItem exp = {siBuffer, pubExp, 3};
+  ScopedRSAPrivateKey key(RSA_NewKey(2048, &exp));
+  ASSERT_TRUE(key);
+  uint8_t out[10] = {0};
+  uint8_t in_small[100] = {0};
+  unsigned int outputLen = 0;
+  unsigned int maxOutputLen = sizeof(out);
+
+  // This should fail because input the same size as the modulus (256).
+  SECStatus rv = RSA_DecryptBlock(key.get(), out, &outputLen, maxOutputLen,
+                                  in_small, sizeof(in_small));
+  EXPECT_EQ(SECFailure, rv);
+
+  uint8_t in[256] = {0};
+  // This should fail because the padding checks will fail.
+  rv = RSA_DecryptBlock(key.get(), out, &outputLen, maxOutputLen, in,
+                        sizeof(in));
+  EXPECT_EQ(SECFailure, rv);
+  // outputLen should be maxOutputLen.
+  EXPECT_EQ(maxOutputLen, outputLen);
+
+  // This should fail because the padding checks will fail.
+  uint8_t out_long[260] = {0};
+  maxOutputLen = sizeof(out_long);
+  rv = RSA_DecryptBlock(key.get(), out_long, &outputLen, maxOutputLen, in,
+                        sizeof(in));
+  EXPECT_EQ(SECFailure, rv);
+  // outputLen should <= 256-11=245.
+  EXPECT_LE(outputLen, 245u);
+  // Everything over 256 must be 0 in the output.
+  uint8_t out_long_test[4] = {0};
+  EXPECT_EQ(0, memcmp(out_long_test, &out_long[256], 4));
+}
--- a/security/nss/gtests/ssl_gtest/ssl_custext_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_custext_unittest.cc
@@ -127,17 +127,17 @@ TEST_F(TlsConnectStreamTls13, CustomExte
 }
 
 TEST_F(TlsConnectStreamTls13, CustomExtensionEmptyWriterServer) {
   EnsureTlsSetup();
   InstallManyWriters(server_, EmptyExtensionWriter);
   // Sending extensions that the client doesn't expect leads to extensions
   // appearing even if the client didn't send one, or in the wrong messages.
   client_->ExpectSendAlert(kTlsAlertUnsupportedExtension);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
 }
 
 // Install an writer to disable sending of a natively-supported extension.
 TEST_F(TlsConnectStreamTls13, CustomExtensionWriterDisable) {
   EnsureTlsSetup();
 
   // This option enables sending the extension via the native support.
@@ -345,17 +345,17 @@ TEST_F(TlsConnectStreamTls13, CustomExte
       server_->ssl_fd(), extension_code, NonsenseExtensionWriter, server_.get(),
       NoopExtensionHandler, nullptr);
   EXPECT_EQ(SECSuccess, rv);
 
   // Capture it to see what we got.
   auto capture = MakeTlsFilter<TlsExtensionCapture>(server_, extension_code);
 
   client_->ExpectSendAlert(kTlsAlertUnsupportedExtension);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
 
   EXPECT_TRUE(capture->captured());
   EXPECT_EQ(DataBuffer(kNonsenseExtension, sizeof(kNonsenseExtension)),
             capture->extension());
 }
 
 SECStatus RejectExtensionHandler(PRFileDesc *fd, SSLHandshakeType message,
@@ -396,17 +396,17 @@ TEST_F(TlsConnectStreamTls13, CustomExte
 
   // Have the server send nonsense.
   rv = SSL_InstallExtensionHooks(server_->ssl_fd(), extension_code,
                                  EmptyExtensionWriter, nullptr,
                                  NoopExtensionHandler, nullptr);
   EXPECT_EQ(SECSuccess, rv);
 
   client_->ExpectSendAlert(kTlsAlertHandshakeFailure);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
 }
 
 static const uint8_t kCustomAlert = 0xf6;
 
 SECStatus AlertExtensionHandler(PRFileDesc *fd, SSLHandshakeType message,
                                 const PRUint8 *data, unsigned int len,
                                 SSLAlertDescription *alert, void *arg) {
@@ -446,17 +446,17 @@ TEST_F(TlsConnectStreamTls13, CustomExte
 
   // Have the server send nonsense.
   rv = SSL_InstallExtensionHooks(server_->ssl_fd(), extension_code,
                                  EmptyExtensionWriter, nullptr,
                                  NoopExtensionHandler, nullptr);
   EXPECT_EQ(SECSuccess, rv);
 
   client_->ExpectSendAlert(kCustomAlert);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
 }
 
 // Configure a custom extension hook badly.
 TEST_F(TlsConnectStreamTls13, CustomExtensionOnlyWriter) {
   EnsureTlsSetup();
 
   // This installs an override that sends nothing but expects nonsense.
--- a/security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc
@@ -558,72 +558,72 @@ TEST_P(TlsExtensionTest13, EmptyClientKe
 // These tests only work in stream mode because the client sends a
 // cleartext alert which causes a MAC error on the server. With
 // stream this causes handshake failure but with datagram, the
 // packet gets dropped.
 TEST_F(TlsExtensionTest13Stream, DropServerKeyShare) {
   EnsureTlsSetup();
   MakeTlsFilter<TlsExtensionDropper>(server_, ssl_tls13_key_share_xtn);
   client_->ExpectSendAlert(kTlsAlertMissingExtension);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
   EXPECT_EQ(SSL_ERROR_MISSING_KEY_SHARE, client_->error_code());
-  EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code());
+  EXPECT_EQ(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, server_->error_code());
 }
 
 TEST_F(TlsExtensionTest13Stream, WrongServerKeyShare) {
   const uint16_t wrong_group = ssl_grp_ec_secp384r1;
 
   static const uint8_t key_share[] = {
       wrong_group >> 8,
       wrong_group & 0xff,  // Group we didn't offer.
       0x00,
       0x02,  // length = 2
       0x01,
       0x02};
   DataBuffer buf(key_share, sizeof(key_share));
   EnsureTlsSetup();
   MakeTlsFilter<TlsExtensionReplacer>(server_, ssl_tls13_key_share_xtn, buf);
   client_->ExpectSendAlert(kTlsAlertIllegalParameter);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
   EXPECT_EQ(SSL_ERROR_RX_MALFORMED_KEY_SHARE, client_->error_code());
-  EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code());
+  EXPECT_EQ(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, server_->error_code());
 }
 
 TEST_F(TlsExtensionTest13Stream, UnknownServerKeyShare) {
   const uint16_t wrong_group = 0xffff;
 
   static const uint8_t key_share[] = {
       wrong_group >> 8,
       wrong_group & 0xff,  // Group we didn't offer.
       0x00,
       0x02,  // length = 2
       0x01,
       0x02};
   DataBuffer buf(key_share, sizeof(key_share));
   EnsureTlsSetup();
   MakeTlsFilter<TlsExtensionReplacer>(server_, ssl_tls13_key_share_xtn, buf);
   client_->ExpectSendAlert(kTlsAlertIllegalParameter);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
   EXPECT_EQ(SSL_ERROR_RX_MALFORMED_KEY_SHARE, client_->error_code());
-  EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code());
+  EXPECT_EQ(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, server_->error_code());
 }
 
 TEST_F(TlsExtensionTest13Stream, AddServerSignatureAlgorithmsOnResumption) {
   SetupForResume();
   DataBuffer empty;
   MakeTlsFilter<TlsExtensionInjector>(server_, ssl_signature_algorithms_xtn,
                                       empty);
   client_->ExpectSendAlert(kTlsAlertUnsupportedExtension);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
   EXPECT_EQ(SSL_ERROR_EXTENSION_DISALLOWED_FOR_VERSION, client_->error_code());
-  EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code());
+  EXPECT_EQ(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, server_->error_code());
 }
 
 struct PskIdentity {
   DataBuffer identity;
   uint32_t obfuscated_ticket_age;
 };
 
 class TlsPreSharedKeyReplacer;
@@ -1020,17 +1020,17 @@ class TlsBogusExtensionTest13 : public T
   void FailWithAlert(uint8_t alert) {
     StartConnect();
     client_->Handshake();  // ClientHello
     server_->Handshake();  // ServerHello
 
     client_->ExpectSendAlert(alert);
     client_->Handshake();
     if (variant_ == ssl_variant_stream) {
-      server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+      server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
     }
     server_->Handshake();
   }
 };
 
 TEST_P(TlsBogusExtensionTestPre13, AddBogusExtensionServerHello) {
   Run(kTlsHandshakeServerHello);
 }
--- a/security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc
@@ -919,20 +919,20 @@ TEST_F(TlsConnectStreamTls13, RetryWithD
   // Force a HelloRetryRequest.
   static const std::vector<SSLNamedGroup> groups = {ssl_grp_ec_secp384r1};
   server_->ConfigNamedGroups(groups);
   // Then switch out the default suite (TLS_AES_128_GCM_SHA256).
   MakeTlsFilter<SelectedCipherSuiteReplacer>(server_,
                                              TLS_CHACHA20_POLY1305_SHA256);
 
   client_->ExpectSendAlert(kTlsAlertIllegalParameter);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
   EXPECT_EQ(SSL_ERROR_RX_MALFORMED_SERVER_HELLO, client_->error_code());
-  EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code());
+  EXPECT_EQ(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, server_->error_code());
 }
 
 // This tests that the second attempt at sending a ClientHello (after receiving
 // a HelloRetryRequest) is correctly retransmitted.
 TEST_F(TlsConnectDatagram13, DropClientSecondFlightWithHelloRetry) {
   static const std::vector<SSLNamedGroup> groups = {ssl_grp_ec_secp384r1,
                                                     ssl_grp_ec_secp521r1};
   server_->ConfigNamedGroups(groups);
--- a/security/nss/gtests/ssl_gtest/ssl_loopback_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_loopback_unittest.cc
@@ -125,17 +125,17 @@ TEST_P(TlsConnectTls13, CaptureAlertClie
   StartConnect();
 
   client_->Handshake();
   client_->ExpectSendAlert(kTlsAlertDecodeError);
   server_->Handshake();
   client_->Handshake();
   if (variant_ == ssl_variant_stream) {
     // DTLS just drops the alert it can't decrypt.
-    server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+    server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   }
   server_->Handshake();
   EXPECT_EQ(kTlsAlertFatal, alert_recorder->level());
   EXPECT_EQ(kTlsAlertDecodeError, alert_recorder->description());
 }
 
 TEST_P(TlsConnectGenericPre13, ConnectFalseStart) {
   client_->EnableFalseStart();
@@ -521,16 +521,36 @@ TEST_P(TlsConnectTls13, AlertWrongLevel)
   server_->ExpectSendAlert(kTlsAlertUnexpectedMessage, kTlsAlertWarning);
   client_->ExpectReceiveAlert(kTlsAlertUnexpectedMessage, kTlsAlertWarning);
   SSLInt_SendAlert(server_->ssl_fd(), kTlsAlertWarning,
                    kTlsAlertUnexpectedMessage);
   client_->ExpectReadWriteError();
   client_->WaitForErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT, 2000);
 }
 
+TEST_P(TlsConnectTls13, UnknownRecord) {
+  static const uint8_t kUknownRecord[] = {
+      0xff, SSL_LIBRARY_VERSION_TLS_1_2 >> 8,
+      SSL_LIBRARY_VERSION_TLS_1_2 & 0xff, 0, 0};
+
+  Connect();
+  if (variant_ == ssl_variant_stream) {
+    // DTLS just drops the record with an invalid type.
+    server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
+  }
+  client_->SendDirect(DataBuffer(kUknownRecord, sizeof(kUknownRecord)));
+  server_->ExpectReadWriteError();
+  server_->ReadBytes();
+  if (variant_ == ssl_variant_stream) {
+    EXPECT_EQ(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, server_->error_code());
+  } else {
+    EXPECT_EQ(SSL_ERROR_RX_UNKNOWN_RECORD_TYPE, server_->error_code());
+  }
+}
+
 TEST_F(TlsConnectStreamTls13, Tls13FailedWriteSecondFlight) {
   EnsureTlsSetup();
   StartConnect();
   client_->Handshake();
   server_->Handshake();  // Send first flight.
   client_->adapter()->SetWriteError(PR_IO_ERROR);
   client_->Handshake();  // This will get an error, but shouldn't crash.
   client_->CheckErrorCode(SSL_ERROR_SOCKET_WRITE_FAILURE);
--- a/security/nss/gtests/ssl_gtest/ssl_resumption_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_resumption_unittest.cc
@@ -648,26 +648,26 @@ TEST_P(TlsConnectStream, TestResumptionO
 
   Reset();
   ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
   MakeTlsFilter<SelectedCipherSuiteReplacer>(server_,
                                              ChooseAnotherCipher(version_));
 
   if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
     client_->ExpectSendAlert(kTlsAlertIllegalParameter);
-    server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+    server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   } else {
     ExpectAlert(client_, kTlsAlertHandshakeFailure);
   }
   ConnectExpectFail();
   client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
   if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
     // The reason this test is stream only: the server is unable to decrypt
     // the alert that the client sends, see bug 1304603.
-    server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
+    server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE);
   } else {
     server_->CheckErrorCode(SSL_ERROR_HANDSHAKE_FAILURE_ALERT);
   }
 }
 
 class SelectedVersionReplacer : public TlsHandshakeFilter {
  public:
   SelectedVersionReplacer(const std::shared_ptr<TlsAgent>& a, uint16_t version)
@@ -1041,20 +1041,20 @@ TEST_F(TlsConnectTest, TestTls13Resumpti
       server_, ssl_tls13_pre_shared_key_xtn));
   server_->SetFilter(std::make_shared<ChainedPacketFilter>(filters));
 
   // The client here generates an unexpected_message alert when it receives an
   // encrypted handshake message from the server (EncryptedExtension).  The
   // client expects to receive an unencrypted TLS 1.2 Certificate message.
   // The server can't decrypt the alert.
   client_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);  // Server can't read
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);  // Server can't read
   ConnectExpectFail();
   client_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_APPLICATION_DATA);
-  server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
+  server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE);
 }
 
 TEST_P(TlsConnectGenericResumption, ReConnectTicket) {
   ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
   server_->EnableSingleCipher(ChooseOneCipher(version_));
   Connect();
   SendReceive();
   CheckKeys(ssl_kea_ecdh, ssl_grp_ec_curve25519, ssl_auth_rsa_sign,
--- a/security/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
@@ -294,21 +294,21 @@ class TlsSessionIDInjectFilter : public 
   }
 };
 
 TEST_F(TlsConnectTest, TLS13NonCompatModeSessionID) {
   ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
 
   MakeTlsFilter<TlsSessionIDInjectFilter>(server_);
   client_->ExpectSendAlert(kTlsAlertIllegalParameter);
-  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
   ConnectExpectFail();
 
   client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
-  server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
+  server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE);
 }
 
 static const uint8_t kCannedCcs[] = {
     ssl_ct_change_cipher_spec,
     SSL_LIBRARY_VERSION_TLS_1_2 >> 8,
     SSL_LIBRARY_VERSION_TLS_1_2 & 0xff,
     0,
     1,  // length
@@ -357,16 +357,29 @@ TEST_F(TlsConnectStreamTls13, ChangeCiph
                            SSL_LIBRARY_VERSION_TLS_1_2);
   // Client sends CCS before starting the handshake.
   client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs)));
   ConnectExpectAlert(server_, kTlsAlertUnexpectedMessage);
   server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_CHANGE_CIPHER);
   client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
 }
 
+TEST_F(TlsConnectStreamTls13, ChangeCipherSpecAfterFinished13) {
+  EnsureTlsSetup();
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  SendReceive(10);
+  // Client sends CCS after the handshake.
+  client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs)));
+  server_->ExpectSendAlert(kTlsAlertUnexpectedMessage);
+  server_->ExpectReadWriteError();
+  server_->ReadBytes();
+  EXPECT_EQ(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, server_->error_code());
+}
+
 TEST_F(TlsConnectDatagram13, CompatModeDtlsClient) {
   EnsureTlsSetup();
   client_->SetOption(SSL_ENABLE_TLS13_COMPAT_MODE, PR_TRUE);
   auto client_records = MakeTlsFilter<TlsRecordRecorder>(client_);
   auto server_records = MakeTlsFilter<TlsRecordRecorder>(server_);
   Connect();
 
   ASSERT_EQ(2U, client_records->count());  // CH, Fin
--- a/security/nss/lib/freebl/mpi/mpi.c
+++ b/security/nss/lib/freebl/mpi/mpi.c
@@ -4770,46 +4770,69 @@ mp_to_signed_octets(const mp_int *mp, un
     }
     if (!pos)
         str[pos++] = 0;
     return pos;
 } /* end mp_to_signed_octets() */
 /* }}} */
 
 /* {{{ mp_to_fixlen_octets(mp, str) */
-/* output a buffer of big endian octets exactly as long as requested. */
+/* output a buffer of big endian octets exactly as long as requested.
+   constant time on the value of mp. */
 mp_err
 mp_to_fixlen_octets(const mp_int *mp, unsigned char *str, mp_size length)
 {
-    int ix, pos = 0;
+    int ix, jx;
     unsigned int bytes;
 
-    ARGCHK(mp != NULL && str != NULL && !SIGN(mp), MP_BADARG);
-
-    bytes = mp_unsigned_octet_size(mp);
-    ARGCHK(bytes <= length, MP_BADARG);
-
-    /* place any needed leading zeros */
-    for (; length > bytes; --length) {
-        *str++ = 0;
+    ARGCHK(mp != NULL, MP_BADARG);
+    ARGCHK(str != NULL, MP_BADARG);
+    ARGCHK(!SIGN(mp), MP_BADARG);
+    ARGCHK(length > 0, MP_BADARG);
+
+    /* Constant time on the value of mp.  Don't use mp_unsigned_octet_size. */
+    bytes = USED(mp) * MP_DIGIT_SIZE;
+
+    /* If the output is shorter than the native size of mp, then check that any
+     * bytes not written have zero values.  This check isn't constant time on
+     * the assumption that timing-sensitive callers can guarantee that mp fits
+     * in the allocated space. */
+    ix = USED(mp) - 1;
+    if (bytes > length) {
+        unsigned int zeros = bytes - length;
+
+        while (zeros >= MP_DIGIT_SIZE) {
+            ARGCHK(DIGIT(mp, ix) == 0, MP_BADARG);
+            zeros -= MP_DIGIT_SIZE;
+            ix--;
+        }
+
+        if (zeros > 0) {
+            mp_digit d = DIGIT(mp, ix);
+            mp_digit m = ~0ULL << ((MP_DIGIT_SIZE - zeros) * CHAR_BIT);
+            ARGCHK((d & m) == 0, MP_BADARG);
+            for (jx = MP_DIGIT_SIZE - zeros - 1; jx >= 0; jx--) {
+                *str++ = d >> (jx * CHAR_BIT);
+            }
+            ix--;
+        }
+    } else if (bytes < length) {
+        /* Place any needed leading zeros. */
+        unsigned int zeros = length - bytes;
+        memset(str, 0, zeros);
+        str += zeros;
     }
 
-    /* Iterate over each digit... */
-    for (ix = USED(mp) - 1; ix >= 0; ix--) {
+    /* Iterate over each whole digit... */
+    for (; ix >= 0; ix--) {
         mp_digit d = DIGIT(mp, ix);
-        int jx;
 
         /* Unpack digit bytes, high order first */
-        for (jx = sizeof(mp_digit) - 1; jx >= 0; jx--) {
-            unsigned char x = (unsigned char)(d >> (jx * CHAR_BIT));
-            if (!pos && !x) /* suppress leading zeros */
-                continue;
-            str[pos++] = x;
+        for (jx = MP_DIGIT_SIZE - 1; jx >= 0; jx--) {
+            *str++ = d >> (jx * CHAR_BIT);
         }
     }
-    if (!pos)
-        str[pos++] = 0;
     return MP_OKAY;
 } /* end mp_to_fixlen_octets() */
 /* }}} */
 
 /*------------------------------------------------------------------------*/
 /* HERE THERE BE DRAGONS                                                  */
--- a/security/nss/lib/freebl/mpi/mpi.h
+++ b/security/nss/lib/freebl/mpi/mpi.h
@@ -123,17 +123,18 @@ typedef long long mp_sword;
 #endif /* !defined(MP_NO_MP_WORD) */
 
 #if !defined(MP_WORD_MAX) && defined(MP_DEFINE_SMALL_WORD)
 typedef unsigned int mp_word;
 typedef int mp_sword;
 #define MP_WORD_MAX UINT_MAX
 #endif
 
-#define MP_DIGIT_BIT (CHAR_BIT * sizeof(mp_digit))
+#define MP_DIGIT_SIZE sizeof(mp_digit)
+#define MP_DIGIT_BIT (CHAR_BIT * MP_DIGIT_SIZE)
 #define MP_WORD_BIT (CHAR_BIT * sizeof(mp_word))
 #define MP_RADIX (1 + (mp_word)MP_DIGIT_MAX)
 
 #define MP_HALF_DIGIT_BIT (MP_DIGIT_BIT / 2)
 #define MP_HALF_RADIX (1 + (mp_digit)MP_HALF_DIGIT_MAX)
 /* MP_HALF_RADIX really ought to be called MP_SQRT_RADIX, but it's named
 ** MP_HALF_RADIX because it's the radix for MP_HALF_DIGITs, and it's
 ** consistent with the other _HALF_ names.
--- a/security/nss/lib/freebl/rsapkcs.c
+++ b/security/nss/lib/freebl/rsapkcs.c
@@ -933,58 +933,66 @@ failure:
 SECStatus
 RSA_DecryptBlock(RSAPrivateKey *key,
                  unsigned char *output,
                  unsigned int *outputLen,
                  unsigned int maxOutputLen,
                  const unsigned char *input,
                  unsigned int inputLen)
 {
-    SECStatus rv;
+    PRInt8 rv;
     unsigned int modulusLen = rsa_modulusLen(&key->modulus);
     unsigned int i;
-    unsigned char *buffer;
+    unsigned char *buffer = NULL;
+    unsigned int outLen = 0;
+    unsigned int copyOutLen = modulusLen - 11;
 
-    if (inputLen != modulusLen)
-        goto failure;
+    if (inputLen != modulusLen || modulusLen < 10) {
+        return SECFailure;
+    }
 
-    buffer = (unsigned char *)PORT_Alloc(modulusLen + 1);
-    if (!buffer)
-        goto failure;
+    if (copyOutLen > maxOutputLen) {
+        copyOutLen = maxOutputLen;
+    }
 
-    rv = RSA_PrivateKeyOp(key, buffer, input);
-    if (rv != SECSuccess)
-        goto loser;
+    // Allocate enough space to decrypt + copyOutLen to allow copying outLen later.
+    buffer = PORT_ZAlloc(modulusLen + 1 + copyOutLen);
+    if (!buffer) {
+        return SECFailure;
+    }
 
-    /* XXX(rsleevi): Constant time */
-    if (buffer[0] != RSA_BLOCK_FIRST_OCTET ||
-        buffer[1] != (unsigned char)RSA_BlockPublic) {
-        goto loser;
+    // rv is 0 if everything is going well and 1 if an error occurs.
+    rv = RSA_PrivateKeyOp(key, buffer, input) != SECSuccess;
+    rv |= (buffer[0] != RSA_BLOCK_FIRST_OCTET) |
+          (buffer[1] != (unsigned char)RSA_BlockPublic);
+
+    // There have to be at least 8 bytes of padding.
+    for (i = 2; i < 10; i++) {
+        rv |= buffer[i] == RSA_BLOCK_AFTER_PAD_OCTET;
     }
-    *outputLen = 0;
-    for (i = 2; i < modulusLen; i++) {
-        if (buffer[i] == RSA_BLOCK_AFTER_PAD_OCTET) {
-            *outputLen = modulusLen - i - 1;
-            break;
-        }
+
+    for (i = 10; i < modulusLen; i++) {
+        unsigned int newLen = modulusLen - i - 1;
+        unsigned int c = (buffer[i] == RSA_BLOCK_AFTER_PAD_OCTET) & (outLen == 0);
+        outLen = constantTimeCondition(c, newLen, outLen);
     }
-    if (*outputLen == 0)
-        goto loser;
-    if (*outputLen > maxOutputLen)
-        goto loser;
+    rv |= outLen == 0;
+    rv |= outLen > maxOutputLen;
 
-    PORT_Memcpy(output, buffer + modulusLen - *outputLen, *outputLen);
+    // Note that output is set even if SECFailure is returned.
+    PORT_Memcpy(output, buffer + modulusLen - outLen, copyOutLen);
+    *outputLen = constantTimeCondition(outLen > maxOutputLen, maxOutputLen,
+                                       outLen);
 
     PORT_Free(buffer);
-    return SECSuccess;
 
-loser:
-    PORT_Free(buffer);
-failure:
-    return SECFailure;
+    for (i = 1; i < sizeof(rv) * 8; i <<= 1) {
+        rv |= rv << i;
+    }
+    return (SECStatus)rv;
 }
 
 /*
  * Encode a RSA-PSS signature.
  * Described in RFC 3447, section 9.1.1.
  * We use mHash instead of M as input.
  * emBits from the RFC is just modBits - 1, see section 8.1.1.
  * We only support MGF1 as the MGF.
--- a/security/nss/lib/ssl/SSLerrs.h
+++ b/security/nss/lib/ssl/SSLerrs.h
@@ -556,8 +556,11 @@ ER3(SSL_ERROR_DH_KEY_TOO_LONG, (SSL_ERRO
 ER3(SSL_ERROR_RX_MALFORMED_ESNI_KEYS, (SSL_ERROR_BASE + 176),
     "SSL received a malformed ESNI keys structure")
 
 ER3(SSL_ERROR_RX_MALFORMED_ESNI_EXTENSION, (SSL_ERROR_BASE + 177),
     "SSL received a malformed ESNI extension")
 
 ER3(SSL_ERROR_MISSING_ESNI_EXTENSION, (SSL_ERROR_BASE + 178),
     "SSL did not receive an ESNI extension")
+
+ER3(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE, (SSL_ERROR_BASE + 179),
+    "SSL received an unexpected record type.")
--- a/security/nss/lib/ssl/ssl3con.c
+++ b/security/nss/lib/ssl/ssl3con.c
@@ -9774,16 +9774,33 @@ ssl3_GenerateRSAPMS(sslSocket *ss, ssl3C
     if (!serverKeySlot)
         PK11_FreeSlot(slot);
     if (pms == NULL) {
         ssl_MapLowLevelError(SSL_ERROR_CLIENT_KEY_EXCHANGE_FAILURE);
     }
     return pms;
 }
 
+static void
+ssl3_CSwapPK11SymKey(PK11SymKey **x, PK11SymKey **y, PRBool c)
+{
+    uintptr_t mask = (uintptr_t)c;
+    unsigned int i;
+    for (i = 1; i < sizeof(uintptr_t) * 8; i <<= 1) {
+        mask |= mask << i;
+    }
+    uintptr_t x_ptr = (uintptr_t)*x;
+    uintptr_t y_ptr = (uintptr_t)*y;
+    uintptr_t tmp = (x_ptr ^ y_ptr) & mask;
+    x_ptr = x_ptr ^ tmp;
+    y_ptr = y_ptr ^ tmp;
+    *x = (PK11SymKey *)x_ptr;
+    *y = (PK11SymKey *)y_ptr;
+}
+
 /* Note: The Bleichenbacher attack on PKCS#1 necessitates that we NEVER
  * return any indication of failure of the Client Key Exchange message,
  * where that failure is caused by the content of the client's message.
  * This function must not return SECFailure for any reason that is directly
  * or indirectly caused by the content of the client's encrypted PMS.
  * We must not send an alert and also not drop the connection.
  * Instead, we generate a random PMS.  This will cause a failure
  * in the processing the finished message, which is exactly where
@@ -9794,19 +9811,19 @@ ssl3_GenerateRSAPMS(sslSocket *ss, ssl3C
 static SECStatus
 ssl3_HandleRSAClientKeyExchange(sslSocket *ss,
                                 PRUint8 *b,
                                 PRUint32 length,
                                 sslKeyPair *serverKeyPair)
 {
     SECStatus rv;
     SECItem enc_pms;
-    PK11SymKey *tmpPms[2] = { NULL, NULL };
-    PK11SlotInfo *slot;
-    int useFauxPms = 0;
+    PK11SymKey *pms = NULL;
+    PK11SymKey *fauxPms = NULL;
+    PK11SlotInfo *slot = NULL;
 
     PORT_Assert(ss->opt.noLocks || ssl_HaveRecvBufLock(ss));
     PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
     PORT_Assert(ss->ssl3.prSpec->epoch == ss->ssl3.pwSpec->epoch);
 
     enc_pms.data = b;
     enc_pms.len = length;
 
@@ -9817,21 +9834,16 @@ ssl3_HandleRSAClientKeyExchange(sslSocke
             PORT_SetError(SSL_ERROR_CLIENT_KEY_EXCHANGE_FAILURE);
             return SECFailure;
         }
         if ((unsigned)kLen < enc_pms.len) {
             enc_pms.len = kLen;
         }
     }
 
-#define currentPms tmpPms[!useFauxPms]
-#define unusedPms tmpPms[useFauxPms]
-#define realPms tmpPms[1]
-#define fauxPms tmpPms[0]
-
     /*
      * Get as close to algorithm 2 from RFC 5246; Section 7.4.7.1
      * as we can within the constraints of the PKCS#11 interface.
      *
      * 1. Unconditionally generate a bogus PMS (what RFC 5246
      *    calls R).
      * 2. Attempt the RSA decryption to recover the PMS (what
      *    RFC 5246 calls M).
@@ -9876,50 +9888,43 @@ ssl3_HandleRSAClientKeyExchange(sslSocke
     }
 
     /*
      * unwrap pms out of the incoming buffer
      * Note: CKM_SSL3_MASTER_KEY_DERIVE is NOT the mechanism used to do
      *  the unwrap.  Rather, it is the mechanism with which the
      *      unwrapped pms will be used.
      */
-    realPms = PK11_PubUnwrapSymKey(serverKeyPair->privKey, &enc_pms,
-                                   CKM_SSL3_MASTER_KEY_DERIVE, CKA_DERIVE, 0);
+    pms = PK11_PubUnwrapSymKey(serverKeyPair->privKey, &enc_pms,
+                               CKM_SSL3_MASTER_KEY_DERIVE, CKA_DERIVE, 0);
     /* Temporarily use the PMS if unwrapping the real PMS fails. */
-    useFauxPms |= (realPms == NULL);
+    ssl3_CSwapPK11SymKey(&pms, &fauxPms, pms == NULL);
 
     /* Attempt to derive the MS from the PMS. This is the only way to
      * check the version field in the RSA PMS. If this fails, we
      * then use the faux PMS in place of the PMS. Note that this
      * operation should never fail if we are using the faux PMS
      * since it is correctly formatted. */
-    rv = ssl3_ComputeMasterSecret(ss, currentPms, NULL);
-
-    /* If we succeeded, then select the true PMS and discard the
-     * FPMS. Else, select the FPMS and select the true PMS */
-    useFauxPms |= (rv != SECSuccess);
-
-    if (unusedPms) {
-        PK11_FreeSymKey(unusedPms);
-    }
+    rv = ssl3_ComputeMasterSecret(ss, pms, NULL);
+
+    /* If we succeeded, then select the true PMS, else select the FPMS. */
+    ssl3_CSwapPK11SymKey(&pms, &fauxPms, (rv != SECSuccess) & (fauxPms != NULL));
 
     /* This step will derive the MS from the PMS, among other things. */
-    rv = ssl3_InitPendingCipherSpecs(ss, currentPms, PR_TRUE);
-    PK11_FreeSymKey(currentPms);
+    rv = ssl3_InitPendingCipherSpecs(ss, pms, PR_TRUE);
+
+    /* Clear both PMS. */
+    PK11_FreeSymKey(pms);
+    PK11_FreeSymKey(fauxPms);
 
     if (rv != SECSuccess) {
         (void)SSL3_SendAlert(ss, alert_fatal, handshake_failure);
         return SECFailure; /* error code set by ssl3_InitPendingCipherSpec */
     }
 
-#undef currentPms
-#undef unusedPms
-#undef realPms
-#undef fauxPms
-
     return SECSuccess;
 }
 
 static SECStatus
 ssl3_HandleDHClientKeyExchange(sslSocket *ss,
                                PRUint8 *b,
                                PRUint32 length,
                                sslKeyPair *serverKeyPair)
--- a/security/nss/lib/ssl/sslerr.h
+++ b/security/nss/lib/ssl/sslerr.h
@@ -262,15 +262,16 @@ typedef enum {
     SSL_ERROR_TOO_MANY_KEY_UPDATES = (SSL_ERROR_BASE + 171),
     SSL_ERROR_HANDSHAKE_FAILED = (SSL_ERROR_BASE + 172),
     SSL_ERROR_BAD_RESUMPTION_TOKEN_ERROR = (SSL_ERROR_BASE + 173),
     SSL_ERROR_RX_MALFORMED_DTLS_ACK = (SSL_ERROR_BASE + 174),
     SSL_ERROR_DH_KEY_TOO_LONG = (SSL_ERROR_BASE + 175),
     SSL_ERROR_RX_MALFORMED_ESNI_KEYS = (SSL_ERROR_BASE + 176),
     SSL_ERROR_RX_MALFORMED_ESNI_EXTENSION = (SSL_ERROR_BASE + 177),
     SSL_ERROR_MISSING_ESNI_EXTENSION = (SSL_ERROR_BASE + 178),
+    SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE = (SSL_ERROR_BASE + 179),
     SSL_ERROR_END_OF_LIST   /* let the c compiler determine the value of this. */
 } SSLErrorCodes;
 #endif /* NO_SECURITY_ERROR_ENUM */
 
 /* clang-format on */
 
 #endif /* __SSL_ERR_H_ */
--- a/security/nss/lib/ssl/tls13con.c
+++ b/security/nss/lib/ssl/tls13con.c
@@ -5011,36 +5011,36 @@ tls13_UnprotectRecord(sslSocket *ss,
 
     *alert = bad_record_mac; /* Default alert for most issues. */
 
     PORT_Assert(spec->direction == CipherSpecRead);
     SSL_TRC(3, ("%d: TLS13[%d]: spec=%d epoch=%d (%s) unprotect 0x%0llx len=%u",
                 SSL_GETPID(), ss->fd, spec, spec->epoch, spec->phase,
                 cText->seqNum, cText->buf->len));
 
-    /* We can perform this test in variable time because the record's total
-     * length and the ciphersuite are both public knowledge. */
-    if (cText->buf->len < cipher_def->tag_size) {
-        SSL_TRC(3,
-                ("%d: TLS13[%d]: record too short to contain valid AEAD data",
-                 SSL_GETPID(), ss->fd));
-        PORT_SetError(SSL_ERROR_BAD_MAC_READ);
-        return SECFailure;
-    }
-
     /* Verify that the content type is right, even though we overwrite it.
      * Also allow the DTLS short header in TLS 1.3. */
     if (!(cText->hdr[0] == ssl_ct_application_data ||
           (IS_DTLS(ss) &&
            ss->version >= SSL_LIBRARY_VERSION_TLS_1_3 &&
            (cText->hdr[0] & 0xe0) == 0x20))) {
         SSL_TRC(3,
                 ("%d: TLS13[%d]: record has invalid exterior type=%2.2x",
                  SSL_GETPID(), ss->fd, cText->hdr[0]));
-        /* Do we need a better error here? */
+        PORT_SetError(SSL_ERROR_RX_UNEXPECTED_RECORD_TYPE);
+        *alert = unexpected_message;
+        return SECFailure;
+    }
+
+    /* We can perform this test in variable time because the record's total
+     * length and the ciphersuite are both public knowledge. */
+    if (cText->buf->len < cipher_def->tag_size) {
+        SSL_TRC(3,
+                ("%d: TLS13[%d]: record too short to contain valid AEAD data",
+                 SSL_GETPID(), ss->fd));
         PORT_SetError(SSL_ERROR_BAD_MAC_READ);
         return SECFailure;
     }
 
     /* Check the version number in the record. Stream only. */
     if (!IS_DTLS(ss)) {
         SSL3ProtocolVersion version =
             ((SSL3ProtocolVersion)cText->hdr[1] << 8) |