From 2c7d12ef940cbe142bdee8203c50328a6ebd2f75 Mon Sep 17 00:00:00 2001
From: Kate McKinley <kmckinley@mozilla.com>
Date: Wed, 10 May 2017 11:48:48 -0700
Subject: [PATCH] Bug 1351314 - Add length check for SSL context for TLS 1.2 or
 prior, r=mt

Summary:
RFC 5705 requires that the context length passed to
SSL_ExportKeyingMaterial be no longer than will fit in
a 16-bit integer (uint16).

Reviewers: franziskus, mt

Reviewed By: mt

Differential Revision: https://nss-review.dev.mozaws.net/D309

--HG--
extra : amend_source : 8c2ce106dcffcae9d8cc3dd9f0fcc811a68158bb
---
 gtests/ssl_gtest/ssl_exporter_unittest.cc | 28 +++++++++++++++++++++++
 lib/ssl/sslinfo.c                         |  8 +++++++
 2 files changed, 36 insertions(+)

diff --git a/gtests/ssl_gtest/ssl_exporter_unittest.cc b/gtests/ssl_gtest/ssl_exporter_unittest.cc
index 047d535bba..be407b42ea 100644
--- a/gtests/ssl_gtest/ssl_exporter_unittest.cc
+++ b/gtests/ssl_gtest/ssl_exporter_unittest.cc
@@ -77,6 +77,33 @@ TEST_P(TlsConnectTls13, ExporterContextEmptyIsSameAsNone) {
   ExportAndCompare(client_, server_, false);
 }
 
+TEST_P(TlsConnectGenericPre13, ExporterContextLengthTooLong) {
+  static const uint8_t kExporterContextTooLong[PR_UINT16_MAX] = {
+      0x12, 0x34, 0x56, 0x78, 0x9A, 0xBC, 0xDE, 0xFF};
+
+  EnsureTlsSetup();
+  Connect();
+  CheckKeys();
+
+  static const size_t exporter_len = 10;
+  uint8_t client_value[exporter_len] = {0};
+  EXPECT_EQ(SECFailure,
+            SSL_ExportKeyingMaterial(client_->ssl_fd(), kExporterLabel,
+                                     strlen(kExporterLabel), PR_TRUE,
+                                     kExporterContextTooLong,
+                                     sizeof(kExporterContextTooLong),
+                                     client_value, sizeof(client_value)));
+  EXPECT_EQ(PORT_GetError(), SEC_ERROR_INVALID_ARGS);
+  uint8_t server_value[exporter_len] = {0xff};
+  EXPECT_EQ(SECFailure,
+            SSL_ExportKeyingMaterial(server_->ssl_fd(), kExporterLabel,
+                                     strlen(kExporterLabel), PR_TRUE,
+                                     kExporterContextTooLong,
+                                     sizeof(kExporterContextTooLong),
+                                     server_value, sizeof(server_value)));
+  EXPECT_EQ(PORT_GetError(), SEC_ERROR_INVALID_ARGS);
+}
+
 // This has a weird signature so that it can be passed to the SNI callback.
 int32_t RegularExporterShouldFail(TlsAgent* agent, const SECItem* srvNameArr,
                                   PRUint32 srvNameArrSize) {
@@ -99,6 +126,7 @@ TEST_P(TlsConnectTls13, EarlyExporter) {
   client_->Handshake();  // Send ClientHello.
   uint8_t client_value[10] = {0};
   RegularExporterShouldFail(client_.get(), nullptr, 0);
+
   EXPECT_EQ(SECSuccess,
             SSL_ExportEarlyKeyingMaterial(
                 client_->ssl_fd(), kExporterLabel, strlen(kExporterLabel),
diff --git a/lib/ssl/sslinfo.c b/lib/ssl/sslinfo.c
index 54558345d6..88162d8146 100644
--- a/lib/ssl/sslinfo.c
+++ b/lib/ssl/sslinfo.c
@@ -233,6 +233,9 @@ SSL_GetPreliminaryChannelInfo(PRFileDesc *fd,
 #define F_NFIPS_NSTD 0, 0, 1, 0 /* i.e., trash */
 #define F_EXPORT 0, 1, 0, 0     /* i.e., trash */
 
+// RFC 5705
+#define MAX_CONTEXT_LEN PR_UINT16_MAX - 1
+
 static const SSLCipherSuiteInfo suiteInfo[] = {
     /* <------ Cipher suite --------------------> <auth> <KEA>  <bulk cipher> <MAC> <FIPS> */
     { 0, CS_(TLS_AES_128_GCM_SHA256), S_ANY, K_ANY, C_AESGCM, B_128, M_AEAD_128, F_FIPS_STD, A_ANY },
@@ -439,6 +442,11 @@ SSL_ExportKeyingMaterial(PRFileDesc *fd,
                               out, outLen);
     }
 
+    if (hasContext && contextLen > MAX_CONTEXT_LEN) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return SECFailure;
+    }
+
     /* construct PRF arguments */
     valLen = SSL3_RANDOM_LENGTH * 2;
     if (hasContext) {