Closed Bug 1352064 Opened 3 years ago Closed 3 years ago

certutil --extNC includes an explicit encoding for minimum value zero

Categories

(NSS :: Tools, enhancement)

3.31
enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

Attachments

(1 file)

When using certutil --extNC to create a name constraints extension, the encoding looks like this:

  5  12:     SEQUENCE {
  7   7:       [2] '.abc.de'
 16   1:       [0] 00
       :       }
 19  12:     SEQUENCE {
 21   7:       [2] '.fgh.ij'
 30   1:       [0] 00
       :       }

In bug 1349705 comment 16 Ryan says this is an invalid encoding.


Function AddNameConstraints in cmd/certutil/certext.c explicitly encodes the minimum value zero.

I've traced the changelogs, this code was already present in Bob's "Initial NSS open source checkin" from year 2000.


Looking at RFC 5280 section 4.2.1.10:

  "Within this profile, the minimum and maximum fields are not used with
   any name forms, thus, the minimum MUST be zero, and maximum MUST be
   absent."

If the minimum field is required to be absent, then why didn't the document say so? As it is written, in my opinion the meaning is:
  "minimum field is allowed to be present".

Maybe that ambiguity had resulted in the original code to always encode it.


However, Ryan suggests that encoding a default value is invalid.

If that's true, then we should fix NSS.
Attached patch 1352064-v1.patchSplinter Review
Attachment #8852904 - Flags: review?(rrelyea)
Assignee: nobody → kaie
Because X.690 specifies the behavior I described, which is specific to DER. The text is explicit about maximum because it is merely OPTIONAL with no DEFAULT, while minimum, having a DEFAULT, is already implicitly omitted.
Comment on attachment 8852904 [details] [diff] [review]
1352064-v1.patch

Review of attachment 8852904 [details] [diff] [review]:
-----------------------------------------------------------------

r+

Whether invalid or not, the encoding should follow the accepted conventions.

Since it is DER encoding, I have more confidence that it is in fact invalid. DER requires a single possible encoding for any given value (at least that's the goal). That would been if the field in optional with a default meaning, that meaning should never be explicitly encoded.
Attachment #8852904 - Flags: review?(rrelyea) → review+
https://hg.mozilla.org/projects/nss/rev/1feb89a254de8d4ecd669b2c7bf80ff26784aadf
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.31
You need to log in before you can comment on or make changes to this bug.