Skip to content

8261513: Various BasicConstraintsExtension issues #20224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

8261513: Various BasicConstraintsExtension issues #20224

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
911bd22
Changed constructor, getPathLen, and toString to return a canonical r…
blperez01 Jul 17, 2024
c04afb0
Added IOException for negative pathLenConstraint
blperez01 Jan 31, 2025
64398d5
Merge branch 'master' into JDK-8261513
blperez01 Feb 3, 2025
6ba4482
changed toString wording, no longer set critical to ca
blperez01 Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/java.base/share/classes/sun/security/x509/BasicConstraintsExtension.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 143, we should throw an IOException if the decoded pathLenConstraint field is < 0. This is point #1 in the bug report.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -52,7 +52,7 @@ public class BasicConstraintsExtension extends Extension {
public static final String NAME = "BasicConstraints";

// Private data members
private boolean ca = false;
private boolean ca = false;
private int pathLen = -1;

// Encode this extension value
Expand Down Expand Up @@ -91,7 +91,7 @@ public BasicConstraintsExtension(boolean ca, int len) {
*/
public BasicConstraintsExtension(Boolean critical, boolean ca, int len) {
this.ca = ca;
this.pathLen = len;
this.pathLen = (len < 0 || len == Integer.MAX_VALUE) ? -1 : len;
this.extensionId = PKIXExtensions.BasicConstraints_Id;
this.critical = critical.booleanValue();
encodeThis();
Expand Down Expand Up @@ -140,6 +140,11 @@ public BasicConstraintsExtension(Boolean critical, Object value)
if (opt.tag != DerValue.tag_Integer) {
throw new IOException("Invalid encoding of BasicConstraints");
}

if (opt.getInteger() < 0) {
throw new IOException("Invalid encoding of BasicConstraints: " +
"pathLenConstraint cannot be negative");
}
this.pathLen = opt.getInteger();
/*
* Activate this check once again after PKIX profiling
Expand All @@ -158,9 +163,7 @@ public BasicConstraintsExtension(Boolean critical, Object value)
*/
public String toString() {
String pathLenAsString;
if (pathLen < 0) {
pathLenAsString = " undefined";
} else if (pathLen == Integer.MAX_VALUE) {
if (pathLen < 0 || pathLen == Integer.MAX_VALUE) {
pathLenAsString = " no limit";
} else {
pathLenAsString = String.valueOf(pathLen);
Expand All @@ -180,7 +183,6 @@ public String toString() {
public void encode(DerOutputStream out) {
if (extensionValue == null) {
this.extensionId = PKIXExtensions.BasicConstraints_Id;
critical = ca;
encodeThis();
}
super.encode(out);
Expand All @@ -191,7 +193,7 @@ public boolean isCa() {
}

public int getPathLen() {
return pathLen;
return (pathLen < 0) ? Integer.MAX_VALUE : Integer.valueOf(pathLen);
}

/**
Expand Down