RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]

[Valerie Peng]

On Fri, 16 Oct 2020 02:30:55 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/pkcs/PKCS7.java line 818:
>> 
>>> 816:             DerOutputStream derSigAlg = new DerOutputStream();
>>> 817:             sigAlgID.derEncode(derSigAlg);
>>> 818:             derAlgs.writeImplicit((byte)0xA1, derSigAlg);
>> 
>> Are you sure that this context specific tag value is implicit? In RFC 6211, some other ASN.1 definition uses IMPLICIT
>> keyword after the [x] which seems to suggest that the default is explicit unless specified. Besides, the layman's guide
>> sec2.3 also states "The keyword [class number] alone is the same as explicit tagging, except when the "module" in which
>> the ASN.1 type is defined has implicit tagging by default." So, it seems that explicit tagging should be the default?
>
> In the formal definition at https://tools.ietf.org/html/rfc6211#appendix-A, you can see `DEFINITIONS IMPLICIT TAGS`
> covers from BEGIN to END. Those explicit IMPLICIT tags you see are CMS ASN.1 definitions, and it looks in its own RFC
> at https://tools.ietf.org/html/rfc5652#section-12, IMPLICIT and EXPLICIT are always written out.  I can confirm both
> OpenSSL and BC use IMPLICIT.

Ah, I see. There is a line about implicit tags as you pointed out. Good~

>> src/java.base/share/classes/sun/security/pkcs/PKCS7.java line 172:
>> 
>>> 170:         throws IOException
>>> 171:     {
>>> 172:         ContentInfo block = new ContentInfo(derin, oldStyle);
>> 
>> With this change, i.e. using a local variable instead of setting the field 'contentInfo', the 'contentInfo' field seems
>> to left unset when contentType equals to ContentInfo.NETSCAPE_CERT_SEQUENCE_OID?
>
> I'll see what the best code is, but I don't like the way contentInfo is assigned twice, once as the whole block and
> once as the content inside. I'd rather add a `contentInfo = block` in its else if block.

Right, I also dislike the double assignment. Just making sure that contentInfo is set somewhere.

-------------

PR: https://git.openjdk.java.net/jdk/pull/322