Bug 1295405 - Reject status_request extension with content, r=ekr

5 files changed, 42 insertions, 21 deletions

diff --git a/external_tests/ssl_gtest/ssl_cert_ext_unittest.cc b/external_tests/ssl_gtest/ssl_cert_ext_unittest.cc
index b246c0760..932ca0e90 100644
--- a/external_tests/ssl_gtest/ssl_cert_ext_unittest.cc
+++ b/external_tests/ssl_gtest/ssl_cert_ext_unittest.cc
@@ -168,6 +168,22 @@ TEST_P(TlsConnectGeneric, OcspNotProvided) {
   Connect();
 }
 
+TEST_P(TlsConnectGenericPre13, OcspMangled) {
+  EnsureTlsSetup();
+  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
+                                      SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
+  EXPECT_TRUE(
+      server_->ConfigServerCert(TlsAgent::kServerRsa, true, &kOcspExtraData));
+
+  static const uint8_t val[] = { 1 };
+  auto replacer = new TlsExtensionReplacer(ssl_cert_status_xtn,
+                                           DataBuffer(val, sizeof(val)));
+  server_->SetPacketFilter(replacer);
+  ConnectExpectFail();
+  client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+}
+
 TEST_P(TlsConnectGeneric, OcspSuccess) {
   EnsureTlsSetup();
   EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
diff --git a/external_tests/ssl_gtest/ssl_extension_unittest.cc b/external_tests/ssl_gtest/ssl_extension_unittest.cc
index e10885c41..8d43fd68a 100644
--- a/external_tests/ssl_gtest/ssl_extension_unittest.cc
+++ b/external_tests/ssl_gtest/ssl_extension_unittest.cc
@@ -75,26 +75,6 @@ class TlsExtensionDamager : public TlsExtensionFilter {
   size_t index_;
 };
 
-class TlsExtensionReplacer : public TlsExtensionFilter {
- public:
-  TlsExtensionReplacer(uint16_t extension, const DataBuffer& data)
-      : extension_(extension), data_(data) {}
-  virtual PacketFilter::Action FilterExtension(uint16_t extension_type,
-                                               const DataBuffer& input,
-                                               DataBuffer* output) {
-    if (extension_type != extension_) {
-      return KEEP;
-    }
-
-    *output = data_;
-    return CHANGE;
-  }
-
- private:
-  const uint16_t extension_;
-  const DataBuffer data_;
-};
-
 class TlsExtensionInjector : public TlsHandshakeFilter {
  public:
   TlsExtensionInjector(uint16_t ext, DataBuffer& data)
diff --git a/external_tests/ssl_gtest/tls_filter.cc b/external_tests/ssl_gtest/tls_filter.cc
index 8814aa1d2..60bbfde5e 100644
--- a/external_tests/ssl_gtest/tls_filter.cc
+++ b/external_tests/ssl_gtest/tls_filter.cc
@@ -429,6 +429,16 @@ PacketFilter::Action TlsExtensionCapture::FilterExtension(
   return KEEP;
 }
 
+PacketFilter::Action TlsExtensionReplacer::FilterExtension(
+    uint16_t extension_type, const DataBuffer& input, DataBuffer* output) {
+  if (extension_type != extension_) {
+    return KEEP;
+  }
+
+  *output = data_;
+  return CHANGE;
+}
+
 PacketFilter::Action AfterRecordN::FilterRecord(const RecordHeader& header,
                                                 const DataBuffer& body,
                                                 DataBuffer* out) {
diff --git a/external_tests/ssl_gtest/tls_filter.h b/external_tests/ssl_gtest/tls_filter.h
index c72991f64..07e72c141 100644
--- a/external_tests/ssl_gtest/tls_filter.h
+++ b/external_tests/ssl_gtest/tls_filter.h
@@ -233,6 +233,19 @@ class TlsExtensionCapture : public TlsExtensionFilter {
   DataBuffer data_;
 };
 
+class TlsExtensionReplacer : public TlsExtensionFilter {
+ public:
+  TlsExtensionReplacer(uint16_t extension, const DataBuffer& data)
+      : extension_(extension), data_(data) {}
+  PacketFilter::Action FilterExtension(uint16_t extension_type,
+                                       const DataBuffer& input,
+                                       DataBuffer* output) override;
+
+ private:
+  const uint16_t extension_;
+  const DataBuffer data_;
+};
+
 class TlsAgent;
 typedef std::function<void(void)> VoidFunction;
 
diff --git a/lib/ssl/ssl3ext.c b/lib/ssl/ssl3ext.c
index ca781695b..4ebd0310c 100644
--- a/lib/ssl/ssl3ext.c
+++ b/lib/ssl/ssl3ext.c
@@ -1201,7 +1201,9 @@ ssl3_ClientHandleStatusRequestXtn(sslSocket *ss, PRUint16 ex_type,
             return SECFailure; /* code already set */
         }
     } else if (data->len != 0) {
-        return SECSuccess; /* Ignore the extension. */
+        (void)SSL3_SendAlert(ss, alert_fatal, illegal_parameter);
+        PORT_SetError(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
+        return SECFailure;
     }
 
     /* Keep track of negotiated extensions. */