From: Jakub Jirutka Date: Mon, 10 Jan 2022 22:57:26 +0100 Subject: [PATCH] Disable obsolete ciphers in signatures (patch from Thunderbird) This patch is based on [1] from Thunderbird, but I also modified the tests to make them pass. It's quite a hack, a proper solution is being discussed in: * https://github.com/rnpgp/rnp/issues/1281 * https://github.com/rnpgp/rnp/pull/1524 [1]: https://hg.mozilla.org/comm-central/file/tip/third_party/patches/rnp/disable_obsolete_ciphers.patch --- rnp-0.15.2.orig/src/lib/crypto/signatures.cpp +++ rnp-0.15.2/src/lib/crypto/signatures.cpp @@ -193,6 +193,59 @@ } } +static bool is_hash_alg_allowed_in_sig(const pgp_hash_alg_t hash_alg) +{ + switch (hash_alg) { + case PGP_HASH_SHA1: + case PGP_HASH_RIPEMD: + case PGP_HASH_SHA256: + case PGP_HASH_SHA384: + case PGP_HASH_SHA512: + case PGP_HASH_SHA224: + case PGP_HASH_SHA3_256: + case PGP_HASH_SHA3_512: + return true; + + case PGP_HASH_MD5: + case PGP_HASH_SM3: + case PGP_HASH_UNKNOWN: + default: + return false; + } +} + +static bool is_pubkey_alg_allowed_in_sig(const pgp_pubkey_alg_t pubkey_alg) { + switch (pubkey_alg) { + case PGP_PKA_RSA: + case PGP_PKA_RSA_ENCRYPT_ONLY: + case PGP_PKA_RSA_SIGN_ONLY: + case PGP_PKA_ELGAMAL: + case PGP_PKA_DSA: + case PGP_PKA_ECDH: + case PGP_PKA_ECDSA: + case PGP_PKA_ELGAMAL_ENCRYPT_OR_SIGN: + case PGP_PKA_EDDSA: + return true; + + case PGP_PKA_RESERVED_DH: + case PGP_PKA_NOTHING: + case PGP_PKA_SM2: + case PGP_PKA_PRIVATE00: + case PGP_PKA_PRIVATE01: + case PGP_PKA_PRIVATE02: + case PGP_PKA_PRIVATE03: + case PGP_PKA_PRIVATE04: + case PGP_PKA_PRIVATE05: + case PGP_PKA_PRIVATE06: + case PGP_PKA_PRIVATE07: + case PGP_PKA_PRIVATE08: + case PGP_PKA_PRIVATE09: + case PGP_PKA_PRIVATE10: + default: + return false; + } +} + rnp_result_t signature_validate(const pgp_signature_t *sig, const pgp_key_material_t *key, pgp_hash_t *hash) { @@ -201,9 +254,16 @@ rnp_result_t ret = RNP_ERROR_GENERIC; const pgp_hash_alg_t hash_alg = pgp_hash_alg_type(hash); + if (!is_hash_alg_allowed_in_sig(hash_alg)) { + return RNP_ERROR_SIGNATURE_INVALID; + } if (!key) { return RNP_ERROR_NULL_POINTER; + } + + if (!is_pubkey_alg_allowed_in_sig(sig->palg)) { + return RNP_ERROR_SIGNATURE_INVALID; } if (sig->palg != key->alg) { --- rnp-0.15.2.orig/src/tests/generatekey.cpp +++ rnp-0.15.2/src/tests/generatekey.cpp @@ -96,19 +96,19 @@ * Sign a message, then verify it */ - const char *hashAlg[] = {"SHA1", + const char *hashAlg[] = { "SHA224", "SHA256", "SHA384", "SHA512", - "SM3", - "sha1", + + "sha224", "sha256", "sha384", "sha512", - "sm3", - NULL}; + + NULL}; // XXX-Patched: obsolete algs removed int pipefd[2] = {-1, -1}; char memToSign[] = "A simple test message"; cli_rnp_t rnp; @@ -270,20 +270,20 @@ /* Generate key for each of the hash algorithms. Check whether key was generated * successfully */ - const char *hashAlg[] = {"MD5", + const char *hashAlg[] = { "SHA1", "SHA256", "SHA384", "SHA512", "SHA224", - "SM3", - "md5", + + "sha1", "sha256", "sha384", "sha512", - "sha224", - "sm3"}; + "sha224" + }; // XXX-Patched: obsolete algs removed const char *keystores[] = {RNP_KEYSTORE_GPG, RNP_KEYSTORE_GPG21, RNP_KEYSTORE_KBX}; cli_rnp_t rnp = {}; --- rnp-0.15.2.orig/src/tests/key-validate.cpp +++ rnp-0.15.2/src/tests/key-validate.cpp @@ -77,7 +77,8 @@ pubring = new rnp_key_store_t(PGP_KEY_STORE_GPG, "data/keyrings/2/pubring.gpg"); assert_true(rnp_key_store_load_from_path(pubring, NULL)); - assert_true(all_keys_valid(pubring)); + assert_false(all_keys_valid(pubring)); // XXX-Patched: insecure keys (s/assert_true/assert_false/) + delete pubring; secring = new rnp_key_store_t(PGP_KEY_STORE_GPG, "data/keyrings/2/secring.gpg"); @@ -99,7 +100,7 @@ pubring = new rnp_key_store_t(PGP_KEY_STORE_GPG, "data/keyrings/4/pubring.pgp"); assert_true(rnp_key_store_load_from_path(pubring, NULL)); - assert_true(all_keys_valid(pubring)); + assert_false(all_keys_valid(pubring)); // XXX-Patched: insecure keys (s/assert_true/assert_false/) delete pubring; secring = new rnp_key_store_t(PGP_KEY_STORE_GPG, "data/keyrings/4/secring.pgp"); --- rnp-0.15.2.orig/src/tests/streams.cpp +++ rnp-0.15.2/src/tests/streams.cpp @@ -1012,25 +1012,25 @@ assert_true(rng_init(&rng, RNG_SYSTEM)); /* v3 public key */ - pubring = new rnp_key_store_t(PGP_KEY_STORE_GPG, "data/keyrings/4/rsav3-p.asc"); - assert_true(rnp_key_store_load_from_path(pubring, NULL)); - assert_rnp_success(init_file_src(&keysrc, "data/keyrings/4/rsav3-p.asc")); - assert_rnp_success(process_pgp_keys(&keysrc, keyseq, false)); - src_close(&keysrc); - assert_int_equal(keyseq.keys.size(), 1); - assert_non_null(key = &keyseq.keys.front()); - assert_non_null(uid = &key->userids.front()); - assert_non_null(sig = &uid->signatures.front()); - assert_non_null(pkey = rnp_key_store_get_key_by_id(pubring, sig->keyid(), NULL)); - /* check certification signature */ - assert_true(signature_hash_certification(sig, &key->key, &uid->uid, &hash)); - assert_rnp_success(signature_validate(sig, &pkey->material(), &hash)); - /* modify userid and check signature */ - uid->uid.uid[2] = '?'; - assert_true(signature_hash_certification(sig, &key->key, &uid->uid, &hash)); - assert_rnp_failure(signature_validate(sig, &pkey->material(), &hash)); - delete pubring; + /* XXX-Patched: obsolete key format + + + + + + + + + + + + + + + + */ + /* keyring */ pubring = new rnp_key_store_t(PGP_KEY_STORE_GPG, "data/keyrings/1/pubring.gpg"); assert_true(rnp_key_store_load_from_path(pubring, NULL)); @@ -1103,13 +1103,13 @@ pgp_key_t * pkey = NULL; /* v3 public key */ - pubring = new rnp_key_store_t(PGP_KEY_STORE_GPG, "data/keyrings/4/rsav3-p.asc"); - assert_true(rnp_key_store_load_from_path(pubring, NULL)); - assert_int_equal(rnp_key_store_get_key_count(pubring), 1); - pkey = &pubring->keys.front(); - pkey->validate(*pubring); - assert_true(pkey->valid()); - delete pubring; + /* XXX-Patched: obsolete key format + + + + + + */ /* keyring */ pubring = new rnp_key_store_t(PGP_KEY_STORE_GPG, "data/keyrings/1/pubring.gpg"); --- rnp-0.15.2/src/tests/cli_tests.py +++ rnp-0.15.2/src/tests/cli_tests.py @@ -1624,10 +1624,10 @@ '--secret', '--with-sigs']) compare_file_any(allow_y2k38_on_32bit(path + 'keyring_1_list_sigs_sec'), out, 'keyring 1 sec sig listing failed') - _, out, _ = run_proc(RNPK, ['--homedir', data_path('keyrings/2'), '--list-keys']) - compare_file(path + 'keyring_2_list_keys', out, 'keyring 2 key listing failed') - _, out, _ = run_proc(RNPK, ['--homedir', data_path('keyrings/2'), '-l', '--with-sigs']) - compare_file(path + 'keyring_2_list_sigs', out, 'keyring 2 sig listing failed') + #_, out, _ = run_proc(RNPK, ['--homedir', data_path('keyrings/2'), '--list-keys']) XXX-Patched: obsolete keys + #compare_file(path + 'keyring_2_list_keys', out, 'keyring 2 key listing failed') + #_, out, _ = run_proc(RNPK, ['--homedir', data_path('keyrings/2'), '-l', '--with-sigs']) + #compare_file(path + 'keyring_2_list_sigs', out, 'keyring 2 sig listing failed') _, out, _ = run_proc(RNPK, ['--homedir', data_path('keyrings/3'), '--list-keys']) compare_file_any(allow_y2k38_on_32bit(path + 'keyring_3_list_keys'), out, 'keyring 3 key listing failed')