Closed Bug 519255 Opened 13 years ago Closed 13 years ago

Add the PK11_ExportDERPrivateKeyInfo function

Categories

(NSS :: Libraries, enhancement, P2)

3.12
enhancement

Tracking

(Not tracked)

RESOLVED FIXED
3.16.1

People

(Reporter: wtc, Assigned: wtc)

Details

Attachments

(2 files, 4 obsolete files)

Attached patch Proposed patch (obsolete) — Splinter Review
This bug report is FYI only.  You don't need to review
the patch or comment on it.

The attached patch implements a function for exporting
a private key in the PrivateKeyInfo format.  I submitted
the patch in the hope that it could be useful to someone.
May I infer from this that someone at Google believes that exporting 
private keys in the clear is in the security interests of its users?
This was an intellectual exercise for me.  I saw an
unimplemented function PK11_ExportPrivateKeyInfo in
our API.  I was curious as to how much work it would
take to implement it, so I spent a couple hours of a
Friday night to implement it.  The patch works and
has no memory leaks under valgrind.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
Attached patch Proposed patch v2 (obsolete) — Splinter Review
The previous patch has a bug: the "version" local variable in
PK11_ExportPrivKeyInfo should be static.

In this patch, I fixed that bug and some coding style issues,
and updated the patch to the current NSS trunk.
Attachment #403292 - Attachment is obsolete: true
Attached patch Proposed patch v2.1 (obsolete) — Splinter Review
Removed a space at the end of a line.
Attachment #8393861 - Attachment is obsolete: true
Comment on attachment 8393865 [details] [diff] [review]
Proposed patch v2.1

Review of attachment 8393865 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/pk11wrap/pk11pk12.c
@@ +542,5 @@
> +{
> +    /* PrivateKeyInfo version (always zero) */
> +    static const unsigned char pkiVersion = 0;
> +    /* RSAPrivateKey version (always zero) */
> +    static const unsigned char rsaVersion = 0;

Note that rsaVersion doesn't need to be static. Only
pkiVersion needs to be static because the returned |pki|
points to it.
Attached patch Proposed patch v3 (obsolete) — Splinter Review
This patch was reviewed at https://codereview.chromium.org/205343004.

Richard: do you need this function?

The current NSS policy is to only export EncryptedPrivateKeyInfo.
If you need to export unencrypted PrivateKeyInfo for WebCrypto, I'll
check in this patch.
Attachment #8393865 - Attachment is obsolete: true
Attachment #8398713 - Flags: feedback?(rlb)
Attachment #8398713 - Attachment is obsolete: true
Attachment #8398713 - Flags: feedback?(rlb)
Attachment #8398718 - Flags: feedback?(rlb)
Yes, there is a need for exporting unencrypted PrivateKeyInfo for WebCrypto.  

I would note that this doesn't really change the capabilities that NSS exposes, since you could do basically the same thing outside of NSS.  But just being able to call PK11_ExportDERPrivateKeyInfo would be easier.
Attachment #8398718 - Flags: feedback?(rlb) → feedback+
Comment on attachment 8398718 [details] [diff] [review]
Proposed patch v3 (PK11_ReadAttribute comment change excluded)

Patch checked in: https://hg.mozilla.org/projects/nss/rev/a751a5146718
Attachment #8398718 - Flags: checked-in+
Priority: -- → P2
Resolution: WONTFIX → FIXED
Target Milestone: --- → 3.16.1
You need to log in before you can comment on or make changes to this bug.