On Wed, 12 Jan 2022 02:15:45 GMT, Hai-May Chao <[email protected]> wrote:
> `keytool` currently uses a simpler scheme in `DisabledAlgorithmConstraints`
> class when performing algorithm constraints checks. This change is to enhance
> `keytool` to make use of the new methods
> `DisabledAlgorithmConstraints.permits` with `CertPathConstraintsParameters`
> and `checkKey` parameters. For the keyusage in the EE certificate of a
> certificate chains, set the variant accordingly when calling
> `CertPathConstraintsParameters` constructor.
Some comments so far. Will continue reviewing later.
src/java.base/share/classes/sun/security/tools/keytool/Main.java line 187:
> 185: private List<String> weakWarnings = new ArrayList<>();
> 186:
> 187: Set<X509Certificate> trustedCerts = new HashSet<>();
Make private.
src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1129:
> 1127: }
> 1128:
> 1129: buildTrustedCerts();
Can we reuse the keystore loaded by `buildTrustedCerts()` instead of reloading
it again on line 1138?
src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1558:
> 1556: checkWeakConstraint(rb.getString("the.issuer"),
> 1557: keyStore.getCertificateChain(alias));
> 1558: cpcp = new CertPathConstraintsParameters(cert, null, null,
> null);
You could also specify the variant parameter if the certificate contains an EKU
extension. The mapping from EKU to variant would be:
- anyExtendedKeyUsage -> VAR_GENERIC
- serverAuth -> VAR_TLS_SERVER
- clientAuth -> VAR_TLS_CLIENT
- codeSigning -> VAR_CODE_SIGNING
- emailProtection -> none defined, so just use null or VAR_GENERIC
- timeStamping -> VAR_TSA_SERVER
- OCSPSigning -> none defined, so just use null or VAR_GENERIC
src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1706:
> 1704: }
> 1705: dumpCert(cert, out);
> 1706: CertPathConstraintsParameters cpcp =
Here you could also specify the variant if the cert contains an EKU extension.
Same comment applies to other places where you are checking the algorithm
constraints of a certificate.
src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5039:
> 5037:
> trustedCerts.add((X509Certificate)caks.getCertificate(a));
> 5038: } catch (Exception e2) {
> 5039: // ignore, when a SecretkeyEntry does not
> include a cert
Not sure I understand this comment, as these should not be SecretKeyEntries.
You should still ignore it but this should not throw an Exception unless the
KeyStore was not loaded/initialized properly (which normally won't occur).
-------------
PR: https://git.openjdk.java.net/jdk/pull/7039
