Bug 1354152 - Reject records with plaintext >2^14, r=ttaubert NSS_3_32_BRANCH
authorMartin Thomson <martin.thomson@gmail.com>
Fri, 02 Jun 2017 10:21:51 +1000
branchNSS_3_32_BRANCH
changeset 14089 0713bc0a60b9642a52e55a1c373c1a4c8c76ce1c
parent 13584 0cb197c219061cb11dac4e1768ab4638029beea7
push id2861
push usermartin.thomson@gmail.com
push dateMon, 06 Nov 2017 02:07:48 +0000
reviewersttaubert
bugs1354152
Bug 1354152 - Reject records with plaintext >2^14, r=ttaubert
gtests/ssl_gtest/ssl_record_unittest.cc
lib/ssl/ssl3con.c
--- a/gtests/ssl_gtest/ssl_record_unittest.cc
+++ b/gtests/ssl_gtest/ssl_record_unittest.cc
@@ -5,16 +5,18 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nss.h"
 #include "ssl.h"
 #include "sslimpl.h"
 
 #include "databuffer.h"
 #include "gtest_utils.h"
+#include "tls_connect.h"
+#include "tls_filter.h"
 
 namespace nss_test {
 
 const static size_t kMacSize = 20;
 
 class TlsPaddingTest
     : public ::testing::Test,
       public ::testing::WithParamInterface<std::tuple<size_t, bool>> {
@@ -46,18 +48,18 @@ class TlsPaddingTest
   }
 
   void Unpad(bool expect_success) {
     std::cerr << "Content length=" << plaintext_len_
               << " padding length=" << pad_len_
               << " total length=" << plaintext_.len() << std::endl;
     std::cerr << "Plaintext: " << plaintext_ << std::endl;
     sslBuffer s;
-    s.buf = const_cast<unsigned char *>(
-        static_cast<const unsigned char *>(plaintext_.data()));
+    s.buf = const_cast<unsigned char*>(
+        static_cast<const unsigned char*>(plaintext_.data()));
     s.len = plaintext_.len();
     SECStatus rv = ssl_RemoveTLSCBCPadding(&s, kMacSize);
     if (expect_success) {
       EXPECT_EQ(SECSuccess, rv);
       EXPECT_EQ(plaintext_len_, static_cast<size_t>(s.len));
     } else {
       EXPECT_EQ(SECFailure, rv);
     }
@@ -94,16 +96,85 @@ TEST_P(TlsPaddingTest, FirstByteOfPadWro
 TEST_P(TlsPaddingTest, LastByteOfPadWrong) {
   if (pad_len_) {
     plaintext_.Write(plaintext_.len() - 2,
                      plaintext_.data()[plaintext_.len() - 1] + 1, 1);
     Unpad(false);
   }
 }
 
+class RecordReplacer : public TlsRecordFilter {
+ public:
+  RecordReplacer(size_t size)
+      : TlsRecordFilter(), enabled_(false), size_(size) {}
+
+  PacketFilter::Action FilterRecord(const TlsRecordHeader& header,
+                                    const DataBuffer& data,
+                                    DataBuffer* changed) override {
+    if (!enabled_) {
+      return KEEP;
+    }
+
+    EXPECT_EQ(kTlsApplicationDataType, header.content_type());
+    changed->Allocate(size_);
+
+    for (size_t i = 0; i < size_; ++i) {
+      changed->data()[i] = i & 0xff;
+    }
+
+    enabled_ = false;
+    return CHANGE;
+  }
+
+  void Enable() { enabled_ = true; }
+
+ private:
+  bool enabled_;
+  size_t size_;
+};
+
+TEST_F(TlsConnectStreamTls13, LargeRecord) {
+  EnsureTlsSetup();
+
+  const size_t record_limit = 16384;
+  auto replacer = std::make_shared<RecordReplacer>(record_limit);
+  client_->SetTlsRecordFilter(replacer);
+  replacer->EnableDecryption();
+  Connect();
+
+  replacer->Enable();
+  client_->SendData(10);
+  WAIT_(server_->received_bytes() == record_limit, 2000);
+  ASSERT_EQ(record_limit, server_->received_bytes());
+}
+
+TEST_F(TlsConnectStreamTls13, TooLargeRecord) {
+  EnsureTlsSetup();
+
+  const size_t record_limit = 16384;
+  auto replacer = std::make_shared<RecordReplacer>(record_limit + 1);
+  client_->SetTlsRecordFilter(replacer);
+  replacer->EnableDecryption();
+  Connect();
+
+  replacer->Enable();
+  ExpectAlert(server_, kTlsAlertRecordOverflow);
+  client_->SendData(10);  // This is expanded.
+
+  uint8_t buf[record_limit + 2];
+  PRInt32 rv = PR_Read(server_->ssl_fd(), buf, sizeof(buf));
+  EXPECT_GT(0, rv);
+  EXPECT_EQ(SSL_ERROR_RX_RECORD_TOO_LONG, PORT_GetError());
+
+  // Read the server alert.
+  rv = PR_Read(client_->ssl_fd(), buf, sizeof(buf));
+  EXPECT_GT(0, rv);
+  EXPECT_EQ(SSL_ERROR_RECORD_OVERFLOW_ALERT, PORT_GetError());
+}
+
 const static size_t kContentSizesArr[] = {
     1, kMacSize - 1, kMacSize, 30, 31, 32, 36, 256, 257, 287, 288};
 
 auto kContentSizes = ::testing::ValuesIn(kContentSizesArr);
 const static bool kTrueFalseArr[] = {true, false};
 auto kTrueFalse = ::testing::ValuesIn(kTrueFalseArr);
 
 INSTANTIATE_TEST_CASE_P(TlsPadding, TlsPaddingTest,
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -12693,17 +12693,17 @@ ssl3_HandleRecord(sslSocket *ss, SSL3Cip
         }
 
         sslBuffer_Clear(&temp_buf);
     }
 
     /*
     ** Having completed the decompression, check the length again.
     */
-    if (isTLS && databuf->len > (MAX_FRAGMENT_LENGTH + 1024)) {
+    if (isTLS && databuf->len > MAX_FRAGMENT_LENGTH) {
         SSL3_SendAlert(ss, alert_fatal, record_overflow);
         PORT_SetError(SSL_ERROR_RX_RECORD_TOO_LONG);
         return SECFailure;
     }
 
     /* Application data records are processed by the caller of this
     ** function, not by this function.
     */