gpg: Cleanup use of make_keysig_packet.
authorWerner Koch <wk@gnupg.org>
Mon, 13 May 2019 10:38:32 +0000 (12:38 +0200)
committerWerner Koch <wk@gnupg.org>
Mon, 13 May 2019 10:39:17 +0000 (12:39 +0200)
* 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
g10/gpgcompose.c
g10/keyedit.c
g10/keygen.c
g10/packet.h
g10/revoke.c
g10/sign.c

index d3b7be5..df807fa 100644 (file)
@@ -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
-that GnuPG supports but other OpenPGP implementations do not, then some
-users will not be able to use the key signatures you make, or quite
-possibly your entire key.
+list of supported algorithms.  Be aware that if you choose an
+algorithm that GnuPG supports but other OpenPGP implementations do
+not, then some users will not be able to use the key signatures you
+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
index e882fa8..9e6d51a 100644 (file)
@@ -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);
index c28a565..7f4c5a5 100644 (file)
@@ -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)
            {
index 22ac6f0..ac6bcc8 100644 (file)
@@ -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)
index d05a8f0..d787bde 100644 (file)
@@ -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,
index e8ce354..e63060c 100644 (file)
@@ -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)
index 176940b..132b941 100644 (file)
@@ -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,31 +1613,22 @@ 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.  */
+    digest_algo = opt.cert_digest_algo;
+  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 /* Meet ECDSA requirements. */
+           || pksk->pubkey_algo == PUBKEY_ALGO_EDDSA)
     {
-      /* 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)
-        digest_algo = match_dsa_hash (gcry_mpi_get_nbits (pksk->pkey[1])/8);
-      else if (pksk->pubkey_algo == PUBKEY_ALGO_ECDSA
-               || pksk->pubkey_algo == PUBKEY_ALGO_EDDSA)
-        {
-          if (openpgp_oid_is_ed25519 (pksk->pkey[0]))
-            digest_algo = DIGEST_ALGO_SHA256;
-          else
-            digest_algo = match_dsa_hash
-              (ecdsa_qbits_from_Q (gcry_mpi_get_nbits (pksk->pkey[1]))/8);
-        }
+      if (openpgp_oid_is_ed25519 (pksk->pkey[0]))
+        digest_algo = DIGEST_ALGO_SHA256;
       else
-        digest_algo = DEFAULT_DIGEST_ALGO;
+        digest_algo = match_dsa_hash
+          (ecdsa_qbits_from_Q (gcry_mpi_get_nbits (pksk->pkey[1]))/8);
     }
+  else /* Use the default.  */
+    digest_algo = DEFAULT_DIGEST_ALGO;
 
   if (gcry_md_open (&md, digest_algo, 0))
     BUG ();