Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
On 5/22/20 10:30 AM, Weijun Wang wrote: Please take a review at CSR : https://bugs.openjdk.java.net/browse/JDK-8245274 webrev : http://cr.openjdk.java.net/~weijun/8242068/webrev.00/ Major points in CSR: - new sigalg "RSASSA-PSS", "EdDSA", "Ed25519" and "Ed448" can be used in jarsigner In the CSR, it says "In fact, the new -sigalg option values are quite useless and do not need to specified." What happens if you specify anything other than the defaults? In the Supported Algorithms, what does keysize: (empty) mean? Do you mean "any size" as in the current table for DSA? - The ".RSA" and ".EC" block extension types (PKCS #7 SignedData inside a signed JAR) are reused for new signature algorithms I think the CSR should have the proposed changes to the JAR specification instead of just saying it will be described. Also, we never defined the "EC" type, so I think this CSR is a good opportunity to also fix that and add that extension to the JAR spec. --Sean major code changes: - 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 Next I'll do some basic interop tests with openssl and BouncyCastle. Thanks, Max
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
> On May 23, 2020, at 4:44 AM, Sean Mullan wrote: > > On 5/22/20 10:30 AM, Weijun Wang wrote: >> Please take a review at >> CSR : https://bugs.openjdk.java.net/browse/JDK-8245274 >>webrev : http://cr.openjdk.java.net/~weijun/8242068/webrev.00/ >> Major points in CSR: >> - new sigalg "RSASSA-PSS", "EdDSA", "Ed25519" and "Ed448" can be used in >> jarsigner > > In the CSR, it says "In fact, the new -sigalg option values are quite useless > and do not need to specified." > > What happens if you specify anything other than the defaults? If not compatible with the key alg, an error will be reported. This is implemented in SignatureUtil.checkKeyAndSigAlgMatch. Precisely, if the key alg is RSASSA-PSS, you can only set "-siglag RSASSA-PSS". For Ed25519, everything other than "-sigalg EdDSA" or "-sigalg Ed25519" will lead to an error, and these 2 are actually the same. > > In the Supported Algorithms, what does keysize: (empty) mean? Do you mean > "any size" as in the current table for DSA? You can say that (RSASSA-PSS can be any size, EdDSA can be any of those 2 sizes). Of course, it's not unrelated, and I've added "using the same parameters of the key" in the default siglag cells. For EdDSA, it's more precise to say "using the same key size". > >> - The ".RSA" and ".EC" block extension types (PKCS #7 SignedData inside a >> signed JAR) are reused for new signature algorithms > > I think the CSR should have the proposed changes to the JAR specification > instead of just saying it will be described. Also, we never defined the "EC" > type, so I think this CSR is a good opportunity to also fix that and add that > extension to the JAR spec. OK. Thanks, Max > > --Sean > >> major code changes: >> - 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 >> Next I'll do some basic interop tests with openssl and BouncyCastle. >> Thanks, >> Max
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
>> >> In the Supported Algorithms, what does keysize: (empty) mean? Do you mean >> "any size" as in the current table for DSA? > > You can say that (RSASSA-PSS can be any size, EdDSA can be any of those 2 > sizes). Of course, it's not unrelated, and I've added "using the same > parameters of the key" in the default siglag cells. For EdDSA, it's more > precise to say "using the same key size". > I just realized that an RSASSA-PSS key could have no params in its AlgorithmId. In this case, the key size will be used to determine the params of the Signature similar to that of RSA (i.e. 2048 key leads to RSASSA-PSS using SHA-256 as hash and MGF1 algorithms). I'll find out a way to describe this. Thanks, Max
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
CSR updated at https://bugs.openjdk.java.net/browse/JDK-8245274 with the full patch on docs. Thanks, Max > On May 23, 2020, at 9:45 AM, Weijun Wang wrote: > >>> >>> In the Supported Algorithms, what does keysize: (empty) mean? Do you mean >>> "any size" as in the current table for DSA? >> >> You can say that (RSASSA-PSS can be any size, EdDSA can be any of those 2 >> sizes). Of course, it's not unrelated, and I've added "using the same >> parameters of the key" in the default siglag cells. For EdDSA, it's more >> precise to say "using the same key size". >> > > I just realized that an RSASSA-PSS key could have no params in its > AlgorithmId. In this case, the key size will be used to determine the params > of the Signature similar to that of RSA (i.e. 2048 key leads to RSASSA-PSS > using SHA-256 as hash and MGF1 algorithms). > > I'll find out a way to describe this. > > Thanks, > Max >
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
Webrev updated at
http://cr.openjdk.java.net/~weijun/8242068/webrev.01/
Two major changes:
1. Always use the signature algorithm directly in
SignerInfo::signatureAlgorithm:
In PKCS7 SignerInfo
SignerInfo ::= SEQUENCE {
version CMSVersion,
sid SignerIdentifier,
digestAlgorithm DigestAlgorithmIdentifier,
signedAttrs [0] IMPLICIT SignedAttributes OPTIONAL,
signatureAlgorithm SignatureAlgorithmIdentifier,
signature SignatureValue,
unsignedAttrs [1] IMPLICIT UnsignedAttributes OPTIONAL }
We have always been putting "SHA-1" etc into DigestAlgorithmIdentifier, and
"RSA", "DSA", "EC" into signatureAlgorithm.
The latest https://tools.ietf.org/html/rfc5652#section-10.1.2 claims it to be
The SignatureAlgorithmIdentifier type identifies a signature
algorithm, and it can also identify a message digest algorithm.
Examples include RSA, DSA, DSA with SHA-1, ECDSA, and ECDSA with
SHA-256.
It's complicated to always divide a signature algorithm into a digest algorithm
and an encryption algorithm (and with the new RSASSA-PSS and EdDSA it's not
easy to define it), therefore I decide to use the signature algorithm directly
from now on. Fortunately Java has been able to parse this for a very long time
so there is no compatibility issue. I noticed BouncyCastle has been doing the
same, and OpenSSL too except for RSA.
2. Support both SHAKE256 and SHAKE256-LEN while parsing a Ed448 SignerInfo.
They are both described in https://www.rfc-editor.org/rfc/rfc8419.html although
it's a little complicated. To be standard compliant
(https://www.rfc-editor.org/rfc/rfc8419.html#section-3.2 and we don't use
Signed Attributes), by default Java will use SHAKE256 as the digestAlgorithm.
However I noticed BouncyCastle does not recognize it, so if you set the system
property "jdk.security.pkcs7.ed448.digalg.haslen" to "true", it will use
SHAKE256-LEN (len == 512). I haven't described this system property in the CSR
yet.
Thanks,
Max
> On May 22, 2020, at 10:30 PM, Weijun Wang wrote:
>
> Please take a review at
>
> CSR : https://bugs.openjdk.java.net/browse/JDK-8245274
> webrev : http://cr.openjdk.java.net/~weijun/8242068/webrev.00/
>
> Major points in CSR:
>
> - 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
>
> major code changes:
>
> - 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
>
> Next I'll do some basic interop tests with openssl and BouncyCastle.
>
> Thanks,
> Max
>
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
On 5/23/20 4:21 AM, Weijun Wang wrote: CSR updated at https://bugs.openjdk.java.net/browse/JDK-8245274 with the full patch on docs. Good to remove all of the DSA references :) I had a few comments, mostly wording suggestions: - In the Problem section, you should add references to the RFCs. - Solution section: RSASSA-RSS and EdDSA keys can be used to signed a JAR file typo: signed/signed - Comments on the Specification section: Some of this wording where you discuss what is going to be done in jarsigner and the tool doc seems more like it belongs in the Solution section, but it may be ok either way (not sure, I guess Joe will let you know if it is an issue). +RSASSA-PSS \<= 3072 RSASSA-PSS using SHA-256 +\<= 7680 RSASSA-PSS using SHA-384 +\> 7680 RSASSA-PSS using SHA-512 It might be better to be more specific, for example "RSASSA-PSS with the SHA-256 message digest algorithm" I used the term "with" as that is the word we use in the standard signature algorithm strings. +pair using `-keyalg EdDSA`, user can specify `-keysize 255` or `-keysize 448` s/user/a user/ +Ed25519 key pair is generated. User can also directly specifies `-keyalg Ed25519` s/User/A user/ s/specifies/specify/ + By default, the `jarsigner` command signs a JAR file using one of the following + algorithms files depending on the type and size of the private key: Not your changes, but there is a grammar error above. I would also mention block files, since you have added it to the table, so how about: "By default, the `jarsigner` command signs a JAR file using the following algorithms and block file extension depending on the type and size of the private key:" +will use a new `PSSParameterSpec` parameters that is determined by the key's s/a new/new/ +for keysize \<= 3072 bits, use a `PSSParameterSpec` with SHA-256 as the hash s/use a `PSSParameterSpec`/a `PSSParameterSpec` will be used/ +and MGF1 algorithms; for keysize \<= 7680 bits, use SHA-384; for +keysize \>= 7680 bits, use SHA-512. s/use SHA-384/SHA-384 will be used/ s/use SHA-512/SHA-512 will be used/ --Sean
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
Replies inline below. I've removed the items I agree with for simplicity. > On May 30, 2020, at 3:32 AM, Sean Mullan wrote: > > On 5/23/20 4:21 AM, Weijun Wang wrote: >> CSR updated at https://bugs.openjdk.java.net/browse/JDK-8245274 with the >> full patch on docs. > > - Comments on the Specification section: > > Some of this wording where you discuss what is going to be done in jarsigner > and the tool doc seems more like it belongs in the Solution section, but it > may be ok either way (not sure, I guess Joe will let you know if it is an > issue). > > +RSASSA-PSS \<= 3072 RSASSA-PSS using SHA-256 > +\<= 7680 RSASSA-PSS using SHA-384 > +\> 7680 RSASSA-PSS using SHA-512 > > It might be better to be more specific, for example "RSASSA-PSS with the > SHA-256 message digest algorithm" I used the term "with" as that is the word > we use in the standard signature algorithm strings. Can I simply say "RSASSA-PSS with SHA-256"? Otherwise the text is much longer than the other rows. That said, we have enough width and no need to wrap. Also, we used to say HASHwithENC, but here it's "ENC with HASH". Hopefully this will not make people laugh. > > > +for keysize \<= 3072 bits, use a `PSSParameterSpec` with SHA-256 as the hash > > s/use a `PSSParameterSpec`/a `PSSParameterSpec` will be used/ > > +and MGF1 algorithms; for keysize \<= 7680 bits, use SHA-384; for > +keysize \>= 7680 bits, use SHA-512. > > s/use SHA-384/SHA-384 will be used/ > s/use SHA-512/SHA-512 will be used/ My original text is Precisely, for keysize \<= 3072 bits, use a `PSSParameterSpec` with SHA-256 as the hash and MGF1 algorithms; for keysize \<= 7680 bits, use SHA-384; for keysize \>= 7680 bits, use SHA-512. So the object of "use" here is the PSSParameterSpec. If I change it to passive voice, it will be a `PSSParameterSpec` with SHA-256 as the hash and MGF1 algorithms will be used, and `PSSParameterSpec` (instead of SHA-256) will be the noun. The "SHA-384 will be used" in the next sentence might not be 100% grammatically correct in this sense but I think it's OK and nobody will misunderstand it. (In fact, there is the same problem in my original text). Thanks, Max p.s. This is probably the only RFE I can add into jdk15 before rdp1 now. The strong p12 algorithms and new CACERTS keystore type won't be ready. I do have several bug fixes that can wait till rdp2. > > --Sean >
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
On 5/31/20 9:26 PM, Weijun Wang wrote: Replies inline below. I've removed the items I agree with for simplicity. On May 30, 2020, at 3:32 AM, Sean Mullan wrote: On 5/23/20 4:21 AM, Weijun Wang wrote: CSR updated at https://bugs.openjdk.java.net/browse/JDK-8245274 with the full patch on docs. - Comments on the Specification section: Some of this wording where you discuss what is going to be done in jarsigner and the tool doc seems more like it belongs in the Solution section, but it may be ok either way (not sure, I guess Joe will let you know if it is an issue). +RSASSA-PSS \<= 3072 RSASSA-PSS using SHA-256 +\<= 7680 RSASSA-PSS using SHA-384 +\> 7680 RSASSA-PSS using SHA-512 It might be better to be more specific, for example "RSASSA-PSS with the SHA-256 message digest algorithm" I used the term "with" as that is the word we use in the standard signature algorithm strings. Can I simply say "RSASSA-PSS with SHA-256"? Otherwise the text is much longer than the other rows. That said, we have enough width and no need to wrap. How about "RSASSA-PSS (with SHA-256)" I think the parentheses helps. Also, we used to say HASHwithENC, but here it's "ENC with HASH". Hopefully this will not make people laugh. +for keysize \<= 3072 bits, use a `PSSParameterSpec` with SHA-256 as the hash s/use a `PSSParameterSpec`/a `PSSParameterSpec` will be used/ +and MGF1 algorithms; for keysize \<= 7680 bits, use SHA-384; for +keysize \>= 7680 bits, use SHA-512. s/use SHA-384/SHA-384 will be used/ s/use SHA-512/SHA-512 will be used/ My original text is Precisely, for keysize \<= 3072 bits, use a `PSSParameterSpec` with SHA-256 as the hash and MGF1 algorithms; for keysize \<= 7680 bits, use SHA-384; for keysize \>= 7680 bits, use SHA-512. So the object of "use" here is the PSSParameterSpec. If I change it to passive voice, it will be a `PSSParameterSpec` with SHA-256 as the hash and MGF1 algorithms will be used, and `PSSParameterSpec` (instead of SHA-256) will be the noun. The "SHA-384 will be used" in the next sentence might not be 100% grammatically correct in this sense but I think it's OK and nobody will misunderstand it. (In fact, there is the same problem in my original text). When you say "use a `PSSParameterSpec` with SHA-256" it sounds to me as you are telling the reader they need to take action to do that. But this is what jarsigner itself will be doing. Another option which I will throw out, is to change the RSASSA-PSS rows in the table above it to be more like the keytool table, so that it includes the keysize ranges, ex: +RSASSA-PSS \<= 3072 RSASSA-PSS (with SHA-256) +\<= 7680 RSASSA-PSS (with SHA-384) +\> 7680 RSASSA-PSS (with SHA-512) Then you could simplify the following text as something like: * If an RSASSA-PSS key is encoded with parameters, then the signature will use the same parameters. Otherwise, the signature will use parameters that are determined by the size of the key as specified in the table above. For example, an 3072-bit RSASSA-PSS key will use RSASSA-PSS as the signature algorithm and SHA-256 as the hash and MGF1 algorithms. I don't think you need to mention PSSParameterSpec at all. It seems like a detail that is more for an application developer and doesn't need to be detailed here. Thanks, Max p.s. This is probably the only RFE I can add into jdk15 before rdp1 now. The strong p12 algorithms and new CACERTS keystore type won't be ready. I do have several bug fixes that can wait till rdp2. Yes, I hope there is still time. I have added my name as Reviewer, so you should submit the CSR soon. Also, I view all of my comments as wording changes/tweaks and not changes to the specification which is basically to add support for these algorithms to jarsigner. --Sean
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
Updated again at
https://cr.openjdk.java.net/~weijun/8242068/webrev.02
Major changes this time:
1. Use Signed Attributes in generateNewSignedData(direct==false). This means
for Ed448 we will use SHAKE256-LEN so there is no interop issue (BTW, BC
recently added support to SHAKE256 in 166b07), hence removed the newly added
system property in webrev.01.
2. Better interop with providers using Ed448 and Ed25519 as key algorithm names.
Major changes previously:
1. Always encode the whole signature algorithm identifier (instead of
encryption algorithmid) in PKCS7 SignerInfo.
2. New SignatureUtil methods
Thanks,
Max
> On May 27, 2020, at 8:21 AM, Weijun Wang wrote:
>
> Webrev updated at
>
>http://cr.openjdk.java.net/~weijun/8242068/webrev.01/
>
> Two major changes:
>
> 1. Always use the signature algorithm directly in
> SignerInfo::signatureAlgorithm:
>
> In PKCS7 SignerInfo
>
> SignerInfo ::= SEQUENCE {
>version CMSVersion,
>sid SignerIdentifier,
>digestAlgorithm DigestAlgorithmIdentifier,
>signedAttrs [0] IMPLICIT SignedAttributes OPTIONAL,
>signatureAlgorithm SignatureAlgorithmIdentifier,
>signature SignatureValue,
>unsignedAttrs [1] IMPLICIT UnsignedAttributes OPTIONAL }
>
> We have always been putting "SHA-1" etc into DigestAlgorithmIdentifier, and
> "RSA", "DSA", "EC" into signatureAlgorithm.
>
> The latest https://tools.ietf.org/html/rfc5652#section-10.1.2 claims it to be
>
> The SignatureAlgorithmIdentifier type identifies a signature
> algorithm, and it can also identify a message digest algorithm.
> Examples include RSA, DSA, DSA with SHA-1, ECDSA, and ECDSA with
> SHA-256.
>
> It's complicated to always divide a signature algorithm into a digest
> algorithm and an encryption algorithm (and with the new RSASSA-PSS and EdDSA
> it's not easy to define it), therefore I decide to use the signature
> algorithm directly from now on. Fortunately Java has been able to parse this
> for a very long time so there is no compatibility issue. I noticed
> BouncyCastle has been doing the same, and OpenSSL too except for RSA.
>
> 2. Support both SHAKE256 and SHAKE256-LEN while parsing a Ed448 SignerInfo.
> They are both described in https://www.rfc-editor.org/rfc/rfc8419.html
> although it's a little complicated. To be standard compliant
> (https://www.rfc-editor.org/rfc/rfc8419.html#section-3.2 and we don't use
> Signed Attributes), by default Java will use SHAKE256 as the digestAlgorithm.
> However I noticed BouncyCastle does not recognize it, so if you set the
> system property "jdk.security.pkcs7.ed448.digalg.haslen" to "true", it will
> use SHAKE256-LEN (len == 512). I haven't described this system property in
> the CSR yet.
>
> Thanks,
> Max
>
>
>> On May 22, 2020, at 10:30 PM, Weijun Wang wrote:
>>
>> Please take a review at
>>
>> CSR : https://bugs.openjdk.java.net/browse/JDK-8245274
>> webrev : http://cr.openjdk.java.net/~weijun/8242068/webrev.00/
>>
>> Major points in CSR:
>>
>> - 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
>>
>> major code changes:
>>
>> - 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
>>
>> Next I'll do some basic interop tests with openssl and BouncyCastle.
>>
>> Thanks,
>> Max
>>
>
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
Ping again. CSR already approved and rdp1 is near...
Thanks,
Max
> On Jun 2, 2020, at 4:07 PM, Weijun Wang wrote:
>
> Updated again at
>
> https://cr.openjdk.java.net/~weijun/8242068/webrev.02
>
> Major changes this time:
>
> 1. Use Signed Attributes in generateNewSignedData(direct==false). This means
> for Ed448 we will use SHAKE256-LEN so there is no interop issue (BTW, BC
> recently added support to SHAKE256 in 166b07), hence removed the newly added
> system property in webrev.01.
>
> 2. Better interop with providers using Ed448 and Ed25519 as key algorithm
> names.
>
> Major changes previously:
>
> 1. Always encode the whole signature algorithm identifier (instead of
> encryption algorithmid) in PKCS7 SignerInfo.
>
> 2. New SignatureUtil methods
>
> Thanks,
> Max
>
>> On May 27, 2020, at 8:21 AM, Weijun Wang wrote:
>>
>> Webrev updated at
>>
>> http://cr.openjdk.java.net/~weijun/8242068/webrev.01/
>>
>> Two major changes:
>>
>> 1. Always use the signature algorithm directly in
>> SignerInfo::signatureAlgorithm:
>>
>> In PKCS7 SignerInfo
>>
>> SignerInfo ::= SEQUENCE {
>> version CMSVersion,
>> sid SignerIdentifier,
>> digestAlgorithm DigestAlgorithmIdentifier,
>> signedAttrs [0] IMPLICIT SignedAttributes OPTIONAL,
>> signatureAlgorithm SignatureAlgorithmIdentifier,
>> signature SignatureValue,
>> unsignedAttrs [1] IMPLICIT UnsignedAttributes OPTIONAL }
>>
>> We have always been putting "SHA-1" etc into DigestAlgorithmIdentifier, and
>> "RSA", "DSA", "EC" into signatureAlgorithm.
>>
>> The latest https://tools.ietf.org/html/rfc5652#section-10.1.2 claims it to be
>>
>> The SignatureAlgorithmIdentifier type identifies a signature
>> algorithm, and it can also identify a message digest algorithm.
>> Examples include RSA, DSA, DSA with SHA-1, ECDSA, and ECDSA with
>> SHA-256.
>>
>> It's complicated to always divide a signature algorithm into a digest
>> algorithm and an encryption algorithm (and with the new RSASSA-PSS and EdDSA
>> it's not easy to define it), therefore I decide to use the signature
>> algorithm directly from now on. Fortunately Java has been able to parse this
>> for a very long time so there is no compatibility issue. I noticed
>> BouncyCastle has been doing the same, and OpenSSL too except for RSA.
>>
>> 2. Support both SHAKE256 and SHAKE256-LEN while parsing a Ed448 SignerInfo.
>> They are both described in https://www.rfc-editor.org/rfc/rfc8419.html
>> although it's a little complicated. To be standard compliant
>> (https://www.rfc-editor.org/rfc/rfc8419.html#section-3.2 and we don't use
>> Signed Attributes), by default Java will use SHAKE256 as the
>> digestAlgorithm. However I noticed BouncyCastle does not recognize it, so if
>> you set the system property "jdk.security.pkcs7.ed448.digalg.haslen" to
>> "true", it will use SHAKE256-LEN (len == 512). I haven't described this
>> system property in the CSR yet.
>>
>> Thanks,
>> Max
>>
>>
>>> On May 22, 2020, at 10:30 PM, Weijun Wang wrote:
>>>
>>> Please take a review at
>>>
>>>CSR : https://bugs.openjdk.java.net/browse/JDK-8245274
>>> webrev : http://cr.openjdk.java.net/~weijun/8242068/webrev.00/
>>>
>>> Major points in CSR:
>>>
>>> - 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
>>>
>>> major code changes:
>>>
>>> - 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
>>>
>>> Next I'll do some basic interop tests with openssl and BouncyCastle.
>>>
>>> Thanks,
>>> Max
>>>
>>
>
Re: RFR 8242068: Signed JAR support for RSASSA-PSS and EdDSA
New webrev at
http://cr.openjdk.java.net/~weijun/8242068/webrev.03/
Changes since webrev.02:
- Does not add OID.1.2.3.4 into KnownOIDs's name2enum map. Instead, check for
the "OID." prefix in SignatureUtil::checkName.
- Add a new "directsign" property to JarSigner.Builder::setProperty.
- s/signManifest/sectionsonly/ and s/externalSF/internalsf/ in JarSigner, to be
consistent with jarsigner option names.
- When directsign == false, always uses PKCS7.generateNewSignedData
- A new functional test to check several JarSigner properties
CSR (withdrawn and) updated on the new JarSigner property at
https://bugs.openjdk.java.net/browse/JDK-8245274
Thanks,
Max
> On Jun 2, 2020, at 4:07 PM, Weijun Wang wrote:
>
> Updated again at
>
> https://cr.openjdk.java.net/~weijun/8242068/webrev.02
>
> Major changes this time:
>
> 1. Use Signed Attributes in generateNewSignedData(direct==false). This means
> for Ed448 we will use SHAKE256-LEN so there is no interop issue (BTW, BC
> recently added support to SHAKE256 in 166b07), hence removed the newly added
> system property in webrev.01.
>
> 2. Better interop with providers using Ed448 and Ed25519 as key algorithm
> names.
>
> Major changes previously:
>
> 1. Always encode the whole signature algorithm identifier (instead of
> encryption algorithmid) in PKCS7 SignerInfo.
>
> 2. New SignatureUtil methods
>
> Thanks,
> Max
>
>> On May 27, 2020, at 8:21 AM, Weijun Wang wrote:
>>
>> Webrev updated at
>>
>> http://cr.openjdk.java.net/~weijun/8242068/webrev.01/
>>
>> Two major changes:
>>
>> 1. Always use the signature algorithm directly in
>> SignerInfo::signatureAlgorithm:
>>
>> In PKCS7 SignerInfo
>>
>> SignerInfo ::= SEQUENCE {
>> version CMSVersion,
>> sid SignerIdentifier,
>> digestAlgorithm DigestAlgorithmIdentifier,
>> signedAttrs [0] IMPLICIT SignedAttributes OPTIONAL,
>> signatureAlgorithm SignatureAlgorithmIdentifier,
>> signature SignatureValue,
>> unsignedAttrs [1] IMPLICIT UnsignedAttributes OPTIONAL }
>>
>> We have always been putting "SHA-1" etc into DigestAlgorithmIdentifier, and
>> "RSA", "DSA", "EC" into signatureAlgorithm.
>>
>> The latest https://tools.ietf.org/html/rfc5652#section-10.1.2 claims it to be
>>
>> The SignatureAlgorithmIdentifier type identifies a signature
>> algorithm, and it can also identify a message digest algorithm.
>> Examples include RSA, DSA, DSA with SHA-1, ECDSA, and ECDSA with
>> SHA-256.
>>
>> It's complicated to always divide a signature algorithm into a digest
>> algorithm and an encryption algorithm (and with the new RSASSA-PSS and EdDSA
>> it's not easy to define it), therefore I decide to use the signature
>> algorithm directly from now on. Fortunately Java has been able to parse this
>> for a very long time so there is no compatibility issue. I noticed
>> BouncyCastle has been doing the same, and OpenSSL too except for RSA.
>>
>> 2. Support both SHAKE256 and SHAKE256-LEN while parsing a Ed448 SignerInfo.
>> They are both described in https://www.rfc-editor.org/rfc/rfc8419.html
>> although it's a little complicated. To be standard compliant
>> (https://www.rfc-editor.org/rfc/rfc8419.html#section-3.2 and we don't use
>> Signed Attributes), by default Java will use SHAKE256 as the
>> digestAlgorithm. However I noticed BouncyCastle does not recognize it, so if
>> you set the system property "jdk.security.pkcs7.ed448.digalg.haslen" to
>> "true", it will use SHAKE256-LEN (len == 512). I haven't described this
>> system property in the CSR yet.
>>
>> Thanks,
>> Max
>>
>>
>>> On May 22, 2020, at 10:30 PM, Weijun Wang wrote:
>>>
>>> Please take a review at
>>>
>>>CSR : https://bugs.openjdk.java.net/browse/JDK-8245274
>>> webrev : http://cr.openjdk.java.net/~weijun/8242068/webrev.00/
>>>
>>> Major points in CSR:
>>>
>>> - 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
>>>
>>> major code changes:
>>>
>>> - 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
>>>
>>> Next I'll do some basic interop tests with openssl and BouncyCastle.
>>>
>>> Thanks,
>>> Max
>>>
>>
>
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA
On Wed, 23 Sep 2020 14:41:59 GMT, Weijun Wang 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
Changes requested by alanb (Reviewer).
test/lib/jdk/test/lib/util/JarUtils.java line 90:
> 88: String name = toJarEntryName(entry);
> 89: jos.putNextEntry(new JarEntry(name));
> 90: if (Files.exists(dir.resolve(entry))) {
This is test infrastructure that we use in several areas and changing it to
allow file paths to files that don't exist
be problematic. Is there any reason why the jarsigner can't create an empty or
dummy file to put into the JAR file?
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA
On Sun, 4 Oct 2020 08:41:28 GMT, Alan Bateman 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
>
> test/lib/jdk/test/lib/util/JarUtils.java line 90:
>
>> 88: String name = toJarEntryName(entry);
>> 89: jos.putNextEntry(new JarEntry(name));
>> 90: if (Files.exists(dir.resolve(entry))) {
>
> This is test infrastructure that we use in several areas and changing it to
> allow file paths to files that don't exist
> be problematic. Is there any reason why the jarsigner can't create an empty
> or dummy file to put into the JAR file?
Sorry, I'll revert the change and create files myself. I just thought any
existing call to this method should have the
file already created there, but it could be a problem if the creation is not
trivial and might fail.
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v2]
> 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 with a new target base due to a merge or a rebase. The pull request now contains three commits: - extract some methods, rollback JarUtils, deperated version from 15 to 16. - Merge - 8242068: Signed JAR support for RSASSA-PSS and EdDSA - Changes: https://git.openjdk.java.net/jdk/pull/322/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=01 Stats: 1696 lines in 21 files changed: 972 ins; 555 del; 169 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v3]
> 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 refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: extract some methods, rollback JarUtils, deprecated version from 15 to 16. - Changes: - all: https://git.openjdk.java.net/jdk/pull/322/files - new: https://git.openjdk.java.net/jdk/pull/322/files/c05af70a..5d455bae Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=01-02 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v3]
On Sun, 4 Oct 2020 08:41:40 GMT, Alan Bateman wrote: >> Weijun Wang has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental >> views will show differences compared to the previous content of the PR. > > Changes requested by alanb (Reviewer). Note: I force pushed a new commit to correct a typo in the summary line. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v4]
> 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: fotgot to read one property - Changes: - all: https://git.openjdk.java.net/jdk/pull/322/files - new: https://git.openjdk.java.net/jdk/pull/322/files/5d455bae..fecf30d7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=02-03 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v5]
> 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: support rfc5652 CMSAlgorithmProtection - Changes: - all: https://git.openjdk.java.net/jdk/pull/322/files - new: https://git.openjdk.java.net/jdk/pull/322/files/fecf30d7..81513907 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=03-04 Stats: 72 lines in 5 files changed: 65 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
On Tue, 13 Oct 2020 13:24:50 GMT, Weijun Wang wrote: >> Note: I force pushed a new commit to correct a typo in the summary line. > > Add support for [RFC 6211: Cryptographic Message Syntax (CMS) Algorithm > Identifier Protection > Attribute](https://tools.ietf.org/html/rfc6211) to protect against algorithm > substitution attacks. This attribute is > signed and it contains copies of digestAlgorithm and signatureAlgorithm which > are unprotected in SignerInfo. Before > this enhancement, the two algorithms can be implied from the signature itself > (i.e. if you change any of them the > signature size would not match or the key will not decrypt). However, with > the introduction of RSASSA-PSS, the > signature algorithm can be modified and it still looks like the signature is > valid. This particular case is [described > in the RFC](https://tools.ietf.org/html/rfc6211#page-5): >signatureAlgorithm has been protected by implication in the past. > The use of an RSA public key implied that the RSA v1.5 signature > algorithm was being used. The hash algorithm and this fact could > be checked by the internal padding defined. This is no longer > true with the addition of the RSA-PSS signature algorithms. A force push to fix the RFC number typo in the latest commit. No content update. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
> 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 refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: support rfc6211 CMSAlgorithmProtection - Changes: - all: https://git.openjdk.java.net/jdk/pull/322/files - new: https://git.openjdk.java.net/jdk/pull/322/files/81513907..ffaae532 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=04-05 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
On Sun, 4 Oct 2020 14:09:26 GMT, Weijun Wang wrote: >> Changes requested by alanb (Reviewer). > > Note: I force pushed a new commit to correct a typo in the summary line. Add support for [RFC 6211: Cryptographic Message Syntax (CMS) Algorithm Identifier Protection Attribute](https://tools.ietf.org/html/rfc6211) to protect against algorithm substitution attacks. This attribute is signed and it contains copies of digestAlgorithm and signatureAlgorithm which are unprotected in SignerInfo. Before this enhancement, the two algorithms can be implied from the signature itself (i.e. if you change any of them the signature size would not match or the key will not decrypt). However, with the introduction of RSASSA-PSS, the signature algorithm can be modified and it still looks like the signature is valid. This particular case is [described in the RFC](https://tools.ietf.org/html/rfc6211#page-5): signatureAlgorithm has been protected by implication in the past. The use of an RSA public key implied that the RSA v1.5 signature algorithm was being used. The hash algorithm and this fact could be checked by the internal padding defined. This is no longer true with the addition of the RSA-PSS signature algorithms. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/322/files - new: https://git.openjdk.java.net/jdk/pull/322/files/ffaae532..734fd034 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=05-06 Stats: 53 lines in 5 files changed: 42 ins; 0 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
On Tue, 13 Oct 2020 13:34:27 GMT, Weijun Wang 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 refreshed the contents of this pull request, and previous
> commits have been removed. The incremental
> views will show differences compared to the previous content of the PR.
src/java.base/share/classes/sun/security/x509/AlgorithmId.java line 113:
> 111: }
> 112:
> 113: public AlgorithmId(ObjectIdentifier oid, DerValue params)
Now that this is public, can we add a javadoc spec for it?
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 50:
> 48:
> 49: /**
> 50: * Convent OID.1.2.3.4 or 1.2.3.4 to the matched stdName.
Convent => Convert? matched => matching?
Or maybe just "Match OID. to the stdName"
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 53:
> 51: *
> 52: * @param algName input, could be in any form
> 53: * @return the original name is it does not look like an OID,
is => if?
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 54:
> 52: * @param algName input, could be in any form
> 53: * @return the original name is it does not look like an OID,
> 54: * the matched name for an OID, or the OID itself
Maybe use "the OID value"? The term "itself" reminds me of the specified
argument somehow. Just nit.
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 94:
> 92: * @return an AlgorithmParameterSpec object
> 93: * @throws ProviderException
> 94: */
Well, I am a bit unsure about your changes to this method. With your change, it
returns default parameter spec (instead
of null) when the specified AlgorithmParameters object is null. This may not be
desirable for all cases? Existing
callers would have to check for (params != null) before calling this method.
The javadoc description also seems a bit
strange with the to-be-converted AlgorithmParameters object being optional.
Maybe add a separate method like
`getParamSpecWithDefault` on top of this method or add a separate boolean
argument `useDefault`?
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 164:
> 162: }
> 163: } else {
> 164: paramSpec = getDefaultAlgorithmParameterSpec(sigName, null);
Same comment here as for the getParamSpec(String, AlgorithmParameters) above.
Given that there are two methods named
getParamSpec(...), maybe add a boolean argument to indicate whether to return
default value if the to-be-converted
object is null?
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 290:
> 288: * that must be initialized with a AlgorithmParameterSpec now.
> 289: */
> 290: public static AlgorithmParameterSpec
> getDefaultAlgorithmParameterSpec(
Maybe we can just use "getDefaultParamSpec" to match other methods' name in
this class. Just a suggestion.
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 511:
> 509: }
> 510:
> 511: // Same values for RSA and DSA
Update the comment with "Return the default message digest algorithm with the
same security strength as the specified
IFC/FFC key size"?
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 499:
> 497: }
> 498:
> 499: // Values from SP800-57 part 1 rev 4 tables 2 and 3
Update the comment with "Return the default message digest algorithm with the
same security strength as the specified
EC key size"?
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 367:
> 365: * @param sigEngine the signature object
> 366: * @param key the private key
> 367: * @return the AlgorithmIA, not null
IA => Id?
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 268:
> 266:
> 267: /**
> 268: * Extracts the diges
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Wed, 14 Oct 2020 03:51:23 GMT, Weijun Wang 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/util/KnownOIDs.java line 147:
> 145: SHAKE128("2.16.840.1.101.3.4.2.11"),
> 146: SHAKE256("2.16.840.1.101.3.4.2.12"),
> 147: SHAKE256_LEN("2.16.840.1.101.3.4.2.18", "SHAKE256-LEN"),
Can we move this down a little? The ordering within the section is based on the
oid value. It's easier to check for
unlisted/unsupported oid value this way. Why not also add SHAKE128_LEN?
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Wed, 14 Oct 2020 03:51:23 GMT, Weijun Wang 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/util/SignatureUtil.java line 210:
> 208: new DerValue((byte) 2, new byte[]{2, 0})); //
> int 512
> 209: } catch (IOException | NoSuchAlgorithmException e) {
> 210: throw new AssertionError("Shoudl not happen", e);
Shoudl => Should
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 215:
> 213: }
> 214: /**
> 215: * Determines the digestEncryptionAlgorithmId in PKCS& SignerInfo.
&=>7
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 217:
> 215: * Determines the digestEncryptionAlgorithmId in PKCS& SignerInfo.
> 216: *
> 217: * @param signer Signature object that tells you RSASA-PSS params
RSASA=>RSASSA
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 221:
> 219: * @param privateKey key tells you EdDSA params
> 220: * @param directsign Ed448 uses different digest algs depending on
> this
> 221: * @return the digest alg
alg => algid
src/java.base/share/classes/sun/security/util/SignatureUtil.java line 218:
> 216: *
> 217: * @param signer Signature object that tells you RSASA-PSS params
> 218: * @param sigalg Signature algorithm tells you who with who
who with who?
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Tue, 13 Oct 2020 13:29:39 GMT, Weijun Wang wrote: >> Add support for [RFC 6211: Cryptographic Message Syntax (CMS) Algorithm >> Identifier Protection >> Attribute](https://tools.ietf.org/html/rfc6211) to protect against algorithm >> substitution attacks. This attribute is >> signed and it contains copies of digestAlgorithm and signatureAlgorithm >> which are unprotected in SignerInfo. Before >> this enhancement, the two algorithms can be implied from the signature >> itself (i.e. if you change any of them the >> signature size would not match or the key will not decrypt). However, with >> the introduction of RSASSA-PSS, the >> signature algorithm can be modified and it still looks like the signature is >> valid. This particular case is [described >> in the RFC](https://tools.ietf.org/html/rfc6211#page-5): >>signatureAlgorithm has been protected by implication in the past. >> The use of an RSA public key implied that the RSA v1.5 signature >> algorithm was being used. The hash algorithm and this fact could >> be checked by the internal padding defined. This is no longer >> true with the addition of the RSA-PSS signature algorithms. > > A force push to fix the RFC number typo in the latest commit. No content > update. this one has nothing to do with javac so the `compiler` label should be removed - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Thu, 15 Oct 2020 18:15:35 GMT, Vicente Romero wrote: > this one has nothing to do with javac so the `compiler` label should be > removed @vicente-romero-oracle Sorry for the noise, I should have removed it earlier. All files in the `jdk.jartool` module are under `compiler` in https://github.com/openjdk/skara/blob/master/config/mailinglist/rules/jdk.json#L120. You can make it more specific. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Wed, 14 Oct 2020 03:51:23 GMT, Weijun Wang 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
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
On Tue, 13 Oct 2020 23:50:05 GMT, Valerie Peng wrote: >> Weijun Wang has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental >> views will show differences compared to the previous content of the PR. > > src/java.base/share/classes/sun/security/util/SignatureUtil.java line 94: > >> 92: * @return an AlgorithmParameterSpec object >> 93: * @throws ProviderException >> 94: */ > > Well, I am a bit unsure about your changes to this method. With your change, > it returns default parameter spec (instead > of null) when the specified AlgorithmParameters object is null. This may not > be desirable for all cases? Existing > callers would have to check for (params != null) before calling this method. > The javadoc description also seems a bit > strange with the to-be-converted AlgorithmParameters object being optional. > Maybe add a separate method like > `getParamSpecWithDefault` on top of this method or add a separate boolean > argument `useDefault`? I cannot remember why I need to return a default. The only default we current have is for RSASSA-PSS, and in all RSASSA-PSS AlgorithmId for signature I see there is always the params. (When it's for a key the params can be missing). All 3 callers of this method is on a signature AlgorithmId so the params should not be null. I'll remove the default return value and do more testing. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
On Tue, 13 Oct 2020 13:34:27 GMT, Weijun Wang 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 refreshed the contents of this pull request, and previous
> commits have been removed. The incremental
> views will show differences compared to the previous content of the PR.
test/jdk/jdk/security/jarsigner/Spec.java line 128:
> 126: npe(()->b1.setProperty("sectionsonly", null));
> 127: iae(()->b1.setProperty("sectionsonly", "OK"));
> 128: npe(()->b1.setProperty("sectionsonly", null));
Is 'altsigner' support removed? But I saw it being used in later part of this
test. The javadoc for JarSigner.Builder
only lists a subset of above properties. Are those not in javadoc internal and
can be removed any time, just curious?
Nit: maybe add 8242068 to `@bug line` for existing regression tests?
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
On Wed, 14 Oct 2020 00:00:38 GMT, Valerie Peng wrote:
>> Weijun Wang has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental
>> views will show differences compared to the previous content of the PR.
>
> src/java.base/share/classes/sun/security/util/SignatureUtil.java line 164:
>
>> 162: }
>> 163: } else {
>> 164: paramSpec = getDefaultAlgorithmParameterSpec(sigName, null);
>
> Same comment here as for the getParamSpec(String, AlgorithmParameters) above.
> Given that there are two methods named
> getParamSpec(...), maybe add a boolean argument to indicate whether to return
> default value if the to-be-converted
> object is null?
Same reply. I'll see if it's OK to return a null.
> src/java.base/share/classes/sun/security/util/SignatureUtil.java line 290:
>
>> 288: * that must be initialized with a AlgorithmParameterSpec now.
>> 289: */
>> 290: public static AlgorithmParameterSpec
>> getDefaultAlgorithmParameterSpec(
>
> Maybe we can just use "getDefaultParamSpec" to match other methods' name in
> this class. Just a suggestion.
Sure.
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Wed, 14 Oct 2020 04:03:46 GMT, Valerie Peng wrote:
>> 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/util/KnownOIDs.java line 147:
>
>> 145: SHAKE128("2.16.840.1.101.3.4.2.11"),
>> 146: SHAKE256("2.16.840.1.101.3.4.2.12"),
>> 147: SHAKE256_LEN("2.16.840.1.101.3.4.2.18", "SHAKE256-LEN"),
>
> Can we move this down a little? The ordering within the section is based on
> the oid value. It's easier to check for
> unlisted/unsupported oid value this way. Why not also add SHAKE128_LEN?
I'll add one.
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Wed, 14 Oct 2020 03:51:23 GMT, Weijun Wang 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 test/lib/jdk/test/lib/security/timestamp/TsaSigner.java line 221: > 219: new X500Name(issuerName), > 220: signerEntry.cert.getSerialNumber(), > 221: > AlgorithmId.get(SignatureUtil.extractDigestAlgFromDwithE(sigAlgo)), So, sigAlgo would never be RSASSA-PSS, EDDSA, ED25519, or ED448? - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Wed, 14 Oct 2020 05:31:33 GMT, Valerie Peng wrote: >> 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/util/SignatureUtil.java line 218: > >> 216: * >> 217: * @param signer Signature object that tells you RSASA-PSS params >> 218: * @param sigalg Signature algorithm tells you who with who > > who with who? Maybe I was thinking about SHA1withRSA, but here it can be the new algorithms. Just remove the confusing words. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Wed, 14 Oct 2020 19:18:04 GMT, Valerie Peng wrote:
>> 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?
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.
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Thu, 15 Oct 2020 02:03:13 GMT, Valerie Peng wrote: >> 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 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. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Thu, 15 Oct 2020 20:42:30 GMT, Valerie Peng wrote:
>> 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/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-".
Good suggestion. I'll also try some tests.
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
On Fri, 16 Oct 2020 01:40:48 GMT, Valerie Peng wrote:
>> Weijun Wang has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental
>> views will show differences compared to the previous content of the PR.
>
> test/jdk/jdk/security/jarsigner/Spec.java line 128:
>
>> 126: npe(()->b1.setProperty("sectionsonly", null));
>> 127: iae(()->b1.setProperty("sectionsonly", "OK"));
>> 128: npe(()->b1.setProperty("sectionsonly", null));
>
> Is 'altsigner' support removed? But I saw it being used in later part of this
> test. The javadoc for JarSigner.Builder
> only lists a subset of above properties. Are those not in javadoc internal
> and can be removed any time, just curious?
> Nit: maybe add 8242068 to `@bug line` for existing regression tests?
It's deprecated. I remember we are already thinking about deprecating the
altSign mechanism when creating the JarSigner
API and therefore we didn't add them to the spec from the beginning.
Bug id added.
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Fri, 16 Oct 2020 02:15:08 GMT, Valerie Peng wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> signing time, jarsigner -directsign, and digest algorithm check > > test/lib/jdk/test/lib/security/timestamp/TsaSigner.java line 221: > >> 219: new X500Name(issuerName), >> 220: signerEntry.cert.getSerialNumber(), >> 221: >> AlgorithmId.get(SignatureUtil.extractDigestAlgFromDwithE(sigAlgo)), > > So, sigAlgo would never be RSASSA-PSS, EDDSA, ED25519, or ED448? There were no getDigestAlgInPkcs7SignerInfo in my initial version. I'll use this new method instead. I'll probably need to try if the new algorithms really work here. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Fri, 16 Oct 2020 02:34:35 GMT, Weijun Wang wrote:
>> 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-".
>
> Good suggestion. I'll also try some tests.
In fact, since now I directly write the signature algorithm into the
`SignerInfo.digestEncryptionAlgorithmId` field,
the code above is not used at all. The `makeSigAlg` method directly returns the
`encAlgId` argument if it has "with"
inside.
I'll fix it anyway. I've confirmed that if I still write only the key algorithm
there (Ex: "EC") then the verification
process will see a problem without your suggested change.
-
PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v8]
> 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: no default getParamSpec, renames, comments, more testing - Changes: - all: https://git.openjdk.java.net/jdk/pull/322/files - new: https://git.openjdk.java.net/jdk/pull/322/files/734fd034..bd3a1596 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=06-07 Stats: 101 lines in 9 files changed: 60 ins; 12 del; 29 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v7]
On Fri, 16 Oct 2020 02:30:55 GMT, Weijun Wang 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
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v6]
On Fri, 16 Oct 2020 01:38:44 GMT, Weijun Wang wrote: >> src/java.base/share/classes/sun/security/util/SignatureUtil.java line 94: >> >>> 92: * @return an AlgorithmParameterSpec object >>> 93: * @throws ProviderException >>> 94: */ >> >> Well, I am a bit unsure about your changes to this method. With your change, >> it returns default parameter spec (instead >> of null) when the specified AlgorithmParameters object is null. This may not >> be desirable for all cases? Existing >> callers would have to check for (params != null) before calling this method. >> The javadoc description also seems a bit >> strange with the to-be-converted AlgorithmParameters object being optional. >> Maybe add a separate method like >> `getParamSpecWithDefault` on top of this method or add a separate boolean >> argument `useDefault`? > > I cannot remember why I need to return a default. The only default we current > have is for RSASSA-PSS, and in all > RSASSA-PSS AlgorithmId for signature I see there is always the params. (When > it's for a key the params can be missing). > All 3 callers of this method is on a signature AlgorithmId so the params > should not be null. I'll remove the default > return value and do more testing. Sounds good. RSASSA-PSS sig algorithm id always have params, it's required. - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v8]
On Fri, 16 Oct 2020 15:51:25 GMT, Weijun Wang 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: > > no default getParamSpec, renames, comments, more testing Marked as reviewed by valeriep (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/322
Re: RFR: 8242068: Signed JAR support for RSASSA-PSS and EdDSA [v9]
> 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: tiny help screen change - Changes: - all: https://git.openjdk.java.net/jdk/pull/322/files - new: https://git.openjdk.java.net/jdk/pull/322/files/bd3a1596..251373f4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=322&range=07-08 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/322.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/322/head:pull/322 PR: https://git.openjdk.java.net/jdk/pull/322
