Closed Bug 1413596 Opened 6 years ago Closed 6 years ago

export of RSA-PSS key to PKCS#12 drops the RSA-PSS identifier

Categories

(NSS :: Libraries, defect)

3.34
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hkario, Assigned: ueno)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20171006080222

Steps to reproduce:

mkdir nssdb
certutil -N -d sql:./nssdb --empty-password
pk12util -i attachment 8845944 [details] -d sql:./nssdb -W ''
pk12util -o file.p12 -d sql:./nssdb -W '' -n server
openssl pkcs12 -in file.p12 -passin pass: -nodes > decoded.pem
awk 'BEGIN { p = 0 } /BEGIN PRIVATE KEY/ { p = 1 } { if ( p == 1 ) print $0 } /END PRIVATE KEY/ { p = 0 }' < decoded.pem > private.pem
openssl asn1parse -in private.pem -inform pem


Actual results:

    0:d=0  hl=4 l=1212 cons: SEQUENCE          
    4:d=1  hl=2 l=   1 prim: INTEGER           :00
    7:d=1  hl=2 l=  13 cons: SEQUENCE          
    9:d=2  hl=2 l=   9 prim: OBJECT            :rsaEncryption
   20:d=2  hl=2 l=   0 prim: NULL 


Expected results:

    0:d=0  hl=4 l=1210 cons: SEQUENCE          
    4:d=1  hl=2 l=   1 prim: INTEGER           :00
    7:d=1  hl=2 l=  11 cons: SEQUENCE          
    9:d=2  hl=2 l=   9 prim: OBJECT            :rsassaPss
Blocks: 158750
The RSA-PSS private-key information is lost when importing a key.  I can think of two options to deal with this:

- record the private-key information when importing
- derive the private-key information from the matching certificate, when exporting

In either case, it would require changes to the softoken's C_WrapKey() and C_UnwrapKey() implementation.

As a starting point, I have filed a patch in the former approach:
https://phabricator.services.mozilla.com/D198
(In reply to Daiki Ueno [:ueno] from comment #1)
> As a starting point, I have filed a patch in the former approach:
> https://phabricator.services.mozilla.com/D198

Bob, it has been more than one month, and I don't see any review feedback from you on that link.

Bob, just checking, do you receive the review request email from phabricator?
Assignee: nobody → dueno
Flags: needinfo?(rrelyea)
I don't have any access to phabricator. It's a long standing complaint I have with Mozilla. Until I do, you'll have to use splinter if you need my feedback.
Flags: needinfo?(rrelyea)
OK, I am attaching the patch here.
Attachment #8939511 - Flags: review?(rrelyea)
Comment on attachment 8939511 [details] [diff] [review]
cka-public-key-info.patch

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

There are no functional errors, I did point out a few things I'd like fixed. You can fix the comments and the switch-> if statement without getting a new review from me. If you decide to move the SUBJECTPublicKey Template to nssutil, that should get reviewed. It's probably the right long term thing to do, but it could be tricky.

::: lib/softoken/lowkey.c
@@ +52,5 @@
> +      SEC_ASN1_SUB(SECOID_AlgorithmIDTemplate) },
> +    { SEC_ASN1_BIT_STRING,
> +      offsetof(NSSLOWKEYSubjectPublicKeyInfo, subjectPublicKey) },
> +    { 0 }
> +};

It's preferable to have a single copy of these templates, so if we need it in softoken, we should probably move it to nssutil.

::: lib/softoken/lowkeyti.h
@@ +25,5 @@
>  
>  extern const SEC_ASN1Template nsslowkey_PrivateKeyInfoTemplate[];
>  extern const SEC_ASN1Template nsslowkey_EncryptedPrivateKeyInfoTemplate[];
> +extern const SEC_ASN1Template nsslowkey_SubjectPublicKeyInfoTemplate[];
> +extern const SEC_ASN1Template nsslowkey_RSAPublicKeyTemplate[];

It's preferable to have a single copy of these templates, so if we need it in softoken, we should probably move it to nssutil.

::: lib/softoken/pkcs11c.c
@@ +5324,5 @@
>              prepare_low_rsa_priv_key_for_asn1(lk);
>              dummy = SEC_ASN1EncodeItem(arena, &pki->privateKey, lk,
>                                         nsslowkey_RSAPrivateKeyTemplate);
> +
> +            /* Prefer the value of CKA_PUBLIC_KEY_INFO, if present */

Comment should be 'determine RSA key type from the CKA_PUBLIC_KEY_INFO if present' 'the value' is to ambiguous.

@@ +5365,5 @@
> +                    *crvp = CKR_HOST_MEMORY;
> +                    goto loser;
> +                }
> +                sftk_FreeAttribute(attribute);
> +            } else {

Add comment: default to PKCS 1 .

@@ +5850,5 @@
> +    if (crv != CKR_OK) {
> +        goto loser;
> +    }
> +
> +    switch (SECOID_GetAlgorithmTag(&pki->algorithm)) {

Why is this a switch and not just an if?

::: lib/softoken/sftkdb.c
@@ -1592,1 @@
>  };

Note: this code doesn't help dbm, I think that's OK, but it needs to be called out. PSS keys will encode as PKCS #1 keys if they are stored in dbm: databases.
Attachment #8939511 - Flags: review?(rrelyea) → review+
Comment on attachment 8957084 [details]
Bug 1413596, Preserve private-key info in PKCS#8 when wrapping

Robert Relyea has approved the revision.

https://phabricator.services.mozilla.com/D198
Attachment #8957084 - Flags: review+
Landed as:
https://hg.mozilla.org/projects/nss/rev/d7331353ee37
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.37
You need to log in before you can comment on or make changes to this bug.