Bug 1372001 - Refactor out encrypt-to-self into a self-contained

interface. r=mt Summary: This cleans up the ticket encryption code and also prepares us for encrypt-to-self for hash state with HRR. Note that I eventually plan to move the self encrypt management functions from sslsnce.c, but I want to do it in a followup patch to make seeing the changes here (mostly rename in that block) easier. Reviewers: mt Differential Revision: https://nss-review.dev.mozaws.net/D329
sailfishos-mirror · Jun 11, 2017 · 8cb4c93 · 8cb4c93
1 parent 1a5850c
commit 8cb4c93
Show file tree

Hide file tree

Showing 21 changed files with 958 additions and 548 deletions.
diff --git a/gtests/ssl_gtest/libssl_internals.c b/gtests/ssl_gtest/libssl_internals.c
@@ -10,6 +10,7 @@
 #include "nss.h"
 #include "pk11pub.h"
 #include "seccomon.h"
+#include "selfencrypt.h"
 
 SECStatus SSLInt_IncrementClientHandshakeVersion(PRFileDesc *fd) {
   sslSocket *ss = ssl_FindSocket(fd);
@@ -55,7 +56,16 @@ PRBool SSLInt_ExtensionNegotiated(PRFileDesc *fd, PRUint16 ext) {
   return (PRBool)(ss && ssl3_ExtensionNegotiated(ss, ext));
 }
 
-void SSLInt_ClearSessionTicketKey() { ssl_ResetSessionTicketKeys(); }
+void SSLInt_ClearSelfEncryptKey() { ssl_ResetSelfEncryptKeys(); }
+
+sslSelfEncryptKeys *ssl_GetSelfEncryptKeysInt();
+
+void SSLInt_SetSelfEncryptMacKey(PK11SymKey *key) {
+  sslSelfEncryptKeys *keys = ssl_GetSelfEncryptKeysInt();
+
+  PK11_FreeSymKey(keys->macKey);
+  keys->macKey = key;
+}
 
 SECStatus SSLInt_SetMTU(PRFileDesc *fd, PRUint16 mtu) {
   sslSocket *ss = ssl_FindSocket(fd);

diff --git a/gtests/ssl_gtest/libssl_internals.h b/gtests/ssl_gtest/libssl_internals.h
@@ -22,7 +22,8 @@ SECStatus SSLInt_UpdateSSLv2ClientRandom(PRFileDesc *fd, uint8_t *rnd,
                                          size_t msg_len);
 
 PRBool SSLInt_ExtensionNegotiated(PRFileDesc *fd, PRUint16 ext);
-void SSLInt_ClearSessionTicketKey();
+void SSLInt_ClearSelfEncryptKey();
+void SSLInt_SetSelfEncryptMacKey(PK11SymKey *key);
 PRInt32 SSLInt_CountTls13CipherSpecs(PRFileDesc *fd);
 void SSLInt_PrintTls13CipherSpecs(PRFileDesc *fd);
 void SSLInt_ForceTimerExpiry(PRFileDesc *fd);

diff --git a/gtests/ssl_gtest/manifest.mn b/gtests/ssl_gtest/manifest.mn
@@ -40,6 +40,7 @@ CPPSRCS = \
       ssl_v2_client_hello_unittest.cc \
       ssl_version_unittest.cc \
       ssl_versionpolicy_unittest.cc \
+      selfencrypt_unittest.cc \
       test_io.cc \
       tls_agent.cc \
       tls_connect.cc \

diff --git a/gtests/ssl_gtest/selfencrypt_unittest.cc b/gtests/ssl_gtest/selfencrypt_unittest.cc
@@ -0,0 +1,279 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* 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 <functional>
+#include <memory>
+#include "nss.h"
+#include "pk11pub.h"
+#include "prerror.h"
+#include "secerr.h"
+#include "sslerr.h"
+extern "C" {
+#include "selfencrypt.h"
+}
+
+#include "databuffer.h"
+#include "gtest_utils.h"
+#include "scoped_ptrs.h"
+
+namespace nss_test {
+
+static const uint8_t kAesKey1Buf[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05,
+                                      0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
+                                      0x0c, 0x0d, 0x0e, 0x0f};
+static const DataBuffer kAesKey1(kAesKey1Buf, sizeof(kAesKey1Buf));
+
+static const uint8_t kAesKey2Buf[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15,
+                                      0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b,
+                                      0x1c, 0x1d, 0x1e, 0x1f};
+static const DataBuffer kAesKey2(kAesKey2Buf, sizeof(kAesKey2Buf));
+
+static const uint8_t kHmacKey1Buf[] = {
+    0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a,
+    0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15,
+    0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f};
+static const DataBuffer kHmacKey1(kHmacKey1Buf, sizeof(kHmacKey1Buf));
+
+static const uint8_t kHmacKey2Buf[] = {
+    0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a,
+    0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23, 0x24, 0x25,
+    0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f};
+static const DataBuffer kHmacKey2(kHmacKey2Buf, sizeof(kHmacKey2Buf));
+
+static const uint8_t* kKeyName1 =
+    reinterpret_cast<const unsigned char*>("KEY1KEY1KEY1KEY1");
+static const uint8_t* kKeyName2 =
+    reinterpret_cast<const uint8_t*>("KEY2KEY2KEY2KEY2");
+
+static void ImportKey(const DataBuffer& key, PK11SlotInfo* slot,
+                      CK_MECHANISM_TYPE mech, CK_ATTRIBUTE_TYPE cka,
+                      ScopedPK11SymKey* to) {
+  SECItem key_item = {siBuffer, const_cast<uint8_t*>(key.data()),
+                      static_cast<unsigned int>(key.len())};
+
+  PK11SymKey* inner =
+      PK11_ImportSymKey(slot, mech, PK11_OriginUnwrap, cka, &key_item, nullptr);
+  ASSERT_NE(nullptr, inner);
+  to->reset(inner);
+}
+
+extern "C" {
+extern char ssl_trace;
+extern FILE* ssl_trace_iob;
+}
+
+class SelfEncryptTestBase : public ::testing::Test {
+ public:
+  SelfEncryptTestBase(size_t message_size)
+      : aes1_(),
+        aes2_(),
+        hmac1_(),
+        hmac2_(),
+        message_(),
+        slot_(PK11_GetInternalSlot()) {
+    EXPECT_NE(nullptr, slot_);
+    char* ev = getenv("SSLTRACE");
+    if (ev && ev[0]) {
+      ssl_trace = atoi(ev);
+      ssl_trace_iob = stderr;
+    }
+    message_.Allocate(message_size);
+    for (size_t i = 0; i < message_.len(); ++i) {
+      message_.data()[i] = i;
+    }
+  }
+
+  void SetUp() {
+    message_.Allocate(100);
+    for (size_t i = 0; i < 100; ++i) {
+      message_.data()[i] = i;
+    }
+    ImportKey(kAesKey1, slot_.get(), CKM_AES_CBC, CKA_ENCRYPT, &aes1_);
+    ImportKey(kAesKey2, slot_.get(), CKM_AES_CBC, CKA_ENCRYPT, &aes2_);
+    ImportKey(kHmacKey1, slot_.get(), CKM_SHA256_HMAC, CKA_SIGN, &hmac1_);
+    ImportKey(kHmacKey2, slot_.get(), CKM_SHA256_HMAC, CKA_SIGN, &hmac2_);
+  }
+
+  void SelfTest(
+      const uint8_t* writeKeyName, const ScopedPK11SymKey& writeAes,
+      const ScopedPK11SymKey& writeHmac, const uint8_t* readKeyName,
+      const ScopedPK11SymKey& readAes, const ScopedPK11SymKey& readHmac,
+      PRErrorCode protect_error_code = 0, PRErrorCode unprotect_error_code = 0,
+      std::function<void(uint8_t* ciphertext, unsigned int* ciphertext_len)>
+          mutate = nullptr) {
+    uint8_t ciphertext[1000];
+    unsigned int ciphertext_len;
+    uint8_t plaintext[1000];
+    unsigned int plaintext_len;
+
+    SECStatus rv = ssl_SelfEncryptProtectInt(
+        writeAes.get(), writeHmac.get(), writeKeyName, message_.data(),
+        message_.len(), ciphertext, &ciphertext_len, sizeof(ciphertext));
+    if (rv != SECSuccess) {
+      std::cerr << "Error: " << PORT_ErrorToName(PORT_GetError()) << std::endl;
+    }
+    if (protect_error_code) {
+      ASSERT_EQ(protect_error_code, PORT_GetError());
+      return;
+    }
+    ASSERT_EQ(SECSuccess, rv);
+
+    if (mutate) {
+      mutate(ciphertext, &ciphertext_len);
+    }
+    rv = ssl_SelfEncryptUnprotectInt(readAes.get(), readHmac.get(), readKeyName,
+                                     ciphertext, ciphertext_len, plaintext,
+                                     &plaintext_len, sizeof(plaintext));
+    if (rv != SECSuccess) {
+      std::cerr << "Error: " << PORT_ErrorToName(PORT_GetError()) << std::endl;
+    }
+    if (!unprotect_error_code) {
+      ASSERT_EQ(SECSuccess, rv);
+      EXPECT_EQ(message_.len(), plaintext_len);
+      EXPECT_EQ(0, memcmp(message_.data(), plaintext, message_.len()));
+    } else {
+      ASSERT_EQ(SECFailure, rv);
+      EXPECT_EQ(unprotect_error_code, PORT_GetError());
+    }
+  }
+
+ protected:
+  ScopedPK11SymKey aes1_;
+  ScopedPK11SymKey aes2_;
+  ScopedPK11SymKey hmac1_;
+  ScopedPK11SymKey hmac2_;
+  DataBuffer message_;
+
+ private:
+  ScopedPK11SlotInfo slot_;
+};
+
+class SelfEncryptTestVariable : public SelfEncryptTestBase,
+                                public ::testing::WithParamInterface<size_t> {
+ public:
+  SelfEncryptTestVariable() : SelfEncryptTestBase(GetParam()) {}
+};
+
+class SelfEncryptTest128 : public SelfEncryptTestBase {
+ public:
+  SelfEncryptTest128() : SelfEncryptTestBase(128) {}
+};
+
+TEST_P(SelfEncryptTestVariable, SuccessCase) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_);
+}
+
+TEST_P(SelfEncryptTestVariable, WrongMacKey) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac2_, 0,
+           SEC_ERROR_BAD_DATA);
+}
+
+TEST_P(SelfEncryptTestVariable, WrongKeyName) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName2, aes1_, hmac1_, 0,
+           SEC_ERROR_NOT_A_RECIPIENT);
+}
+
+TEST_P(SelfEncryptTestVariable, AddAByte) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             (*ciphertext_len)++;
+           });
+}
+
+TEST_P(SelfEncryptTestVariable, SubtractAByte) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             (*ciphertext_len)--;
+           });
+}
+
+TEST_P(SelfEncryptTestVariable, BogusIv) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             ciphertext[16]++;
+           });
+}
+
+TEST_P(SelfEncryptTestVariable, BogusCiphertext) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             ciphertext[32]++;
+           });
+}
+
+TEST_P(SelfEncryptTestVariable, BadMac) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             ciphertext[*ciphertext_len - 1]++;
+           });
+}
+
+TEST_F(SelfEncryptTest128, DISABLED_BadPadding) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes2_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA);
+}
+
+TEST_F(SelfEncryptTest128, ShortKeyName) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             *ciphertext_len = 15;
+           });
+}
+
+TEST_F(SelfEncryptTest128, ShortIv) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             *ciphertext_len = 31;
+           });
+}
+
+TEST_F(SelfEncryptTest128, ShortCiphertextLen) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             *ciphertext_len = 32;
+           });
+}
+
+TEST_F(SelfEncryptTest128, ShortCiphertext) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, hmac1_, 0,
+           SEC_ERROR_BAD_DATA,
+           [](uint8_t* ciphertext, unsigned int* ciphertext_len) {
+             *ciphertext_len -= 17;
+           });
+}
+
+TEST_F(SelfEncryptTest128, MacWithAESKeyEncrypt) {
+  SelfTest(kKeyName1, aes1_, aes1_, kKeyName1, aes1_, hmac1_,
+           SEC_ERROR_LIBRARY_FAILURE);
+}
+
+TEST_F(SelfEncryptTest128, AESWithMacKeyEncrypt) {
+  SelfTest(kKeyName1, hmac1_, hmac1_, kKeyName1, aes1_, hmac1_,
+           SEC_ERROR_INVALID_KEY);
+}
+
+TEST_F(SelfEncryptTest128, MacWithAESKeyDecrypt) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, aes1_, aes1_, 0,
+           SEC_ERROR_LIBRARY_FAILURE);
+}
+
+TEST_F(SelfEncryptTest128, AESWithMacKeyDecrypt) {
+  SelfTest(kKeyName1, aes1_, hmac1_, kKeyName1, hmac1_, hmac1_, 0,
+           SEC_ERROR_INVALID_KEY);
+}
+
+INSTANTIATE_TEST_CASE_P(VariousSizes, SelfEncryptTestVariable,
+                        ::testing::Values(0, 15, 16, 31, 255, 256, 257));
+
+}  // namespace nss_test
diff --git a/gtests/ssl_gtest/ssl_fuzz_unittest.cc b/gtests/ssl_gtest/ssl_fuzz_unittest.cc
@@ -281,13 +281,8 @@ FUZZ_P(TlsConnectGeneric, UnencryptedSessionTickets) {
     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 */
-
+  offset += 2 + /* ticket 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));

diff --git a/gtests/ssl_gtest/ssl_gtest.gyp b/gtests/ssl_gtest/ssl_gtest.gyp
@@ -12,6 +12,7 @@
       'type': 'executable',
       'sources': [
         'libssl_internals.c',
+        'selfencrypt_unittest.cc',
         'ssl_0rtt_unittest.cc',
         'ssl_agent_unittest.cc',
         'ssl_auth_unittest.cc',

diff --git a/gtests/ssl_gtest/ssl_resumption_unittest.cc b/gtests/ssl_gtest/ssl_resumption_unittest.cc
@@ -258,6 +258,30 @@ TEST_P(TlsConnectGeneric, ConnectWithExpiredTicketAtServer) {
   CheckConnected();
 }
 
+TEST_P(TlsConnectGeneric, ConnectResumeCorruptTicket) {
+  // This causes a ticket resumption.
+  ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
+  Connect();
+  SendReceive();
+
+  Reset();
+  static const uint8_t kHmacKey1Buf[32] = {0};
+  static const DataBuffer kHmacKey1(kHmacKey1Buf, sizeof(kHmacKey1Buf));
+
+  SECItem key_item = {siBuffer, const_cast<uint8_t*>(kHmacKey1Buf),
+                      sizeof(kHmacKey1Buf)};
+
+  ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
+  PK11SymKey* hmac_key =
+      PK11_ImportSymKey(slot.get(), CKM_SHA256_HMAC, PK11_OriginUnwrap,
+                        CKA_SIGN, &key_item, nullptr);
+  ASSERT_NE(nullptr, hmac_key);
+  SSLInt_SetSelfEncryptMacKey(hmac_key);
+  ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
+  ConnectExpectAlert(server_, illegal_parameter);
+  server_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_CLIENT_HELLO);
+}
+
 // This callback switches out the "server" cert used on the server with
 // the "client" certificate, which should be the same type.
 static int32_t SwitchCertificates(TlsAgent* agent, const SECItem* srvNameArr,
@@ -627,7 +651,7 @@ TEST_F(TlsConnectTest, TestTls13ResumptionDuplicateNST) {
   Connect();
 
   // Clear the session ticket keys to invalidate the old ticket.
-  SSLInt_ClearSessionTicketKey();
+  SSLInt_ClearSelfEncryptKey();
   SSLInt_SendNewSessionTicket(server_->ssl_fd());
 
   SendReceive();  // Need to read so that we absorb the session tickets.