294554 - unexported api calls in p12.h

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Jason Reid]

The following api calls in p12.h are not found in a .def file.

SEC_PKCS12CreatePubKeyEncryptedSafe NOT exported
SEC_PKCS12AddPublicKeyIntegrity NOT exported
SEC_PKCS12AddKeyForCert NOT exported
SEC_PKCS12AddDERCertAndEncryptedKey NOT exported
SEC_PKCS12CreateNestedSafeContents NOT exported

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

So, we need to decide:
a) are these functions intended to be "public"? 

then if so, we need to add them to the relevant .def file,
and if not, we need to move these declarations to another file, or 
enclose them in an #ifdef with some symbol like PKCS12_private.

Comment 2

[Julien Pierre]

Retargetting all P2s to 3.11.3 .

Comment 3

[Neil Williams]

Attachment: move non-exported pkcs12 functions to p12local.h

Moved declarations of 7 PKCS12 functions from distributed .h files into p12local.h. Added 3 function to smime.def since they pertain to testing PFX algorithms.

Comment 4

[Neil Williams]

(In reply to comment #3)
> Created an attachment (id=271306) [details]
> move non-exported pkcs12 functions to p12local.h
> 
> Moved declarations of 7 PKCS12 functions from distributed .h files into
> p12local.h. Added 3 function to smime.def since they pertain to testing PFX
> algorithms.
> 
This patch applies to bug # 294555 and bug # 294557 as well.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 271306 [details] [diff] [review]
move non-exported pkcs12 functions to p12local.h

SEC_PKCS12CreatePubKeyEncryptedSafe is dead code.
SEC_PKCS12AddPublicKeyIntegrity     is dead code.
SEC_PKCS12AddDERCertAndEncryptedKey is dead code.
SEC_PKCS12CreateNestedSafeContents  is dead code.

SEC_PKCS12AddKeyForCert is called from SEC_PKCS12AddCertAndKey, an exported 
public function.

We should get completely rid of dead code.  Don't just delete the declarations
from the public header files, but get rid of the functions themselves.
For the one other function, I suggest exporting it.  It's in use, so it is 
tested.  It was clearly intended to be part of the public API, so might as
well export it.

This patch also makes changes for other bugs.  I would prefer if separate
patches were made for each of those bugs.

Comment 6

[Neil Williams]

Comment on attachment 271306 [details] [diff] [review]
move non-exported pkcs12 functions to p12local.h

On further examination it makes better sense to patch each bug separately rather than lump them together in this patch.

Comment 7

[Neil Williams]

Bug 314331(In reply to comment #5)
> (From update of attachment 271306 [details] [diff] [review])
> SEC_PKCS12CreatePubKeyEncryptedSafe is dead code.
> SEC_PKCS12AddPublicKeyIntegrity     is dead code.

Let's call them infant code. They code looks reasonable and bug 314331 was opened about testing pub key encrypted p12 files and testing these routines would be part of that effort.

> SEC_PKCS12AddDERCertAndEncryptedKey is dead code.

I agree this one is dead.

> SEC_PKCS12CreateNestedSafeContents  is dead code.
> 
> SEC_PKCS12AddKeyForCert is called from SEC_PKCS12AddCertAndKey, an exported 
> public function.
> 
> ...
> This patch also makes changes for other bugs.  I would prefer if separate
> patches were made for each of those bugs.  
> 
Done. Partly because bug 294557 has 3 files worth of dead code, whereas, with this bug, I think we should leave the source for the pub key functions in and export them when I can get around to bug 314331.