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 <[email protected]>

3 files changed, 58 insertions, 38 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 */
-
-		    if (hint.digest_length<temp_hashlen)
-		      hint.digest_length=temp_hashlen;
+		       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 (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
-	       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;