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
sailfishos-mirror · May 10, 2017 · 2c7d12e · 2c7d12e
1 parent 2269169
commit 2c7d12e
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 0 deletions.
diff --git 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
@@ -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) {