Bug 1247021 - Add protective mechanisms to SSL_GetChannelInfo. Macro patch v3 with improvements by Martin Thomson, r=rrelyea

5 files changed, 44 insertions, 5 deletions

diff --git a/cmd/selfserv/selfserv.c b/cmd/selfserv/selfserv.c
index 9a1572837..dc7cb4973 100644
--- a/cmd/selfserv/selfserv.c
+++ b/cmd/selfserv/selfserv.c
@@ -412,6 +412,11 @@ printSecurityInfo(PRFileDesc *fd)
         result = SSL_GetCipherSuiteInfo(channel.cipherSuite,
                                         &suite, sizeof suite);
         if (result == SECSuccess) {
+            const char *EMSUsed = "?";
+            if (SSL_CHANNEL_INFO_FIELD_EXISTS(channel, extendedMasterSecretUsed)) {
+                EMSUsed = SSL_CHANNEL_INFO_FIELD_GET(channel, extendedMasterSecretUsed) ?
+                    "Yes": "No";
+            }
             FPRINTF(stderr,
                     "selfserv: SSL version %d.%d using %d-bit %s with %d-bit %s MAC\n",
                     channel.protocolVersion >> 8, channel.protocolVersion & 0xff,
@@ -423,7 +428,7 @@ printSecurityInfo(PRFileDesc *fd)
                     channel.authKeyBits, suite.authAlgorithmName,
                     channel.keaKeyBits, suite.keaTypeName,
                     channel.compressionMethodName,
-                    channel.extendedMasterSecretUsed ? "Yes" : "No");
+                    EMSUsed);
         }
     }
     if (verbose) {
diff --git a/cmd/tstclnt/tstclnt.c b/cmd/tstclnt/tstclnt.c
index 4b0d06302..246c0a482 100644
--- a/cmd/tstclnt/tstclnt.c
+++ b/cmd/tstclnt/tstclnt.c
@@ -118,6 +118,11 @@ printSecurityInfo(PRFileDesc *fd)
         result = SSL_GetCipherSuiteInfo(channel.cipherSuite,
                                         &suite, sizeof suite);
         if (result == SECSuccess) {
+            const char *EMSUsed = "?";
+            if (SSL_CHANNEL_INFO_FIELD_EXISTS(channel, extendedMasterSecretUsed)) {
+                EMSUsed = SSL_CHANNEL_INFO_FIELD_GET(channel, extendedMasterSecretUsed) ?
+                  "Yes": "No";
+            }
             FPRINTF(stderr,
                     "tstclnt: SSL version %d.%d using %d-bit %s with %d-bit %s MAC\n",
                     channel.protocolVersion >> 8, channel.protocolVersion & 0xff,
@@ -129,7 +134,7 @@ printSecurityInfo(PRFileDesc *fd)
                     channel.authKeyBits, suite.authAlgorithmName,
                     channel.keaKeyBits, suite.keaTypeName,
                     channel.compressionMethodName,
-                    channel.extendedMasterSecretUsed ? "Yes" : "No");
+                    EMSUsed);
         }
     }
     cert = SSL_RevealCert(fd);
diff --git a/external_tests/ssl_gtest/tls_agent.cc b/external_tests/ssl_gtest/tls_agent.cc
index 2afd3cf92..22d6828a7 100644
--- a/external_tests/ssl_gtest/tls_agent.cc
+++ b/external_tests/ssl_gtest/tls_agent.cc
@@ -534,7 +534,9 @@ void TlsAgent::CheckExtendedMasterSecret(bool expected) {
   if (version() >= SSL_LIBRARY_VERSION_TLS_1_3) {
     expected = PR_TRUE;
   }
-  ASSERT_EQ(expected, info_.extendedMasterSecretUsed != PR_FALSE)
+  ASSERT_TRUE(SSL_CHANNEL_INFO_FIELD_EXISTS(info_, extendedMasterSecretUsed));
+  ASSERT_EQ(expected,
+    SSL_CHANNEL_INFO_FIELD_GET(info_, extendedMasterSecretUsed) != PR_FALSE)
       << "unexpected extended master secret state for " << name_;
 }
 
diff --git a/lib/ssl/sslinfo.c b/lib/ssl/sslinfo.c
index ba9d0f040..5920023d7 100644
--- a/lib/ssl/sslinfo.c
+++ b/lib/ssl/sslinfo.c
@@ -20,6 +20,10 @@ ssl_GetCompressionMethodName(SSLCompressionMethod compression)
     }
 }
 
+#define SSL_CHANNEL_INFO_FIELD_SET(info,field,value) \
+    if (SSL_CHANNEL_INFO_FIELD_EXISTS(info,field)) \
+        {info.UseMacroToAccess_##field = value;}
+
 SECStatus
 SSL_GetChannelInfo(PRFileDesc *fd, SSLChannelInfo *info, PRUintn len)
 {
@@ -71,16 +75,20 @@ SSL_GetChannelInfo(PRFileDesc *fd, SSLChannelInfo *info, PRUintn len)
         }
         if (sid) {
             unsigned int sidLen;
+            PRBool EMSValue;
 
             inf.creationTime = sid->creationTime;
             inf.lastAccessTime = sid->lastAccessTime;
             inf.expirationTime = sid->expirationTime;
-            inf.extendedMasterSecretUsed =
+
+            EMSValue = 
                 (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3 ||
                  sid->u.ssl3.keys.extendedMasterSecretUsed)
                     ? PR_TRUE
                     : PR_FALSE;
 
+            SSL_CHANNEL_INFO_FIELD_SET(inf, extendedMasterSecretUsed, EMSValue)
+
             sidLen = sid->u.ssl3.sessionIDLength;
             sidLen = PR_MIN(sidLen, sizeof inf.sessionID);
             inf.sessionIDLength = sidLen;
diff --git a/lib/ssl/sslt.h b/lib/ssl/sslt.h
index daacb239d..7c7226cec 100644
--- a/lib/ssl/sslt.h
+++ b/lib/ssl/sslt.h
@@ -181,13 +181,32 @@ typedef struct SSLChannelInfoStr {
     const char* compressionMethodName;
     SSLCompressionMethod compressionMethod;
 
+    /*
+     * Any attributes that follow MUST NOT accessed directly using their
+     * names. Instead, you must use the accessor macros
+     *   SSL_CHANNEL_INFO_FIELD_GET and SSL_CHANNEL_INFO_FIELD_SET,
+     * which check at runtime that the dynamically linked version of NSS
+     * supports the attribute.
+     * The names of the attributes must start with "UseMacroToAccess_".
+     * (Only internal NSS library code may access the attributes directly,
+     *  but NSS tools MUST use the macros.)
+     */
+
     /* The following fields are added in NSS 3.21.
      * This field only has meaning in TLS < 1.3 and will be set to
      *  PR_FALSE in TLS 1.3.
      */
-    PRBool extendedMasterSecretUsed;
+    PRBool UseMacroToAccess_extendedMasterSecretUsed;
 } SSLChannelInfo;
 
+/* Use these macros to access the entries in SSLChannelInfo that are named
+ * starting with "UseMacroToAccess_" */
+#define SSL_CHANNEL_INFO_FIELD_EXISTS(info, field) \
+   ((info).length >= (offsetof(SSLChannelInfo, UseMacroToAccess_##field) + sizeof((info).UseMacroToAccess_##field)))
+
+#define SSL_CHANNEL_INFO_FIELD_GET(info,field) \
+   (SSL_CHANNEL_INFO_FIELD_EXISTS(info,field) ? info.UseMacroToAccess_##field : -1)
+
 /* Preliminary channel info */
 #define ssl_preinfo_version (1U << 0)
 #define ssl_preinfo_cipher_suite (1U << 1)