gpg: Cleanup use of make_keysig_packet.

* g10/sign.c (make_keysig_packet): Remove obsolete arg diegst_algo
which was always passed as 0.  Change all callers.

* g10/gpgcompose.c (signature): Warn when trying to set a digest algo.
--

Signed-off-by: Werner Koch <wk@gnupg.org>

doc/gpg.texi

@@ -3081,10 +3081,14 @@ the same thing.
 @opindex cert-digest-algo
 Use @var{name} as the message digest algorithm used when signing a
 key. Running the program with the command @option{--version} yields a
-list of supported algorithms. Be aware that if you choose an algorithm
+list of supported algorithms.  Be aware that if you choose an
-that GnuPG supports but other OpenPGP implementations do not, then some
+algorithm that GnuPG supports but other OpenPGP implementations do
-users will not be able to use the key signatures you make, or quite
+not, then some users will not be able to use the key signatures you
-possibly your entire key.
+make, or quite possibly your entire key.  Note also that a public key
+algorithm must be compatible with the specified digest algorithm; thus
+selecting an arbitrary digest algorithm may result in error messages
+from lower crypto layers or lead to security flaws.
+
 
 @item --disable-cipher-algo @var{name}
 @opindex disable-cipher-algo

g10/gpgcompose.c

@@ -1799,12 +1799,19 @@ signature (const char *option, int argc, char *argv[], void *cookie)
       keyid_copy (si.issuer_pk->keyid, pk_keyid (pripk));
     }
 
+  /* The reuse of core gpg stuff by this tool is questionable when it
+   * requires adding extra code to the actual gpg code.  It does not
+   * make sense to pass an extra parameter and in particular not given
+   * that gpg already has opt.cert_digest_algo to override it.  */
+  if (si.digest_algo)
+    log_info ("note: digest algo can't be passed to make_keysig_packet\n");
+
   /* Changing the issuer's key id is fragile.  Check to make sure
      make_keysig_packet didn't recompute the keyid.  */
   keyid_copy (keyid, si.issuer_pk->keyid);
   err = make_keysig_packet (global_ctrl,
                             &sig, si.pk, si.uid, si.sk, si.issuer_pk,
-                            si.class, si.digest_algo,
+                            si.class,
                             si.timestamp, si.expiration,
                             mksubpkt_callback, &si, NULL);
   log_assert (keyid_cmp (keyid, si.issuer_pk->keyid) == 0);

g10/keyedit.c

@@ -1012,7 +1012,8 @@ sign_uids (ctrl_t ctrl, estream_t fp,
 					 node->pkt->pkt.user_id,
 					 NULL,
 					 pk,
-					 0x13, 0, 0, 0,
+					 0x13,
+                                         0, 0,
 					 keygen_add_std_prefs, primary_pk,
                                          NULL);
 	      else
@@ -1020,7 +1021,7 @@ sign_uids (ctrl_t ctrl, estream_t fp,
 					 node->pkt->pkt.user_id,
 					 NULL,
 					 pk,
-					 class, 0,
+					 class,
 					 timestamp, duration,
 					 sign_mk_attrib, &attrib,
                                          NULL);
@@ -3991,7 +3992,7 @@ menu_adduid (ctrl_t ctrl, kbnode_t pub_keyblock,
       return 0;
     }
 
-  err = make_keysig_packet (ctrl, &sig, pk, uid, NULL, pk, 0x13, 0, 0, 0,
+  err = make_keysig_packet (ctrl, &sig, pk, uid, NULL, pk, 0x13, 0, 0,
                             keygen_add_std_prefs, pk, NULL);
   if (err)
     {
@@ -4374,7 +4375,7 @@ menu_addrevoker (ctrl_t ctrl, kbnode_t pub_keyblock, int sensitive)
       break;
     }
 
-  rc = make_keysig_packet (ctrl, &sig, pk, NULL, NULL, pk, 0x1F, 0, 0, 0,
+  rc = make_keysig_packet (ctrl, &sig, pk, NULL, NULL, pk, 0x1F, 0, 0,
 			   keygen_add_revkey, &revkey, NULL);
   if (rc)
     {
@@ -5898,7 +5899,7 @@ reloop:			/* (must use this, because we are modifying the list) */
 	}
       rc = make_keysig_packet (ctrl, &sig, primary_pk,
 			       unode->pkt->pkt.user_id,
-			       NULL, signerkey, 0x30, 0, 0, 0,
+			       NULL, signerkey, 0x30, 0, 0,
                                sign_mk_attrib, &attrib, NULL);
       free_public_key (signerkey);
       if (rc)
@@ -5977,11 +5978,11 @@ core_revuid (ctrl_t ctrl, kbnode_t keyblock, KBNODE node,
           memset (&attrib, 0, sizeof attrib);
           /* should not need to cast away const here; but
              revocation_reason_build_cb needs to take a non-const
-             void* in order to meet the function signtuare for the
+             void* in order to meet the function signutare for the
              mksubpkt argument to make_keysig_packet */
           attrib.reason = (struct revocation_reason_info *)reason;
 
-          rc = make_keysig_packet (ctrl, &sig, pk, uid, NULL, pk, 0x30, 0,
+          rc = make_keysig_packet (ctrl, &sig, pk, uid, NULL, pk, 0x30,
                                    timestamp, 0,
                                    sign_mk_attrib, &attrib, NULL);
           if (rc)
@@ -6111,7 +6112,7 @@ menu_revkey (ctrl_t ctrl, kbnode_t pub_keyblock)
     return 0;
 
   rc = make_keysig_packet (ctrl, &sig, pk, NULL, NULL, pk,
-			   0x20, 0, 0, 0,
+			   0x20, 0, 0,
 			   revocation_reason_build_cb, reason, NULL);
   if (rc)
     {
@@ -6173,7 +6174,7 @@ menu_revsubkey (ctrl_t ctrl, kbnode_t pub_keyblock)
 
 	  node->flag &= ~NODFLG_SELKEY;
 	  rc = make_keysig_packet (ctrl, &sig, mainpk, NULL, subpk, mainpk,
-				   0x28, 0, 0, 0, sign_mk_attrib, &attrib,
+				   0x28, 0, 0, sign_mk_attrib, &attrib,
                                    NULL);
 	  if (rc)
 	    {

g10/keygen.c

@@ -1022,7 +1022,7 @@ make_backsig (ctrl_t ctrl, PKT_signature *sig, PKT_public_key *pk,
   cache_public_key (sub_pk);
 
   err = make_keysig_packet (ctrl, &backsig, pk, NULL, sub_pk, sub_psk, 0x19,
-                            0, timestamp, 0, NULL, NULL, cache_nonce);
+                            timestamp, 0, NULL, NULL, cache_nonce);
   if (err)
     log_error ("make_keysig_packet failed for backsig: %s\n",
                gpg_strerror (err));
@@ -1130,7 +1130,7 @@ write_direct_sig (ctrl_t ctrl, kbnode_t root, PKT_public_key *psk,
 
   /* Make the signature.  */
   err = make_keysig_packet (ctrl, &sig, pk, NULL,NULL, psk, 0x1F,
-                            0, timestamp, 0,
+                            timestamp, 0,
                             keygen_add_revkey, revkey, cache_nonce);
   if (err)
     {
@@ -1185,7 +1185,7 @@ write_selfsigs (ctrl_t ctrl, kbnode_t root, PKT_public_key *psk,
 
   /* Make the signature.  */
   err = make_keysig_packet (ctrl, &sig, pk, uid, NULL, psk, 0x13,
-                            0, timestamp, 0,
+                            timestamp, 0,
                             keygen_add_std_prefs, pk, cache_nonce);
   if (err)
     {
@@ -1245,7 +1245,7 @@ write_keybinding (ctrl_t ctrl, kbnode_t root,
   oduap.usage = use;
   oduap.pk = sub_pk;
   err = make_keysig_packet (ctrl, &sig, pri_pk, NULL, sub_pk, pri_psk, 0x18,
-                            0, timestamp, 0,
+                            timestamp, 0,
                             keygen_add_key_flags_and_expire, &oduap,
                             cache_nonce);
   if (err)

g10/packet.h

@@ -931,7 +931,7 @@ int ask_for_detached_datafile( gcry_md_hd_t md, gcry_md_hd_t md2,
 int make_keysig_packet (ctrl_t ctrl,
                         PKT_signature **ret_sig, PKT_public_key *pk,
 			PKT_user_id *uid, PKT_public_key *subpk,
-			PKT_public_key *pksk, int sigclass, int digest_algo,
+			PKT_public_key *pksk, int sigclass,
 			u32 timestamp, u32 duration,
 			int (*mksubpkt)(PKT_signature *, void *),
 			void *opaque,

g10/revoke.c

@@ -343,7 +343,7 @@ gen_desig_revoke (ctrl_t ctrl, const char *uname, strlist_t locusr)
 	    push_armor_filter (afx, out);
 
 	    /* create it */
-	    rc = make_keysig_packet (ctrl, &sig, pk, NULL, NULL, pk2, 0x20, 0,
+	    rc = make_keysig_packet (ctrl, &sig, pk, NULL, NULL, pk2, 0x20,
 				     0, 0,
 				     revocation_reason_build_cb, reason,
                                      NULL);
@@ -474,7 +474,7 @@ create_revocation (ctrl_t ctrl,
   afx->hdrlines = "Comment: This is a revocation certificate\n";
   push_armor_filter (afx, out);
 
-  rc = make_keysig_packet (ctrl, &sig, psk, NULL, NULL, psk, 0x20, 0,
+  rc = make_keysig_packet (ctrl, &sig, psk, NULL, NULL, psk, 0x20,
                            0, 0,
                            revocation_reason_build_cb, reason, cache_nonce);
   if (rc)

g10/sign.c

@@ -1593,7 +1593,7 @@ make_keysig_packet (ctrl_t ctrl,
                     PKT_signature **ret_sig, PKT_public_key *pk,
 		    PKT_user_id *uid, PKT_public_key *subpk,
 		    PKT_public_key *pksk,
-		    int sigclass, int digest_algo,
+		    int sigclass,
                     u32 timestamp, u32 duration,
 		    int (*mksubpkt)(PKT_signature *, void *), void *opaque,
                     const char *cache_nonce)
@@ -1601,6 +1601,7 @@ make_keysig_packet (ctrl_t ctrl,
   PKT_signature *sig;
   int rc = 0;
   int sigversion;
+  int digest_algo;
   gcry_md_hd_t md;
 
   log_assert ((sigclass >= 0x10 && sigclass <= 0x13) || sigclass == 0x1F
@@ -1612,20 +1613,12 @@ make_keysig_packet (ctrl_t ctrl,
   else
     sigversion = 4;
 
-  if (!digest_algo)
+  /* Select the digest algo to use. */
-    {
+  if (opt.cert_digest_algo)     /* Forceful override by the user.  */
-      /* Basically, this means use SHA1 always unless the user
-       * specified something (use whatever they said), or it's DSA
-       * (use the best match).  They still can't pick an inappropriate
-       * hash for DSA or the signature will fail.  Note that this
-       * still allows the caller of make_keysig_packet to override the
-       * user setting if it must. */
-
-      if (opt.cert_digest_algo)
     digest_algo = opt.cert_digest_algo;
-      else if (pksk->pubkey_algo == PUBKEY_ALGO_DSA)
+  else if (pksk->pubkey_algo == PUBKEY_ALGO_DSA) /* Meet DSA requirements.  */
     digest_algo = match_dsa_hash (gcry_mpi_get_nbits (pksk->pkey[1])/8);
-      else if (pksk->pubkey_algo == PUBKEY_ALGO_ECDSA
+  else if (pksk->pubkey_algo == PUBKEY_ALGO_ECDSA /* Meet ECDSA requirements. */
            || pksk->pubkey_algo == PUBKEY_ALGO_EDDSA)
     {
       if (openpgp_oid_is_ed25519 (pksk->pkey[0]))
@@ -1634,9 +1627,8 @@ make_keysig_packet (ctrl_t ctrl,
         digest_algo = match_dsa_hash
           (ecdsa_qbits_from_Q (gcry_mpi_get_nbits (pksk->pkey[1]))/8);
     }
-      else
+  else /* Use the default.  */
     digest_algo = DEFAULT_DIGEST_ALGO;
-    }
 
   if (gcry_md_open (&md, digest_algo, 0))
     BUG ();