gpg: Make the use of "--verify FILE" for detached sigs harder.

* g10/openfile.c (open_sigfile): Factor some code out to ...
(get_matching_datafile): new function.
* g10/plaintext.c (hash_datafiles): Do not try to find matching file
in batch mode.
* g10/mainproc.c (check_sig_and_print): Print a warning if a possibly
matching data file is not used by a standard signatures.
--

Allowing to use the abbreviated form for detached signatures is a long
standing bug which has only been noticed by the public with the
release of 2.1.0.  :-(

What we do is to remove the ability to check detached signature in
--batch using the one file abbreviated mode.  This should exhibit
problems in scripts which use this insecure practice.  We also print a
warning if a matching data file exists but was not considered because
the detached signature was actually a standard signature:

  gpgv: Good signature from "Werner Koch (dist sig)"
  gpgv: WARNING: not a detached signature; \
  file 'gnupg-2.1.0.tar.bz2' was NOT verified!

We can only print a warning because it is possible that a standard
signature is indeed to be verified but by coincidence a file with a
matching name is stored alongside the standard signature.

Reported-by: Simon Nicolussi (to gnupg-users on Nov 7)
Signed-off-by: Werner Koch <wk@gnupg.org>

doc/gpg.texi

@@ -214,16 +214,22 @@ files which don't begin with an encrypted message.
 
 @item --verify
 @opindex verify
-Assume that the first argument is a signed file or a detached signature
-and verify it without generating any output. With no arguments, the
-signature packet is read from STDIN. If only a sigfile is given, it may
-be a complete signature or a detached signature, in which case the
-signed stuff is expected in a file without the ".sig" or ".asc"
-extension.  With more than 1 argument, the first should be a detached
-signature and the remaining files are the signed stuff. To read the
-signed stuff from STDIN, use @samp{-} as the second filename.  For
-security reasons a detached signature cannot read the signed material
-from STDIN without denoting it in the above way.
+Assume that the first argument is a signed file and verify it without
+generating any output.  With no arguments, the signature packet is
+read from STDIN.  If only a one argument is given, it is expected to
+be a complete signature.
+
+With more than 1 argument, the first should be a detached signature
+and the remaining files ake up the the signed data. To read the signed
+data from STDIN, use @samp{-} as the second filename.  For security
+reasons a detached signature cannot read the signed material from
+STDIN without denoting it in the above way.
+
+Note: If the option @option{--batch} is not used, @command{@gpgname}
+may assume that a single argument is a file with a detached signature
+and it will try to find a matching data file by stripping certain
+suffixes.  Using this historical feature to verify a detached
+signature is strongly discouraged; always specify the data file too.
 
 Note: When verifying a cleartext signature, @command{gpg} verifies
 only what makes up the cleartext signed data and not any extra data

g10/main.h

@@ -286,7 +286,8 @@ char *make_outfile_name( const char *iname );
 char *ask_outfile_name( const char *name, size_t namelen );
 int open_outfile (int inp_fd, const char *iname, int mode,
                   int restrictedperm, iobuf_t *a);
-iobuf_t open_sigfile( const char *iname, progress_filter_context_t *pfx );
+char *get_matching_datafile (const char *sigfilename);
+iobuf_t open_sigfile (const char *sigfilename, progress_filter_context_t *pfx);
 void try_make_homedir( const char *fname );
 char *get_openpgp_revocdir (const char *home);
 
@@ -374,7 +375,7 @@ void decrypt_messages (ctrl_t ctrl, int nfiles, char *files[]);
 
 /*-- plaintext.c --*/
 int hash_datafiles( gcry_md_hd_t md, gcry_md_hd_t md2,
-		    strlist_t files, const char *sigfilename, int textmode );
+		    strlist_t files, const char *sigfilename, int textmode);
 int hash_datafile_by_fd ( gcry_md_hd_t md, gcry_md_hd_t md2, int data_fd,
                           int textmode );
 PKT_plaintext *setup_plaintext_name(const char *filename,IOBUF iobuf);

g10/mainproc.c

@@ -2008,6 +2008,44 @@ check_sig_and_print (CTX c, kbnode_t node)
                   *pkstrbuf?_(", key algorithm "):"",
                   pkstrbuf);
 
+      if (!rc && !c->signed_data.used)
+        {
+          /* Signature is basically good but we test whether the
+             deprecated command
+               gpg --verify FILE.sig
+             was used instead of
+               gpg --verify FILE.sig FILE
+             to verify a detached signature.  If we figure out that a
+             data file with a matching name exists, we print a warning.
+
+             The problem is that the first form would also verify a
+             standard signature.  This behavior could be used to
+             create a made up .sig file for a tarball by creating a
+             standard signature from a valid detached signature packet
+             (for example from a signed git tag).  Then replace the
+             sig file on the FTP server along with a changed tarball.
+             Using the first form the verify command would correctly
+             verify the signature but don't even consider the tarball.  */
+          kbnode_t n;
+          char *dfile;
+
+          dfile = get_matching_datafile (c->sigfilename);
+          if (dfile)
+            {
+              for (n = c->list; n; n = n->next)
+                if (n->pkt->pkttype != PKT_SIGNATURE)
+                  break;
+              if (n)
+                {
+                  /* Not only signature packets in the tree thus this
+                     is not a detached signature.  */
+                  log_info (_("WARNING: not a detached signature; "
+                              "file '%s' was NOT verified!\n"), dfile);
+                }
+              xfree (dfile);
+            }
+        }
+
       if (rc)
         g10_errors_seen = 1;
       if (opt.batch && rc)

g10/openfile.c

@@ -310,40 +310,64 @@ open_outfile (int inp_fd, const char *iname, int mode, int restrictedperm,
 }
 
 
+/* Find a matching data file for the signature file SIGFILENAME and
+   return it as a malloced string.  If no matching data file is found,
+   return NULL.  */
+char *
+get_matching_datafile (const char *sigfilename)
+{
+  char *fname = NULL;
+  size_t len;
+
+  if (iobuf_is_pipe_filename (sigfilename))
+    return NULL;
+
+  len = strlen (sigfilename);
+  if (len > 4
+      && (!strcmp (sigfilename + len - 4, EXTSEP_S "sig")
+          || (len > 5 && !strcmp(sigfilename + len - 5, EXTSEP_S "sign"))
+          || !strcmp(sigfilename + len - 4, EXTSEP_S "asc")))
+    {
+
+      fname = xstrdup (sigfilename);
+      fname[len-(fname[len-1]=='n'?5:4)] = 0 ;
+      if (access (fname, R_OK ))
+        {
+          /* Not found or other error.  */
+          xfree (fname);
+          fname = NULL;
+        }
+    }
+
+  return fname;
+}
+
+
 /*
  * Try to open a file without the extension ".sig" or ".asc"
  * Return NULL if such a file is not available.
  */
-IOBUF
-open_sigfile( const char *iname, progress_filter_context_t *pfx )
+iobuf_t
+open_sigfile (const char *sigfilename, progress_filter_context_t *pfx)
 {
-  IOBUF a = NULL;
-  size_t len;
+  iobuf_t a = NULL;
+  char *buf;
 
-  if (!iobuf_is_pipe_filename (iname))
+  buf = get_matching_datafile (sigfilename);
+  if (buf)
     {
-      len = strlen(iname);
-      if( len > 4 && (!strcmp(iname + len - 4, EXTSEP_S "sig")
-                      || (len > 5 && !strcmp(iname + len - 5, EXTSEP_S "sign"))
-                      || !strcmp(iname + len - 4, EXTSEP_S "asc")))
+      a = iobuf_open (buf);
+      if (a && is_secured_file (iobuf_get_fd (a)))
         {
-          char *buf;
-
-          buf = xstrdup(iname);
-          buf[len-(buf[len-1]=='n'?5:4)] = 0 ;
-          a = iobuf_open( buf );
-          if (a && is_secured_file (iobuf_get_fd (a)))
-            {
-              iobuf_close (a);
-              a = NULL;
-              gpg_err_set_errno (EPERM);
-            }
-          if (a && opt.verbose)
-            log_info (_("assuming signed data in '%s'\n"), buf);
-          if (a && pfx)
-            handle_progress (pfx, a, buf);
-          xfree (buf);
-	}
+          iobuf_close (a);
+          a = NULL;
+          gpg_err_set_errno (EPERM);
+        }
+      if (a)
+        log_info (_("assuming signed data in '%s'\n"), buf);
+      if (a && pfx)
+        handle_progress (pfx, a, buf);
+      xfree (buf);
     }
 
   return a;

g10/plaintext.c

@@ -607,10 +607,8 @@ leave:
 
 
 
-/****************
- * Hash the given files and append the hash to hash context md.
- * If FILES is NULL, hash stdin.
- */
+/* Hash the given files and append the hash to hash contexts MD and
+ * MD2.  If FILES is NULL, stdin is hashed.  */
 int
 hash_datafiles (gcry_md_hd_t md, gcry_md_hd_t md2, strlist_t files,
 		const char *sigfilename, int textmode)
@@ -623,15 +621,22 @@ hash_datafiles (gcry_md_hd_t md, gcry_md_hd_t md2, strlist_t files,
 
   if (!files)
     {
-      /* check whether we can open the signed material */
-      fp = open_sigfile (sigfilename, pfx);
-      if (fp)
-	{
-	  do_hash (md, md2, fp, textmode);
-	  iobuf_close (fp);
-	  release_progress_context (pfx);
-	  return 0;
-	}
+      /* Check whether we can open the signed material.  We avoid
+         trying to open a file if run in batch mode.  This assumed
+         data file for a sig file feature is just a convenience thing
+         for the command line and the user needs to read possible
+         warning messages. */
+      if (!opt.batch)
+        {
+          fp = open_sigfile (sigfilename, pfx);
+          if (fp)
+            {
+              do_hash (md, md2, fp, textmode);
+              iobuf_close (fp);
+              release_progress_context (pfx);
+              return 0;
+            }
+        }
       log_error (_("no signed data\n"));
       release_progress_context (pfx);
       return gpg_error (GPG_ERR_NO_DATA);