From e9be8de8035fec859310805ac19c8a9fd14f4e21 Mon Sep 17 00:00:00 2001 From: "J.C. Jones" Date: Thu, 11 May 2017 15:10:00 -0700 Subject: [PATCH] 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 --HG-- extra : rebase_source : c6a736e2bbd0647c7fbae09157a0fb7d26ac6f2a --- cpputil/scoped_ptrs.h | 2 + gtests/certdb_gtest/Makefile | 43 +++++++++++++++++++ gtests/certdb_gtest/alg1485_unittest.cc | 57 +++++++++++++++++++++++++ gtests/certdb_gtest/certdb_gtest.gyp | 29 +++++++++++++ gtests/certdb_gtest/manifest.mn | 22 ++++++++++ gtests/manifest.mn | 1 + lib/certdb/alg1485.c | 12 ++++-- nss.gyp | 1 + tests/gtests/gtests.sh | 2 +- 9 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 gtests/certdb_gtest/Makefile create mode 100644 gtests/certdb_gtest/alg1485_unittest.cc create mode 100644 gtests/certdb_gtest/certdb_gtest.gyp create mode 100644 gtests/certdb_gtest/manifest.mn diff --git a/cpputil/scoped_ptrs.h b/cpputil/scoped_ptrs.h index a2351984d3..39a5e1f05c 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 0000000000..0d547e0803 --- /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 0000000000..a6bd40b4b3 --- /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 + +#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 {}; + +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 0000000000..898102defe --- /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 0000000000..4a3a1fda09 --- /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 633129cf19..d572edad06 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 8d475ff77e..79598c02c5 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 dafc669626..97586b3337 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 6fb7e60df3..8c9d656cbe 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