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

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Alicja Kario]

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

Comment 1

[Daiki Ueno [:ueno]]

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

Comment 2

[Kai Engert [:KaiE:]]

(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?

Comment 3

[Robert Relyea]

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.

Comment 4

[Daiki Ueno [:ueno]]

Attachment: cka-public-key-info.patch

OK, I am attaching the patch here.

Comment 5

[Robert Relyea]

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.

Comment 6

[Phabricator Automation]

Attachment: Bug 1413596, Preserve private-key info in PKCS#8 when wrapping

Comment 7

[Phabricator Automation]

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

Comment 8

[Daiki Ueno [:ueno]]

Landed as:
https://hg.mozilla.org/projects/nss/rev/d7331353ee37