Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On Nov 16, 2021, at 7:28 PM, Michael StJohns
mailto:mstjo...@comcast.net>> wrote:
id-kp-timeStampingOBJECT IDENTIFIER ::= { id-kp 8 }
-- Binding the hash of an object to a time
-- Key usage bits that may be consistent: digitalSignature
-- and/or nonRepudiation
Hm, we may want to check it strictly in this update, by allowing nonRepudiation
alternatively.
Xuelei
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On 11/16/2021 7:46 PM, Weijun Wang wrote:
On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote:
There is no need to check for the KeyUsage extension when validating a TSA
certificate.
A test is modified where a TSA cert has a KeyUsage but without the
DigitalSignature bit.
Weijun Wang has updated the pull request incrementally with one additional
commit since the last revision:
clarify RFC requirement
Really? The TSA ishttp://timestamp.digicert.com and the cert chain is
CN=DigiCert Timestamp 2021, O="DigiCert, Inc.", C=US
KeyUsage: DigitalSignature
ExtendedKeyUsages: timeStamping
CN=DigiCert SHA2 Assured ID Timestamping CA, OU=www.digicert.com, O=DigiCert
Inc, C=US
KeyUsage: DigitalSignature, Key_CertSign, Crl_Sign
ExtendedKeyUsages: timeStamping
You mean this CA can be used for time stamping as well? I understand that when
KU is using you can find out its usage in EKU (vice versa), but here it's a CA
that can sign cert and CRLs. Does it really need to act as the end entity cert
of a TSA server?
-
PR:https://git.openjdk.java.net/jdk/pull/6416
It doesn't need to act as an EE of a TSA server, but with those markings
it can.
Whoever issued these over marked them. I think their intent was to say
that this CA chain would issue time stamp issuing certificates, but
extendedKeyUsage contents are not transitive to the subordinate
certificates so that extension is pretty much extraneous in a CA. That
said, if you got a timestamp verifiable by the public key in this CA
certificate it would be valid (based on the certificate only). The
TSA RFC doesn't actually prohibit having a basicConstraints ca=true
extension. If I were verifying a timestamp, I'd probably filter out
any signatures from certificates that are claiming to be CAs, but that's
not strictly according to standards.
If I were issuing this chain, there would be no extendedKeyUsage
extensions in the intermediate certificate(s), and the keyPurpose would
only be keyCertSign or keyCertSign,cRLSign depending. The EE
certificate would have eku {id-kp-timestamping} and ku {
digitalSignature } as I probably couldn't ensure non-repudiation for the
time stamp (not the data being wrapped by the timestamp which is what
the Rekor chain is trying to claim I think).
Hmm... while I was researching - I found this in RFC5280 - 4.2.1.12
defining extendedKeyUsage oids:
This extension indicates one or more purposes for which the certified
public key may be used, in addition to or in place of the basic
purposes indicated in the key usage extension._In general, this extension
will appear only in end entity certificates._ This
extension is defined as follows
and
id-kp-timeStampingOBJECT IDENTIFIER ::= { id-kp 8 }
-- Binding the hash of an object to a time
-- Key usage bits that may be consistent:_digitalSignature_
-- and/or_nonRepudiation_
I hope that helps.
Mike
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote: >> There is no need to check for the KeyUsage extension when validating a TSA >> certificate. >> >> A test is modified where a TSA cert has a KeyUsage but without the >> DigitalSignature bit. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > clarify RFC requirement Really? The TSA is http://timestamp.digicert.com and the cert chain is CN=DigiCert Timestamp 2021, O="DigiCert, Inc.", C=US KeyUsage: DigitalSignature ExtendedKeyUsages: timeStamping CN=DigiCert SHA2 Assured ID Timestamping CA, OU=www.digicert.com, O=DigiCert Inc, C=US KeyUsage: DigitalSignature, Key_CertSign, Crl_Sign ExtendedKeyUsages: timeStamping You mean this CA can be used for time stamping as well? I understand that when KU is using you can find out its usage in EKU (vice versa), but here it's a CA that can sign cert and CRLs. Does it really need to act as the end entity cert of a TSA server? - PR: https://git.openjdk.java.net/jdk/pull/6416
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On 11/16/2021 6:37 PM, Weijun Wang wrote: On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote: There is no need to check for the KeyUsage extension when validating a TSA certificate. A test is modified where a TSA cert has a KeyUsage but without the DigitalSignature bit. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: clarify RFC requirement I did see an issuer of TSA certs whose own certificate has EKU with id-kp-timeStamping and KU with both DigitialSignature and keyCertsign. This cert should be rejected if it signed a timestamp response. - PR: https://git.openjdk.java.net/jdk/pull/6416 Not quite. The rule is that if there's both an ExtendedKeyUsage and KeyUsage extensions, for any given OID in the EKU there has to be at least one bit in the KeyUsage extenstion that's compatible - there may be more than one. If there's an EKU, and no KeyUsage, then only the EKU needs to have an OID for the key usage purpose - in this case signing a timestamp. The cert you cite would be valid for timestamping. Mike
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On 11/16/2021 5:58 PM, Michael StJohns wrote:
On 11/16/2021 4:05 PM, Weijun Wang wrote:
On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote:
There is no need to check for the KeyUsage extension when validating a TSA
certificate.
A test is modified where a TSA cert has a KeyUsage but without the
DigitalSignature bit.
Weijun Wang has updated the pull request incrementally with one additional
commit since the last revision:
clarify RFC requirement
Hi Michael. Thanks for the comment. That was also our previous understanding
but we are seeing timestamp returning by sigstore.dev (see the `rekor
timestamp` command athttps://github.com/sigstore/rekor) whose cert does not
have the DigitialSignature bit set.
-BEGIN CERTIFICATE-
MIIBvzCCAWWgAwIBAgICBnowCgYIKoZIzj0EAwIwHzEdMBsGA1UEChMUaHR0cHM6
Ly9zaWdzdG9yZS5kZXYwHhcNMjExMTAyMTgxODI5WhcNMzExMTAyMTgxODI5WjAi
MSAwHgYDVQQKExdSZWtvciBUaW1lc3RhbXBpbmcgQ2VydDBZMBMGByqGSM49AgEG
CCqGSM49AwEHA0IABJIsOXOZUdgXhnNmvue9AAsxSYWdp+1HvEQQMYuZUsU0Ylf2
bKqIp3zVrc0a58pGJZvwGjyOHgY5lRevPP1huuOjgY0wgYowDgYDVR0PAQH/BAQD
AgZAMAwGA1UdEwEB/wQCMAAwDgYDVR0OBAcEBQECAwQGMB8GA1UdIwQYMBaAFIiV
AzEbE/rHgP3CA3x7tofqTtIcMCEGA1UdEQQaMBiHBH8AAAGHEAAA
AAEwFgYDVR0lAQH/BAwwCgYIKwYBBQUHAwgwCgYIKoZIzj0EAwIDSAAwRQIg
XdDBMPMTrtXiHmhFJOgQW8DDz/IaHkNZ+hXNL19dmuICIQCw3lE5+52F41kpY3B/
sJaPjuKmeIuEyYZDnMnlhHSusg==
-END CERTIFICATE-
-
PR:https://git.openjdk.java.net/jdk/pull/6416
The certificate is either incorrect, or weird and correct. I actually
think its incorrect, but let's assume the latter and that either of
these two key purposes can be used. Change the check to permit
either of the two KUs in a KeyUsageExtension:
// insert around line 109.
private static final int KU_NON_REPUDIATION = 1;
// replace line 359.
if (checkKeyUsage (cert, KU_SIGNATURE) == false && checkKeyUsage(cert,
KU_NON_REPUDATION) == false) {
From RFC5280:
The digitalSignature bit is asserted when the subject public key
is used for verifying digital signatures, other than signatures on
certificates (bit 5) and CRLs (bit 6), such as those used in an
entity authentication service, a data origin authentication
service, and/or an integrity service.
The nonRepudiation bit is asserted when the subject public key is
used to verify digital signatures, other than signatures on
certificates (bit 5) and CRLs (bit 6), used to provide a non-
repudiation service that protects against the signing entity
falsely denying some action. In the case of later conflict, a
reliable third party may determine the authenticity of the signed
data. (Note that recent editions of X.509 have renamed the
nonRepudiation bit to contentCommitment.)
In any event, if you have a KU extension and it includes neither of
these bits, the certificate is invalid for timestamping. So simply
deleting the check is wrong.
I'll reach out again to my expert and let you know what I find out.
Thanks - Mike
According to the guy who wrote RFC5280, nonRepudiation (aka
contentCommitment) is a valid alternative for a keyUsage that pairs with
an extended key usage of id-kp-timestamping. I'd add a check that
requires one or the other or both if a KeyUsage extension exists.
I added a note to the Rekor CA repository, hopefully at the correct
location suggesting they set both bits going forward. This was code they
published only back in May.
Mike
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote: >> There is no need to check for the KeyUsage extension when validating a TSA >> certificate. >> >> A test is modified where a TSA cert has a KeyUsage but without the >> DigitalSignature bit. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > clarify RFC requirement I did see an issuer of TSA certs whose own certificate has EKU with id-kp-timeStamping and KU with both DigitialSignature and keyCertsign. This cert should be rejected if it signed a timestamp response. - PR: https://git.openjdk.java.net/jdk/pull/6416
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote:
>> There is no need to check for the KeyUsage extension when validating a TSA
>> certificate.
>>
>> A test is modified where a TSA cert has a KeyUsage but without the
>> DigitalSignature bit.
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> clarify RFC requirement
Sean found RFC 5280, 4.2.1.12:
id-kp-timeStampingOBJECT IDENTIFIER ::= { id-kp 8 }
-- Binding the hash of an object to a time
-- Key usage bits that may be consistent: digitalSignature
-- and/or nonRepudiation
so it seems either is OK. That said, it's just a "may", and to me it reads more
like that others are NOT consistent and should be rejected.
-
PR: https://git.openjdk.java.net/jdk/pull/6416
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On 11/16/2021 4:05 PM, Weijun Wang wrote:
On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote:
There is no need to check for the KeyUsage extension when validating a TSA
certificate.
A test is modified where a TSA cert has a KeyUsage but without the
DigitalSignature bit.
Weijun Wang has updated the pull request incrementally with one additional
commit since the last revision:
clarify RFC requirement
Hi Michael. Thanks for the comment. That was also our previous understanding
but we are seeing timestamp returning by sigstore.dev (see the `rekor
timestamp` command athttps://github.com/sigstore/rekor) whose cert does not
have the DigitialSignature bit set.
-BEGIN CERTIFICATE-
MIIBvzCCAWWgAwIBAgICBnowCgYIKoZIzj0EAwIwHzEdMBsGA1UEChMUaHR0cHM6
Ly9zaWdzdG9yZS5kZXYwHhcNMjExMTAyMTgxODI5WhcNMzExMTAyMTgxODI5WjAi
MSAwHgYDVQQKExdSZWtvciBUaW1lc3RhbXBpbmcgQ2VydDBZMBMGByqGSM49AgEG
CCqGSM49AwEHA0IABJIsOXOZUdgXhnNmvue9AAsxSYWdp+1HvEQQMYuZUsU0Ylf2
bKqIp3zVrc0a58pGJZvwGjyOHgY5lRevPP1huuOjgY0wgYowDgYDVR0PAQH/BAQD
AgZAMAwGA1UdEwEB/wQCMAAwDgYDVR0OBAcEBQECAwQGMB8GA1UdIwQYMBaAFIiV
AzEbE/rHgP3CA3x7tofqTtIcMCEGA1UdEQQaMBiHBH8AAAGHEAAA
AAEwFgYDVR0lAQH/BAwwCgYIKwYBBQUHAwgwCgYIKoZIzj0EAwIDSAAwRQIg
XdDBMPMTrtXiHmhFJOgQW8DDz/IaHkNZ+hXNL19dmuICIQCw3lE5+52F41kpY3B/
sJaPjuKmeIuEyYZDnMnlhHSusg==
-END CERTIFICATE-
-
PR:https://git.openjdk.java.net/jdk/pull/6416
The certificate is either incorrect, or weird and correct. I actually
think its incorrect, but let's assume the latter and that either of
these two key purposes can be used. Change the check to permit
either of the two KUs in a KeyUsageExtension:
// insert around line 109.
private static final int KU_NON_REPUDIATION = 1;
// replace line 359.
if (checkKeyUsage (cert, KU_SIGNATURE) == false && checkKeyUsage(cert,
KU_NON_REPUDATION) == false) {
From RFC5280:
The digitalSignature bit is asserted when the subject public key
is used for verifying digital signatures, other than signatures on
certificates (bit 5) and CRLs (bit 6), such as those used in an
entity authentication service, a data origin authentication
service, and/or an integrity service.
The nonRepudiation bit is asserted when the subject public key is
used to verify digital signatures, other than signatures on
certificates (bit 5) and CRLs (bit 6), used to provide a non-
repudiation service that protects against the signing entity
falsely denying some action. In the case of later conflict, a
reliable third party may determine the authenticity of the signed
data. (Note that recent editions of X.509 have renamed the
nonRepudiation bit to contentCommitment.)
In any event, if you have a KU extension and it includes neither of
these bits, the certificate is invalid for timestamping. So simply
deleting the check is wrong.
I'll reach out again to my expert and let you know what I find out.
Thanks - Mike
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote: >> There is no need to check for the KeyUsage extension when validating a TSA >> certificate. >> >> A test is modified where a TSA cert has a KeyUsage but without the >> DigitalSignature bit. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > clarify RFC requirement Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6416
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
On Tue, 16 Nov 2021 21:00:12 GMT, Weijun Wang wrote: >> There is no need to check for the KeyUsage extension when validating a TSA >> certificate. >> >> A test is modified where a TSA cert has a KeyUsage but without the >> DigitalSignature bit. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > clarify RFC requirement Hi Michael. Thanks for the comment. That was also our previous understanding but we are seeing timestamp returning by sigstore.dev (see the `rekor timestamp` command at https://github.com/sigstore/rekor) whose cert does not have the DigitialSignature bit set. -BEGIN CERTIFICATE- MIIBvzCCAWWgAwIBAgICBnowCgYIKoZIzj0EAwIwHzEdMBsGA1UEChMUaHR0cHM6 Ly9zaWdzdG9yZS5kZXYwHhcNMjExMTAyMTgxODI5WhcNMzExMTAyMTgxODI5WjAi MSAwHgYDVQQKExdSZWtvciBUaW1lc3RhbXBpbmcgQ2VydDBZMBMGByqGSM49AgEG CCqGSM49AwEHA0IABJIsOXOZUdgXhnNmvue9AAsxSYWdp+1HvEQQMYuZUsU0Ylf2 bKqIp3zVrc0a58pGJZvwGjyOHgY5lRevPP1huuOjgY0wgYowDgYDVR0PAQH/BAQD AgZAMAwGA1UdEwEB/wQCMAAwDgYDVR0OBAcEBQECAwQGMB8GA1UdIwQYMBaAFIiV AzEbE/rHgP3CA3x7tofqTtIcMCEGA1UdEQQaMBiHBH8AAAGHEAAA AAEwFgYDVR0lAQH/BAwwCgYIKwYBBQUHAwgwCgYIKoZIzj0EAwIDSAAwRQIg XdDBMPMTrtXiHmhFJOgQW8DDz/IaHkNZ+hXNL19dmuICIQCw3lE5+52F41kpY3B/ sJaPjuKmeIuEyYZDnMnlhHSusg== -END CERTIFICATE- - PR: https://git.openjdk.java.net/jdk/pull/6416
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate [v2]
> There is no need to check for the KeyUsage extension when validating a TSA > certificate. > > A test is modified where a TSA cert has a KeyUsage but without the > DigitalSignature bit. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: clarify RFC requirement - Changes: - all: https://git.openjdk.java.net/jdk/pull/6416/files - new: https://git.openjdk.java.net/jdk/pull/6416/files/e7db147e..a5b3bc86 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6416=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6416=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6416.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6416/head:pull/6416 PR: https://git.openjdk.java.net/jdk/pull/6416
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate
On 11/16/2021 2:43 PM, Weijun Wang wrote:
There is no need to check for the KeyUsage extension when validating a TSA
certificate.
A test is modified where a TSA cert has a KeyUsage but without the
DigitalSignature bit.
-
Commit messages:
- 8277246: No need to check about KeyUsage when validating a TSA certificate
Changes:https://git.openjdk.java.net/jdk/pull/6416/files
Webrev:https://webrevs.openjdk.java.net/?repo=jdk=6416=00
Issue:https://bugs.openjdk.java.net/browse/JDK-8277246
Stats: 7 lines in 2 files changed: 0 ins; 6 del; 1 mod
Patch:https://git.openjdk.java.net/jdk/pull/6416.diff
Fetch: git fetchhttps://git.openjdk.java.net/jdk pull/6416/head:pull/6416
PR:https://git.openjdk.java.net/jdk/pull/6416
I just ran this by one of the PKIX folk and this fix isn't correct.
Basically, if a certificate has both an Extended Key Usage and a Key
Usage extension then they both have to be consistent with the actual
purpose and both have to be checked. The former for the specific use
described in the TSA document, and the latter for general use (RFC3280
and 5280).
In this case, checkKeyUsage attempts to find keyUsageExtension and if
it's not present returns true (e.g. key usage is acceptable). Otherwise
it checks to see if there's a digitalSignature bit set, and if it's not
set checkKeyUsage returns false. The code as written (before the
change) is correct. Here's the utility method in EndEntityChecker.java
/**
220 * Utility method checking if bit 'bit' is set in this certificates
221 * key usage extension.
222 * @throws CertificateException if not
223 */
224 private boolean checkKeyUsage(X509Certificate cert, int bit)
225 throws CertificateException {
226 boolean[] keyUsage = cert.getKeyUsage();
227 if (keyUsage == null) {
228 return true;
229 }
230 return (keyUsage.length > bit) && keyUsage[bit];
231 }
It would be acceptable to have a certificate without a
keyUsageExtension, but if the KUE is present, then the digitalSignature
bit needs to be set.
Recommend backing out the change and closing the bug report as mistaken.
Mike
Integrated: 8275811 Incorrect instance to dispose
On Fri, 22 Oct 2021 18:24:22 GMT, Daniel Jeliński wrote: > The current code that changes cipher suites disposes the new suite instead of > the old one, which usually silently fails. This patch fixes the code to > dispose the old instance instead. > > DTLS appears to be unaffected: DTLSOutputRecord keeps 2 ciphers and correctly > [disposes the old > one](https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/src/java.base/share/classes/sun/security/ssl/DTLSOutputRecord.java#L106), > and DTLSInputRecord [doesn't dispose > anything](https://github.com/openjdk/jdk/blob/4b9303b77b43d890ebacbec38b4ac5db7e171886/src/java.base/share/classes/sun/security/ssl/DTLSInputRecord.java#L57) This pull request has now been integrated. Changeset: cddc6ce4 Author:Daniel Jelinski Committer: Xue-Lei Andrew Fan URL: https://git.openjdk.java.net/jdk/commit/cddc6ce44695cba4614c3405eb2b194d7c76489b Stats: 35 lines in 4 files changed: 32 ins; 0 del; 3 mod 8275811: Incorrect instance to dispose Reviewed-by: xuelei - PR: https://git.openjdk.java.net/jdk/pull/6084
Re: RFR: 8275811 Incorrect instance to dispose [v6]
On Wed, 3 Nov 2021 09:16:47 GMT, Daniel Jeliński wrote: >> The current code that changes cipher suites disposes the new suite instead >> of the old one, which usually silently fails. This patch fixes the code to >> dispose the old instance instead. >> >> DTLS appears to be unaffected: DTLSOutputRecord keeps 2 ciphers and >> correctly [disposes the old >> one](https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/src/java.base/share/classes/sun/security/ssl/DTLSOutputRecord.java#L106), >> and DTLSInputRecord [doesn't dispose >> anything](https://github.com/openjdk/jdk/blob/4b9303b77b43d890ebacbec38b4ac5db7e171886/src/java.base/share/classes/sun/security/ssl/DTLSInputRecord.java#L57) > > Daniel Jeliński has updated the pull request incrementally with one > additional commit since the last revision: > > bugfix > /integrate > > tier1, tier2 and jdk_core are all clean now. I think we're good to go. > Thanks, I will sponsor the integration soon. > Do PKCS11 implementations release resources when doFinal is called? Our > documentation for Cipher does not mention that doFinal should be called to > release resources, and our PKCS11 guide doesn't say that providers should > release resources when doFinal is called. That's a pretty old and interesting topic. I don't think doFinal could really have PKCS11 release its resources. As there is not close APIs for key instance, there is not much we can do right now. Maybe, we could consider to support closable keys in the future. > Do we need to dispose GCM ciphers? They always call init and doFinal in > matching pairs, so the extra doFinal in dispose looks superfluous. It makes sense. But it may make it complicated to check different ciphers. In the long run, the doFinal() should be replaced with something like close(). - PR: https://git.openjdk.java.net/jdk/pull/6084
Re: RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate
On Tue, 16 Nov 2021 19:36:11 GMT, Weijun Wang wrote: > There is no need to check for the KeyUsage extension when validating a TSA > certificate. > > A test is modified where a TSA cert has a KeyUsage but without the > DigitalSignature bit. src/java.base/share/classes/sun/security/validator/EndEntityChecker.java line 364: > 362: ValidatorException.T_EE_EXTENSIONS, cert); > 363: } > 364: Could you add a comment here just in case someone else adding the checking back? Otherwise, looks nice to me. - PR: https://git.openjdk.java.net/jdk/pull/6416
RFR: 8277246: No need to check about KeyUsage when validating a TSA certificate
There is no need to check for the KeyUsage extension when validating a TSA certificate. A test is modified where a TSA cert has a KeyUsage but without the DigitalSignature bit. - Commit messages: - 8277246: No need to check about KeyUsage when validating a TSA certificate Changes: https://git.openjdk.java.net/jdk/pull/6416/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6416=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277246 Stats: 7 lines in 2 files changed: 0 ins; 6 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6416.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6416/head:pull/6416 PR: https://git.openjdk.java.net/jdk/pull/6416
Re: RFR: 8275887: jarsigner prints invalid digest/signature algorithm warnings if keysize is weak/disabled [v2]
> When a signature/digest algorithm was being checked, the algorithm > constraints checked both the signature/digest algorithm and the key to see if > they were restricted. This caused duplicate checks and was also problematic > for `jarsigner` (and `keytool`) which need to distinguish these two cases, so > that the output can properly indicate when the key is disabled but the > signature or digest alg is ok. > > To address this issue, a new `checkKey` parameter is added to the > `DisabledAlgorithmConstraints.permits` methods. When `true` the key (alg and > size) is also checked, otherwise it is not. This flag is always set to > `false` by `jarsigner` when checking algs and by the JDK when checking digest > algorithms. Other small changes include changes in `SignerInfo` to use a > record to store info about the algorithms to be checked, and removing an > unnecessary CRL checking method from `AlgorithmChecker`. > > `keytool` will be enhanced in a subsequent CR to call the new methods. Sean Mullan has updated the pull request incrementally with one additional commit since the last revision: Use var in for loop in SignerInfo. In TimestampCheck test, combine/simplify what messages should not be emitted when jar is signed with 512-bit RSA key. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6296/files - new: https://git.openjdk.java.net/jdk/pull/6296/files/6c1f1dd8..ac6d9436 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6296=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6296=00-01 Stats: 13 lines in 2 files changed: 0 ins; 8 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/6296.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6296/head:pull/6296 PR: https://git.openjdk.java.net/jdk/pull/6296
Re: RFR: 8275887: jarsigner prints invalid digest/signature algorithm warnings if keysize is weak/disabled
On Tue, 16 Nov 2021 01:07:55 GMT, Weijun Wang wrote:
>> When a signature/digest algorithm was being checked, the algorithm
>> constraints checked both the signature/digest algorithm and the key to see
>> if they were restricted. This caused duplicate checks and was also
>> problematic for `jarsigner` (and `keytool`) which need to distinguish these
>> two cases, so that the output can properly indicate when the key is disabled
>> but the signature or digest alg is ok.
>>
>> To address this issue, a new `checkKey` parameter is added to the
>> `DisabledAlgorithmConstraints.permits` methods. When `true` the key (alg and
>> size) is also checked, otherwise it is not. This flag is always set to
>> `false` by `jarsigner` when checking algs and by the JDK when checking
>> digest algorithms. Other small changes include changes in `SignerInfo` to
>> use a record to store info about the algorithms to be checked, and removing
>> an unnecessary CRL checking method from `AlgorithmChecker`.
>>
>> `keytool` will be enhanced in a subsequent CR to call the new methods.
>
> src/java.base/share/classes/sun/security/pkcs/SignerInfo.java line 749:
>
>> 747: Set enabledAlgorithms = new HashSet<>();
>> 748: try {
>> 749: for (Map.Entry algorithm :
>
> You can use `var`.
Yes.
> test/jdk/sun/security/tools/jarsigner/TimestampCheck.java line 368:
>
>> 366: .shouldNotContain("The SHA-256 algorithm
>> specified " +
>> 367: "for the -tsadigestalg option is considered
>> a " +
>> 368: "security risk and is disabled")
>
> Maybe just check `.shouldNotContain("option is considered a security risk and
> is disabled")`?
Ok
-
PR: https://git.openjdk.java.net/jdk/pull/6296
Re: RFR: 8275887: jarsigner prints invalid digest/signature algorithm warnings if keysize is weak/disabled
On Tue, 16 Nov 2021 01:41:50 GMT, Weijun Wang wrote:
> I'm feeling we should completely dump checking for algorithms and switch to
> checking algorithmIds. Even if currently it's only RSASSA-PSS, but suppose
> one day we support the SHAKE256-LEN MessageDigest algorithm and I suppose
> that LEN cannot be any number.
Yes, this is a good suggestion, but I think it should be tackled separately.
I'll file a separate RFE though. It would be nice if we made AlgorithmId a
public API too.
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line
> 1491:
>
>> 1489: private static String checkWeakAlg(String alg,
>> CertPathConstraintsParameters cpcp) {
>> 1490: try {
>> 1491: CERTPATH_DISABLED_CHECK.permits(alg, cpcp, false);
>
> Do we need to check AlgorithmParamters as well? Ex: if `alg` is RSASSA-PSS.
We should, but the problem is that jarsigner needs to individually test each
algorithm, so it can properly display which algorithm is restricted. So, I
think it will need to parse the RSSASSA params itself, and then call the
constraints code to check each algorithm. Let me see if I can code up something
that does that.
-
PR: https://git.openjdk.java.net/jdk/pull/6296
