Last Comment Bug 133191 - NSS Does not recognize 'Issuing Distribution Point' CRL extension
: NSS Does not recognize 'Issuing Distribution Point' CRL extension
Status: NEW
[cert]
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.4
: All All
: P3 enhancement with 1 vote (vote)
: ---
Assigned To: Robert Relyea
:
Mentors:
: 133192 373945 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2002-03-24 20:02 PST by Neil Brittliff
Modified: 2007-05-31 20:47 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
implement changes suggested by the bug submittor (1.54 KB, patch)
2002-05-30 14:07 PDT, Nelson Bolyard (seldom reads bugmail)
no flags Details | Diff | Splinter Review

Description Neil Brittliff 2002-03-24 20:02:19 PST
The CRL generated from Baltimore's UNICERT product contain critical attributes 
which NSS fails to understand. 

I can email you a CRL with the Necessary certificates and I am unable to load 
the CRL into a NSS database.  The CRL could be loaded in NSS - 3.3.1
Comment 1 Wan-Teh Chang 2002-03-25 08:25:10 PST
*** Bug 133192 has been marked as a duplicate of this bug. ***
Comment 2 Wan-Teh Chang 2002-03-25 08:32:08 PST
Assigned the bug to Bob.

Mr. Brittliff, please email your CRL to me (wtc@netscape.com)
and Bob (relyea@netscape.com) with the instructions to load
it into a NSS database.  Thanks.
Comment 3 Robert Relyea 2002-03-25 15:57:41 PST
Neither NSS 3.3.1 or NSS 3.4 understand any v2 CRL extensions. In NSS 3.3 there
was a bug where it was accepting CRLs even though they didn't understand the
extensions: namely it was accepting things like delta CRL's blindly.

If you have CRL's which truly have critical extensions, then NSS 3.3.1 has been
incorrectly ignoring those extensions. NSS 3.4 correctly determines that it does
not know how to handle the extention and does not load the CRL.

Anyway the bug actually is describing what is supposed to be corrrect behavior.

The previous bug number for this issue is: bug 103946
Comment 4 Neil Brittliff 2002-04-02 22:15:05 PST
I believe it is a bug since NSS is not recognising the Issuing Distribution 
Point(OID is 2.5.29.28) contained within a CRL.  It should ignore this OID but 
continue the CRL import.  Here is an extract of RFC-2459

5.2.5  Issuing Distribution Point

   The issuing distribution point is a critical CRL extension that
   identifies the CRL distribution point for a particular CRL, and it
   indicates whether the CRL covers revocation for end entity
   certificates only, CA  certificates only, or a limitied set of reason
   codes.  Although the extension is critical, conforming
   implementations are not required to support this extension.

   The CRL is signed using the CA's private key.  CRL Distribution
   Points do not have their own key pairs.  If the CRL is stored in the
   X.500 Directory, it is stored in the Directory entry corresponding to
   the CRL distribution point, which may be different than the Directory
   entry of the CA.

   The reason codes associated with a distribution point shall be
   specified in onlySomeReasons. If onlySomeReasons does not appear, the
   distribution point shall contain revocations for all reason codes.
   CAs may use CRL distribution points to partition the CRL on the basis
   of compromise and routine revocation.  In this case, the revocations
   with reason code keyCompromise (1) and cACompromise (2) appear in one
   distribution point, and the revocations with other reason codes
   appear in another distribution point.

   Where the issuingDistributionPoint extension contains a URL, the
   following semantics MUST be assumed: the object is a pointer to the
   most current CRL issued by this CA.  The URI schemes ftp, http,
   mailto [RFC1738] and ldap [RFC1778] are defined for this purpose.
   The URI MUST be an absolute, not relative, pathname and MUST specify
   the host.

   id-ce-issuingDistributionPoint OBJECT IDENTIFIER ::= { id-ce 28 }

   issuingDistributionPoint ::= SEQUENCE {
        distributionPoint       [0] DistributionPointName OPTIONAL,
        onlyContainsUserCerts   [1] BOOLEAN DEFAULT FALSE,
        onlyContainsCACerts     [2] BOOLEAN DEFAULT FALSE,
        onlySomeReasons         [3] ReasonFlags OPTIONAL,
        indirectCRL             [4] BOOLEAN DEFAULT FALSE } 
Comment 5 Robert Relyea 2002-04-03 08:36:38 PST
Reopening the bug with new title and severity of enhancement.
Comment 6 Wan-Teh Chang 2002-04-25 16:25:01 PDT
Changed the QA contact to Bishakha.
Comment 7 Robert Relyea 2002-05-02 15:13:32 PDT
If I have a patch for this, I might be able to get it into 3.5 otherwise I'll
set the target release to 4.0.
Comment 8 Wan-Teh Chang 2002-05-02 15:54:26 PDT
Let's discuss how much work it is to support this
CRL extension and whether we can get this done in
NSS 3.5.  For now I am setting priority to P1.
Comment 9 Wan-Teh Chang 2002-05-14 15:27:17 PDT
Moved to 3.6.
Comment 10 Neil Brittliff 2002-05-26 23:12:02 PDT
Is it possible to add the following line to secoid.c
    OD( x509BasicConstraints, SEC_OID_X509_BASIC_CONSTRAINTS, 
	"Certificate Basic Constraints",
	CKM_INVALID_MECHANISM, SUPPORTED_CERT_EXTENSION ),

--> OD( x509IssuingDistPoint, SEC_OID_X509_ISSUING_DIST_POINT, 
	"Issuing Distrubution Point	",
	CKM_INVALID_MECHANISM, SUPPORTED_CERT_EXTENSION ), <--

    OD( x509NameConstraints, SEC_OID_X509_NAME_CONSTRAINTS, 
	"Certificate Name Constraints",
	CKM_INVALID_MECHANISM, SUPPORTED_CERT_EXTENSION ),

The definition x509IssuingDistPoint is 
CONST_OID x509IssuingDistPoint[]         	= { ID_CE_OID, 28 };

in addition could you regenerate the secoidt.h header file, or tell me how to 
get the utility yo do so.

Thanks
Comment 11 Wan-Teh Chang 2002-05-30 13:16:38 PDT
Terry, could you look at the proposed change in comment #10?
Comment 12 Nelson Bolyard (seldom reads bugmail) 2002-05-30 14:07:32 PDT
Created attachment 85655 [details] [diff] [review]
implement changes suggested by the bug submittor

As the person who has most recently worked on the secoid code, I thought 
I might jump in here.

This patch implements the changes suggested by the submittor of this bug.
However, unless SEC_OID_X509_ISSUING_DIST_POINT is added to some other
code in NSS, it's unclear to me that that is going to have any change on
the problem he's experiencing.	

Submittor, please feel free to test the above patch and let us know if 
it has any effect.
Comment 13 Robert Relyea 2002-08-09 10:36:38 PDT
OK, so looking at the spec, it is not safe for NSS to ignor this extension. It
tells us that this is only a partial CRL, not a full CRL. NSS 3.6 does not
support CRL distribution points, so moving target to future.

bob
Comment 14 Robert Relyea 2003-05-14 14:39:07 PDT
Moving Priority to P3 based on our current criteria
Comment 15 Hanfei Yu 2005-03-21 05:39:19 PST
Is there any plan for implementing CRL issuing distribution point
decoding/encoding?  Enclosed is the codes I came out for CRL issuing
distribution point decodeing.  Hope it helps.

********************************************************************

******* in util/secoidt.h and util/secoid.c
add entries for SEC_OID_X509_CRL_ISSUING_DIST_POINTS

******* in lib/certdb/certt.h

typedef struct CERTCrlIssuingDistributionPointStr CERTCrlIssuingDistributionPoint;

******* in lib/certhigh/crlv2.c

SECStatus CERT_FindCRLIssuingDistPointExten (CERTCrl *crl,
CERTCrlIssuingDistributionPoint **pValue)
{
    SECItem encodedExtenValue;
    SECStatus rv;

    rv = cert_FindExtension
	    (crl->extensions, SEC_OID_X509_CRL_ISSUING_DIST_POINTS,
	    &encodedExtenValue);

    if ( rv != SECSuccess )
	return (rv);

    rv = CERT_DecodeCRLIssuingDistributionPoint
	    (crl->arena, &encodedExtenValue, pValue);

    PORT_Free (encodedExtenValue.data);

    return (rv);
}

******* in certhigh/xcrldist.c

/* Templates for CRL Issuing Distribution Point */
static const SEC_ASN1Template CRLIDPFullNameTemplate[] = {
    {SEC_ASN1_CONTEXT_SPECIFIC | SEC_ASN1_CONSTRUCTED | 0,
     offsetof (CERTCrlIssuingDistributionPoint,derFullName),
     CERT_GeneralNamesTemplate}
};

static const SEC_ASN1Template CRLIDPRelativeNameTemplate[] = {
    {SEC_ASN1_CONTEXT_SPECIFIC | SEC_ASN1_CONSTRUCTED | 1, 
     offsetof (CERTCrlIssuingDistributionPoint,
	       distPointName.distPoint.relativeName),
     CERT_RDNTemplate}
};
	 
static const SEC_ASN1Template CRLIssuingDistributionPointTemplate[] = {
    { SEC_ASN1_SEQUENCE, 0, NULL, sizeof(CERTCrlIssuingDistributionPoint) },
	{ SEC_ASN1_OPTIONAL | SEC_ASN1_CONTEXT_SPECIFIC |
	    SEC_ASN1_CONSTRUCTED | SEC_ASN1_EXPLICIT | 0,
	    offsetof(CERTCrlIssuingDistributionPoint,derDistPoint), SEC_AnyTemplate},
        { SEC_ASN1_OPTIONAL | SEC_ASN1_CONTEXT_SPECIFIC | 1,
	  offsetof(CERTCrlIssuingDistributionPoint,onlyContainsUserCerts),
	  SEC_BooleanTemplate },

        { SEC_ASN1_OPTIONAL | SEC_ASN1_CONTEXT_SPECIFIC | 2,
	  offsetof(CERTCrlIssuingDistributionPoint,onlyContainsCACerts),
	  SEC_BooleanTemplate },

	{ SEC_ASN1_OPTIONAL | SEC_ASN1_CONTEXT_SPECIFIC | 3,
	    offsetof(CERTCrlIssuingDistributionPoint,bitsmap),
	    SEC_BitStringTemplate},
        { SEC_ASN1_OPTIONAL | SEC_ASN1_CONTEXT_SPECIFIC | 4,
	  offsetof(CERTCrlIssuingDistributionPoint,indirectCrl),
	  SEC_BooleanTemplate },

        { SEC_ASN1_OPTIONAL | SEC_ASN1_CONTEXT_SPECIFIC | 5,
	  offsetof(CERTCrlIssuingDistributionPoint,onlyContainsAttrCerts),
	   SEC_BooleanTemplate },

    { 0 }
};

........

SECStatus
CERT_DecodeCRLIssuingDistributionPoint (PRArenaPool *arena, SECItem
*encodedValue, CERTCrlIssuingDistributionPoint **pValue)
{
   CERTCrlIssuingDistributionPoint *value = NULL;    
   SECStatus rv;
   SECItem newEncodedValue;

   PORT_Assert (arena);
   do {
	value = (CERTCrlIssuingDistributionPoint*)PORT_ArenaZAlloc
		(arena, sizeof (*value));
	if (value == NULL) {
	    rv = SECFailure;
	    break;
	}

        /* copy the DER into the arena, since Quick DER returns data that points
           into the DER input, which may get freed by the caller */
        rv = SECITEM_CopyItem(arena, &newEncodedValue, encodedValue);
        if ( rv != SECSuccess ) {
	    break;
        }

	rv = SEC_QuickDERDecodeItem
	     (arena, value, CRLIssuingDistributionPointTemplate,
	      &newEncodedValue);
	if (rv != SECSuccess)
	    break;

	/* get the data if the distributionPointName is not omitted */
	if (value->derDistPoint.data != NULL) {
	    value->distPointName.distPointType = (DistributionPointTypes)
		    ((value->derDistPoint.data[0] & 0x1f) +1);
	    if (value->distPointName.distPointType == generalName) {
		SECItem innerDER;
		
		innerDER.data = NULL;
		rv = SEC_QuickDERDecodeItem
			(arena, value, CRLIDPFullNameTemplate,
			 &value->derDistPoint);
		if (rv != SECSuccess)
		    break;

		value->distPointName.distPoint.fullName = cert_DecodeGeneralNames
			(arena, value->derFullName);

		if (!value->distPointName.distPoint.fullName)
		    break;
	    }
	    else if (value->distPointName.distPointType == relativeDistinguishedName) {
		rv = SEC_QuickDERDecodeItem
			(arena, value, CRLIDPRelativeNameTemplate,
			 &(value->derDistPoint));
		if (rv != SECSuccess)
		    break;
	    }
	    else {
		PORT_SetError (SEC_ERROR_EXTENSION_VALUE_INVALID);
		break;
	    }
	}

	/* Get the onlySomeReasons if it's not omitted in the encoding */
	if (value->bitsmap.data != NULL) {
	    value->onlySomeReasons.data = (unsigned char*) PORT_ArenaAlloc
		    (arena, (value->bitsmap.len + 7) >> 3);
	    if (!value->onlySomeReasons.data) {
		rv = SECFailure;
		break;
	    }
	    value->onlySomeReasons.len = ((value->bitsmap.len + 7) >> 3);
	    PORT_Memcpy (value->onlySomeReasons.data,
			 value->bitsmap.data,
			 value->onlySomeReasons.len);
	}

   } while (0);

   *pValue = value; /* consider failure free XXX */

   return (rv);
}
Comment 16 Nelson Bolyard (seldom reads bugmail) 2007-03-14 10:45:31 PDT
*** Bug 373945 has been marked as a duplicate of this bug. ***
Comment 17 Nelson Bolyard (seldom reads bugmail) 2007-05-06 00:51:50 PDT
Comment on attachment 85655 [details] [diff] [review]
implement changes suggested by the bug submittor

Note that this patch was effectively contained in the patch for bug 371685.
That feature now is conditionally compiled.
Comment 18 Gervase Markham [:gerv] 2007-05-23 08:26:22 PDT
Nelson: are the standard NSS builds going to ship this feature? Do you happen to know if the NSS build flags used by Firefox, Thunderbird or Seamonkey switch this on?

I would like to avoid the situation where I approve a root for inclusion (bug 373174) but we can't read its CRL.

Gerv
Comment 19 Nelson Bolyard (seldom reads bugmail) 2007-05-23 09:41:48 PDT
In comment 1, the reporter wrote:

> The CRL generated from Baltimore's UNICERT product contain critical 
> attributes which NSS fails to understand. 

Actually, they're extensions, not attributes.  And Yes, NSS does not
understand them (at present) for patent reasons.

In comment 4, the reporter writes:
>  NSS is not recognising the Issuing Distribution Point(OID is 2.5.29.28) 
> contained within a CRL.  It should ignore this OID but continue the CRL 
> import. 

However RFC 3280 says this about CRL extensions:

   Each extension in a CRL may be designated as
   critical or non-critical.  A CRL validation MUST fail if it
   encounters a critical extension which it does not know how to
   process.  However, an unrecognized non-critical extension may be
   ignored. 

NSS follows that MUST sentence to the letter.  

In Comment 4, the reporter quotes RFC 2459 (the predecessor of RFC 3280)
as saying:

   Although the extension is critical, conforming
   implementations are not required to support this extension.

That does not mean that conforming implementations are free to ignore this 
extensions even when it is designated as critical.  Rather it means that
implementations (such as NSS) that do not support this extension may 
nonetheless legitimately claim to be RFC 3280 conformant, provided that 
they do not ignore critical extensions, but rather fail the CRL validation
(that is, treat the CRL as an invalid CRL) for unsupported critical 
extensions.  

Full support for CRL DPs (in certs) and Issuing DPs (in CRLs) involves a 
lot more than simply extracting a URL and using it.  Because of their
potentially large size, CRLs may be broken up into pieces, separated by
type of cert (CA or EE), and by the particular reasons for inclusion in
the CRL (e.g. revoked, suspended, etc.).  A complete CRL DP or IDP 
implementation must understand the type of cert it wishes to check for 
revocation, and also the reason codes in which it is interested, so that 
it can determine which parts of the CRL are needed to do the revocation 
check, and then fetch those parts according to their URLs in the DP extension.  
This presupposes automated CRL fetching on demand at cert validation time.  

NSS presently does not support partial CRLs.  (Repeat the P word here. :)
There is some question as to whether it can claim to support this extension 
without having full support for partial CRLs.  I believe that it is 
precisely because of the patent that the RFC says that conformant 
implementations need not support this extension.  (IETF standards-track 
RFCs seldom require implementation of patented technologies.)  
Hope springs eternal (for up to 17 years :) that a patent licensing 
agreement can be worked out.

The conditionally compiled code in comment 17, when enabled, causes NSS to
ignore critical DistributionPoint extensions, in violation of the MUST 
sentence quoted from RFC 3280 above.  This allows certain products that 
build their own copies of NSS and wish to violate the RFC to do so, 
without necessitating that all NSS users do so.  

(I'm sure I've written this all before in some other bug, which is probably
a duplicate of this one.)
Comment 20 Wan-Teh Chang 2007-05-23 10:15:55 PDT
Nelson, I believe that was bug 321755, on the "CRL distribution point"
*certificate* extension.
Comment 21 Nelson Bolyard (seldom reads bugmail) 2007-05-23 10:32:30 PDT
Thanks, Wan-Teh.  My (possibly mistaken) recollection is that the 
primary difference between those two extensions are their names and the
names of the documents in which they appear.  I believe the obstacles to 
implementing one are also the obstacles to the other.

I'm expecting Mr. CRL to jump in here with some additional thoughts,
considerations, and maybe corrections, later today.
Comment 22 Julien Pierre 2007-05-23 16:39:16 PDT
Gerv,

It's not always obvious where to obtain the CRL for each root. Checking that the root issuer's CRLs work with the NSS code not been part of the process for adding roots to mozilla in the past. CRLs are reissued periodically by the root CA, they are not a static attribute and not part of the root CA cert itself. So, whether or not the CRL is compatible with mozilla shouldn't be a condition of acceptance.
Currently, NSS has a limited implementation of RFC3280 in the CRL area, due partly to the patent issue. But CAs don't have to use that feature, and many don't.

In addition, the CRLs that are of interest to applications are usually those that are relevant to the end-entity certs, most of which are not directly issued by the root CA but rather by some intermediate CA. Of course it's always possible for the intermediate CAs themselves to be compromised, in which case the root's CRL would be needed, but it's less likely.

The CDP cert extension and the IDP CRL extension are closely related. The cert contains the CDP extension which lets you locate the CRL(s), and then that CRL may contain an IDP extension if it is not a full CRL, which DistributionPointName should match one of the values from the cert CDP extension.
Comment 23 Gervase Markham [:gerv] 2007-05-24 07:09:35 PDT
So basically, you are telling me that I shouldn't worry about the incompatibility, in terms of getting the root added?

Gerv
Comment 24 Julien Pierre 2007-05-24 16:43:39 PDT
Yes, since the CA can choose at any time to stop using or start using those CRL features; and especially as work is underway in NSS to support them in the future anyway.

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