Last Comment Bug 430916 - add sustaining asserts
: add sustaining asserts
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: All All
: P2 normal (vote)
: 3.12.1
Assigned To: Julien Pierre
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-04-25 21:38 PDT by Julien Pierre
Modified: 2008-05-01 18:27 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
assert on SEC_ERROR_REUSED_ISSUER_AND_SERIAL_NUMBER (508 bytes, patch)
2008-04-25 21:40 PDT, Julien Pierre
nelson: review+
Details | Diff | Review

Description Julien Pierre 2008-04-25 21:38:49 PDT
Normally as a rule, we try to only assert on coding errors, not bad external data. I feel however that there are some exceptions for certain particularly egregious errors, in the case of reused issuer and serial number. Often with some products, the errors are not properly reported all the way to the end user. And we can spend a lot of time debugging this kind of problem for no good reason.
I have attached a patch that is #ifdef'ed just for me. But if you agree, I would prefer to have that without ifdef.
Comment 1 Julien Pierre 2008-04-25 21:40:46 PDT
Created attachment 317835 [details] [diff] [review]
assert on SEC_ERROR_REUSED_ISSUER_AND_SERIAL_NUMBER

Nelson,

Please let me know also if you are in favor of having this non-ifdef'ed.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2008-04-26 15:19:29 PDT
Comment on attachment 317835 [details] [diff] [review]
assert on SEC_ERROR_REUSED_ISSUER_AND_SERIAL_NUMBER

Julien, I'm sure it would not be appropriate to put that assertion into
Mozilla browsers.  Personally, I would not want to encounter it in NSS tools, 
either.  So, I do not favor putting THIS assertion in unconditionally,
but I'm not opposed to it in its present ifdef'ed form.

Is there some particular product or family of products you have in mind
that fails to report this error?  
Can we detect that NSS is being used by those products?
Comment 3 Julien Pierre 2008-04-27 23:51:52 PDT
Nelson,

Our own pk12util still fails to report proper errors. Somebody sent me a p12 file with 2 certs with same issuer & serial in it. All I got was a generic error, and I had to trace the code to figure it out.

I suspect mozilla PKCS#12 import would do the same.

I thought about surrounding this with a getenv, but I think that would be way too expensive given how common PORT_SetError is.
Comment 4 Nelson Bolyard (seldom reads bugmail) 2008-04-28 10:58:45 PDT
In reply to comment 3, please file a bug against our PKCS#12 code about this.
I'd suggest filing it against the utility first, and then that can be changed
to be against the library in case it is found to be the fault of the library
code.
Comment 5 Julien Pierre 2008-04-28 16:50:54 PDT
Nelson,

I already did, over 7 years ago. See bug 72290 .
Comment 6 Julien Pierre 2008-05-01 18:27:28 PDT
Checking in secport.c;
/cvsroot/mozilla/security/nss/lib/util/secport.c,v  <--  secport.c
new revision: 1.22; previous revision: 1.21
done

Note You need to log in before you can comment on or make changes to this bug.