Open
Bug 1064337
Opened 10 years ago
Updated 1 year ago
certutil fails to create crl distribution point extension
Categories
(NSS :: Tools, defect, P5)
Tracking
(Not tracked)
NEW
People
(Reporter: KaiE, Assigned: KaiE)
Details
Attachments
(1 file)
1.74 KB,
patch
|
rrelyea
:
review-
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee: nobody → kaie
Attachment #8485771 -
Flags: review?(rrelyea)
Assignee | ||
Comment 2•10 years ago
|
||
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".
Comment 4•10 years ago
|
||
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-
Comment 5•9 years ago
|
||
clearing need info, I effective answered kaie's question with the review in comment 4
Flags: needinfo?(rrelyea)
Updated•2 years ago
|
Severity: normal → S3
Updated•1 year ago
|
Severity: S3 → S4
Priority: -- → P5
You need to log in
before you can comment on or make changes to this bug.
Description
•