Open Bug 1064337 Opened 10 years ago Updated 1 year ago

certutil fails to create crl distribution point extension

Categories

(NSS :: Tools, defect, P5)

3.16.4

Tracking

(Not tracked)

People

(Reporter: KaiE, Assigned: KaiE)

Details

Attachments

(1 file)

Although certutil contains parameters and code to add a CRL distribution point extension, I've failed to use it. The code incorrectly checks the result of PORT_GetError() and aborts wrongfully.
Attached patch patch v1Splinter Review
Assignee: nobody → kaie
Attachment #8485771 - Flags: review?(rrelyea)
Using the attached patch, I was able to add an extension with a single general name (URI), and selecting "finish" in both additional questions that asked for "reason flag" and "crl issuer name".
Bob, do you have time to review this?
Flags: needinfo?(rrelyea)
Comment on attachment 8485771 [details] [diff] [review] patch v1 Review of attachment 8485771 [details] [diff] [review]: ----------------------------------------------------------------- r-. but the hard part and base idea of the patch is correct. ::: cmd/certutil/certext.c @@ +1169,5 @@ > current->distPointType = intValue; > current->distPoint.fullName = CreateGeneralName (arena); > + if (!current->distPoint.fullName) { > + rv = PORT_GetError(); > + } rv = SECFailure; not rv = PORT_GetError(); This is a bug in the original code. rv is a SECStatus (SECSuccess, SECFailure, SECWouldBlock). PORT_GetError() returns a SECError (lots of values, none of which are SECFailure). @@ +1230,3 @@ > current->crlIssuer = CreateGeneralName (arena); > + if (current->crlIssuer == NULL && PORT_GetError() != 0) { > + rv = SECFailure; Good, you fixed the same issue here. I actually don't think we want to do the PORT_GetError() here unless we need this code to execute and not fail if user does not select a valid general name type on the first call. I think we want to fail in that case as well, but I'll leave it to you. Did you have a case where you got '0' from PORT_GetError() but current->crlIssue was NULL?
Attachment #8485771 - Flags: review?(rrelyea) → review-
clearing need info, I effective answered kaie's question with the review in comment 4
Flags: needinfo?(rrelyea)
Severity: normal → S3
Severity: S3 → S4
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: