389904: avoid multiple decoding/encoding while creating and using PKIX_PL_X500Name. r=nelson

security/coreconf/config.mk

@@ -185,3 +185,7 @@ endif
 ifdef NSS_ALLOW_UNSUPPORTED_CRITICAL
 DEFINES += -DNSS_ALLOW_UNSUPPORTED_CRITICAL
 endif
+
+ifdef BUILD_LIBPKIX_TESTS
+DEFINES += -DBUILD_LIBPKIX_TESTS
+endif

security/nss/lib/libpkix/include/pkix_errorstrings.h

@@ -141,6 +141,7 @@ PKIX_ERRORENTRY(CERTCHECKNAMECONSTRAINTSFAILED,PKIX_PL_Cert_CheckNameConstraints
 PKIX_ERRORENTRY(CERTCHECKVALIDITYFAILED,PKIX_PL_Cert_CheckValidity failed),
 PKIX_ERRORENTRY(CERTCOMPLETECRLDECODEDENTRIESFAILED,CERT_CompleteCRLDecodedEntries failed),
 PKIX_ERRORENTRY(CERTCOPYNAMECONSTRAINTFAILED,CERT_CopyNameConstraint failed),
+PKIX_ERRORENTRY(CERTCOPYNAMEFAILED,CERT_CopyName failed),
 PKIX_ERRORENTRY(CERTCREATEFAILED,PKIX_PL_Cert_Create failed),
 PKIX_ERRORENTRY(CERTCREATEGENERALNAMELISTFAILED,CERT_CreateGeneralNameList failed),
 PKIX_ERRORENTRY(CERTCREATETOLISTFAILED,pkix_pl_Cert_CreateToList failed),
@@ -834,6 +835,7 @@ PKIX_ERRORENTRY(POLICYNODEGETPOLICYQUALIFIERSFAILED,PKIX_PolicyNode_GetPolicyQua
 PKIX_ERRORENTRY(POLICYNODEGETVALIDPOLICYFAILED,PKIX_PolicyNode_GetValidPolicy failed),
 PKIX_ERRORENTRY(POLICYNODEISCRITICALFAILED,PKIX_PolicyNode_IsCritical failed),
 PKIX_ERRORENTRY(POLICYNODEPRUNEFAILED,pkix_PolicyNode_Prune failed),
+PKIX_ERRORENTRY(PORTARENAALLOCFAILED,PORT_ArenaAlloc failed),
 PKIX_ERRORENTRY(PORTARENAZNEWFAILED,PORT_ArenaZNew failed),
 PKIX_ERRORENTRY(PORTNEWARENAFAILED,PORT_NewArena failed),
 PKIX_ERRORENTRY(PORTUCS2UTF8CONVERSIONFAILED,PORT_UCS2_UTF8Conversion failed.),
@@ -911,6 +913,7 @@ PKIX_ERRORENTRY(SECONDFIELDMUSTBEBETWEEN039,Second field must be between 0-39),
 PKIX_ERRORENTRY(SECONDKEYDSAPUBLICKEY,Second key is a DSA public key but has null parameters),
 PKIX_ERRORENTRY(SECONDKEYNOTDSAPUBLICKEY,Second key is not a DSA public key),
 PKIX_ERRORENTRY(SECONDPUBKEYTYPENULLKEY,secondPubKeyType is nullKey),
+PKIX_ERRORENTRY(SECQUICKDERDECODERFAILED,SEC_QuickDERDecodeItem failed),
 PKIX_ERRORENTRY(SECREADPKCS7CERTSFAILED,SEC_ReadPKCS7Certs failed),
 PKIX_ERRORENTRY(SELECTORMATCHFAILED,selectorMatch failed),
 PKIX_ERRORENTRY(SESSIONNOTANHTTPDEFAULTCLIENT,session is not an HttpDefaultClient),
@@ -1027,6 +1030,7 @@ PKIX_ERRORENTRY(VERSIONVALUEMUSTBEV1ORV2,Version value must be V1(0) or V2(1)),
 PKIX_ERRORENTRY(VERSIONVALUEMUSTBEV1V2ORV3,Version value must be v1(0) v2(1) or v3(2)),
 PKIX_ERRORENTRY(X500NAMECOMPAREDERBYTESFAILED,pkix_pl_X500Name_CompareDERBytes failed),
 PKIX_ERRORENTRY(X500NAMECREATEFAILED,PKIX_PL_X500Name_Create failed),
+PKIX_ERRORENTRY(X500NAMECREATEFROMCERTNAMEFAILED,pkix_pl_X500Name_CreateFromCERTName failed),
 PKIX_ERRORENTRY(X500NAMECREATEFROMUTF8FAILED,pkix_pl_X500Name_CreateFromUtf8 failed),
 PKIX_ERRORENTRY(X500NAMEEQUALSFAILED,PKIX_PL_X500Name_Equals failed),
 PKIX_ERRORENTRY(X500NAMEGETCOMMONNAMEFAILED,pkix_pl_X500Name_GetCommonName failed),

security/nss/lib/libpkix/include/pkix_pl_pki.h

@@ -44,6 +44,8 @@
 #define _PKIX_PL_PKI_H
 
 #include "pkixt.h"
+#include "seccomon.h"
+#include "certt.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -2103,6 +2105,7 @@ PKIX_PL_CRLEntry_GetCriticalExtensionOIDs(
         PKIX_List **pExtensions,  /* list of PKIX_PL_OID */
         void *plContext);
 
+#ifdef BUILD_LIBPKIX_TESTS
 /*
  * FUNCTION: PKIX_PL_X500Name_Create
  * DESCRIPTION:
@@ -2154,6 +2157,42 @@ PKIX_PL_X500Name_Create (
         PKIX_PL_X500Name **pName,
         void *plContext);
 
+#endif /* BUILD_LIBPKIX_TESTS */
+
+/*
+ * FUNCTION: PKIX_PL_X500Name_CreateFromCERTName
+ * DESCRIPTION:
+ * 
+ * The function creates x500Name using der encoded DN and/or pointer to
+ * CERTName. If arument "name" is NULL, but derName is supplied when
+ * the function generates nssDN(CERTName type) from der data. If derName
+ * is not supplied, CERTName *name will not be used to generate DN DER
+ * encoding.
+ *
+ * PARAMETERS:
+ *  "derName"
+ *      Address of DER representation of X500Name. Can be NULL
+ *  "name"
+ *      Address of CERTName representation of X500Name. Can be NULL
+ *  "pName"
+ *      Address where object pointer will be stored. Must be non-NULL.
+ *  "plContext"
+ *      Platform-specific context pointer.
+ * THREAD SAFETY:
+ *  Thread Safe (see Thread Safety Definitions in Programmer's Guide)
+ * RETURNS:
+ *  Returns NULL if the function succeeds.
+ *  Returns an X500Name Error if the function fails in a non-fatal way.
+ *  Returns a Fatal Error if the function fails in an unrecoverable way.
+ */
+PKIX_Error *
+PKIX_PL_X500Name_CreateFromCERTName(
+        SECItem *derName,
+        CERTName *name,
+        PKIX_PL_X500Name **pName,
+        void *plContext);
+
+
 /*
  * TYPE: PKIX_PL_X500Name_Match
  * DESCRIPTION:
@@ -2275,6 +2314,7 @@ PKIX_PL_Date_Create_CurrentOffBySeconds(
         PKIX_PL_Date **pDate,
         void *plContext);
 
+#ifdef BUILD_LIBPKIX_TESTS
 /*
  * FUNCTION: PKIX_PL_GeneralName_Create
  * DESCRIPTION:
@@ -2302,6 +2342,9 @@ PKIX_PL_Date_Create_CurrentOffBySeconds(
  *      iPAddress                       [7]     OCTET STRING,
  *      registeredID                    [8]     OBJECT IDENTIFIER }
  *
+ *
+ * NOTE: This function is allowed to be called only by pkix tests programs.
+ * 
  * PARAMETERS:
  *  "nameType"
  *      Type of GeneralName to be created. This must be one of the GeneralName
@@ -2325,6 +2368,7 @@ PKIX_PL_GeneralName_Create (
         PKIX_PL_String *stringRep,
         PKIX_PL_GeneralName **pGName,
         void *plContext);
+#endif /* BUILD_LIBPKIX_TESTS */
 
 /*
  * FUNCTION: PKIX_PL_CertNameConstraints_CheckNamesInNameSpace

security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_pk11certstore.c

@@ -191,7 +191,7 @@ pkix_pl_Pk11CertStore_CertQuery(
                 arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
                 if (arena) {
 
-                        PKIX_CHECK(pkix_pl_X500Name_GetSECName
+                        PKIX_CHECK(pkix_pl_X500Name_GetDERName
                                 (subjectName, arena, &nameItem, plContext),
                                 PKIX_X500NAMEGETSECNAMEFAILED);
 
@@ -379,7 +379,7 @@ pkix_pl_Pk11CertStore_CrlQuery(
                         (PKIX_PL_Object **)&issuer,
                         plContext),
                         PKIX_LISTGETITEMFAILED);
-                    PKIX_CHECK(pkix_pl_X500Name_GetSECName
+                    PKIX_CHECK(pkix_pl_X500Name_GetDERName
                         (issuer, arena, &nameItem, plContext),
                         PKIX_X500NAMEGETSECNAMEFAILED);
                     if (nameItem) {

security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_cert.c

@@ -1714,8 +1714,8 @@ PKIX_PL_Cert_GetSubject(
         void *plContext)
 {
         PKIX_PL_X500Name *pkixSubject = NULL;
-        CERTCertificate *nssCert = NULL;
+        CERTName *subjName = NULL;
-        char *utf8Subject = NULL;
+        SECItem  *derSubjName = NULL;
 
         PKIX_ENTER(CERT, "PKIX_PL_Cert_GetSubject");
         PKIX_NULLCHECK_THREE(cert, cert->nssCert, pCertSubject);
@@ -1727,19 +1727,19 @@ PKIX_PL_Cert_GetSubject(
 
                 if (cert->subject == NULL){
 
-                        nssCert = cert->nssCert;
+                        subjName = &cert->nssCert->subject;
-                        utf8Subject = nssCert->subjectName;
+                        derSubjName = &cert->nssCert->derSubject;
 
                         /* if there is no subject name */
-                        if (utf8Subject == NULL) {
+                        if (derSubjName->data == NULL) {
 
                                 pkixSubject = NULL;
 
                         } else {
-
+                                PKIX_CHECK(PKIX_PL_X500Name_CreateFromCERTName
-                                PKIX_CHECK(pkix_pl_X500Name_CreateFromUtf8
+                                    (derSubjName, subjName, &pkixSubject,
-                                    (utf8Subject, &pkixSubject, plContext),
+                                     plContext),
-                                    PKIX_X500NAMECREATEFROMUTF8FAILED);
+                                    PKIX_X500NAMECREATEFROMCERTNAMEFAILED);
 
                         }
                         /* save a cached copy in case it is asked for again */
@@ -1767,8 +1767,8 @@ PKIX_PL_Cert_GetIssuer(
         void *plContext)
 {
         PKIX_PL_X500Name *pkixIssuer = NULL;
-        CERTCertificate *nssCert = NULL;
+        SECItem  *derIssuerName = NULL;
-        char *utf8Issuer = NULL;
+        CERTName *issuerName = NULL;
 
         PKIX_ENTER(CERT, "PKIX_PL_Cert_GetIssuer");
         PKIX_NULLCHECK_THREE(cert, cert->nssCert, pCertIssuer);
@@ -1780,12 +1780,14 @@ PKIX_PL_Cert_GetIssuer(
 
                 if (cert->issuer == NULL){
 
-                        nssCert = cert->nssCert;
+                        issuerName = &cert->nssCert->issuer;
-                        utf8Issuer = nssCert->issuerName;
+                        derIssuerName = &cert->nssCert->derIssuer;
 
-                        PKIX_CHECK(pkix_pl_X500Name_CreateFromUtf8
+                        /* if there is no subject name */
-                                    (utf8Issuer, &pkixIssuer, plContext),
+                        PKIX_CHECK(PKIX_PL_X500Name_CreateFromCERTName
-                                    PKIX_X500NAMECREATEFROMUTF8FAILED);
+                                    (derIssuerName, issuerName,
+                                     &pkixIssuer, plContext),
+                                    PKIX_X500NAMECREATEFROMCERTNAMEFAILED);
 
                         /* save a cached copy in case it is asked for again */
                         cert->issuer = pkixIssuer;

security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_crl.c

@@ -965,8 +965,8 @@ PKIX_PL_CRL_GetIssuer(
 {
         PKIX_PL_String *crlString = NULL;
         PKIX_PL_X500Name *issuer = NULL;
-        char *utf8Issuer = NULL;
+        SECItem  *derIssuerName = NULL;
-        PKIX_UInt32 utf8Length;
+        CERTName *issuerName = NULL;
 
         PKIX_ENTER(CRL, "PKIX_PL_CRL_GetIssuer");
         PKIX_NULLCHECK_THREE(crl, crl->nssSignedCrl, pCRLIssuer);
@@ -978,27 +978,16 @@ PKIX_PL_CRL_GetIssuer(
 
                 if (crl->issuer == NULL) {
 
-                        PKIX_CRL_DEBUG("\t\tCalling CERT_NameToAscii\n");
+                        issuerName = &crl->nssSignedCrl->crl.name;
-                        utf8Issuer = CERT_NameToAscii
+                        derIssuerName = &crl->nssSignedCrl->crl.derName;
-                                (&(crl->nssSignedCrl->crl.name));
-
-                        PKIX_CRL_DEBUG("\t\tCalling PL_strlen\n");
-                        utf8Length = PL_strlen(utf8Issuer);
-
-                        PKIX_CHECK(PKIX_PL_String_Create
-                                    (PKIX_UTF8,
-                                    utf8Issuer,
-                                    utf8Length,
-                                    &crlString,
-                                    plContext),
-                                    PKIX_UNABLETOCREATECRLSTRING);
-
-                        PKIX_CHECK(PKIX_PL_X500Name_Create
-                                    (crlString,
-                                    &issuer,
-                                    plContext),
-                                    PKIX_UNABLETOCREATEISSUER);
 
+                        PKIX_CHECK(
+                            PKIX_PL_X500Name_CreateFromCERTName(derIssuerName,
+                                                                issuerName,
+                                                                &issuer,
+                                                                plContext),
+                            PKIX_X500NAMECREATEFROMCERTNAMEFAILED);
+                        
                         /* save a cached copy in case it is asked for again */
                         crl->issuer = issuer;
                 }
@@ -1013,12 +1002,6 @@ PKIX_PL_CRL_GetIssuer(
 
 cleanup:
 
-        if (utf8Issuer != NULL) {
-                PKIX_CRL_DEBUG("\t\tCalling PORT_Free\n");
-                PR_Free(utf8Issuer);
-                utf8Issuer = NULL;
-        }
-
         PKIX_DECREF(crlString);
 
         PKIX_RETURN(CRL);

security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_generalname.c

@@ -186,6 +186,7 @@ pkix_pl_DirectoryName_Create(
         void *plContext)
 {
         PKIX_PL_X500Name *pkixDN = NULL;
+        CERTName *dirName = NULL;
         PKIX_PL_String *pkixDNString = NULL;
         char *utf8String = NULL;
         PKIX_UInt32 utf8Length;
@@ -193,27 +194,11 @@ pkix_pl_DirectoryName_Create(
         PKIX_ENTER(GENERALNAME, "pkix_pl_DirectoryName_Create");
         PKIX_NULLCHECK_TWO(nssAltName, pX500Name);
 
-        PKIX_GENERALNAME_DEBUG("\t\tCalling CERT_NameToAscii).\n");
+        dirName = &nssAltName->name.directoryName;
-        /* should be called CERT_NameToUTF8 */
-        utf8String = CERT_NameToAscii(&nssAltName->name.directoryName);
 
-        if (!utf8String){
+        PKIX_CHECK(PKIX_PL_X500Name_CreateFromCERTName(NULL, dirName, 
-                PKIX_ERROR(PKIX_CERTNAMETOASCIIFAILED);
+                                                       &pkixDN, plContext),
-        }
+                   PKIX_X500NAMECREATEFROMCERTNAMEFAILED);
-
-        PKIX_GENERALNAME_DEBUG("\t\tCalling PL_strlen).\n");
-        utf8Length = PL_strlen(utf8String);
-
-        PKIX_CHECK(PKIX_PL_String_Create
-                    (PKIX_UTF8,
-                    utf8String,
-                    utf8Length,
-                    &pkixDNString,
-                    plContext),
-                    PKIX_STRINGCREATEFAILED);
-
-        PKIX_CHECK(PKIX_PL_X500Name_Create(pkixDNString, &pkixDN, plContext),
-                    PKIX_X500NAMECREATEFAILED);
 
         *pX500Name = pkixDN;
 
@@ -810,6 +795,7 @@ pkix_pl_GeneralName_RegisterSelf(void *plContext)
 
 /* --Public-Functions------------------------------------------------------- */
 
+#ifdef BUILD_LIBPKIX_TESTS
 /*
  * FUNCTION: PKIX_PL_GeneralName_Create (see comments in pkix_pl_pki.h)
  */
@@ -869,11 +855,6 @@ PKIX_PL_GeneralName_Create(
 
         case certDirectoryName:
 
-
-
-
-
-
                 PKIX_CHECK(PKIX_PL_X500Name_Create
                             (stringRep, &pkixDN, plContext),
                             PKIX_X500NAMECREATEFAILED);
@@ -946,3 +927,5 @@ cleanup:
 
         PKIX_RETURN(GENERALNAME);
 }
+
+#endif /* BUILD_LIBPKIX_TESTS */

security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_x500name.c

@@ -45,123 +45,6 @@
 
 /* --Private-X500Name-Functions------------------------------------- */
 
-/*
- * FUNCTION: pkix_pl_X500Name_CompareDERBytes
- * DESCRIPTION:
- *
- *  Checks whether the DER encoding of the X500Name pointed to by
- *  "firstX500Name" is byte-for-byte equal with the DER encoding of the
- *  X500Name pointed to by "secondX500Name" and stores the Boolean result at
- *  "pResult".
- *
- * PARAMETERS
- *  "firstX500Name"
- *      Address of first X500Name. Must be non-NULL.
- *  "secondX500Name"
- *      Address of second X500Name. Must be non-NULL.
- *  "pResult"
- *      Address where Boolean will be stored. Must be non-NULL.
- *  "plContext"
- *      Platform-specific context pointer.
- * THREAD SAFETY:
- *  Thread Safe (see Thread Safety Definitions in Programmer's Guide)
- * RETURNS:
- *  Returns NULL if the function succeeds.
- *  Returns a X500Name Error if the function fails in a non-fatal way.
- *  Returns a Fatal Error if the function fails in an unrecoverable way.
- */
-static PKIX_Error *
-pkix_pl_X500Name_CompareDERBytes(
-        PKIX_PL_X500Name *firstX500Name,
-        PKIX_PL_X500Name *secondX500Name,
-        PKIX_Boolean *pResult,
-        void *plContext)
-{
-        CERTName *firstName = NULL;
-        CERTName *secondName = NULL;
-        CERTRDN **ardns = NULL;
-        CERTRDN **brdns = NULL;
-        CERTRDN *ardn = NULL;
-        CERTRDN *brdn = NULL;
-        CERTAVA **aavas = NULL;
-        CERTAVA **bavas = NULL;
-        CERTAVA *aava = NULL;
-        CERTAVA *bava = NULL;
-        PKIX_UInt32 ac, bc;
-        SECComparison rv;
-
-        PKIX_ENTER(X500NAME, "pkix_pl_X500Name_CompareDERBytes");
-        PKIX_NULLCHECK_THREE(firstX500Name, secondX500Name, pResult);
-
-        firstName = firstX500Name->nssDN;
-        secondName = secondX500Name->nssDN;
-
-        PKIX_NULLCHECK_TWO(firstName, secondName);
-
-        ardns = firstName->rdns;
-        brdns = secondName->rdns;
-
-        /* if array of rdn's not same length, names can't be equal */
-        ac = pkix_countArray((void**) ardns);
-        bc = pkix_countArray((void**) brdns);
-        if (ac != bc){
-                *pResult = PKIX_FALSE;
-                goto cleanup;
-        }
-
-        for (;;) {
-                PKIX_NULLCHECK_TWO(ardns, brdns);
-                ardn = *ardns++;
-                brdn = *brdns++;
-                if (!ardn) {
-                        break;
-                }
-
-                PKIX_NULLCHECK_TWO(ardn, brdn);
-                aavas = ardn->avas;
-                bavas = brdn->avas;
-
-                /* if array of ava's not same length, names can't be equal */
-                ac = pkix_countArray((void**) aavas);
-                bc = pkix_countArray((void**) bavas);
-                if (ac != bc){
-                        *pResult = PKIX_FALSE;
-                        goto cleanup;
-                }
-
-                for (;;) {
-                        PKIX_NULLCHECK_TWO(aavas, bavas);
-                        aava = *aavas++;
-                        bava = *bavas++;
-                        if (!aava) {
-                                break;
-                        }
-
-                        PKIX_X500NAME_DEBUG("\t\tCalling "
-                                            "SECITEM_CompareItem).\n");
-                        rv = SECITEM_CompareItem(&aava->type, &bava->type);
-                        if (rv != SECEqual){
-                                *pResult = PKIX_FALSE;
-                                goto cleanup;
-                        }
-
-                        PKIX_X500NAME_DEBUG("\t\tCalling "
-                                            "SECITEM_CompareItem).\n");
-                        rv = SECITEM_CompareItem(&aava->value, &bava->value);
-                        if (rv != SECEqual){
-                                *pResult = PKIX_FALSE;
-                                goto cleanup;
-                        }
-                }
-        }
-
-        *pResult = (rv == SECEqual);
-
-cleanup:
-
-        PKIX_RETURN(X500NAME);
-}
-
 /*
  * FUNCTION: pkix_pl_X500Name_ToString_Helper
  * DESCRIPTION:
@@ -195,7 +78,7 @@ pkix_pl_X500Name_ToString_Helper(
 
         PKIX_ENTER(X500NAME, "pkix_pl_X500Name_ToString_Helper");
         PKIX_NULLCHECK_TWO(name, pString);
-        nssDN = name->nssDN;
+        nssDN = &name->nssDN;
 
         PKIX_X500NAME_DEBUG("\t\tCalling CERT_NameToAscii).\n");
         /* this should really be called CERT_NameToUTF8 */
@@ -237,9 +120,12 @@ pkix_pl_X500Name_Destroy(
 
         name = (PKIX_PL_X500Name *)object;
 
-        PKIX_X500NAME_DEBUG("\t\tCalling CERT_DestroyName).\n");
+        /* PORT_FreeArena will destroy arena, and, allocated on it, CERTName
-        CERT_DestroyName(name->nssDN);
+         * and SECItem */
-        name->nssDN = NULL;
+        if (name->arena) {
+            PORT_FreeArena(name->arena, PR_FALSE);
+            name->arena = NULL;
+        }
 
 cleanup:
 
@@ -289,11 +175,8 @@ pkix_pl_X500Name_Hashcode(
         void *plContext)
 {
         PKIX_PL_X500Name *name = NULL;
-        SECItem *resultSecItem = NULL;
+        SECItem *derBytes = NULL;
-        PRArenaPool *arena = NULL;
-        CERTName *nssDN = NULL;
         PKIX_UInt32 nameHash;
-        SECItem *derBytes;
 
         PKIX_ENTER(X500NAME, "pkix_pl_X500Name_Hashcode");
         PKIX_NULLCHECK_TWO(object, pHashcode);
@@ -305,27 +188,7 @@ pkix_pl_X500Name_Hashcode(
 
         /* we hash over the bytes in the DER encoding */
 
-        nssDN = name->nssDN;
+        derBytes = &name->derName;
-
-        PKIX_X500NAME_DEBUG("\t\tCalling PORT_NewArena).\n");
-        arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-        if (arena == NULL) {
-                PKIX_ERROR(PKIX_PORTNEWARENAFAILED);
-        }
-
-        PKIX_X500NAME_DEBUG("\t\tCalling PORT_ArenaZNew).\n");
-        derBytes = PORT_ArenaZNew(arena, SECItem);
-        if (derBytes == NULL) {
-                PKIX_ERROR(PKIX_PORTARENAZNEWFAILED);
-        }
-
-        PKIX_X500NAME_DEBUG("\t\tCalling SEC_ASN1EncodeItem).\n");
-        resultSecItem =
-                SEC_ASN1EncodeItem(arena, derBytes, nssDN, CERT_NameTemplate);
-
-        if (resultSecItem == NULL){
-                PKIX_ERROR(PKIX_SECASN1ENCODEITEMFAILED);
-        }
 
         PKIX_CHECK(pkix_hash
                     (derBytes->data, derBytes->len, &nameHash, plContext),
@@ -335,12 +198,6 @@ pkix_pl_X500Name_Hashcode(
 
 cleanup:
 
-        if (arena){
-                /* Note that freeing the arena also frees derBytes */
-                PKIX_X500NAME_DEBUG("\t\tCalling PORT_FreeArena).\n");
-                PORT_FreeArena(arena, PR_FALSE);
-        }
-
         PKIX_RETURN(X500NAME);
 }
 
@@ -356,8 +213,6 @@ pkix_pl_X500Name_Equals(
         PKIX_Boolean *pResult,
         void *plContext)
 {
-        PKIX_PL_X500Name *firstX500Name = NULL;
-        PKIX_PL_X500Name *secondX500Name = NULL;
         PKIX_UInt32 secondType;
 
         PKIX_ENTER(X500NAME, "pkix_pl_X500Name_Equals");
@@ -386,13 +241,11 @@ pkix_pl_X500Name_Equals(
                     PKIX_COULDNOTGETTYPEOFSECONDARGUMENT);
         if (secondType != PKIX_X500NAME_TYPE) goto cleanup;
 
-        firstX500Name = (PKIX_PL_X500Name *)firstObject;
+        PKIX_CHECK(
-        secondX500Name = (PKIX_PL_X500Name *)secondObject;
+            PKIX_PL_X500Name_Match((PKIX_PL_X500Name *)firstObject,
-
+                                   (PKIX_PL_X500Name *)secondObject,
-        /* we simply do byte comparison on DER encodings of DN's */
+                                   pResult, plContext),
-        PKIX_CHECK(pkix_pl_X500Name_CompareDERBytes
+            PKIX_X500NAMEMATCHFAILED);
-                    (firstX500Name, secondX500Name, pResult, plContext),
-                    PKIX_X500NAMECOMPAREDERBYTESFAILED);
 
 cleanup:
 
@@ -432,162 +285,7 @@ pkix_pl_X500Name_RegisterSelf(void *plContext)
         PKIX_RETURN(X500NAME);
 }
 
-/* --Public-Functions------------------------------------------------------- */
+#ifdef BUILD_LIBPKIX_TESTS
-
-/*
- * FUNCTION: PKIX_PL_X500Name_Create (see comments in pkix_pl_pki.h)
- */
-PKIX_Error *
-PKIX_PL_X500Name_Create(
-        PKIX_PL_String *stringRep,
-        PKIX_PL_X500Name **pName,
-        void *plContext)
-{
-        PKIX_PL_X500Name *x500Name = NULL;
-        CERTName *nssDN = NULL;
-        char *utf8String = NULL;
-        PKIX_UInt32 utf8Length;
-
-        PKIX_ENTER(X500NAME, "PKIX_PL_X500Name_Create()");
-        PKIX_NULLCHECK_TWO(pName, stringRep);
-
-        /*
-         * convert the input PKIX_PL_String to PKIX_UTF8_NULL_TERM.
-         * we need to use this format specifier because
-         * CERT_AsciiToName expects a NULL-terminated UTF8 string.
-         * Since UTF8 allow NUL characters in the middle of the
-         * string, this is buggy. However, as a workaround, using
-         * PKIX_UTF8_NULL_TERM gives us a NULL-terminated UTF8 string.
-         */
-
-        PKIX_CHECK(PKIX_PL_String_GetEncoded
-                    (stringRep,
-                    PKIX_UTF8_NULL_TERM,
-                    (void **)&utf8String,
-                    &utf8Length,
-                    plContext),
-                    PKIX_STRINGGETENCODEDFAILED);
-
-        PKIX_X500NAME_DEBUG("\t\tCalling CERT_AsciiToName).\n");
-        /* this should be really be called CERT_UTF8ToName */
-        nssDN = CERT_AsciiToName(utf8String);
-        if (nssDN == NULL) {
-                PKIX_ERROR(PKIX_COULDNOTCREATENSSDN);
-        }
-
-        /* create a PKIX_PL_X500Name object */
-        PKIX_CHECK(PKIX_PL_Object_Alloc
-                    (PKIX_X500NAME_TYPE,
-                    sizeof (PKIX_PL_X500Name),
-                    (PKIX_PL_Object **)&x500Name,
-                    plContext),
-                    PKIX_COULDNOTCREATEX500NAMEOBJECT);
-
-        /* populate the nssDN field */
-        x500Name->nssDN = nssDN;
-
-        *pName = x500Name;
-
-cleanup:
-
-        PKIX_FREE(utf8String);
-
-        if (nssDN && PKIX_ERROR_RECEIVED){
-                PKIX_X500NAME_DEBUG("\t\tCalling CERT_DestroyName).\n");
-                CERT_DestroyName(nssDN);
-                nssDN = NULL;
-        }
-
-        PKIX_RETURN(X500NAME);
-}
-
-/*
- * FUNCTION: PKIX_PL_X500Name_Match (see comments in pkix_pl_pki.h)
- */
-PKIX_Error *
-PKIX_PL_X500Name_Match(
-        PKIX_PL_X500Name *firstX500Name,
-        PKIX_PL_X500Name *secondX500Name,
-        PKIX_Boolean *pResult,
-        void *plContext)
-{
-        CERTName *firstName = NULL;
-        CERTName *secondName = NULL;
-        SECComparison cmpResult;
-
-        PKIX_ENTER(X500NAME, "PKIX_PL_X500Name_Equals");
-        PKIX_NULLCHECK_THREE(firstX500Name, secondX500Name, pResult);
-
-        if (firstX500Name == secondX500Name){
-                *pResult = PKIX_TRUE;
-                goto cleanup;
-        }
-
-        firstName = firstX500Name->nssDN;
-        secondName = secondX500Name->nssDN;
-
-        PKIX_NULLCHECK_TWO(firstName, secondName);
-
-        PKIX_X500NAME_DEBUG("\t\tCalling CERT_CompareName).\n");
-        cmpResult = CERT_CompareName(firstName, secondName);
-
-        *pResult = (cmpResult == SECEqual);
-
-cleanup:
-
-        PKIX_RETURN(X500NAME);
-}
-
-/*
- * FUNCTION: pkix_pl_X500Name_GetSECName
- *
- * DESCRIPTION:
- *  Encodes as a SECItem the CERTName embodied by the X500Name object pointed
- *  to by "xname", using the arena pointed to by "arena", and stores the result
- *  at "pSECName". If the name cannot be successfully encoded, NULL is stored
- *  at "pSECName".
- *
- * PARAMETERS:
- *  "xname"
- *      Address of X500Name whose CERTName flag is to be encoded. Must be
- *      non-NULL.
- *  "arena"
- *      Address of the PRArenaPool to be used in the encoding, and in which
- *      "pSECName" will be allocated. Must be non-NULL.
- *  "pSECName"
- *      Address where result will be stored. Must be non-NULL.
- *  "plContext"
- *      Platform-specific context pointer.
- *
- * THREAD SAFETY:
- *  Thread Safe (see Thread Safety Definitions in Programmer's Guide)
- *
- * RETURNS:
- *  Returns NULL if the function succeeds.
- *  Returns a Fatal Error if the function fails in an unrecoverable way.
- */
-PKIX_Error *
-pkix_pl_X500Name_GetSECName(
-        PKIX_PL_X500Name *xname,
-        PRArenaPool *arena,
-        SECItem **pSECName,
-        void *plContext)
-{
-
-        PKIX_ENTER(X500NAME, "pkix_pl_X500Name_GetCertName");
-
-        PKIX_NULLCHECK_THREE(xname, arena, pSECName);
-
-        /*
-         * SEC_ASN1EncodeItem returns NULL if unsuccessful. We just
-         * store the NULL result.
-         */
-        PKIX_PL_NSSCALLRV(X500NAME, *pSECName, SEC_ASN1EncodeItem,
-                (arena, NULL, (void *)xname->nssDN, CERT_NameTemplate));
-
-        PKIX_RETURN(X500NAME);
-}
-
 /*
  * FUNCTION: pkix_pl_X500Name_CreateFromUtf8
  *
@@ -596,6 +294,9 @@ pkix_pl_X500Name_GetSECName(
  *  to by "stringRep", and stores the result at "pName". If the string cannot
  *  be successfully converted, a non-fatal error is returned.
  *
+ * NOTE: ifdefed BUILD_LIBPKIX_TESTS function: this function is allowed to be
+ * called only by pkix tests programs.
+ * 
  * PARAMETERS:
  *  "stringRep"
  *      Address of the RFC1485 string to be converted. Must be non-NULL.
@@ -619,17 +320,20 @@ pkix_pl_X500Name_CreateFromUtf8(
         void *plContext)
 {
         PKIX_PL_X500Name *x500Name = NULL;
+        PRArenaPool *arena = NULL;
         CERTName *nssDN = NULL;
-
+        SECItem *resultSecItem = NULL;
+        
         PKIX_ENTER(X500NAME, "pkix_pl_X500Name_CreateFromUtf8");
         PKIX_NULLCHECK_TWO(pName, stringRep);
 
-        PKIX_PL_NSSCALLRV(X500NAME, nssDN, CERT_AsciiToName, (stringRep));
+        nssDN = CERT_AsciiToName(stringRep);
-
         if (nssDN == NULL) {
-                PKIX_ERROR(PKIX_COULDNOTCREATENSSDN);
+            PKIX_ERROR(PKIX_COULDNOTCREATENSSDN);
         }
 
+        arena = nssDN->arena;
+
         /* create a PKIX_PL_X500Name object */
         PKIX_CHECK(PKIX_PL_Object_Alloc
                     (PKIX_X500NAME_TYPE,
@@ -639,20 +343,291 @@ pkix_pl_X500Name_CreateFromUtf8(
                     PKIX_COULDNOTCREATEX500NAMEOBJECT);
 
         /* populate the nssDN field */
-        x500Name->nssDN = nssDN;
+        x500Name->arena = arena;
+        x500Name->nssDN.arena = arena;
+        x500Name->nssDN.rdns = nssDN->rdns;
+        
+        resultSecItem =
+            SEC_ASN1EncodeItem(arena, &x500Name->derName, nssDN,
+                               CERT_NameTemplate);
+        
+        if (resultSecItem == NULL){
+            PKIX_ERROR(PKIX_SECASN1ENCODEITEMFAILED);
+        }
 
         *pName = x500Name;
 
 cleanup:
 
-        if (nssDN && PKIX_ERROR_RECEIVED){
+        if (PKIX_ERROR_RECEIVED){
-                PKIX_X500NAME_DEBUG("\t\tCalling CERT_DestroyName).\n");
+            if (x500Name) {
+                PKIX_PL_Object_DecRef((PKIX_PL_Object*)x500Name,
+                                      plContext);
+            } else if (nssDN) {
                 CERT_DestroyName(nssDN);
-                nssDN = NULL;
+            }
         }
 
         PKIX_RETURN(X500NAME);
 }
+#endif /* BUILD_LIBPKIX_TESTS */
+
+/*
+ * FUNCTION: pkix_pl_X500Name_GetCERTName
+ *
+ * DESCRIPTION:
+ * 
+ * Returns the pointer to CERTName member of X500Name structure.
+ *
+ * Returned pointed should not be freed.2
+ *
+ * PARAMETERS:
+ *  "xname"
+ *      Address of X500Name whose OrganizationName is to be extracted. Must be
+ *      non-NULL.
+ *  "pCERTName"
+ *      Address where result will be stored. Must be non-NULL.
+ *  "plContext"
+ *      Platform-specific context pointer.
+ *
+ * THREAD SAFETY:
+ *  Thread Safe (see Thread Safety Definitions in Programmer's Guide)
+ *
+ * RETURNS:
+ *  Returns NULL if the function succeeds.
+ *  Returns a Fatal Error if the function fails in an unrecoverable way.
+ */
+PKIX_Error *
+pkix_pl_X500Name_GetCERTName(
+        PKIX_PL_X500Name *xname,
+        CERTName **pCERTName,
+        void *plContext)
+{
+        PKIX_ENTER(X500NAME, "pkix_pl_X500Name_GetCERTName");
+        PKIX_NULLCHECK_TWO(xname, pCERTName);
+
+        *pCERTName = &xname->nssDN;
+
+        PKIX_RETURN(X500NAME);
+}
+
+/* --Public-Functions------------------------------------------------------- */
+
+/*
+ * FUNCTION: PKIX_PL_X500Name_CreateFromCERTName (see comments in pkix_pl_pki.h)
+ */
+
+PKIX_Error *
+PKIX_PL_X500Name_CreateFromCERTName(
+        SECItem *derName,
+        CERTName *name, 
+        PKIX_PL_X500Name **pName,
+        void *plContext)
+{
+        PRArenaPool *arena = NULL;
+        SECStatus rv = SECFailure;
+        PKIX_PL_X500Name *x500Name = NULL;
+
+        PKIX_ENTER(X500NAME, "PKIX_PL_X500Name_CreateFromCERTName");
+        PKIX_NULLCHECK_ONE(pName);
+        if (derName == NULL && name == NULL) {
+            PKIX_ERROR(PKIX_NULLARGUMENT);
+        }
+
+        arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
+        if (arena == NULL) {
+            PKIX_ERROR(PKIX_PORTARENAALLOCFAILED);
+        }
+        
+        PKIX_CHECK(PKIX_PL_Object_Alloc
+                    (PKIX_X500NAME_TYPE,
+                    sizeof (PKIX_PL_X500Name),
+                    (PKIX_PL_Object **)&x500Name,
+                    plContext),
+                    PKIX_COULDNOTCREATEX500NAMEOBJECT);
+
+        x500Name->arena = arena;
+        x500Name->nssDN.arena = NULL;
+
+        if (derName != NULL) {
+            rv = SECITEM_CopyItem(arena, &x500Name->derName, derName);
+            if (rv == SECFailure) {
+                PKIX_ERROR(PKIX_SECITEMCOPYITEMFAILED);
+            }
+        }            
+
+        if (name != NULL) {
+            rv = CERT_CopyName(arena, &x500Name->nssDN, name);
+            if (rv == SECFailure) {
+                PKIX_ERROR(PKIX_CERTCOPYNAMEFAILED);
+            }
+        } else {
+            rv = SEC_QuickDERDecodeItem(arena, &x500Name->nssDN,
+                                        CERT_NameTemplate,
+                                        &x500Name->derName);
+            if (rv == SECFailure) {
+                PKIX_ERROR(PKIX_SECQUICKDERDECODERFAILED);
+            }
+        }
+
+        *pName = x500Name;
+
+cleanup:
+        if (PKIX_ERROR_RECEIVED) {
+            if (x500Name) {
+                PKIX_PL_Object_DecRef((PKIX_PL_Object*)x500Name,
+                                      plContext);
+            } else if (arena) {                
+                PORT_FreeArena(arena, PR_FALSE);
+            }
+        }
+
+        PKIX_RETURN(X500NAME);
+}
+
+#ifdef BUILD_LIBPKIX_TESTS
+/*
+ * FUNCTION: PKIX_PL_X500Name_Create (see comments in pkix_pl_pki.h)
+ *
+ * NOTE: ifdefed BUILD_LIBPKIX_TESTS function: this function is allowed
+ * to be called only by pkix tests programs.
+ */
+PKIX_Error *
+PKIX_PL_X500Name_Create(
+        PKIX_PL_String *stringRep,
+        PKIX_PL_X500Name **pName,
+        void *plContext)
+{
+        char *utf8String = NULL;
+        PKIX_UInt32 utf8Length = 0;
+
+        PKIX_ENTER(X500NAME, "PKIX_PL_X500Name_Create");
+        PKIX_NULLCHECK_TWO(pName, stringRep);
+
+        /*
+         * convert the input PKIX_PL_String to PKIX_UTF8_NULL_TERM.
+         * we need to use this format specifier because
+         * CERT_AsciiToName expects a NULL-terminated UTF8 string.
+         * Since UTF8 allow NUL characters in the middle of the
+         * string, this is buggy. However, as a workaround, using
+         * PKIX_UTF8_NULL_TERM gives us a NULL-terminated UTF8 string.
+         */
+
+        PKIX_CHECK(PKIX_PL_String_GetEncoded
+                    (stringRep,
+                    PKIX_UTF8_NULL_TERM,
+                    (void **)&utf8String,
+                    &utf8Length,
+                    plContext),
+                    PKIX_STRINGGETENCODEDFAILED);
+
+        PKIX_CHECK(
+            pkix_pl_X500Name_CreateFromUtf8(utf8String,
+                                            pName, plContext),
+            PKIX_X500NAMECREATEFROMUTF8FAILED);
+
+cleanup:
+        PKIX_FREE(utf8String);
+
+        PKIX_RETURN(X500NAME);
+}
+#endif /* BUILD_LIBPKIX_TESTS */
+
+/*
+ * FUNCTION: PKIX_PL_X500Name_Match (see comments in pkix_pl_pki.h)
+ */
+PKIX_Error *
+PKIX_PL_X500Name_Match(
+        PKIX_PL_X500Name *firstX500Name,
+        PKIX_PL_X500Name *secondX500Name,
+        PKIX_Boolean *pResult,
+        void *plContext)
+{
+        SECItem *firstDerName = NULL;
+        SECItem *secondDerName = NULL;
+        SECComparison cmpResult;
+
+        PKIX_ENTER(X500NAME, "PKIX_PL_X500Name_Match");
+        PKIX_NULLCHECK_THREE(firstX500Name, secondX500Name, pResult);
+
+        if (firstX500Name == secondX500Name){
+                *pResult = PKIX_TRUE;
+                goto cleanup;
+        }
+
+        firstDerName = &firstX500Name->derName;
+        secondDerName = &secondX500Name->derName;
+
+        PKIX_NULLCHECK_TWO(firstDerName->data, secondDerName->data);
+
+        cmpResult = SECITEM_CompareItem(firstDerName, secondDerName);
+        if (cmpResult != SECEqual) {
+            cmpResult = CERT_CompareName(&firstX500Name->nssDN,
+                                         &secondX500Name->nssDN);
+        }
+
+        *pResult = (cmpResult == SECEqual);
+                   
+cleanup:
+
+        PKIX_RETURN(X500NAME);
+}
+
+/*
+ * FUNCTION: pkix_pl_X500Name_GetSECName
+ *
+ * DESCRIPTION:
+ *  Returns a copy of CERTName DER representation allocated on passed in arena.
+ *  If allocation on arena can not be done, NULL is stored at "pSECName".
+ *
+ * PARAMETERS:
+ *  "xname"
+ *      Address of X500Name whose CERTName flag is to be encoded. Must be
+ *      non-NULL.
+ *  "arena"
+ *      Address of the PRArenaPool to be used in the encoding, and in which
+ *      "pSECName" will be allocated. Must be non-NULL.
+ *  "pSECName"
+ *      Address where result will be stored. Must be non-NULL.
+ *  "plContext"
+ *      Platform-specific context pointer.
+ *
+ * THREAD SAFETY:
+ *  Thread Safe (see Thread Safety Definitions in Programmer's Guide)
+ *
+ * RETURNS:
+ *  Returns NULL if the function succeeds.
+ *  Returns a Fatal Error if the function fails in an unrecoverable way.
+ */
+PKIX_Error *
+pkix_pl_X500Name_GetDERName(
+        PKIX_PL_X500Name *xname,
+        PRArenaPool *arena,
+        SECItem **pDERName,
+        void *plContext)
+{
+        SECItem *derName = NULL;
+
+        PKIX_ENTER(X500NAME, "pkix_pl_X500Name_GetDERName");
+
+        PKIX_NULLCHECK_THREE(xname, arena, pDERName);
+
+        /* Return NULL is X500Name was not created from DER  */
+        if (xname->derName.data == NULL) {
+            *pDERName = NULL;
+            goto cleanup;
+        }
+
+        derName = SECITEM_ArenaDupItem(arena, &xname->derName);
+        if (derName == NULL) {
+            PKIX_ERROR(PKIX_SECITEMCOPYITEMFAILED);
+        }
+
+        *pDERName = derName;
+cleanup:
+
+        PKIX_RETURN(X500NAME);
+}
 
 /*
  * FUNCTION: pkix_pl_X500Name_GetCommonName
@@ -689,11 +664,7 @@ pkix_pl_X500Name_GetCommonName(
         PKIX_ENTER(X500NAME, "pkix_pl_X500Name_GetCommonName");
         PKIX_NULLCHECK_TWO(xname, pCommonName);
 
-        PKIX_PL_NSSCALLRV
+        *pCommonName = (unsigned char *)CERT_GetCommonName(&xname->nssDN);
-                (X500NAME,
-                *pCommonName,
-                (unsigned char *)CERT_GetCommonName,
-                (xname->nssDN));
 
         PKIX_RETURN(X500NAME);
 }
@@ -733,11 +704,7 @@ pkix_pl_X500Name_GetCountryName(
         PKIX_ENTER(X500NAME, "pkix_pl_X500Name_GetCountryName");
         PKIX_NULLCHECK_TWO(xname, pCountryName);
 
-        PKIX_PL_NSSCALLRV
+        *pCountryName = (unsigned char*)CERT_GetCountryName(&xname->nssDN);
-                (X500NAME,
-                *pCountryName,
-                (unsigned char *)CERT_GetCountryName,
-                (xname->nssDN));
 
         PKIX_RETURN(X500NAME);
 }
@@ -777,24 +744,7 @@ pkix_pl_X500Name_GetOrgName(
         PKIX_ENTER(X500NAME, "pkix_pl_X500Name_GetOrgName");
         PKIX_NULLCHECK_TWO(xname, pOrgName);
 
-        PKIX_PL_NSSCALLRV
+        *pOrgName = (unsigned char*)CERT_GetOrgName(&xname->nssDN);
-                (X500NAME,
-                *pOrgName,
-                (unsigned char *)CERT_GetOrgName,
-                (xname->nssDN));
-
-        PKIX_RETURN(X500NAME);
-}
-PKIX_Error *
-pkix_pl_X500Name_GetCERTName(
-        PKIX_PL_X500Name *xname,
-        CERTName **pCERTName,
-        void *plContext)
-{
-        PKIX_ENTER(X500NAME, "pkix_pl_X500Name_GetCERTName");
-        PKIX_NULLCHECK_TWO(xname, pCERTName);
-
-        *pCERTName = xname->nssDN;
 
         PKIX_RETURN(X500NAME);
 }

security/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_x500name.h

@@ -50,24 +50,32 @@
 extern "C" {
 #endif
 
+
 struct PKIX_PL_X500NameStruct{
-        CERTName *nssDN;
+        PRArenaPool *arena; /* X500Name arena. Shared arena with nssDN
+                             * and derName */
+        CERTName nssDN;
+        SECItem derName;    /* adding DER encoded CERTName to the structure
+                             * to avoid unnecessary name encoding when pass
+                             * der name to cert finder */
 };
 
 /* see source file for function documentation */
 
 PKIX_Error *pkix_pl_X500Name_RegisterSelf(void *plContext);
 
-PKIX_Error *pkix_pl_X500Name_GetSECName(
+PKIX_Error *pkix_pl_X500Name_GetDERName(
         PKIX_PL_X500Name *xname,
         PRArenaPool *arena,
         SECItem **pSECName,
         void *plContext);
 
+#ifdef BUILD_LIBPKIX_TESTS
 PKIX_Error * pkix_pl_X500Name_CreateFromUtf8(
         char *stringRep,
         PKIX_PL_X500Name **pName,
         void *plContext);
+#endif /* BUILD_LIBPKIX_TESTS */
 
 PKIX_Error *pkix_pl_X500Name_GetCommonName(
         PKIX_PL_X500Name *xname,