From aeed0b93ff660fe271d8f98f8d5ce60aa5bf3ebe Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 13 Nov 2020 15:43:30 +0100 Subject: [PATCH] gpg: Fix the encrypt+sign hash algo preference selection for ECDSA. * g10/keydb.h (pref_hint): Change from union to struct and add field 'exact'. Adjust callers. * g10/pkclist.c (algo_available): Take care of the exact hint. * g10/sign.c (sign_file): Fix indentation. Rework the hash from recipient prefs. -- This fixes a encrypt+sign case like: One recipient key has SHA512 as highest ranked hash preference but the the signing key is a 256 bit curve. Because we don't want to use a truncated hash with ECDSA, we need to have an exact match - this is in particular important for smartcard which check that the hash matches the curves. Signed-off-by: Werner Koch --- g10/keydb.h | 9 +++---- g10/pkclist.c | 20 ++++++++++++---- g10/sign.c | 65 +++++++++++++++++++++++++++++---------------------- 3 files changed, 57 insertions(+), 37 deletions(-) diff --git a/g10/keydb.h b/g10/keydb.h index f61515f42..e1a316cca 100644 --- a/g10/keydb.h +++ b/g10/keydb.h @@ -128,9 +128,10 @@ struct pubkey_find_info { /* Helper type for preference functions. */ -union pref_hint +struct pref_hint { - int digest_length; + int digest_length; /* We want at least this digest length. */ + int exact; /* We need to use exactly this length. */ }; @@ -262,9 +263,9 @@ gpg_error_t find_and_check_key (ctrl_t ctrl, pk_list_t *pk_list_addr); int algo_available( preftype_t preftype, int algo, - const union pref_hint *hint ); + const struct pref_hint *hint ); int select_algo_from_prefs( PK_LIST pk_list, int preftype, - int request, const union pref_hint *hint); + int request, const struct pref_hint *hint); int select_mdc_from_pklist (PK_LIST pk_list); void warn_missing_mdc_from_pklist (PK_LIST pk_list); void warn_missing_aes_from_pklist (PK_LIST pk_list); diff --git a/g10/pkclist.c b/g10/pkclist.c index 542164949..bfc4f84e4 100644 --- a/g10/pkclist.c +++ b/g10/pkclist.c @@ -1369,7 +1369,7 @@ build_pk_list (ctrl_t ctrl, strlist_t rcpts, PK_LIST *ret_pk_list) preference list, so I'm including it. -dms */ int -algo_available( preftype_t preftype, int algo, const union pref_hint *hint) +algo_available( preftype_t preftype, int algo, const struct pref_hint *hint) { if( preftype == PREFTYPE_SYM ) { @@ -1395,16 +1395,26 @@ algo_available( preftype_t preftype, int algo, const union pref_hint *hint) { if (hint && hint->digest_length) { - if (hint->digest_length!=20 || opt.flags.dsa2) + unsigned int n = gcry_md_get_algo_dlen (algo); + + if (hint->exact) + { + /* For example ECDSA requires an exact hash value so + * that we do not truncate. For DSA we allow truncation + * and thus exact is not set. */ + if (hint->digest_length != n) + return 0; + } + else if (hint->digest_length!=20 || opt.flags.dsa2) { /* If --enable-dsa2 is set or the hash isn't 160 bits (which implies DSA2), then we'll accept a hash that is larger than we need. Otherwise we won't accept any hash that isn't exactly the right size. */ - if (hint->digest_length > gcry_md_get_algo_dlen (algo)) + if (hint->digest_length > n) return 0; } - else if (hint->digest_length != gcry_md_get_algo_dlen (algo)) + else if (hint->digest_length != n) return 0; } @@ -1441,7 +1451,7 @@ algo_available( preftype_t preftype, int algo, const union pref_hint *hint) */ int select_algo_from_prefs(PK_LIST pk_list, int preftype, - int request, const union pref_hint *hint) + int request, const struct pref_hint *hint) { PK_LIST pkr; u32 bits[8]; diff --git a/g10/sign.c b/g10/sign.c index b437190af..6a90fe16e 100644 --- a/g10/sign.c +++ b/g10/sign.c @@ -905,7 +905,8 @@ write_signature_packets (ctrl_t ctrl, * and ignore the detached mode. Sign the file with all secret keys * which can be taken from LOCUSR, if this is NULL, use the default one * If ENCRYPTFLAG is true, use REMUSER (or ask if it is NULL) to encrypt the - * signed data for these users. + * signed data for these users. If ENCRYPTFLAG is 2 symmetric encryption + * is also used. * If OUTFILE is not NULL; this file is used for output and the function * does not ask for overwrite permission; output is then always * uncompressed, non-armored and in binary mode. @@ -1035,17 +1036,16 @@ sign_file (ctrl_t ctrl, strlist_t filenames, int detached, strlist_t locusr, select_algo_from_prefs(pk_list,PREFTYPE_HASH, opt.def_digest_algo, NULL)!=opt.def_digest_algo) - log_info(_("WARNING: forcing digest algorithm %s (%d)" - " violates recipient preferences\n"), - gcry_md_algo_name (opt.def_digest_algo), - opt.def_digest_algo ); + log_info(_("WARNING: forcing digest algorithm %s (%d)" + " violates recipient preferences\n"), + gcry_md_algo_name (opt.def_digest_algo), + opt.def_digest_algo ); } else { - int algo, smartcard=0; - union pref_hint hint; - - hint.digest_length = 0; + int algo; + int conflict = 0; + struct pref_hint hint = { 0 }; /* Of course, if the recipient asks for something unreasonable (like the wrong hash for a DSA key) then @@ -1073,31 +1073,40 @@ sign_file (ctrl_t ctrl, strlist_t filenames, int detached, strlist_t locusr, (sk_rover->pk->pkey[1])); if (sk_rover->pk->pubkey_algo == PUBKEY_ALGO_ECDSA) - temp_hashlen = ecdsa_qbits_from_Q (temp_hashlen); - temp_hashlen = (temp_hashlen+7)/8; + { + temp_hashlen = ecdsa_qbits_from_Q (temp_hashlen); + if (!temp_hashlen) + conflict = 1; /* Better don't use the prefs. */ + temp_hashlen = (temp_hashlen+7)/8; + /* Fixup for that funny nistp521 (yes, 521) + * were we need to use a 512 bit hash algo. */ + if (temp_hashlen == 66) + temp_hashlen = 64; + } + else + temp_hashlen = (temp_hashlen+7)/8; /* Pick a hash that is large enough for our - largest q */ + largest q or matches our Q but if tehreare + several of them we run into a conflict and + don't use the preferences. */ - if (hint.digest_lengthpk->pubkey_algo == PUBKEY_ALGO_ECDSA) + { + if (hint.exact) + conflict = 1; + hint.exact = 1; + } + hint.digest_length = temp_hashlen; + } } - /* FIXME: need to check gpg-agent for this. */ - /* else if (sk_rover->pk->is_protected */ - /* && sk_rover->pk->protect.s2k.mode == 1002) */ - /* smartcard = 1; */ } - /* Current smartcards only do 160-bit hashes. If we have - to have a >160-bit hash, then we can't use the - recipient prefs as we'd need both =160 and >160 at the - same time and recipient prefs currently require a - single hash for all signatures. All this may well have - to change as the cards add algorithms. */ - - if ((!smartcard || (smartcard && hint.digest_length==20)) - && (algo = select_algo_from_prefs(pk_list,PREFTYPE_HASH, - -1,&hint)) > 0) + if (!conflict + && (algo = select_algo_from_prefs (pk_list,PREFTYPE_HASH, + -1,&hint)) > 0) { /* Note that we later check that the algo is not weak. */ recipient_digest_algo = algo;