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 <wk@gnupg.org>
This commit is contained in:
Werner Koch 2020-11-13 15:43:30 +01:00
parent f400ff4e7d
commit aeed0b93ff
No known key found for this signature in database
GPG Key ID: E3FDFF218E45B72B
3 changed files with 57 additions and 37 deletions

View File

@ -128,9 +128,10 @@ struct pubkey_find_info {
/* Helper type for preference functions. */ /* 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); pk_list_t *pk_list_addr);
int algo_available( preftype_t preftype, int algo, 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 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); int select_mdc_from_pklist (PK_LIST pk_list);
void warn_missing_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); void warn_missing_aes_from_pklist (PK_LIST pk_list);

View File

@ -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 */ preference list, so I'm including it. -dms */
int 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 ) 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 && 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 /* If --enable-dsa2 is set or the hash isn't 160 bits
(which implies DSA2), then we'll accept a hash that (which implies DSA2), then we'll accept a hash that
is larger than we need. Otherwise we won't accept is larger than we need. Otherwise we won't accept
any hash that isn't exactly the right size. */ 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; return 0;
} }
else if (hint->digest_length != gcry_md_get_algo_dlen (algo)) else if (hint->digest_length != n)
return 0; return 0;
} }
@ -1441,7 +1451,7 @@ algo_available( preftype_t preftype, int algo, const union pref_hint *hint)
*/ */
int int
select_algo_from_prefs(PK_LIST pk_list, int preftype, 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; PK_LIST pkr;
u32 bits[8]; u32 bits[8];

View File

@ -905,7 +905,8 @@ write_signature_packets (ctrl_t ctrl,
* and ignore the detached mode. Sign the file with all secret keys * 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 * 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 * 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 * If OUTFILE is not NULL; this file is used for output and the function
* does not ask for overwrite permission; output is then always * does not ask for overwrite permission; output is then always
* uncompressed, non-armored and in binary mode. * 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, select_algo_from_prefs(pk_list,PREFTYPE_HASH,
opt.def_digest_algo, opt.def_digest_algo,
NULL)!=opt.def_digest_algo) NULL)!=opt.def_digest_algo)
log_info(_("WARNING: forcing digest algorithm %s (%d)" log_info(_("WARNING: forcing digest algorithm %s (%d)"
" violates recipient preferences\n"), " violates recipient preferences\n"),
gcry_md_algo_name (opt.def_digest_algo), gcry_md_algo_name (opt.def_digest_algo),
opt.def_digest_algo ); opt.def_digest_algo );
} }
else else
{ {
int algo, smartcard=0; int algo;
union pref_hint hint; int conflict = 0;
struct pref_hint hint = { 0 };
hint.digest_length = 0;
/* Of course, if the recipient asks for something /* Of course, if the recipient asks for something
unreasonable (like the wrong hash for a DSA key) then 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])); (sk_rover->pk->pkey[1]));
if (sk_rover->pk->pubkey_algo == PUBKEY_ALGO_ECDSA) 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 /* 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_length<temp_hashlen) if (hint.digest_length < temp_hashlen)
hint.digest_length=temp_hashlen; {
if (sk_rover->pk->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 if (!conflict
to have a >160-bit hash, then we can't use the && (algo = select_algo_from_prefs (pk_list,PREFTYPE_HASH,
recipient prefs as we'd need both =160 and >160 at the -1,&hint)) > 0)
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)
{ {
/* Note that we later check that the algo is not weak. */ /* Note that we later check that the algo is not weak. */
recipient_digest_algo = algo; recipient_digest_algo = algo;