On Wed, 12 Jan 2022 02:15:45 GMT, Hai-May Chao <hc...@openjdk.org> 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