On Wed, 14 Oct 2020 03:51:23 GMT, Weijun Wang <wei...@openjdk.org> wrote:

>> Major points in CSR at https://bugs.openjdk.java.net/browse/JDK-8245274:
>> 
>> - new sigalg "RSASSA-PSS", "EdDSA", "Ed25519" and "Ed448" can be used in 
>> jarsigner
>> 
>> - The ".RSA" and ".EC" block extension types (PKCS #7 SignedData inside a 
>> signed JAR) are reused for new signature
>>   algorithms
>> 
>> - A new JarSigner property "directsign"
>> 
>> - Updating the jarsigner tool doc
>> 
>> Major code changes:
>> 
>> - Always use the signature algorithm directly as 
>> SignerInfo::signatureAlgorithm. We used to use the encryption algorithm
>>   there like RSA, DSA, and EC. Now it's always SHA1withRSA or RSASSA-PSS.
>> 
>> - Move signature related utilities methods from AlgorithmId.java to 
>> SignatureUtil.java
>> 
>> - Add new SignatureUtil methods fromKey() and fromSignature() to simplify 
>> creating Signature and getting its AlgorithmId
>> 
>> - Use the new methods in PKCS10, X509CertImpl, and X509CRLImpl signing
>> 
>> - Add a new (and intuitive, IMHO) PKCS7::generateNewSignedData capable of 
>> all old and new signature algorithms
>> 
>> - Mark all -altsign related code deprecated and they can be removed once 
>> ContentSigner is removed
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   signing time, jarsigner -directsign, and digest algorithm check

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?

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?

src/java.base/share/classes/sun/security/pkcs/SignerInfo.java line 508:

> 506:      * name and the encryption algorithm name inside a PKCS7 SignerInfo.
> 507:      *
> 508:      * For old style PKCS7 files where we use RSA, DSA, EC asencAlgId

asencAlgId => as encAlgId

src/java.base/share/classes/sun/security/pkcs/SignerInfo.java line 549:

> 547:                 return encAlg;
> 548:             default:
> 549:                 String digAlg = digAlgId.getName().replace("-", "");

This may be incorrect if the digest algorithm is in the SHA3 family. Maybe we 
should check and apply this conversion
only when digest algorithm starts with "SHA-".

src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java line 858:

> 856:         byte[] content = baos.toByteArray();
> 857:
> 858:         // Use new method is directSign is false or it's a modern

is => if

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

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

Reply via email to