<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">On 2/9/2021 9:02 PM, Weijun Wang wrote:<br>
</div>
<blockquote type="cite"
cite="mid:UdF4adB35kKMC5qDX5wfzcQRG5w9nDXb4WooETEO8c0=.0c17442f-3649-46e6-a647-0807a410d925@github.com">
<pre class="moz-quote-pre" wrap="">On Wed, 10 Feb 2021 01:39:15 GMT, Weijun Wang <a class="moz-txt-link-rfc2396E" href="mailto:weijun@openjdk.org"><weijun@openjdk.org></a> wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Print out "no limit" instead. This is the words RFC 5280 uses: "Where pathLenConstraint does not appear, no limit is imposed".
No regression test. Trivial.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
This pull request has now been integrated.
Changeset: 4619f372
Author: Weijun Wang <a class="moz-txt-link-rfc2396E" href="mailto:weijun@openjdk.org"><weijun@openjdk.org></a>
URL: <a class="moz-txt-link-freetext" href="https://git.openjdk.java.net/jdk/commit/4619f372">https://git.openjdk.java.net/jdk/commit/4619f372</a>
Stats: 11 lines in 1 file changed: 8 ins; 1 del; 2 mod
8261472: BasicConstraintsExtension::toString shows "PathLen:2147483647" if there is no pathLenConstraint
Reviewed-by: jnimeh
-------------
PR: <a class="moz-txt-link-freetext" href="https://git.openjdk.java.net/jdk/pull/2493">https://git.openjdk.java.net/jdk/pull/2493</a>
</pre>
</blockquote>
<p><br>
</p>
<p>Sorry - not quite right, it's not quite that trivial a fix.</p>
<p>The definition for BasicConstraints is</p>
<p>
<blockquote type="cite">
<pre class="newpage">BasicConstraints ::= SEQUENCE {
cA BOOLEAN DEFAULT FALSE,
pathLenConstraint INTEGER (0..MAX) OPTIONAL }</pre>
</blockquote>
</p>
<p>If pathLenConstraint is not present, then the path length is
infinite. The flag value for that looks like it was encoded as
both any negative number (and set to -1 to start) and
Integer.MAX_VALUE. I'm not quite sure why it was done that way,
but there's a problem doing that - actually a bunch of them.</p>
<p>You really ought to get the same encoding coming and going (e.g.
creating an object from DER should encode back to the exact same
DER). The current code does not do that.<br>
</p>
<p>1) It's not valid to encode or decode pathLenConstraint in the
DER as a negative number. This isn't a problem for encoding, but
the BasicConstraintsException(Boolean critical, Object value)
needs a value check after line 157 to make sure that the decoded
pathLengthConstraint field is positive - i'd throw an IOException
on failure. I'd also change line 149 to just return - the
initial value of pathLen is -1 and that's an appropriate flag for
a missing field.</p>
<p>2) I'm not sure why the set/get methods were added. I think it
was to provide access for the PathValidation stuff? Given that
they are present, I'd insert a line after line 222 (set) : "if
(pathLen < 0) pathLen = -1;" // treat any negative value as
unconstrained path length<br>
</p>
<p>3) And since the only place pathLen is available externally is in
the get method, I'd change line 237 to "return (pathLen < 0) ?
Integer.MAX_VALUE : Integer.valueOf(pathLen);" I think this is
more correct than returning -1.<br>
</p>
<p>4) And to fix the problem that led to this discussion, change
line 176 to 'pathLenAsString = " unconstrained"' and delete lines
177-178. E.g. there's no such thing as undefined here - both a
negative number and MAX_VALUE mean unconstrained length in the
previous version of the code.<br>
</p>
<p>5) One more - in the other constructor, change line 108 to
"this.pathLen = (len < 0 || len == Integer.MAX_VALUE) ? -1 :
len;"<br>
</p>
<p>6) *sigh* Delete lines 197-201. I have no idea why they are
overriding the specified value of critical based on whether ca is
true or not, but it's wrong. Conversely, the call to the
constructor at line 95 is correct.<br>
</p>
Mike<br>
<p><br>
</p>
</body>
</html>