255552 - CERTAuthKeyIDTemplate does not follow rfc 3280

Status: RESOLVED INVALID

(NSS :: Libraries, defect)

Description

[Emil Assarsson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413 Debian/1.6-5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413 Debian/1.6-5

rfc 3280 (4.2.1.1) states:

   AuthorityKeyIdentifier ::= SEQUENCE {
      keyIdentifier             [0] KeyIdentifier           OPTIONAL,
      authorityCertIssuer       [1] GeneralNames            OPTIONAL,
      authorityCertSerialNumber [2] CertificateSerialNumber OPTIONAL  }

   KeyIdentifier ::= OCTET STRING


Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Actual Results:  
NSS encodes this as:

    0:d=0  hl=2 l=  65 cons: SEQUENCE
    2:d=1  hl=2 l=  60 cons:  cont [ 1 ]
    4:d=2  hl=2 l=  58 cons:   cont [ 4 ]
    6:d=3  hl=2 l=  56 cons:    SEQUENCE
    8:d=4  hl=2 l=  11 cons:     SET
   10:d=5  hl=2 l=   9 cons:      SEQUENCE
   12:d=6  hl=2 l=   3 prim:       OBJECT            :countryName
   17:d=6  hl=2 l=   2 prim:       PRINTABLESTRING   :SE
   21:d=4  hl=2 l=  16 cons:     SET
   23:d=5  hl=2 l=  14 cons:      SEQUENCE
   25:d=6  hl=2 l=   3 prim:       OBJECT            :organizationName
   30:d=6  hl=2 l=   7 prim:       PRINTABLESTRING   :Test AB
   39:d=4  hl=2 l=  23 cons:     SET
   41:d=5  hl=2 l=  21 cons:      SEQUENCE
   43:d=6  hl=2 l=   3 prim:       OBJECT            :commonName
   48:d=6  hl=2 l=  14 prim:       PRINTABLESTRING   :Emil Assarsson
   64:d=1  hl=2 l=   1 prim:  cont [ 2 ]


Expected Results:  
    0:d=0  hl=2 l=  67 cons: SEQUENCE
    2:d=1  hl=2 l=  60 cons:  cont [ 1 ]
    4:d=2  hl=2 l=  58 cons:   cont [ 4 ]
    6:d=3  hl=2 l=  56 cons:    SEQUENCE
    8:d=4  hl=2 l=  11 cons:     SET
   10:d=5  hl=2 l=   9 cons:      SEQUENCE
   12:d=6  hl=2 l=   3 prim:       OBJECT            :countryName
   17:d=6  hl=2 l=   2 prim:       PRINTABLESTRING   :SE
   21:d=4  hl=2 l=  16 cons:     SET
   23:d=5  hl=2 l=  14 cons:      SEQUENCE
   25:d=6  hl=2 l=   3 prim:       OBJECT            :organizationName
   30:d=6  hl=2 l=   7 prim:       PRINTABLESTRING   :Test AB
   39:d=4  hl=2 l=  23 cons:     SET
   41:d=5  hl=2 l=  21 cons:      SEQUENCE
   43:d=6  hl=2 l=   3 prim:       OBJECT            :commonName
   48:d=6  hl=2 l=  14 prim:       PRINTABLESTRING   :Emil Assarsson
   64:d=1  hl=2 l=   3 cons:  cont [ 2 ]
   66:d=2  hl=2 l=   1 prim:   INTEGER           :64


IMHO this should be encoded with EXPLICIT tagging, but I'm not sure.
At the matter of fact OpenSSL does this to. And Windows XP does display it like
it was correct.

Comment 1

[Emil Assarsson]

Attachment: Patch for xauthkid.c

Comment 2

[Wan-Teh Chang]

Nelson, Julien, could you look at this bug and
Emil's patch?

Emil, could you regenerate the patch using
"cvs diff -u"?

Comment 3

[Julien Pierre]

Emil,

AuthorityKeyID is defined in section A.2 of RFC3280, which is an "implicitly
tagged module". What this means is that all tags are implicit, unless
specifically declared EXPLICIT . As you can see, in this case, they aren't. So I
think we are correct to use implicit tagging here, and it would make sense that
the other products you mentioned also do.

Nelson, do you agree ?

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 156059 [details] [diff] [review]
Patch for xauthkid.c

Emil,
I gather that you think that NSS has not properly encoded the implicitly
encoded integer value (64), and you are proposing to fix it my making the
integer be explicitly encoded.	That is definitely not the correct solution,
so I give this patch a '-' on that basis.  But you may or may not have found
a bug in NSS here.  

I would like to see a raw hex dump of NSS's encoder output, and also of 
the data input to the encoder for that integer, including the entire
SECItem (all 3 fields) and the content of the buffer pointed to by the
SECItem's data pointer.  Then I can better determine if there is an encoding
error here, and if so, what it is.

However, while looking at the code, I did find something that seems wrong.
This would not explain an incorrectly encoded integer, but it may be 
a separate bug found by code inspection.

The particular template that your patch changes (seen at
http://lxr.mozilla.org/mozilla/source/security/nss/lib/certdb/xauthkid.c#55 )
presently appears to represent the following ASN.1 syntax:

   AuthorityKeyIdentifier ::= SEQUENCE {
      keyIdentifier		[0] KeyIdentifier	    OPTIONAL,
      authorityCertIssuer	[1] EXPLICIT GeneralNames   OPTIONAL,
      authorityCertSerialNumber [2] CertificateSerialNumber OPTIONAL  }

That EXPLICIT declaration for the GeneralNames seems wrong.  If the item
was of type GenerateName (signular, not plural), then I would agree that
the EXPLICIT is correct, because GeneralName is a CHOICE, and a CHOICE
is always explicitly tagged, even when the ASN syntax says it is IMPLICIT.
But this type is GeneralNames (plural), and is a SEQUENCE OF GeneralName.
As such, it should be implicitly tagged.  

Now, as I interpret the printout above of the "Actual" encoded output, it 
appears not to be explicitly tagged (although I'd rather see the raw hex
data to be sure).  So, it may be that the encoder is generating an implicit
tag, even though the template CLEARLY says it should generate an explicit
one.  But if that is so, that is probably a bug too.  So, I think that 
perhaps it will be necessary to modify the template for the 
authorityCertIssuer to remove the EXPLICIT declaration there.  

Julien, any thoughts on that issue?

Comment 5

[Emil Assarsson]

Nelson,
Armed with the new knowledge of the IMPLICIT in ASN1 header I agree I was wrong. 
Sorry for any extra work. 
I made a hex dump off the AKI part.

This is from a cert that is generated with original 3.9 code.
---
0000000 4130 3ca1 3aa4 3830 0b31 0930 0306 0455
0000010 1306 5302 3145 3010 060e 5503 0a04 0713
0000020 6554 7473 4120 3142 3017 0615 5503 0304
0000030 0e13 6d45 6c69 4120 7373 7261 7373 6e6f
0000040 0182 0064
0000043
---

I guess that you reacted at the empty serial number? Well it's not empty but
openssl asn1parse can't handle implicit things (openssl sucks ;-)

Here is that integer just before the encoding:
---
(gdb) print authKeyID->authCertSerialNumber
$2 = {type = siBuffer,
  data = 0x8067c97 "d0\r\006\t*\206H\206÷\r\001\001\004\005", len = 1}
---
... looks OK to me.

Regarding the EXPLICIT tag on generalNames:
I don't find the explicit sequence.
---
SEQUENCE
 cont [ 1 ] ----> This is an implicit tagged sequence - generalNames
  cont [ 4 ] ----> here is the generalName
   SEQUENCE
    SET
     SEQUENCE
      OBJECT            :countryName
      PRINTABLESTRING   :SE
    SET
     SEQUENCE
      OBJECT            :organizationName
      PRINTABLESTRING   :Test AB
    SET
     SEQUENCE
      OBJECT            :commonName
      PRINTABLESTRING   :Emil Assarsson
 cont [ 2 ] ----> implicit tagged integer
---

I think that this bug is INVALID

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Resolved invalid per previous comment.