On Mon, 8 Nov 2021 14:04:15 GMT, Sean Mullan <mul...@openjdk.org> 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.
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.
src/java.base/share/classes/sun/security/pkcs/SignerInfo.java line 749:
> 747: Set<String> enabledAlgorithms = new HashSet<>();
> 748: try {
> 749: for (Map.Entry<AlgorithmId, AlgorithmInfo> algorithm :
You can use `var`.
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.
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")`?
-------------
PR: https://git.openjdk.java.net/jdk/pull/6296
