Last Comment Bug 287563 - Please make cert_CompareNameWithConstraints a non-static function
: Please make cert_CompareNameWithConstraints a non-static function
Status: RESOLVED FIXED
PKIX
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.10
: All All
: P2 normal (vote)
: 3.12
Assigned To: Alexei Volkov
:
Mentors:
Depends on:
Blocks: 358785
  Show dependency treegraph
 
Reported: 2005-03-24 07:21 PST by Hanfei Yu
Modified: 2007-08-16 11:49 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Split CERT_CompareNameSpace in genname.c into two functions. Also provide a new one. (2.90 KB, text/plain)
2005-04-15 04:25 PDT, Hanfei Yu
no flags Details
Updated patch that includes header file changes (10.26 KB, patch)
2005-07-15 10:18 PDT, Julien Pierre
no flags Details | Diff | Splinter Review

Description Hanfei Yu 2005-03-24 07:21:03 PST
User-Agent:       Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.4) Gecko/20041214
Build Identifier: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.4) Gecko/20041214

This function in lib/certdb/genname.c is useful for checking NameConstraints. It
can save duplicate codes in our component, please make it non-static.

Or even better, the function CERT_CompareNameSpace can also be separated into
two functions. One for getting NameConstraintsExten (name such as
CERT_FindNameConstraintsExten) and the other for doing the actual
checking/comparison.

Reproducible: Always
Comment 1 Nelson Bolyard (seldom reads bugmail) 2005-03-29 10:33:18 PST
Hanfei, You proposed splitting this function.  Would you care to contribute
a patch implementing that suggestion?
Comment 2 Hanfei Yu 2005-04-14 07:18:55 PDT
Here are the two functions CERT_FindNameConstraintsExten() and
CERT_CheckNameSpace() that split CERT_CompareNameSpace(). I also include here
another function CERT_AddNameConstraintByGeneralName() that might be useful for
 creating/adding a CERTNameConstraint's permitted or excluded list from a 
CERTGeneralName.

Please also add their extern declarations in cert.h.  Thanks.

----------- genname.c --------
/* This function adds and links a CERTGeneralName to a CERTNameConstraint
** list. Most likely the CERTNameConstraint passed in is either the permited
** list or the excluded list of a CERTNameConstraints.
*/
SECStatus
CERT_AddNameConstraintByGeneralName(PLArenaPool *arena,
				    CERTNameConstraint **constraints,
				    CERTGeneralName *name)
{
    SECStatus rv;
    CERTNameConstraint *current = NULL;
    CERTNameConstraint *first = *constraints;
    CERTNameConstraint *last = *constraints;

    current = PORT_ArenaZNew(arena, CERTNameConstraint);
    if (current == NULL) {
	return SECFailure;
    }

    rv = cert_CopyOneGeneralName(arena, &current->name, name);
    if (rv != SECSuccess) {
	return SECFailure;
    }

    current->name.l.prev = current->name.l.next = &(current->name.l);

    if (first == NULL) {
	first = last = current;
	*constraints = current;
    }
    current->l.prev = &(last->l);
    current->l.next = last->l.next;
    last->l.next = &(current->l);

    return SECSuccess;

}

/* This function extracts the name constraints extension from the CA cert.
*/
SECStatus
CERT_FindNameConstraintsExten(PRArenaPool      *arena,
			      CERTCertificate  *cert,
			      CERTNameConstraints **constraints)
{
    SECStatus            rv;
    SECItem              constraintsExtension;

    *constraints = NULL;

    rv = CERT_FindCertExtension(cert, SEC_OID_X509_NAME_CONSTRAINTS, 
                                &constraintsExtension);
    if (rv != SECSuccess) {
	if (PORT_GetError() == SEC_ERROR_EXTENSION_NOT_FOUND) {
	    rv = SECSuccess;
	}
	goto done;
    }

    /* TODO: mark arena */
    *constraints = cert_DecodeNameConstraints(arena, &constraintsExtension);

    if (*constraints == NULL) { /* decode failed */
	rv = SECFailure;
	goto done;
    }
    PORT_Free (constraintsExtension.data);

done:
    return rv;
}

/* This function verifies name against all the constraints relevant
** to that type of the name.
*/
SECStatus
CERT_CheckNameSpace(PRArenaPool          *arena,
		    CERTNameConstraints  *constraints,
		    CERTGeneralName      *currentName)
{
    CERTNameConstraint  *matchingConstraints;
    SECStatus            rv = SECSuccess;

    if (constraints->excluded != NULL) {
 	rv = CERT_GetNameConstraintByType(constraints->excluded, 
					  currentName->type, 
					  &matchingConstraints, arena);
	if (rv == SECSuccess && matchingConstraints != NULL) {
 	    rv = cert_CompareNameWithConstraints(currentName, 
						 matchingConstraints,
						 PR_TRUE);
	}
	if (rv != SECSuccess) {
	    return(rv);
	}
    }

    if (constraints->permited != NULL) {
 	rv = CERT_GetNameConstraintByType(constraints->permited, 
					  currentName->type, 
					  &matchingConstraints, arena);
	if (rv == SECSuccess && matchingConstraints != NULL) {
 	    rv = cert_CompareNameWithConstraints(currentName, 
						 matchingConstraints,
						 PR_FALSE);
	}
	if (rv != SECSuccess) {
	    return(rv);
	}
    }

    return(SECSuccess);
}

/* Extract the name constraints extension from the CA cert.
** Test each and every name in namesList against all the constraints
** relevant to that type of name.
** Returns NULL for success, all names are acceptable.
** If some name is not acceptable, returns a pointer to the cert that
** contained that name.
*/
SECStatus
CERT_CompareNameSpace(CERTCertificate  *cert,
		      CERTGeneralName  *namesList,
 		      CERTCertificate **certsList,
 		      PRArenaPool      *arena,
 		      CERTCertificate **pBadCert)
{
    SECStatus            rv;
    CERTNameConstraints  *constraints;
    CERTGeneralName      *currentName;
    int                  count = 0;
    CERTCertificate      *badCert = NULL;

    rv = CERT_FindNameConstraintsExten(arena, cert, &constraints);
    if (rv != SECSuccess) {
	    count = -1;
	    goto done;
    }

    currentName = namesList;
    do {
	rv = CERT_CheckNameSpace(arena, constraints, currentName);
	if (rv != SECSuccess) {
	    break;
	}
 	currentName = CERT_GetNextGeneralName(currentName);
 	count ++;
    } while (currentName != namesList);

done:
    if (rv != SECSuccess) {
	badCert = (count >= 0) ? certsList[count] : cert;
    }
    if (pBadCert)
	*pBadCert = badCert;
    /* TODO: release back to mark */
    return rv;
}
Comment 3 Hanfei Yu 2005-04-15 04:25:16 PDT
Created attachment 180787 [details]
Split CERT_CompareNameSpace in genname.c into two functions. Also provide a new one.
Comment 4 Julien Pierre 2005-07-05 18:37:41 PDT
Retargetting to 3.12 .
Comment 5 Julien Pierre 2005-07-15 10:18:06 PDT
Created attachment 189442 [details] [diff] [review]
Updated patch that includes header file changes

Checking in genname.c;
/cvsroot/mozilla/security/nss/lib/certdb/genname.c,v  <--  genname.c
new revision: 1.28.10.2; previous revision: 1.28.10.1
done
Checking in cert.h;
/cvsroot/mozilla/security/nss/lib/certdb/cert.h,v  <--	cert.h
new revision: 1.53.10.1; previous revision: 1.53
done
Comment 6 Julien Pierre 2005-07-15 10:18:43 PDT
This was checked in on NSS_LIBPKIX_BRANCH only .
Comment 7 Nelson Bolyard (seldom reads bugmail) 2007-03-12 20:06:27 PDT
This bug has a patch that changes an NSS source file that is NOT one of 
the new files for libPKIX.  It does not appear to have been reviewed.  
IINM, the new libPKIX code depends on these changes, and will not build 
without them.  According to comment 6, these changes were committed only
on the libPKIX branch.  So, I suspect that the new libPKIX files that were
recently committed on the trunk do not build without error, since this patch
is not on the trunk.  

So, for NSS 3.12, we need to review this patch very carefully (the name
constraints code is rather tricky) and commit it on the trunk if good.
Comment 8 Alexei Volkov 2007-05-03 11:17:10 PDT
will be integrate as a part of the patch for bug 263373
Comment 9 Alexei Volkov 2007-05-03 13:41:46 PDT
(In reply to comment #8)
> will be integrate as a part of the patch for bug 263373
                                               ^^^^^^^^^^
Correction: bug 358785 


Note You need to log in before you can comment on or make changes to this bug.