Last Comment Bug 419523 - Export Cert_NewTempCertificate.
: Export Cert_NewTempCertificate.
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: 3.12
Assigned To: Wan-Teh Chang
Depends on:
  Show dependency treegraph
Reported: 2008-02-25 13:42 PST by Wan-Teh Chang
Modified: 2008-03-12 20:54 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

Proposed patch (10.14 KB, patch)
2008-02-26 19:16 PST, Wan-Teh Chang
rrelyea: review+
Details | Diff | Splinter Review
Supplemental patch (829 bytes, patch)
2008-03-09 17:20 PDT, Wan-Teh Chang
rrelyea: review+
Details | Diff | Splinter Review
JSS patch (962 bytes, patch)
2008-03-09 17:22 PDT, Wan-Teh Chang
glenbeasley: review+
Details | Diff | Splinter Review
PSM patch (730 bytes, patch)
2008-03-09 17:27 PDT, Wan-Teh Chang
kaie: review+
Details | Diff | Splinter Review

Description Wan-Teh Chang 2008-02-25 13:42:55 PST
We should export Cert_NewTempCertificate from nss.def, and remove it
from nssrenam.h.

For backward compatibility, we need to continue to export the
__CERT_NewTempCertificate symbol.
Comment 1 Wan-Teh Chang 2008-02-26 19:16:16 PST
Created attachment 305909 [details] [diff] [review]
Proposed patch

1. Add CERT_NewTempCertificate to the export list in nss.def.

2. Make CERT_NewTempCertificate the primary function and have
__CERT_NewTempCertificate call it, rather than the other way around.

3. Remove the renaming of CERT_NewTempCertificate from nssrenam.h.
Also remove the renaming of four other functions that are already
exported (for JSS).

3. Remove inclusions of "nssrenam.h" that are no longer needed.
Comment 2 Robert Relyea 2008-03-03 15:34:16 PST
Comment on attachment 305909 [details] [diff] [review]
Proposed patch

r+ rrelyea
Comment 3 Wan-Teh Chang 2008-03-09 17:03:11 PDT
Comment on attachment 305909 [details] [diff] [review]
Proposed patch

I checked in the patch on the NSS trunk for NSS 3.12.
Comment 4 Wan-Teh Chang 2008-03-09 17:20:36 PDT
Created attachment 308342 [details] [diff] [review]
Supplemental patch

This patch finishes the job for NSS.  It makes PK11_GetKeyData
rather than __PK11_GetKeyData the primary function.
Comment 5 Wan-Teh Chang 2008-03-09 17:22:09 PDT
Created attachment 308343 [details] [diff] [review]
JSS patch

Remove manual declarations of three PBE functions.  They are
declared in secpkcs5.h, which this JSS file already includes.
Comment 6 Wan-Teh Chang 2008-03-09 17:27:01 PDT
Created attachment 308346 [details] [diff] [review]
PSM patch

This PSM patch must be checked in after we update the
NSS tag in mozilla/

CERT_NewTempCertificate is just officially exported.  So
no need to use the __CERT_NewTempCertificate symbol now.
Comment 7 Nelson Bolyard (seldom reads bugmail) 2008-03-10 00:27:55 PDT
(In reply to comment #4)

> This patch finishes the job for NSS.  It makes PK11_GetKeyData
> rather than __PK11_GetKeyData the primary function.

Wan-Teh, what is "the job"?  
What does this change for PK11_GetKeyData have to do with the subject 
of this bug?  
Have we decided (somewhere, not recorded in this bug) that we wish to 
make PK11_GetKeyData also now be a supported public function?
Comment 8 Wan-Teh Chang 2008-03-10 10:30:24 PDT
By the "job", I meant not only the subject of this bug but also related problems
I found while working on the bug.

PK11_GetKeyData is already exported in nss.def.  It was exported in NSS 3.3
to make JSS work.
Comment 9 glen beasley 2008-03-10 11:16:18 PDT
Comment on attachment 308343 [details] [diff] [review]
JSS patch

agreed these manual declarations should not be duplicated in PK11KeyGenerator.c when they are declared in secpkcs5.h
Comment 10 Robert Relyea 2008-03-10 13:46:57 PDT
I'd certainly want to make sure appropriate caveats are included with PK11_GetKeyData when we export it. 

The function does not always work, and is guaranteed to fail on FIPS tokens. Users of this function should do some soul searching as to why they need it (it is not good hygene). Usually it means the application is trying to get it's hands to directly into crypto.

Wan-Teh is there actually callers (other than JSS) that use this function? It this point I would prefer to keep it private unless there is a massive need for it to be public.

Comment 11 Robert Relyea 2008-03-10 14:23:30 PDT
Comment on attachment 308342 [details] [diff] [review]
Supplemental patch

I'm going to r- pending a demonstrated need for this function.

Comment 12 Kai Engert (:kaie) (on vacation) 2008-03-10 14:32:43 PDT
Comment on attachment 308346 [details] [diff] [review]
PSM patch

Comment 13 Wan-Teh Chang 2008-03-10 15:33:13 PDT
Comment on attachment 308342 [details] [diff] [review]
Supplemental patch

Bob, we already export both __PK11_GetKeyData and
PK11_GetKeyData in nss.def.  The purpose of this
patch is not to export PK11_GetKeyData.  It is to
reverse which one is defined in terms of the other.

Outside NSS, PK11_GetKeyData is only used by JSS,
without renaming:
Comment 14 Robert Relyea 2008-03-12 11:26:49 PDT
Comment on attachment 308342 [details] [diff] [review]
Supplemental patch

r+ after wtc's explanation.

Comment 15 Wan-Teh Chang 2008-03-12 20:54:27 PDT
I checked in the (NSS) supplemental patch (attachment 308342 [details] [diff] [review])
on the NSS trunk for NSS 3.12.

Checking in pk11skey.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11skey.c,v  <--  pk11skey.c
new revision: 1.111; previous revision: 1.110

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