Bug 1342137 - Permit unknown dotted-decimal X500 Principals r=franziskus,ttaubert

RFC 1485 permits principals with OIDs in either "1.2=Name" or "OID.1.2=Name"
form. This patch permits such forms, for unknown OIDs.

This patch adds disabled tests which should fail, but do not, and need further
cleanup.

Original patch courtesy of Miklos Vajna.

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

9 files changed, 165 insertions, 4 deletions

diff --git a/cpputil/scoped_ptrs.h b/cpputil/scoped_ptrs.h
index a2351984d..39a5e1f05 100644
--- a/cpputil/scoped_ptrs.h
+++ b/cpputil/scoped_ptrs.h
@@ -17,6 +17,7 @@ struct ScopedDelete {
   void operator()(CERTCertificateList* list) {
     CERT_DestroyCertificateList(list);
   }
+  void operator()(CERTName* name) { CERT_DestroyName(name); }
   void operator()(CERTCertList* list) { CERT_DestroyCertList(list); }
   void operator()(CERTSubjectPublicKeyInfo* spki) {
     SECKEY_DestroySubjectPublicKeyInfo(spki);
@@ -48,6 +49,7 @@ struct ScopedMaybeDelete {
 SCOPED(CERTCertificate);
 SCOPED(CERTCertificateList);
 SCOPED(CERTCertList);
+SCOPED(CERTName);
 SCOPED(CERTSubjectPublicKeyInfo);
 SCOPED(PK11SlotInfo);
 SCOPED(PK11SymKey);
diff --git a/gtests/certdb_gtest/Makefile b/gtests/certdb_gtest/Makefile
new file mode 100644
index 000000000..0d547e080
--- /dev/null
+++ b/gtests/certdb_gtest/Makefile
@@ -0,0 +1,43 @@
+#! gmake
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#######################################################################
+# (1) Include initial platform-independent assignments (MANDATORY).   #
+#######################################################################
+
+include manifest.mn
+
+#######################################################################
+# (2) Include "global" configuration information. (OPTIONAL)          #
+#######################################################################
+
+include $(CORE_DEPTH)/coreconf/config.mk
+
+#######################################################################
+# (3) Include "component" configuration information. (OPTIONAL)       #
+#######################################################################
+
+
+#######################################################################
+# (4) Include "local" platform-dependent assignments (OPTIONAL).      #
+#######################################################################
+
+include ../common/gtest.mk
+
+#######################################################################
+# (5) Execute "global" rules. (OPTIONAL)                              #
+#######################################################################
+
+include $(CORE_DEPTH)/coreconf/rules.mk
+
+#######################################################################
+# (6) Execute "component" rules. (OPTIONAL)                           #
+#######################################################################
+
+
+#######################################################################
+# (7) Execute "local" rules. (OPTIONAL).                              #
+#######################################################################
diff --git a/gtests/certdb_gtest/alg1485_unittest.cc b/gtests/certdb_gtest/alg1485_unittest.cc
new file mode 100644
index 000000000..a6bd40b4b
--- /dev/null
+++ b/gtests/certdb_gtest/alg1485_unittest.cc
@@ -0,0 +1,57 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include <stdint.h>
+
+#include "gtest/gtest.h"
+
+#include "nss.h"
+#include "scoped_ptrs.h"
+
+namespace nss_test {
+
+typedef struct AVATestValuesStr {
+  std::string avaString;
+  bool expectedResult;
+} AVATestValues;
+
+class Alg1485Test : public ::testing::Test,
+                    public ::testing::WithParamInterface<AVATestValues> {};
+
+static const AVATestValues kAVATestStrings[] = {
+    {"CN=Marshall T. Rose, O=Dover Beach Consulting, L=Santa Clara, "
+     "ST=California, C=US",
+     true},
+    {"C=HU,L=Budapest,O=Organization,CN=Example - Qualified Citizen "
+     "CA,2.5.4.97=VATHU-10",
+     true},
+    {"C=HU,L=Budapest,O=Example,CN=Example - Qualified Citizen "
+     "CA,OID.2.5.4.97=VATHU-10",
+     true},
+    {"CN=Somebody,L=Set,O=Up,C=US,1=The,2=Bomb", true},
+    {"OID.2.5.4.6=😑", true},
+    {"2.5.4.6=😑", true},
+    {"OID.moocow=😑", false},      // OIDs must be numeric
+    {"3.2=bad", false},           // OIDs cannot be overly large; 3 is too big
+    {"256.257=bad", false},       // Still too big
+    {"YO=LO", false},             // Unknown Tag, 'YO'
+    {"CN=Tester,ZZ=Top", false},  // Unknown tag, 'ZZ'
+    // These tests are disabled pending Bug 1363416
+    // { "01.02.03=Nope", false }, // Numbers not in minimal form
+    // { "000001.0000000001=👌", false },
+    // { "CN=Somebody,L=Set,O=Up,C=US,01=The,02=Bomb", false },
+};
+
+TEST_P(Alg1485Test, TryParsingAVAStrings) {
+  const AVATestValues& param(GetParam());
+
+  ScopedCERTName certName(CERT_AsciiToName(param.avaString.c_str()));
+  ASSERT_EQ(certName != nullptr, param.expectedResult);
+}
+
+INSTANTIATE_TEST_CASE_P(ParseAVAStrings, Alg1485Test,
+                        ::testing::ValuesIn(kAVATestStrings));
+}
diff --git a/gtests/certdb_gtest/certdb_gtest.gyp b/gtests/certdb_gtest/certdb_gtest.gyp
new file mode 100644
index 000000000..898102def
--- /dev/null
+++ b/gtests/certdb_gtest/certdb_gtest.gyp
@@ -0,0 +1,29 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+{
+  'includes': [
+    '../../coreconf/config.gypi',
+    '../common/gtest.gypi',
+  ],
+  'targets': [
+    {
+      'target_name': 'certdb_gtest',
+      'type': 'executable',
+      'sources': [
+        'alg1485_unittest.cc',
+        '<(DEPTH)/gtests/common/gtests.cc'
+      ],
+      'dependencies': [
+        '<(DEPTH)/exports.gyp:nss_exports',
+        '<(DEPTH)/gtests/google_test/google_test.gyp:gtest',
+        '<(DEPTH)/lib/util/util.gyp:nssutil3',
+        '<(DEPTH)/lib/ssl/ssl.gyp:ssl3',
+        '<(DEPTH)/lib/nss/nss.gyp:nss3',
+      ]
+    }
+  ],
+  'variables': {
+    'module': 'nss'
+  }
+}
diff --git a/gtests/certdb_gtest/manifest.mn b/gtests/certdb_gtest/manifest.mn
new file mode 100644
index 000000000..4a3a1fda0
--- /dev/null
+++ b/gtests/certdb_gtest/manifest.mn
@@ -0,0 +1,22 @@
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+CORE_DEPTH = ../..
+DEPTH      = ../..
+MODULE = nss
+
+CPPSRCS = \
+      alg1485_unittest.cc \
+      $(NULL)
+
+INCLUDES += -I$(CORE_DEPTH)/gtests/google_test/gtest/include \
+            -I$(CORE_DEPTH)/gtests/common \
+            -I$(CORE_DEPTH)/cpputil
+
+REQUIRES = nspr nss libdbm gtest
+
+PROGRAM = certdb_gtest
+
+EXTRA_LIBS = $(DIST)/lib/$(LIB_PREFIX)gtest.$(LIB_SUFFIX) $(EXTRA_OBJS) \
+             ../common/$(OBJDIR)/gtests$(OBJ_SUFFIX)
diff --git a/gtests/manifest.mn b/gtests/manifest.mn
index 633129cf1..d572edad0 100644
--- a/gtests/manifest.mn
+++ b/gtests/manifest.mn
@@ -8,6 +8,7 @@ DEPTH      = ..
 DIRS = \
 	google_test \
 	common \
+	certdb_gtest \
 	certhigh_gtest \
 	der_gtest \
 	util_gtest \
diff --git a/lib/certdb/alg1485.c b/lib/certdb/alg1485.c
index 8d475ff77..79598c02c 100644
--- a/lib/certdb/alg1485.c
+++ b/lib/certdb/alg1485.c
@@ -375,6 +375,7 @@ ParseRFC1485AVA(PLArenaPool* arena, const char** pbp, const char* endptr)
     const char* bp;
     int vt = -1;
     int valLen;
+    PRBool isDottedOid = PR_FALSE;
     SECOidTag kind = SEC_OID_UNKNOWN;
     SECStatus rv = SECFailure;
     SECItem derOid = { 0, NULL, 0 };
@@ -401,8 +402,9 @@ ParseRFC1485AVA(PLArenaPool* arena, const char** pbp, const char* endptr)
     }
 
     /* is this a dotted decimal OID attribute type ? */
-    if (!PL_strncasecmp("oid.", tagBuf, 4)) {
+    if (!PL_strncasecmp("oid.", tagBuf, 4) || isdigit(tagBuf[0])) {
         rv = SEC_StringToOID(arena, &derOid, tagBuf, strlen(tagBuf));
+        isDottedOid = (PRBool)(rv == SECSuccess);
     } else {
         for (n2k = name2kinds; n2k->name; n2k++) {
             SECOidData* oidrec;
@@ -428,7 +430,7 @@ ParseRFC1485AVA(PLArenaPool* arena, const char** pbp, const char* endptr)
             goto loser;
         a = CERT_CreateAVAFromRaw(arena, &derOid, &derVal);
     } else {
-        if (kind == SEC_OID_UNKNOWN)
+        if (kind == SEC_OID_UNKNOWN && !isDottedOid)
             goto loser;
         if (kind == SEC_OID_AVA_COUNTRY_NAME && valLen != 2)
             goto loser;
@@ -445,7 +447,11 @@ ParseRFC1485AVA(PLArenaPool* arena, const char** pbp, const char* endptr)
 
         derVal.data = (unsigned char*)valBuf;
         derVal.len = valLen;
-        a = CERT_CreateAVAFromSECItem(arena, kind, vt, &derVal);
+        if (kind == SEC_OID_UNKNOWN && isDottedOid) {
+            a = CERT_CreateAVAFromRaw(arena, &derOid, &derVal);
+        } else {
+            a = CERT_CreateAVAFromSECItem(arena, kind, vt, &derVal);
+        }
     }
     return a;
 
diff --git a/nss.gyp b/nss.gyp
index dafc66962..97586b333 100644
--- a/nss.gyp
+++ b/nss.gyp
@@ -179,6 +179,7 @@
             'cmd/vfyserv/vfyserv.gyp:vfyserv',
             'gtests/certhigh_gtest/certhigh_gtest.gyp:certhigh_gtest',
             'gtests/der_gtest/der_gtest.gyp:der_gtest',
+            'gtests/certdb_gtest/certdb_gtest.gyp:certdb_gtest',
             'gtests/freebl_gtest/freebl_gtest.gyp:prng_gtest',
             'gtests/pk11_gtest/pk11_gtest.gyp:pk11_gtest',
             'gtests/ssl_gtest/ssl_gtest.gyp:ssl_gtest',
diff --git a/tests/gtests/gtests.sh b/tests/gtests/gtests.sh
index 6fb7e60df..8c9d656cb 100755
--- a/tests/gtests/gtests.sh
+++ b/tests/gtests/gtests.sh
@@ -83,7 +83,7 @@ gtest_cleanup()
 }
 
 ################## main #################################################
-GTESTS="prng_gtest certhigh_gtest der_gtest pk11_gtest util_gtest freebl_gtest"
+GTESTS="prng_gtest certhigh_gtest certdb_gtest der_gtest pk11_gtest util_gtest freebl_gtest"
 SOURCE_DIR="$PWD"/../..
 gtest_init $0
 gtest_start