[openssl-dev] [openssl.org #3665] Bug report and a patch forOpenSSL 1.0.1l (and 1.0.1k)

[Uri Blumenthal via RT]

Steve, you’ve made several good points. But please consider:

> The problem is that the two fields containing the signature algorithm do not match.

But they do according to RFC 5754 - I interpret it as declaring that in this particular case (optional parameters of AlgorithmIdentifier) NULL is equivalent to (the same as) absent. In fact, RFC 5754 (page 4) states
that the correct encoding is omitting the empty list altogether, but some uses defined their encoding
as NULL, and it’s OK. It reveals some history of this duality:

The two alternatives arise from the loss of the OPTIONAL associated with the  algorithm identifier parameters when the 1988 syntax for
   AlgorithmIdentifier was translated into the 1997 syntax.  Later, the
   OPTIONAL was recovered via a defect report, but by then many people
   thought that algorithm parameters were mandatory.  Because of this
   history, some implementations encode parameters as a NULL element
   while others omit them entirely.

> Now whether "the same algorithm identifier" means "identical" or "the same meaning" is debatable. Currently it goes with the former and if that is taken to be the case the certificate presented is encoded incorrectly. 

I see your point. However I don’t find it stated anywhere that the same object in a certificate must be encoded the same way if it occurs more than once. Thus, it may be a silly and bad idea to encode AlgorithmIdentifier one way first, and the other way next time - but it doesn’t appear prohibited.

Is 2*2 the same as 4? :-)  Or better, is “2” the same as “two”? :-)

> The question is whether an OpenSSL workaround should be added to tolerate this.

Considering the growing popularity of Apple platforms, my humble opinion is yes - a workaround should be added to tolerate this legal (and ugly) case.

> Before this change OpenSSL completely ignored the signature field (so it could contain anything) and only used the signatureAlgorithm field. 

Exactly. And there are a few certificates deployed that are legal in every aspect of the published RFCs that 1.0.1k patch invalidates unnecessarily.

> In general the ASN.1 NULL value and absent can be very different depending on the ASN.1 structure being represented and "empty" can have a third distinct meaning. Example: imagine an OPTIONAL field where "NULL" represents a status value of some sort. In that case an absent field would indicate no status available while a NULL would indicate a specific status. So in general changing ASN1_TYPE_cmp in the proposed fashion is not a good idea. 

Yes I see your point, and I agree with you. In general it isn’t a good idea to change ASN1_TYPE_cmp(). That means my patch isn’t good as is, and should be re-worked.

> In the very specific case of a certificate and certain digest algorithms absent and NULL do have the same meaning. 

Exactly. I understand you as stating that a patch like what I’m proposing should be limited to this specific narrow case.

> It seems odd that an implementation having decided it should represent an algorithm in one way for one field should then decide to represent an identical algorithm in a different way for another.


Yes it is very odd, and I can only speculate as to how that implementation came about to doing that weird and strange thing. But it is not illegal, because both representations are legal, and there is no explicit requirement to stick with one representation, and no prohibition to mix representations. I’m not saying that it makes sense to do so, merely that (a) it appears legal, and (b) it has already been implemented and deployed by a big vendor - this is not just a theoretical discussion.

> This specific case rejects a certificate where the two AlgorithmIdentifier values in the certificate (signature and signatureAlgorithm) do not match.

Since such a “mixed” representation is legal, according to the definition of that specific object AlgorithmIdentifier, these two do match. Therefore, I think rejecting such a certificate is wrong, and a workaround is necessary. Such a workaround - if correctly implemented - will not detract from OpenSSL security, and will ensure that corner cases are handled correctly (even if we don’t love corner cases very much).

Thanks!
--
Uri Blumenthal
uri at mit.edu


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 1842 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20150120/75218679/attachment.bin>