gpg: Make the use of "--verify FILE" for detached sigs harder.
authorWerner Koch <wk@gnupg.org>
Fri, 14 Nov 2014 08:36:19 +0000 (09:36 +0100)
committerWerner Koch <wk@gnupg.org>
Fri, 14 Nov 2014 08:36:19 +0000 (09:36 +0100)
* 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>
(backported from commit 69384568f66a48eff3968bb1714aa13925580e9f)

g10/main.h
g10/mainproc.c
g10/openfile.c
g10/plaintext.c

index e97b936..b55a184 100644 (file)
@@ -253,7 +253,8 @@ int overwrite_filep( const char *fname );
 char *make_outfile_name( const char *iname );
 char *ask_outfile_name( const char *name, size_t namelen );
 int   open_outfile( const char *iname, int mode, 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 );
 
 /*-- seskey.c --*/
index 551ab58..3abcb15 100644 (file)
@@ -1959,6 +1959,44 @@ check_sig_and_print( CTX c, KBNODE node )
                   sig->sig_class==0x01?_("textmode"):_("unknown"),
                   gcry_md_algo_name (sig->digest_algo));
 
+        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 )
index db5cdc2..dc9dfd0 100644 (file)
@@ -287,41 +287,70 @@ open_outfile( const char *iname, int mode, IOBUF *a )
 }
 
 
+/* 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;
-
-    if( !iobuf_is_pipe_filename (iname) ) {
-       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")) ) {
-           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;
-                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_t a = NULL;
+  char *buf;
+
+  buf = get_matching_datafile (sigfilename);
+  if (buf)
+    {
+      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)
+        log_info (_("assuming signed data in '%s'\n"), buf);
+      if (a && pfx)
+        handle_progress (pfx, a, buf);
+      xfree (buf);
     }
-    return a;
+
+  return a;
 }
 
+
 /****************
  * Copy the option file skeleton to the given directory.
  */
index 3777648..d24c640 100644 (file)
@@ -546,17 +546,25 @@ hash_datafiles( gcry_md_hd_t md, gcry_md_hd_t md2, strlist_t files,
     pfx = new_progress_context ();
 
     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;
-       }
-        log_error (_("no signed data\n"));
-        release_progress_context (pfx);
-        return gpg_error (GPG_ERR_NO_DATA);
+      /* 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);
     }
 
 
@@ -615,7 +623,7 @@ hash_datafile_by_fd ( gcry_md_hd_t md, gcry_md_hd_t md2, int data_fd,
   do_hash ( md, md2, fp, textmode);
 
   iobuf_close(fp);
-  
+
   release_progress_context (pfx);
   return 0;
 }