bug 1301547 - remove ancient workaround in client certificate code r=franziskus
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 16 Jul 2018 16:30:15 -0700
changeset 484735 bbe392227b7d6376bb99843e781e7bd056ddeaba
parent 484709 5196c06ae5add254243c580b4489db7453809468
child 484736 a3a9e9c8a7c77d6105b599d2fd0def2965a91f4b
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfranziskus
bugs1301547
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1301547 - remove ancient workaround in client certificate code r=franziskus Apparently a prehistoric server implementation would send a certificate_authorities field that didn't include the outer DER SEQUENCE tag, so PSM attempted to detect this and work around it. Telemetry indicates this is unnecessary now: https://mzl.la/2Lbi1Lz
security/manager/ssl/nsNSSIOLayer.cpp
security/nss.symbols
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -1962,94 +1962,35 @@ nsConvertCANamesToStrings(const UniquePL
     MOZ_ASSERT(caNameStrings);
     MOZ_ASSERT(caNames);
     if (!arena.get() || !caNameStrings || !caNames) {
         PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
         return SECFailure;
     }
 
     SECItem* dername;
-    SECStatus rv;
-    int headerlen;
-    uint32_t contentlen;
-    SECItem newitem;
     int n;
     char* namestring;
 
     for (n = 0; n < caNames->nnames; n++) {
-        newitem.data = nullptr;
         dername = &caNames->names[n];
-
-        rv = DER_Lengths(dername, &headerlen, &contentlen);
-
-        if (rv != SECSuccess) {
-            goto loser;
-        }
-
-        if (headerlen + contentlen != dername->len) {
-            Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_CLIENT_CERT,
-                                 NS_LITERAL_STRING("compat"), 1);
-            // This must be from an enterprise 2.x server, which sent
-            // incorrectly formatted der without the outer wrapper of type and
-            // length. Fix it up by adding the top level header.
-            if (dername->len <= 127) {
-                newitem.data = (unsigned char*) malloc(dername->len + 2);
-                if (!newitem.data) {
-                    goto loser;
-                }
-                newitem.data[0] = (unsigned char) 0x30;
-                newitem.data[1] = (unsigned char) dername->len;
-                (void) memcpy(&newitem.data[2], dername->data, dername->len);
-            } else if (dername->len <= 255) {
-                newitem.data = (unsigned char*) malloc(dername->len + 3);
-                if (!newitem.data) {
-                    goto loser;
-                }
-                newitem.data[0] = (unsigned char) 0x30;
-                newitem.data[1] = (unsigned char) 0x81;
-                newitem.data[2] = (unsigned char) dername->len;
-                (void) memcpy(&newitem.data[3], dername->data, dername->len);
-            } else {
-                // greater than 256, better be less than 64k
-                newitem.data = (unsigned char*) malloc(dername->len + 4);
-                if (!newitem.data) {
-                    goto loser;
-                }
-                newitem.data[0] = (unsigned char) 0x30;
-                newitem.data[1] = (unsigned char) 0x82;
-                newitem.data[2] = (unsigned char) ((dername->len >> 8) & 0xff);
-                newitem.data[3] = (unsigned char) (dername->len & 0xff);
-                memcpy(&newitem.data[4], dername->data, dername->len);
-            }
-            dername = &newitem;
-        }
-
         namestring = CERT_DerNameToAscii(dername);
         if (!namestring) {
             // XXX - keep going until we fail to convert the name
             caNameStrings[n] = const_cast<char*>("");
         } else {
             caNameStrings[n] = PORT_ArenaStrdup(arena.get(), namestring);
             PR_Free(namestring); // CERT_DerNameToAscii() uses PR_Malloc().
             if (!caNameStrings[n]) {
-                goto loser;
+                return SECFailure;
             }
         }
-
-        if (newitem.data) {
-            free(newitem.data);
-        }
     }
 
     return SECSuccess;
-loser:
-    if (newitem.data) {
-        free(newitem.data);
-    }
-    return SECFailure;
 }
 
 // Possible behaviors for choosing a cert for client auth.
 enum class UserCertChoice {
   // Ask the user to choose a cert.
   Ask = 0,
   // Automatically choose a cert.
   Auto = 1,
--- a/security/nss.symbols
+++ b/security/nss.symbols
@@ -158,17 +158,16 @@ DER_AsciiToTime_Util
 DER_DecodeTimeChoice_Util
 DER_Encode
 DER_EncodeTimeChoice_Util
 DER_Encode_Util
 DER_GeneralizedTimeToTime
 DER_GeneralizedTimeToTime_Util
 DER_GetInteger
 DER_GetInteger_Util
-DER_Lengths
 DER_SetUInteger
 DER_UTCTimeToTime_Util
 DSAU_DecodeDerSigToLen
 DSAU_EncodeDerSigWithLen
 DTLS_GetHandshakeTimeout
 DTLS_ImportFD
 HASH_Begin
 HASH_Create