Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8261513

Various BasicConstraintsExtension issues

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: P4 P4
    • None
    • 25
    • security-libs
    • None

      The definition for BasicConstraints is

      BasicConstraints ::= SEQUENCE {
      cA BOOLEAN DEFAULT FALSE,
      pathLenConstraint INTEGER (0..MAX) OPTIONAL }

      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.? There's a bunch of problems.

      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.

      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.

      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

      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.

      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.

      5) One more - in the other constructor, change line 108 to "this.pathLen
      = (len < 0 || len == Integer.MAX_VALUE) ? -1 : len;"

      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.

            bperez Ben Perez
            weijun Weijun Wang
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: