RFR: 8261513: Various BasicConstraintsExtension issues [v3]

[Sean Mullan]

On Mon, 3 Feb 2025 19:26:27 GMT, Ben Perez <bperez at openjdk.org> wrote:

>> 8261513: Various BasicConstraintsExtension issues
>
> Ben Perez has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8261513
>  - Added IOException for negative pathLenConstraint
>  - Changed constructor, getPathLen, and toString to return a canonical representation for unconstrained pathLens

A couple more comments. You'll need to add an appropriate `noreg` label to the bug if you think it isn't practical to write a test for this.

src/java.base/share/classes/sun/security/x509/BasicConstraintsExtension.java line 1:

> 1: /*

On line 186, it's questionable if we need to set the critical flag to the value of the ca field. This was comment #6 in the bug report. RFC 5280 gives a few cases where it is acceptable to have a non-critical BasicConstraintsExtension with a ca field set to true. I would remove that and make sure all tests still pass.

src/java.base/share/classes/sun/security/x509/BasicConstraintsExtension.java line 167:

> 165:          String pathLenAsString;
> 166:          if (pathLen < 0 || pathLen == Integer.MAX_VALUE) {
> 167:              pathLenAsString = " unconstained";

Typo: s/unconstained/unconstrained/

But I actually prefer the words "no limit" as that is what [RFC 5280, section 4.2.1.9](https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.9) uses, so please restore those words. You can use that term for the "undefined" case as well.

-------------

PR Review: https://git.openjdk.org/jdk/pull/20224#pullrequestreview-2627976730
PR Review Comment: https://git.openjdk.org/jdk/pull/20224#discussion_r1962385195
PR Review Comment: https://git.openjdk.org/jdk/pull/20224#discussion_r1962370853