Closed Bug 1346239 Opened 7 years ago Closed 6 years ago

pk12util can't import RSA-PSS certificate+key file to NSS databse

Categories

(NSS :: Tools, defect, P3)

3.28.2

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hkario, Assigned: ueno)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

Attached file rsa-pss certificate
Importing a PKCS#12 file with RSA-PSS certificate and key pair to NSS database fails:

Reproducer:
mkdir nssdb
certutil -N --empty-password -d sql:./nssdb/
pk12util -i server.p12 -d sql:./nssdb -W ''

Result:
pk12util: PKCS12 decode import bags failed: SEC_ERROR_PKCS12_UNABLE_TO_IMPORT_KEY: Unable to import.  Error attempting to import private key.

Expected:
certificate and key imported to database, available for use

Additional info:
The file is correctly parsed by pk12util -l:
pk12util -l server.p12 -W '' -v
Certificate(has private key):
    Data:
        Version: 3 (0x2)
        Serial Number: 1 (0x1)
        Signature Algorithm: PKCS #1 RSA-PSS Signature
            Parameters:
                Hash algorithm: SHA-256
                Mask algorithm: PKCS #1 MGF1 Mask Generation Function
                Mask hash algorithm: SHA-256
                Salt Length: 222 (0xde)
        Issuer: "CN=CA"
        Validity:
            Not Before: Fri Mar 10 15:08:10 2017
            Not After : Sun Jan 17 15:08:10 2027
        Subject: "CN=localhost"
        Subject Public Key Info:
            Public Key Algorithm: PKCS #1 RSA-PSS Signature
                Parameters:
                    Invalid RSA-PSS parameters
            RSA Public Key:
                Modulus:
                    e5:ce:c8:77:f0:ca:ea:b3:1d:dc:74:e6:1f:14:2a:d0:
                    92:fa:cd:fe:10:cf:04:fc:e8:d5:ee:3e:43:66:e2:ba:
                    f2:15:79:b6:2d:4e:27:1d:2a:89:40:72:e1:2f:12:7d:
                    91:a8:e5:6b:72:6e:70:56:17:64:b6:5a:c3:18:41:c7:
                    9d:aa:2b:f9:0e:a1:8d:18:a7:41:c1:53:7a:3f:8b:d3:
                    e2:84:50:73:8b:52:67:82:1c:09:86:63:00:12:39:07:
                    0b:1d:18:eb:32:4a:9c:5d:98:d1:28:40:a3:5d:6f:bb:
                    bf:a5:3d:39:e8:77:69:c8:2e:27:ea:c4:0e:9b:14:f8:
                    bc:2b:b8:b8:bf:16:76:f6:25:50:89:b1:2a:c7:33:9e:
                    62:f3:fa:64:df:2a:ba:7c:4d:08:6c:ff:fd:6c:5e:1f:
                    ae:34:b0:ff:60:06:72:d8:29:2f:2b:4e:75:ba:26:36:
                    8b:1f:a8:61:a6:1e:fc:12:d0:5c:bd:fc:c7:16:7a:49:
                    c2:9d:c5:6a:bd:11:32:fc:86:a3:a4:85:ac:2e:af:b6:
                    de:99:23:46:05:f4:09:1b:dc:37:df:bb:ca:96:e1:7e:
                    f6:b2:04:45:03:21:05:a4:cf:45:62:16:16:35:c0:08:
                    fa:99:29:23:96:5f:62:e2:02:74:dd:6a:ce:46:c8:7f
                Exponent: 65537 (0x10001)
        Signed Extensions:
            Name: Certificate Basic Constraints
            Data: Is not a CA.

            Name: Certificate Comment
            Comment: "OpenSSL Generated Certificate"

            Name: Certificate Subject Key ID
            Data:
                23:18:28:13:d8:87:23:04:5e:15:0b:39:98:ca:ca:d5:
                20:43:d2:c1

            Name: Certificate Authority Key Identifier
            Key ID:
                66:1b:21:28:fc:c8:35:71:09:56:15:5d:74:93:0a:30:
                d6:84:0e:c6

    Signature Algorithm: PKCS #1 RSA-PSS Signature
        Parameters:
            Hash algorithm: SHA-256
            Mask algorithm: PKCS #1 MGF1 Mask Generation Function
            Mask hash algorithm: SHA-256
            Salt Length: 222 (0xde)
    Signature:
        40:bb:98:7f:8a:98:ad:03:58:b0:6e:c9:15:c4:d8:ad:
        8e:73:87:55:e3:ba:d8:c5:df:de:ef:94:23:59:b8:9e:
        8b:98:5b:13:af:b2:20:72:16:58:87:01:f3:d9:5c:df:
        3d:17:8c:87:89:b2:6d:9c:77:40:30:1a:22:80:f3:f2:
        40:6c:60:2f:39:59:d2:dc:db:fd:a1:bd:3c:d1:f9:17:
        9a:b2:b1:85:fe:62:50:cc:91:c1:34:de:c2:45:33:d8:
        ef:7e:60:67:9d:e6:9a:e2:a9:4d:9b:ef:80:43:9c:5f:
        70:32:1f:b3:56:3a:9f:e1:66:75:3b:7d:7b:8f:e6:4e:
        e6:1f:f5:ce:e4:54:7c:e4:c7:fb:ec:85:b8:fa:68:b0:
        f6:b8:dc:0a:53:b4:f0:91:bd:74:22:c3:d5:a2:ef:50:
        62:44:06:c0:d7:ab:e3:4f:dd:72:ae:b1:1c:3d:bb:e2:
        34:af:51:ef:15:30:7c:4c:ff:54:6a:f5:81:7c:21:d6:
        c8:95:8d:07:2d:a6:88:81:39:ce:7e:a3:02:5f:77:48:
        ad:36:b6:0e:8f:2f:ad:0d:a2:56:cb:36:32:2a:51:13:
        05:49:29:d3:59:35:51:41:4c:8d:0a:2e:7f:17:34:68:
        b6:a0:09:d2:20:52:4c:c6:b8:c3:82:b7:a7:0b:df:ae
    Fingerprint (SHA-256):
        AA:51:B8:88:42:B9:8B:D2:33:43:34:EB:8C:32:6B:E6:5B:6A:17:55:1A:65:B8:94:89:3B:2B:85:58:53:62:E5
    Fingerprint (SHA1):
        F4:71:37:37:3A:36:06:5C:56:DA:56:D9:A7:F0:BB:40:45:0E:0E:B3

    Friendly Name: server

Key(shrouded):
    Friendly Name: server

    Encryption algorithm: PKCS #12 V2 PBE With SHA-1 And 3KEY Triple DES-CBC
        Parameters:
            Salt:
                25:17:5c:2a:fb:8b:58:4f
            Iteration Count: 2048 (0x800)
This is failing in C_UnwrapKey, when RSAPSS is used as an Algorithm Identifier in PKCS#8 PrivateKeyInfo.  The attached patch should fix the issue.
Attachment #8846562 - Flags: review?(kaie)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 8846562 [details] [diff] [review]
pss-unwrap-key.patch

Your patch seems trivial, and you probably have tested that it works, so r=kaie

But I cannot say if your patch is correct. Let's get an additional opinion from Bob, just to give him a chance to comment, if he knows additional things that should be considered.
Attachment #8846562 - Flags: review?(rrelyea)
Attachment #8846562 - Flags: review?(kaie)
Attachment #8846562 - Flags: review+
Assignee: nobody → dueno
(In reply to Kai Engert (:kaie) from comment #2)
> Your patch seems trivial, and you probably have tested that it works, so
> r=kaie

We have tests for pk12util in the tools.sh script, why not add a few more? We really shouldn't be landing code without tests in 2017.
Comment on attachment 8846562 [details] [diff] [review]
pss-unwrap-key.patch

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

Sigh, so to answer Hubert's question. There is no PKCS #11 difference between RSA_PSS and RSA. The keys are the same. This patch is because there is a different tag inside the wrapped PKCS #8 data that PKCS #12 uses. Unfortunately softoken has to parse it because it's only available once the key has been unwrapped.

As you can see from the switch statement, PKCS #11 doesn't treat it any differently than regular RSA.

Oh and r+ for the patch.
Attachment #8846562 - Flags: review?(rrelyea) → review+
(In reply to Tim Taubert [:ttaubert] from comment #3)
> (In reply to Kai Engert (:kaie) from comment #2)
> > Your patch seems trivial, and you probably have tested that it works, so
> > r=kaie
> 
> We have tests for pk12util in the tools.sh script, why not add a few more?
> We really shouldn't be landing code without tests in 2017.

So this gets just ignored? Does any of you have an opinion about the value of tests they want to share? Why do we still land code without tests?
I agree that it should have had tests, sorry for not including them in the patch.

On the other hand, I am not sure if there is a way to create a regular RSA-PSS certificate for testing this, at the moment.
Hubert, do you have any idea?  Or should bug 1341306 be addressed first?
The question is just about being able to import a PKCS#12 file with rsa-pss keys, so I'd say just using the example file I provided earlier, and the one attached above would be enough for the test.
the signature algorithm parameters may have default (implicit) values in the Algorithm Identifier Parameters structure

That's an example file that does that for the minimum salt length
(In reply to Hubert Kario from comment #8)
> Created attachment 8847563 [details]
> PKCS#12 file with rsa-pss public key with parameter restrictions
> 
> The question is just about being able to import a PKCS#12 file with rsa-pss
> keys, so I'd say just using the example file I provided earlier, and the one
> attached above would be enough for the test.

Thank you, I will try to add tests using them.  Can I have the actual openssl commands(?) to reproduce those files for the record?
To create rsa-pss key without restrictions:
openssl req -x509 -newkey rsa-pss -keyout ca.key -out ca.crt -subj /CN=CA -nodes -batch -config /etc/pki/tls/openssl.cnf -pkeyopt rsa_keygen_bits:2048 -sha256

To create rsa-pss key with fully-specified restrictions:
openssl req -x509 -newkey rsa-pss -keyout ca.key -out ca.crt -subj /CN=CA -nodes -batch -config /etc/pki/tls/openssl.cnf -pkeyopt rsa_keygen_bits:2048 -pkeyopt rsa_pss_keygen_md:sha256 -pkeyopt rsa_pss_keygen_mgf1_md:sha256 -pkeyopt rsa_pss_keygen_saltlen:32 -sha256

To create rsa-pss key with some restrictions (salt in this case) set to default values:
openssl req -x509 -newkey rsa-pss -keyout ca.key -out ca.crt -subj /CN=CA -nodes -batch -config /etc/pki/tls/openssl.cnf -pkeyopt rsa_keygen_bits:2048 -pkeyopt rsa_pss_keygen_md:sha256 -pkeyopt rsa_pss_keygen_saltlen:20 -sha256

Then to convert either of the above to a PKCS#12 file:
openssl pkcs12 -export -passout pass: -out ca.p12 -inkey ca.key -in ca.crt -name ca

(Note that this requires at least OpenSSL 1.1.1 to work)
Attachment #8847675 - Flags: review?(ttaubert)
Comment on attachment 8847675 [details] [diff] [review]
pkcs12-pss-tests.patch

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

Thanks!
Attachment #8847675 - Flags: review?(ttaubert) → review+
This fix would be ideally required by Red Hat by NSS 3.33
Daiki, can you please summarize the status of this bug?

Should this patch from March have been checked in?

What is left to be done?
Flags: needinfo?(dueno)
I filed a separate bug 1400844, because the other bugs, e.g. bug 1341306, are the same cause.
Flags: needinfo?(dueno)
Depends on: 1400844
Seems like it is fixed in 3.34 beta, but export mangles the private key (saves as rsaEncryption, not rsassaPss).

Would you prefer to handle it in this bug or a new bug for export support?
Flags: needinfo?(dueno)
I've filed bug 1413596 to track this
Flags: needinfo?(dueno)
I'm closing this, as the export problem is tracked in a separate bug.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: 3.31 → 3.34
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: