From 24b3a5a5794db4bb69b38a1df099d5e59cccf2b3 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 6 Oct 2023 10:57:12 +0200 Subject: [PATCH] sm: Support more HMAC algos in the pkcs#12 parser. * sm/minip12.c (oid_hmacWithSHA1): New. Also for the SHA-2 algos. (digest_algo_from_oid): New. (set_key_iv_pbes2): Add arg digest_algo. (crypt_block): Ditto. (decrypt_block): Ditto. (parse_bag_encrypted_data): Parse the optional prf part and get the hmac algorithm. (parse_shrouded_key_bag): Ditto. (p12_build): Pass SHA1 for digest_algo. * sm/t-minip12.c (run_one_test): Print failed values in verbose mode. * tests/cms/samplekeys/nistp256-openssl-self-signed.p12: New. * tests/cms/samplekeys/Description-p12: Add this one. * tests/cms/Makefile.am (EXTRA_DIST): Ditto. -- This supports the modern algorithms, i.e. using SHA256 for the KDF which is the default in openssl unless the -legacy option is used. GnuPG-bug-id: 6536 --- sm/minip12.c | 131 ++++++++++++++++-- sm/t-minip12.c | 12 +- tests/cms/Makefile.am | 1 + tests/cms/samplekeys/Description-p12 | 6 + .../nistp256-openssl-self-signed.p12 | Bin 0 -> 1232 bytes 5 files changed, 135 insertions(+), 15 deletions(-) create mode 100644 tests/cms/samplekeys/nistp256-openssl-self-signed.p12 diff --git a/sm/minip12.c b/sm/minip12.c index ed80534b6..e63d9a95d 100644 --- a/sm/minip12.c +++ b/sm/minip12.c @@ -83,6 +83,17 @@ static unsigned char const oid_aes128_CBC[9] = { static unsigned char const oid_aes256_CBC[9] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x01, 0x2A }; +static unsigned char const oid_hmacWithSHA1[8] = { + 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x02, 0x07 }; +static unsigned char const oid_hmacWithSHA224[8] = { + 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x02, 0x08 }; +static unsigned char const oid_hmacWithSHA256[8] = { + 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x02, 0x09 }; +static unsigned char const oid_hmacWithSHA384[8] = { + 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x02, 0x0A }; +static unsigned char const oid_hmacWithSHA512[8] = { + 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x02, 0x0B }; + static unsigned char const oid_rsaEncryption[9] = { 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x01 }; static unsigned char const oid_pcPublicKey[7] = { @@ -241,6 +252,32 @@ dump_tag_info (const char *text, struct tag_info *ti) } +static int +digest_algo_from_oid (unsigned char const *oid, size_t oidlen) +{ + int algo; + + if (oidlen == DIM(oid_hmacWithSHA1) && + !memcmp (oid, oid_hmacWithSHA1, oidlen)) + algo = GCRY_MD_SHA1; + else if (oidlen == DIM(oid_hmacWithSHA224) && + !memcmp (oid, oid_hmacWithSHA224, oidlen)) + algo = GCRY_MD_SHA224; + else if (oidlen == DIM(oid_hmacWithSHA256) && + !memcmp (oid, oid_hmacWithSHA256, oidlen)) + algo = GCRY_MD_SHA256; + else if (oidlen == DIM(oid_hmacWithSHA384) && + !memcmp (oid, oid_hmacWithSHA384, oidlen)) + algo = GCRY_MD_SHA384; + else if (oidlen == DIM(oid_hmacWithSHA512) && + !memcmp (oid, oid_hmacWithSHA512, oidlen)) + algo = GCRY_MD_SHA512; + else + algo = 0; + return algo; +} + + /* Wrapper around tlv_builder_add_ptr to add an OID. When we * eventually put the whole tlv_builder stuff into Libksba, we can add * such a function there. Right now we don't do this to avoid a @@ -1029,13 +1066,14 @@ set_key_iv (gcry_cipher_hd_t chd, char *salt, size_t saltlen, int iter, static int set_key_iv_pbes2 (gcry_cipher_hd_t chd, char *salt, size_t saltlen, int iter, - const void *iv, size_t ivlen, const char *pw, int algo) + const void *iv, size_t ivlen, const char *pw, + int cipher_algo, int digest_algo) { unsigned char *keybuf; size_t keylen; int rc; - keylen = gcry_cipher_get_algo_keylen (algo); + keylen = gcry_cipher_get_algo_keylen (cipher_algo); if (!keylen) return -1; keybuf = gcry_malloc_secure (keylen); @@ -1043,7 +1081,7 @@ set_key_iv_pbes2 (gcry_cipher_hd_t chd, char *salt, size_t saltlen, int iter, return -1; rc = gcry_kdf_derive (pw, strlen (pw), - GCRY_KDF_PBKDF2, GCRY_MD_SHA1, + GCRY_KDF_PBKDF2, digest_algo, salt, saltlen, iter, keylen, keybuf); if (rc) { @@ -1074,7 +1112,7 @@ set_key_iv_pbes2 (gcry_cipher_hd_t chd, char *salt, size_t saltlen, int iter, static void crypt_block (unsigned char *buffer, size_t length, char *salt, size_t saltlen, int iter, const void *iv, size_t ivlen, - const char *pw, int cipher_algo, int encrypt) + const char *pw, int cipher_algo, int digest_algo, int encrypt) { gcry_cipher_hd_t chd; int rc; @@ -1088,7 +1126,8 @@ crypt_block (unsigned char *buffer, size_t length, char *salt, size_t saltlen, } if ((cipher_algo == GCRY_CIPHER_AES128 || cipher_algo == GCRY_CIPHER_AES256) - ? set_key_iv_pbes2 (chd, salt, saltlen, iter, iv, ivlen, pw, cipher_algo) + ? set_key_iv_pbes2 (chd, salt, saltlen, iter, iv, ivlen, pw, + cipher_algo, digest_algo) : set_key_iv (chd, salt, saltlen, iter, pw, cipher_algo == GCRY_CIPHER_RFC2268_40? 5:24)) { @@ -1125,7 +1164,7 @@ static void decrypt_block (const void *ciphertext, unsigned char *plaintext, size_t length, char *salt, size_t saltlen, int iter, const void *iv, size_t ivlen, - const char *pw, int cipher_algo, + const char *pw, int cipher_algo, int digest_algo, int (*check_fnc) (const void *, size_t)) { static const char * const charsets[] = { @@ -1197,7 +1236,7 @@ decrypt_block (const void *ciphertext, unsigned char *plaintext, size_t length, } memcpy (plaintext, ciphertext, length); crypt_block (plaintext, length, salt, saltlen, iter, iv, ivlen, - convertedpw? convertedpw:pw, cipher_algo, 0); + convertedpw? convertedpw:pw, cipher_algo, digest_algo, 0); if (check_fnc (plaintext, length)) break; /* Decryption succeeded. */ } @@ -1257,6 +1296,7 @@ parse_bag_encrypted_data (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) int renewed_tlv = 0; int loopcount; unsigned int startlevel; + int digest_algo = GCRY_MD_SHA1; where = "bag.encryptedData"; if (opt_verbose) @@ -1326,6 +1366,8 @@ parse_bag_encrypted_data (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) /*FIXME: This code is duplicated in parse_shrouded_key_bag. */ if (is_pbes2) { + size_t parmlen; /* Remaining length of the parameter sequence. */ + where = "pkcs5PBES2-params"; if (tlv_next (tlv)) goto bailout; @@ -1352,11 +1394,13 @@ parse_bag_encrypted_data (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) goto bailout; if (tlv_expect_sequence (tlv)) goto bailout; + parmlen = tlv->ti.length; if (tlv_next (tlv)) goto bailout; if (tlv_expect_octet_string (tlv, 0, &data, &datalen)) goto bailout; + parmlen -= tlv->ti.length + tlv->ti.nhdr; if (datalen < 8 || datalen > sizeof salt) { log_info ("bad length of salt (%zu)\n", datalen); @@ -1370,6 +1414,7 @@ parse_bag_encrypted_data (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) goto bailout; if ((err = tlv_expect_integer (tlv, &intval))) goto bailout; + parmlen -= tlv->ti.length + tlv->ti.nhdr; if (!intval) /* Not a valid iteration count. */ { err = gpg_error (GPG_ERR_INV_VALUE); @@ -1377,8 +1422,34 @@ parse_bag_encrypted_data (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) } iter = intval; - /* Note: We don't support the optional parameters but assume - that the algorithmIdentifier follows. */ + if (parmlen > 2) /* There is the optional prf. */ + { + if (tlv_next (tlv)) + goto bailout; + if (tlv_expect_sequence (tlv)) + goto bailout; + if (tlv_next (tlv)) + goto bailout; + if (tlv_expect_object_id (tlv, &oid, &oidlen)) + goto bailout; + digest_algo = digest_algo_from_oid (oid, oidlen); + if (!digest_algo) + { + gpgrt_log_printhex (oid, oidlen, "kdf digest algo:"); + err = gpg_error (GPG_ERR_DIGEST_ALGO); + goto bailout; + } + if (opt_verbose > 1) + log_debug ("kdf digest algo = %d\n", digest_algo); + + if (tlv_next (tlv)) + goto bailout; + if (tlv_expect_null (tlv)) + tlv_set_pending (tlv); /* NULL tag missing - ignore this. */ + } + else + digest_algo = GCRY_MD_SHA1; + if (tlv_next (tlv)) goto bailout; if (tlv_expect_sequence (tlv)) @@ -1468,6 +1539,7 @@ parse_bag_encrypted_data (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) iv, is_pbes2?16:0, ctx->password, is_pbes2 ? (is_aes256?GCRY_CIPHER_AES256:GCRY_CIPHER_AES128) : is_3des ? GCRY_CIPHER_3DES : GCRY_CIPHER_RFC2268_40, + digest_algo, bag_decrypted_data_p); /* We do not need the TLV anymore and allocated a new one. */ @@ -1778,6 +1850,7 @@ parse_shrouded_key_bag (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) unsigned char *plain = NULL; int is_pbes2 = 0; int is_aes256 = 0; + int digest_algo = GCRY_MD_SHA1; where = "shrouded_key_bag"; if (opt_verbose) @@ -1819,6 +1892,8 @@ parse_shrouded_key_bag (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) if (is_pbes2) { + size_t parmlen; /* Remaining length of the parameter sequence. */ + where = "shrouded_key_bag.pkcs5PBES2-params"; if (tlv_next (tlv)) goto bailout; @@ -1842,11 +1917,13 @@ parse_shrouded_key_bag (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) goto bailout; if (tlv_expect_sequence (tlv)) goto bailout; + parmlen = tlv->ti.length; if (tlv_next (tlv)) goto bailout; if (tlv_expect_octet_string (tlv, 0, &data, &datalen)) goto bailout; + parmlen -= tlv->ti.length + tlv->ti.nhdr; if (datalen < 8 || datalen > sizeof salt) { log_info ("bad length of salt (%zu) for AES\n", datalen); @@ -1860,6 +1937,7 @@ parse_shrouded_key_bag (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) goto bailout; if ((err = tlv_expect_integer (tlv, &intval))) goto bailout; + parmlen -= tlv->ti.length + tlv->ti.nhdr; if (!intval) /* Not a valid iteration count. */ { err = gpg_error (GPG_ERR_INV_VALUE); @@ -1867,8 +1945,34 @@ parse_shrouded_key_bag (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) } iter = intval; - /* Note: We don't support the optional parameters but assume - that the algorithmIdentifier follows. */ + if (parmlen > 2) /* There is the optional prf. */ + { + if (tlv_next (tlv)) + goto bailout; + if (tlv_expect_sequence (tlv)) + goto bailout; + if (tlv_next (tlv)) + goto bailout; + if (tlv_expect_object_id (tlv, &oid, &oidlen)) + goto bailout; + digest_algo = digest_algo_from_oid (oid, oidlen); + if (!digest_algo) + { + gpgrt_log_printhex (oid, oidlen, "kdf digest algo:"); + err = gpg_error (GPG_ERR_DIGEST_ALGO); + goto bailout; + } + if (opt_verbose > 1) + log_debug ("kdf digest algo = %d\n", digest_algo); + + if (tlv_next (tlv)) + goto bailout; + if (tlv_expect_null (tlv)) + tlv_set_pending (tlv); /* NULL tag missing - ignore this. */ + } + else + digest_algo = GCRY_MD_SHA1; + if (tlv_next (tlv)) goto bailout; if (tlv_expect_sequence (tlv)) @@ -1954,6 +2058,7 @@ parse_shrouded_key_bag (struct p12_parse_ctx_s *ctx, struct tlv_ctx_s *tlv) iv, is_pbes2? 16:0, ctx->password, is_pbes2 ? (is_aes256?GCRY_CIPHER_AES256:GCRY_CIPHER_AES128) : GCRY_CIPHER_3DES, + digest_algo, bag_data_p); @@ -3468,7 +3573,7 @@ p12_build (gcry_mpi_t *kparms, const void *cert, size_t certlen, /* Encrypt it. */ gcry_randomize (salt, 8, GCRY_STRONG_RANDOM); crypt_block (buffer, buflen, salt, 8, 2048, NULL, 0, pw, - GCRY_CIPHER_RFC2268_40, 1); + GCRY_CIPHER_RFC2268_40, GCRY_MD_SHA1, 1); /* Encode the encrypted stuff into a bag. */ seqlist[seqlistidx].buffer = build_cert_bag (buffer, buflen, salt, &n); @@ -3500,7 +3605,7 @@ p12_build (gcry_mpi_t *kparms, const void *cert, size_t certlen, /* Encrypt it. */ gcry_randomize (salt, 8, GCRY_STRONG_RANDOM); crypt_block (buffer, buflen, salt, 8, 2048, NULL, 0, - pw, GCRY_CIPHER_3DES, 1); + pw, GCRY_CIPHER_3DES, GCRY_MD_SHA1, 1); /* Encode the encrypted stuff into a bag. */ if (cert && certlen) diff --git a/sm/t-minip12.c b/sm/t-minip12.c index de6b7e5cc..bf3177ea0 100644 --- a/sm/t-minip12.c +++ b/sm/t-minip12.c @@ -559,13 +559,21 @@ run_one_test (const char *name, const char *desc, const char *pass, else if (!certexpected && certstr) printresult ("FAIL: %s - no certs expected but got one\n", name); else if (certexpected && certstr && strcmp (certexpected, certstr)) - printresult ("FAIL: %s - certs not as expected\n", name); + { + printresult ("FAIL: %s - certs not as expected\n", name); + inf ("cert(exp)=%s", certexpected); + inf ("cert(got)=%s", certstr? certstr:"[null]"); + } else if (keyexpected && !resulthash) printresult ("FAIL: %s - expected key but got none\n", name); else if (!keyexpected && resulthash) printresult ("FAIL: %s - key not expected but got one\n", name); else if (keyexpected && resulthash && strcmp (keyexpected, resulthash)) - printresult ("FAIL: %s - keys not as expected\n", name); + { + printresult ("FAIL: %s - keys not as expected\n", name); + inf ("key(exp)=%s", keyexpected); + inf ("key(got)=%s", resulthash? resulthash:"[null]"); + } else { printresult ("PASS: %s\n", name); diff --git a/tests/cms/Makefile.am b/tests/cms/Makefile.am index d5d753902..b43fb1c91 100644 --- a/tests/cms/Makefile.am +++ b/tests/cms/Makefile.am @@ -100,6 +100,7 @@ EXTRA_DIST = $(XTESTS) $(KEYS) $(CERTS) $(TEST_FILES) \ samplekeys/t5793-openssl.pfx \ samplekeys/t5793-test.pfx \ samplekeys/edward.tester@demo.gnupg.com.p12 \ + samplekeys/nistp256-openssl-self-signed.p12 \ samplemsgs/pwri-sample.cbc.p7m \ samplemsgs/pwri-sample.cbc-2.p7m \ samplemsgs/pwri-sample.gcm.p7m \ diff --git a/tests/cms/samplekeys/Description-p12 b/tests/cms/samplekeys/Description-p12 index 6fbbd82cf..a73998fac 100644 --- a/tests/cms/samplekeys/Description-p12 +++ b/tests/cms/samplekeys/Description-p12 @@ -39,4 +39,10 @@ Pass: abc,123456 Cert: ff810b9281a43c394aa138e9c7fd4c0193216fa6 Key: 94c6d0b067370a8f2a09ae43cfe8d700bbd61e75 +Name: nistp256-openssl-self-signed.p12 +Desc: OpenSSL generated self-signed nistp256 key+cert +Pass: abc +Cert: 5cea0c5bf09ccd92535267c662fc098f6c81c27e +Key: 3cb2fba95d1976df69eb7aa8c65ac5354e15af32 + # eof # diff --git a/tests/cms/samplekeys/nistp256-openssl-self-signed.p12 b/tests/cms/samplekeys/nistp256-openssl-self-signed.p12 new file mode 100644 index 0000000000000000000000000000000000000000..9eeebdae335a133abb7e66166d2691d8999196a2 GIT binary patch literal 1232 zcmXqLVmZUa$ZXKW(!|E8)#lOmotKfFaX}MHF-sFmK2SK@pov)tA;q?!iCGvZ#LvXY z0Hin&GK>b{a1JZ4ftP_Mg3Dtd!@^-9y3zLRjF>_uCJqJz9ySh$ZA_f33Inm3|n4o`T_UqNf1#0qZXKuW{x%%a{Ly!6Xsfp>R#c^6Wrkz~w=Mln|8FOOc z!MHPXA6EQ%zOja7(b5p9&&M~;)_FNu=uKwLnUuS$PaiuuyC8L~k;WAkz7HRcdd^wr zZ==6^%6U`&b!i_PMZ5wmxq{{VaMI007dx~!=2@m|w?Q3RudG?8&)$jEsGLO$@i#k?c@aoL` zM{_1_eHA>h&u~v$)SkI#EUOl-wm8jp+EnE00fVlYw?(+hZpZojKX=Rh{d$306K5yS z6)*U*VCfT?)#tkt#w>bG;>JBmEN%CqQU{Dp1%_Pu5}bL%MI-S;Ucbd&i%%$HDk zKUsa&_N>O2C+zOtiM&!RGylZ{-Tn(t&w3;Yo2EWbpC-2Yb4HbD+w_20hkEkgG>IMR zZT~H$@hI_h_q5G7zH?OB16WP*@9y)!4Y9d5edOk!eBWdxOR|gtE(= z@0mY$bZ(tQFuQ~VyZG%D{JwMc!J-G;Vk=l!8YkS8Ja|s4`A7_FW8twQJ0%3`Hg;Z9 zdHrh6J%&kqc@Kp=R+lkIDXkXKt}0m@)YSFOv*zowyMM(R1b(kyeab%chOy~GQ-y1( zd!t?l#RNy*XFsBOwmItQI-MfLbszTqR`_TX>QG}R@#na~k?9ZRyM=$trw8n1TX|&a zZox