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

Summary: Previously, NSS dropped PKCS #8 PrivateKeyInfo when importing a private key from a PKCS #12 file. This patch attaches the corresponding CKA_PUBLIC_KEY_INFO attribute to a private key when unwrapping it (see PKCS #11 v2.40 4.9). When wrapping it again, the attribute is restored in the encrypted PrivateKeyInfo. Reviewers: rrelyea Reviewed By: rrelyea Bug #: 1413596 Differential Revision: https://phabricator.services.mozilla.com/D198
sailfishos-mirror · Mar 8, 2018 · 9380371 · 9380371
1 parent 8b9313b
commit 9380371
Show file tree

Hide file tree

Showing 13 changed files with 179 additions and 7 deletions.
diff --git a/cmd/certutil/certutil.c b/cmd/certutil/certutil.c
@@ -845,7 +845,7 @@ SECItemToHex(const SECItem *item, char *dst)
 }
 
 static const char *const keyTypeName[] = {
-    "null", "rsa", "dsa", "fortezza", "dh", "kea", "ec"
+    "null", "rsa", "dsa", "fortezza", "dh", "kea", "ec", "rsaPss"
 };
 
 #define MAX_CKA_ID_BIN_LEN 20

diff --git a/lib/pk11wrap/pk11akey.c b/lib/pk11wrap/pk11akey.c
@@ -804,12 +804,30 @@ PK11_MakePrivKey(PK11SlotInfo *slot, KeyType keyType,
     /* don't know? look it up */
     if (keyType == nullKey) {
         CK_KEY_TYPE pk11Type = CKK_RSA;
+        SECItem info;
 
         pk11Type = PK11_ReadULongAttribute(slot, privID, CKA_KEY_TYPE);
         isTemp = (PRBool)!PK11_HasAttributeSet(slot, privID, CKA_TOKEN, PR_FALSE);
         switch (pk11Type) {
             case CKK_RSA:
                 keyType = rsaKey;
+                /* determine RSA key type from the CKA_PUBLIC_KEY_INFO if present */
+                rv = PK11_ReadAttribute(slot, privID, CKA_PUBLIC_KEY_INFO, NULL, &info);
+                if (rv == SECSuccess) {
+                    CERTSubjectPublicKeyInfo *spki;
+
+                    spki = SECKEY_DecodeDERSubjectPublicKeyInfo(&info);
+                    if (spki) {
+                        SECOidTag tag;
+
+                        tag = SECOID_GetAlgorithmTag(&spki->algorithm);
+                        if (tag == SEC_OID_PKCS1_RSA_PSS_SIGNATURE)
+                            keyType = rsaPssKey;
+                        SECKEY_DestroySubjectPublicKeyInfo(spki);
+                    }
+                    SECITEM_FreeItem(&info, PR_FALSE);
+                }
+
                 break;
             case CKK_DSA:
                 keyType = dsaKey;

diff --git a/lib/softoken/lowkey.c b/lib/softoken/lowkey.c
@@ -45,6 +45,23 @@ const SEC_ASN1Template nsslowkey_PrivateKeyInfoTemplate[] = {
     { 0 }
 };
 
+const SEC_ASN1Template nsslowkey_SubjectPublicKeyInfoTemplate[] = {
+    { SEC_ASN1_SEQUENCE, 0, NULL, sizeof(NSSLOWKEYSubjectPublicKeyInfo) },
+    { SEC_ASN1_INLINE | SEC_ASN1_XTRN,
+      offsetof(NSSLOWKEYSubjectPublicKeyInfo, algorithm),
+      SEC_ASN1_SUB(SECOID_AlgorithmIDTemplate) },
+    { SEC_ASN1_BIT_STRING,
+      offsetof(NSSLOWKEYSubjectPublicKeyInfo, subjectPublicKey) },
+    { 0 }
+};
+
+const SEC_ASN1Template nsslowkey_RSAPublicKeyTemplate[] = {
+    { SEC_ASN1_SEQUENCE, 0, NULL, sizeof(NSSLOWKEYPublicKey) },
+    { SEC_ASN1_INTEGER, offsetof(NSSLOWKEYPublicKey, u.rsa.modulus) },
+    { SEC_ASN1_INTEGER, offsetof(NSSLOWKEYPublicKey, u.rsa.publicExponent) },
+    { 0 }
+};
+
 const SEC_ASN1Template nsslowkey_PQGParamsTemplate[] = {
     { SEC_ASN1_SEQUENCE, 0, NULL, sizeof(PQGParams) },
     { SEC_ASN1_INTEGER, offsetof(PQGParams, prime) },
@@ -134,6 +151,13 @@ prepare_low_rsa_priv_key_for_asn1(NSSLOWKEYPrivateKey *key)
     key->u.rsa.coefficient.type = siUnsignedInteger;
 }
 
+void
+prepare_low_rsa_pub_key_for_asn1(NSSLOWKEYPublicKey *key)
+{
+    key->u.rsa.modulus.type = siUnsignedInteger;
+    key->u.rsa.publicExponent.type = siUnsignedInteger;
+}
+
 void
 prepare_low_pqg_params_for_asn1(PQGParams *params)
 {

diff --git a/lib/softoken/lowkeyi.h b/lib/softoken/lowkeyi.h
@@ -27,6 +27,7 @@ extern void prepare_low_dsa_priv_key_export_for_asn1(NSSLOWKEYPrivateKey *key);
 extern void prepare_low_dh_priv_key_for_asn1(NSSLOWKEYPrivateKey *key);
 extern void prepare_low_ec_priv_key_for_asn1(NSSLOWKEYPrivateKey *key);
 extern void prepare_low_ecparams_for_asn1(ECParams *params);
+extern void prepare_low_rsa_pub_key_for_asn1(NSSLOWKEYPublicKey *key);
 
 /*
 ** Destroy a private key object.

diff --git a/lib/softoken/lowkeyti.h b/lib/softoken/lowkeyti.h
@@ -25,6 +25,8 @@ extern const SEC_ASN1Template nsslowkey_ECPrivateKeyTemplate[];
 
 extern const SEC_ASN1Template nsslowkey_PrivateKeyInfoTemplate[];
 extern const SEC_ASN1Template nsslowkey_EncryptedPrivateKeyInfoTemplate[];
+extern const SEC_ASN1Template nsslowkey_SubjectPublicKeyInfoTemplate[];
+extern const SEC_ASN1Template nsslowkey_RSAPublicKeyTemplate[];
 
 /*
  * PKCS #8 attributes
@@ -48,6 +50,13 @@ struct NSSLOWKEYPrivateKeyInfoStr {
 typedef struct NSSLOWKEYPrivateKeyInfoStr NSSLOWKEYPrivateKeyInfo;
 #define NSSLOWKEY_PRIVATE_KEY_INFO_VERSION 0 /* what we *create* */
 
+struct NSSLOWKEYSubjectPublicKeyInfoStr {
+    PLArenaPool *arena;
+    SECAlgorithmID algorithm;
+    SECItem subjectPublicKey;
+};
+typedef struct NSSLOWKEYSubjectPublicKeyInfoStr NSSLOWKEYSubjectPublicKeyInfo;
+
 typedef enum {
     NSSLOWKEYNullKey = 0,
     NSSLOWKEYRSAKey = 1,

diff --git a/lib/softoken/pkcs11c.c b/lib/softoken/pkcs11c.c
@@ -5324,7 +5324,52 @@ sftk_PackagePrivateKey(SFTKObject *key, CK_RV *crvp)
             prepare_low_rsa_priv_key_for_asn1(lk);
             dummy = SEC_ASN1EncodeItem(arena, &pki->privateKey, lk,
                                        nsslowkey_RSAPrivateKeyTemplate);
-            algorithm = SEC_OID_PKCS1_RSA_ENCRYPTION;
+
+            /* determine RSA key type from the CKA_PUBLIC_KEY_INFO if present */
+            attribute = sftk_FindAttribute(key, CKA_PUBLIC_KEY_INFO);
+            if (attribute) {
+                NSSLOWKEYSubjectPublicKeyInfo *publicKeyInfo;
+                SECItem spki;
+
+                spki.data = attribute->attrib.pValue;
+                spki.len = attribute->attrib.ulValueLen;
+
+                publicKeyInfo = PORT_ArenaZAlloc(arena,
+                                                 sizeof(NSSLOWKEYSubjectPublicKeyInfo));
+                if (!publicKeyInfo) {
+                    sftk_FreeAttribute(attribute);
+                    *crvp = CKR_HOST_MEMORY;
+                    rv = SECFailure;
+                    goto loser;
+                }
+                rv = SEC_QuickDERDecodeItem(arena, publicKeyInfo,
+                                            nsslowkey_SubjectPublicKeyInfoTemplate,
+                                            &spki);
+                if (rv != SECSuccess) {
+                    sftk_FreeAttribute(attribute);
+                    *crvp = CKR_KEY_TYPE_INCONSISTENT;
+                    goto loser;
+                }
+                algorithm = SECOID_GetAlgorithmTag(&publicKeyInfo->algorithm);
+                if (algorithm != SEC_OID_PKCS1_RSA_ENCRYPTION &&
+                    algorithm != SEC_OID_PKCS1_RSA_PSS_SIGNATURE) {
+                    sftk_FreeAttribute(attribute);
+                    rv = SECFailure;
+                    *crvp = CKR_KEY_TYPE_INCONSISTENT;
+                    goto loser;
+                }
+                param = SECITEM_DupItem(&publicKeyInfo->algorithm.parameters);
+                if (!param) {
+                    sftk_FreeAttribute(attribute);
+                    rv = SECFailure;
+                    *crvp = CKR_HOST_MEMORY;
+                    goto loser;
+                }
+                sftk_FreeAttribute(attribute);
+            } else {
+                /* default to PKCS #1 */
+                algorithm = SEC_OID_PKCS1_RSA_ENCRYPTION;
+            }
             break;
         case NSSLOWKEYDSAKey:
             prepare_low_dsa_priv_key_export_for_asn1(lk);
@@ -5803,6 +5848,53 @@ sftk_unwrapPrivateKey(SFTKObject *key, SECItem *bpki)
             break;
     }
 
+    if (crv != CKR_OK) {
+        goto loser;
+    }
+
+    /* For RSA-PSS, record the original algorithm parameters so
+     * they can be encrypted altoghether when wrapping */
+    if (SECOID_GetAlgorithmTag(&pki->algorithm) == SEC_OID_PKCS1_RSA_PSS_SIGNATURE) {
+        NSSLOWKEYSubjectPublicKeyInfo spki;
+        NSSLOWKEYPublicKey pubk;
+        SECItem *publicKeyInfo;
+
+        memset(&spki, 0, sizeof(NSSLOWKEYSubjectPublicKeyInfo));
+        rv = SECOID_CopyAlgorithmID(arena, &spki.algorithm, &pki->algorithm);
+        if (rv != SECSuccess) {
+            crv = CKR_HOST_MEMORY;
+            goto loser;
+        }
+
+        prepare_low_rsa_pub_key_for_asn1(&pubk);
+
+        rv = SECITEM_CopyItem(arena, &pubk.u.rsa.modulus, &lpk->u.rsa.modulus);
+        if (rv != SECSuccess) {
+            crv = CKR_HOST_MEMORY;
+            goto loser;
+        }
+        rv = SECITEM_CopyItem(arena, &pubk.u.rsa.publicExponent, &lpk->u.rsa.publicExponent);
+        if (rv != SECSuccess) {
+            crv = CKR_HOST_MEMORY;
+            goto loser;
+        }
+
+        if (SEC_ASN1EncodeItem(arena, &spki.subjectPublicKey,
+                               &pubk, nsslowkey_RSAPublicKeyTemplate) == NULL) {
+            crv = CKR_HOST_MEMORY;
+            goto loser;
+        }
+
+        publicKeyInfo = SEC_ASN1EncodeItem(arena, NULL,
+                                           &spki, nsslowkey_SubjectPublicKeyInfoTemplate);
+        if (!publicKeyInfo) {
+            crv = CKR_HOST_MEMORY;
+            goto loser;
+        }
+        crv = sftk_AddAttributeType(key, CKA_PUBLIC_KEY_INFO,
+                                    sftk_item_expand(publicKeyInfo));
+    }
+
 loser:
     if (lpk) {
         nsslowkey_DestroyPrivateKey(lpk);

diff --git a/lib/softoken/pkcs11u.c b/lib/softoken/pkcs11u.c
@@ -1269,7 +1269,7 @@ static const CK_ULONG ecPubKeyAttrsCount =
 
 static const CK_ATTRIBUTE_TYPE commonPrivKeyAttrs[] = {
     CKA_DECRYPT, CKA_SIGN, CKA_SIGN_RECOVER, CKA_UNWRAP, CKA_SUBJECT,
-    CKA_SENSITIVE, CKA_EXTRACTABLE, CKA_NETSCAPE_DB
+    CKA_SENSITIVE, CKA_EXTRACTABLE, CKA_NETSCAPE_DB, CKA_PUBLIC_KEY_INFO
 };
 static const CK_ULONG commonPrivKeyAttrsCount =
     sizeof(commonPrivKeyAttrs) / sizeof(commonPrivKeyAttrs[0]);

diff --git a/lib/softoken/sdb.c b/lib/softoken/sdb.c
@@ -154,7 +154,8 @@ static const CK_ATTRIBUTE_TYPE known_attributes[] = {
     CKA_TRUST_EMAIL_PROTECTION, CKA_TRUST_IPSEC_END_SYSTEM,
     CKA_TRUST_IPSEC_TUNNEL, CKA_TRUST_IPSEC_USER, CKA_TRUST_TIME_STAMPING,
     CKA_TRUST_STEP_UP_APPROVED, CKA_CERT_SHA1_HASH, CKA_CERT_MD5_HASH,
-    CKA_NETSCAPE_DB, CKA_NETSCAPE_TRUST, CKA_NSS_OVERRIDE_EXTENSIONS
+    CKA_NETSCAPE_DB, CKA_NETSCAPE_TRUST, CKA_NSS_OVERRIDE_EXTENSIONS,
+    CKA_PUBLIC_KEY_INFO
 };
 
 static int known_attributes_size = sizeof(known_attributes) /

diff --git a/lib/softoken/sftkdb.c b/lib/softoken/sftkdb.c
@@ -1591,7 +1591,8 @@ static const CK_ATTRIBUTE_TYPE known_attributes[] = {
     CKA_TRUST_EMAIL_PROTECTION, CKA_TRUST_IPSEC_END_SYSTEM,
     CKA_TRUST_IPSEC_TUNNEL, CKA_TRUST_IPSEC_USER, CKA_TRUST_TIME_STAMPING,
     CKA_TRUST_STEP_UP_APPROVED, CKA_CERT_SHA1_HASH, CKA_CERT_MD5_HASH,
-    CKA_NETSCAPE_DB, CKA_NETSCAPE_TRUST, CKA_NSS_OVERRIDE_EXTENSIONS
+    CKA_NETSCAPE_DB, CKA_NETSCAPE_TRUST, CKA_NSS_OVERRIDE_EXTENSIONS,
+    CKA_PUBLIC_KEY_INFO
 };
 
 static unsigned int known_attributes_size = sizeof(known_attributes) /

diff --git a/lib/util/pkcs11t.h b/lib/util/pkcs11t.h
@@ -466,6 +466,8 @@ typedef CK_ULONG CK_ATTRIBUTE_TYPE;
 #define CKA_EXPONENT_1 0x00000126
 #define CKA_EXPONENT_2 0x00000127
 #define CKA_COEFFICIENT 0x00000128
+/* CKA_PUBLIC_KEY_INFO is new for v2.40 */
+#define CKA_PUBLIC_KEY_INFO 0x00000129
 #define CKA_PRIME 0x00000130
 #define CKA_SUBPRIME 0x00000131
 #define CKA_BASE 0x00000132

diff --git a/tests/common/init.sh b/tests/common/init.sh
@@ -543,8 +543,8 @@ if [ -z "${INIT_SOURCED}" -o "${INIT_SOURCED}" != "TRUE" ]; then
     D_DISTRUST="Distrust.$version"
     D_RSAPSS="RSAPSS.$version"
 
-    # we need relative pathnames of these files abd directories, since our
-    # tools can't handle the unix style absolut pathnames on cygnus
+    # we need relative pathnames of these files and directories, since our
+    # tools can't handle the unix style absolute pathnames on cygnus
 
     R_CADIR=../CA
     R_SERVERDIR=../server
@@ -565,6 +565,7 @@ if [ -z "${INIT_SOURCED}" -o "${INIT_SOURCED}" != "TRUE" ]; then
     R_NOLOGINDIR=../nologin
     R_SSLGTESTDIR=../ssl_gtests
     R_GTESTDIR=../gtests
+    R_RSAPSSDIR=../rsapss
 
     #
     # profiles are either paths or domains depending on the setting of
@@ -581,6 +582,7 @@ if [ -z "${INIT_SOURCED}" -o "${INIT_SOURCED}" != "TRUE" ]; then
     P_R_EXT_SERVERDIR=${R_EXT_SERVERDIR}
     P_R_EXT_CLIENTDIR=${R_EXT_CLIENTDIR}
     P_R_IMPLICIT_INIT_DIR=${R_IMPLICIT_INIT_DIR}
+    P_R_RSAPSSDIR=${R_RSAPSSDIR}
     if [ -n "${MULTIACCESS_DBM}" ]; then
         P_R_CADIR="multiaccess:${D_CA}"
         P_R_ALICEDIR="multiaccess:${D_ALICE}"
@@ -593,6 +595,7 @@ if [ -z "${INIT_SOURCED}" -o "${INIT_SOURCED}" != "TRUE" ]; then
         P_R_EXT_SERVERDIR="multiaccess:${D_EXT_SERVER}"
         P_R_EXT_CLIENTDIR="multiaccess:${D_EXT_CLIENT}"
         P_R_IMPLICIT_INIT_DIR="multiaccess:${D_IMPLICIT_INIT}"
+	P_R_RSAPSSDIR="multiaccess:${D_RSAPSS}"
     fi
 
     R_PWFILE=../tests.pw

diff --git a/tests/tools/TestRSAPSS.p12 b/tests/tools/TestRSAPSS.p12
diff --git a/tests/tools/tools.sh b/tests/tools/tools.sh
@@ -105,6 +105,7 @@ tools_init()
   mkdir -p ${TOOLSDIR}/data
   cp ${QADIR}/tools/TestOldCA.p12 ${TOOLSDIR}/data
   cp ${QADIR}/tools/TestOldAES128CA.p12 ${TOOLSDIR}/data
+  cp ${QADIR}/tools/TestRSAPSS.p12 ${TOOLSDIR}/data
 
   cd ${TOOLSDIR}
 }
@@ -436,6 +437,23 @@ tools_p12_import_old_files()
   check_tmpfile
 }
 
+tools_p12_import_rsa_pss_private_key()
+{
+  echo "$SCRIPTNAME: Importing RSA-PSS private key from PKCS#12 file --------------"
+  ${BINDIR}/pk12util -i ${TOOLSDIR}/data/TestRSAPSS.p12 -d ${P_R_COPYDIR} -k ${R_PWFILE} -W '' 2>&1
+  ret=$?
+  html_msg $ret 0 "Importing RSA-PSS private key from PKCS#12 file"
+  check_tmpfile
+
+  # Check if RSA-PSS identifier is included in the key listing
+  ${BINDIR}/certutil -d ${P_R_COPYDIR} -K -f ${R_PWFILE} | grep '^<[0-9 ]*> *rsaPss'
+  ret=$?
+  html_msg $ret 0 "Listing RSA-PSS private key imported from PKCS#12 file"
+  check_tmpfile
+
+  return $ret
+}
+
 ############################## tools_p12 ###############################
 # local shell function to test basic functionality of pk12util
 ########################################################################
@@ -448,6 +466,9 @@ tools_p12()
   tools_p12_export_with_none_ciphers
   tools_p12_export_with_invalid_ciphers
   tools_p12_import_old_files
+  if [ "${TEST_MODE}" = "SHARED_DB" ] ; then
+    tools_p12_import_rsa_pss_private_key
+  fi
 }
 
 ############################## tools_sign ##############################