Bug 1599514 - Update DTLS 1.3 support to draft-30 r=mt

This patch updates the DTLS 1.3 implementation to draft version 30, including unified header format and sequence number encryption.

Also added are new `SSL_CreateMask` experimental functions.

Differential Revision: https://phabricator.services.mozilla.com/D51014

--HG--
rename : gtests/ssl_gtest/ssl_primitive_unittest.cc => gtests/ssl_gtest/ssl_aead_unittest.cc
extra : moz-landing-system : lando

cpputil/databuffer.h

@@ -23,6 +23,7 @@ class DataBuffer {
   DataBuffer(const DataBuffer& other) : data_(nullptr), len_(0) {
     Assign(other);
   }
+  explicit DataBuffer(size_t l) : data_(nullptr), len_(0) { Allocate(l); }
   ~DataBuffer() { delete[] data_; }
 
   DataBuffer& operator=(const DataBuffer& other) {

cpputil/scoped_ptrs_ssl.h

@@ -12,6 +12,7 @@
 
 struct ScopedDeleteSSL {
   void operator()(SSLAeadContext* ctx) { SSL_DestroyAead(ctx); }
+  void operator()(SSLMaskingContext* ctx) { SSL_DestroyMaskingContext(ctx); }
   void operator()(SSLAntiReplayContext* ctx) {
     SSL_ReleaseAntiReplayContext(ctx);
   }
@@ -34,6 +35,7 @@ struct ScopedMaybeDeleteSSL {
 
 SCOPED(SSLAeadContext);
 SCOPED(SSLAntiReplayContext);
+SCOPED(SSLMaskingContext);
 SCOPED(SSLResumptionTokenInfo);
 
 #undef SCOPED

cpputil/tls_parser.h

@@ -74,6 +74,11 @@ const uint8_t kTlsFakeChangeCipherSpec[] = {
     0x01   // Value
 };
 
+const uint8_t kCtDtlsCiphertext = 0x20;
+const uint8_t kCtDtlsCiphertextMask = 0xE0;
+const uint8_t kCtDtlsCiphertext16bSeqno = 0x08;
+const uint8_t kCtDtlsCiphertextLengthPresent = 0x04;
+
 static const uint8_t kTls13PskKe = 0;
 static const uint8_t kTls13PskDhKe = 1;
 static const uint8_t kTls13PskAuth = 0;

gtests/ssl_gtest/manifest.mn

@@ -14,6 +14,7 @@ CSRCS = \
 CPPSRCS = \
       bloomfilter_unittest.cc \
       ssl_0rtt_unittest.cc \
+      ssl_aead_unittest.cc \
       ssl_agent_unittest.cc \
       ssl_auth_unittest.cc \
       ssl_cert_ext_unittest.cc \
@@ -35,8 +36,8 @@ CPPSRCS = \
       ssl_hrr_unittest.cc \
       ssl_keyupdate_unittest.cc \
       ssl_loopback_unittest.cc \
+      ssl_masking_unittest.cc \
       ssl_misc_unittest.cc \
-      ssl_primitive_unittest.cc \
       ssl_record_unittest.cc \
       ssl_recordsep_unittest.cc \
       ssl_recordsize_unittest.cc \

gtests/ssl_gtest/ssl_primitive_unittest.cc → gtests/ssl_gtest/ssl_aead_unittest.cc

@@ -54,7 +54,7 @@ class AeadTest : public ::testing::Test {
     ASSERT_GE(kMaxSize, ciphertext_len);
     ASSERT_LT(0U, ciphertext_len);
 
-    uint8_t output[kMaxSize];
+    uint8_t output[kMaxSize] = {0};
     unsigned int output_len = 0;
     EXPECT_EQ(SECSuccess, SSL_AeadEncrypt(ctx.get(), 0, kAad, sizeof(kAad),
                                           kPlaintext, sizeof(kPlaintext),
@@ -181,7 +181,7 @@ TEST_F(AeadTest, AeadNoPointer) {
 }
 
 TEST_F(AeadTest, AeadAes128Gcm) {
-  SSLAeadContext *ctxInit;
+  SSLAeadContext *ctxInit = nullptr;
   ASSERT_EQ(SECSuccess,
             SSL_MakeAead(SSL_LIBRARY_VERSION_TLS_1_3, TLS_AES_128_GCM_SHA256,
                          secret_.get(), kLabel, strlen(kLabel), &ctxInit));
@@ -203,7 +203,7 @@ TEST_F(AeadTest, AeadAes256Gcm) {
 }
 
 TEST_F(AeadTest, AeadChaCha20Poly1305) {
-  SSLAeadContext *ctxInit;
+  SSLAeadContext *ctxInit = nullptr;
   ASSERT_EQ(
       SECSuccess,
       SSL_MakeAead(SSL_LIBRARY_VERSION_TLS_1_3, TLS_CHACHA20_POLY1305_SHA256,

gtests/ssl_gtest/ssl_ciphersuite_unittest.cc

@@ -263,6 +263,7 @@ TEST_P(TlsCipherSuiteTest, ResumeCipherSuite) {
 TEST_P(TlsCipherSuiteTest, ReadLimit) {
   SetupCertificate();
   EnableSingleCipher();
+  TlsSendCipherSpecCapturer capturer(client_);
   ConnectAndCheckCipherSuite();
   if (version_ < SSL_LIBRARY_VERSION_TLS_1_3) {
     uint64_t last = last_safe_write();
@@ -295,9 +296,31 @@ TEST_P(TlsCipherSuiteTest, ReadLimit) {
   } else {
     epoch = 0;
   }
-  TlsAgentTestBase::MakeRecord(variant_, ssl_ct_application_data, version_,
-                               payload, sizeof(payload), &record,
-                               (epoch << 48) | record_limit());
+
+  uint64_t seqno = (epoch << 48) | record_limit();
+
+  // DTLS 1.3 masks the sequence number
+  if (variant_ == ssl_variant_datagram &&
+      version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    auto spec = capturer.spec(1);
+    ASSERT_NE(nullptr, spec.get());
+    ASSERT_EQ(3, spec->epoch());
+
+    DataBuffer pt, ct;
+    uint8_t dtls13_ctype = kCtDtlsCiphertext | kCtDtlsCiphertext16bSeqno |
+                           kCtDtlsCiphertextLengthPresent;
+    TlsRecordHeader hdr(variant_, version_, dtls13_ctype, seqno);
+    pt.Assign(payload, sizeof(payload));
+    TlsRecordHeader out_hdr;
+    spec->Protect(hdr, pt, &ct, &out_hdr);
+
+    auto rv = out_hdr.Write(&record, 0, ct);
+    EXPECT_EQ(out_hdr.header_length() + ct.len(), rv);
+  } else {
+    TlsAgentTestBase::MakeRecord(variant_, ssl_ct_application_data, version_,
+                                 payload, sizeof(payload), &record, seqno);
+  }
+
   client_->SendDirect(record);
   server_->ExpectReadWriteError();
   server_->ReadBytes();

gtests/ssl_gtest/ssl_drop_unittest.cc

@@ -619,55 +619,6 @@ TEST_P(TlsDropDatagram13, ReorderServerEE) {
 
 // The client sends an out of order non-handshake message
 // but with the handshake key.
-class TlsSendCipherSpecCapturer {
- public:
-  TlsSendCipherSpecCapturer(const std::shared_ptr<TlsAgent>& agent)
-      : agent_(agent), send_cipher_specs_() {
-    EXPECT_EQ(SECSuccess,
-              SSL_SecretCallback(agent_->ssl_fd(), SecretCallback, this));
-  }
-
-  std::shared_ptr<TlsCipherSpec> spec(size_t i) {
-    if (i >= send_cipher_specs_.size()) {
-      return nullptr;
-    }
-    return send_cipher_specs_[i];
-  }
-
- private:
-  static void SecretCallback(PRFileDesc* fd, PRUint16 epoch,
-                             SSLSecretDirection dir, PK11SymKey* secret,
-                             void* arg) {
-    auto self = static_cast<TlsSendCipherSpecCapturer*>(arg);
-    std::cerr << self->agent_->role_str() << ": capture " << dir
-              << " secret for epoch " << epoch << std::endl;
-
-    if (dir == ssl_secret_read) {
-      return;
-    }
-
-    SSLPreliminaryChannelInfo preinfo;
-    EXPECT_EQ(SECSuccess,
-              SSL_GetPreliminaryChannelInfo(self->agent_->ssl_fd(), &preinfo,
-                                            sizeof(preinfo)));
-    EXPECT_EQ(sizeof(preinfo), preinfo.length);
-    EXPECT_TRUE(preinfo.valuesSet & ssl_preinfo_cipher_suite);
-
-    SSLCipherSuiteInfo cipherinfo;
-    EXPECT_EQ(SECSuccess,
-              SSL_GetCipherSuiteInfo(preinfo.cipherSuite, &cipherinfo,
-                                     sizeof(cipherinfo)));
-    EXPECT_EQ(sizeof(cipherinfo), cipherinfo.length);
-
-    auto spec = std::make_shared<TlsCipherSpec>(true, epoch);
-    EXPECT_TRUE(spec->SetKeys(&cipherinfo, secret));
-    self->send_cipher_specs_.push_back(spec);
-  }
-
-  std::shared_ptr<TlsAgent> agent_;
-  std::vector<std::shared_ptr<TlsCipherSpec>> send_cipher_specs_;
-};
-
 TEST_F(TlsConnectDatagram13, SendOutOfOrderAppWithHandshakeKey) {
   StartConnect();
   // Capturing secrets means that we can't use decrypting filters on the client.
@@ -684,8 +635,10 @@ TEST_F(TlsConnectDatagram13, SendOutOfOrderAppWithHandshakeKey) {
   auto spec = capturer.spec(0);
   ASSERT_NE(nullptr, spec.get());
   ASSERT_EQ(2, spec->epoch());
-  ASSERT_TRUE(client_->SendEncryptedRecord(spec, 0x0002000000000002,
-                                           ssl_ct_application_data,
+
+  uint8_t dtls13_ct = kCtDtlsCiphertext | kCtDtlsCiphertext16bSeqno |
+                      kCtDtlsCiphertextLengthPresent;
+  ASSERT_TRUE(client_->SendEncryptedRecord(spec, 0x0002000000000002, dtls13_ct,
                                            DataBuffer(buf, sizeof(buf))));
 
   // Now have the server consume the bogus message.
@@ -844,7 +797,7 @@ static void GetCipherAndLimit(uint16_t version, uint16_t* cipher,
     // a reasonable amount of time.
     *cipher = TLS_CHACHA20_POLY1305_SHA256;
     // Assume that we are starting with an expected sequence number of 0.
-    *limit = (1ULL << 29) - 1;
+    *limit = (1ULL << 15) - 1;
   }
 }
 
@@ -866,14 +819,14 @@ TEST_P(TlsConnectDatagram, MissLotsOfPackets) {
   SendReceive();
 }
 
-// Send a sequence number of 0xfffffffd and it should be interpreted as that
+// Send a sequence number of 0xfffd and it should be interpreted as that
 // (and not -3 or UINT64_MAX - 2).
 TEST_F(TlsConnectDatagram13, UnderflowSequenceNumber) {
   Connect();
   // This is only valid if short headers are disabled.
   client_->SetOption(SSL_ENABLE_DTLS_SHORT_HEADER, PR_FALSE);
   EXPECT_EQ(SECSuccess,
-            SSLInt_AdvanceWriteSeqNum(client_->ssl_fd(), (1ULL << 30) - 3));
+            SSLInt_AdvanceWriteSeqNum(client_->ssl_fd(), (1ULL << 16) - 3));
   SendReceive();
 }
 
@@ -918,9 +871,13 @@ class TlsReplaceFirstRecordWithJunk : public TlsRecordFilter {
       return KEEP;
     }
     replaced_ = true;
-    TlsRecordHeader out_header(header.variant(), header.version(),
-                               ssl_ct_application_data,
-                               header.sequence_number());
+
+    uint8_t dtls13_ct = kCtDtlsCiphertext | kCtDtlsCiphertext16bSeqno |
+                        kCtDtlsCiphertextLengthPresent;
+    TlsRecordHeader out_header(
+        header.variant(), header.version(),
+        is_dtls13() ? dtls13_ct : ssl_ct_application_data,
+        header.sequence_number());
 
     static const uint8_t junk[] = {1, 2, 3, 4};
     *offset = out_header.Write(output, *offset, DataBuffer(junk, sizeof(junk)));

gtests/ssl_gtest/ssl_gtest.gyp

@@ -15,6 +15,7 @@
         'libssl_internals.c',
         'selfencrypt_unittest.cc',
         'ssl_0rtt_unittest.cc',
+        'ssl_aead_unittest.cc',
         'ssl_agent_unittest.cc',
         'ssl_auth_unittest.cc',
         'ssl_cert_ext_unittest.cc',
@@ -36,8 +37,8 @@
         'ssl_hrr_unittest.cc',
         'ssl_keyupdate_unittest.cc',
         'ssl_loopback_unittest.cc',
+        'ssl_masking_unittest.cc',
         'ssl_misc_unittest.cc',
-        'ssl_primitive_unittest.cc',
         'ssl_record_unittest.cc',
         'ssl_recordsep_unittest.cc',
         'ssl_recordsize_unittest.cc',