Package: gnupg
Version: 1.4.12-7+deb7u6
Severity: important
Tags: patch upstream

OpenPGP signatures come in two forms: detached (where the signature is
in a separate file from the data being signed) and bundled (where one
file contains both the signed message and the signature).

GnuPG is used to verify both detached and bundled OpenPGP signatures.

The canonical way to verify a bundled OpenPGP signature is:

 gpg --verify message.txt

The canonical way to verify a detached OpenPGP signature is:

 gpg --verify message.txt.sig message.txt

GnuPG has traditionally offered a "convenience" mode for verifying
detached OpenPGP signatures:

 gpg --verify message.txt.sig

(in this case, gpg automatically guesses to look for message.txt for
the signed text).

Unfortunately, this convenience mode can be exploited by an attacker
who ships a bundled signature as though it were a detached signature,
tricking the user into believing that any arbitrary file with the
right name is actually signed.

To avoid this threat, detached signatures should always be verified
by:

 gpg --verify SIG FILE

This was noted publicly on gnupg-users about a month ago:

http://lists.gnupg.org/pipermail/gnupg-users/2014-November/051333.html

And upstream is moving to rapidly deprecate the unsafe convenience mode.

The attached patch is how upstream fixes the problem after GnuPG
1.4.18.

        --dkg


-- System Information:
Debian Release: 7.7
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-4-amd64 (SMP w/1 CPU core)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages gnupg depends on:
ii  dpkg          1.16.15
ii  gpgv          1.4.12-7+deb7u6
ii  install-info  4.13a.dfsg.1-10
ii  libbz2-1.0    1.0.6-4
ii  libc6         2.13-38+deb7u6
ii  libreadline6  6.2+dfsg-0.1
ii  libusb-0.1-4  2:0.1.12-20+nmu1
ii  zlib1g        1:1.2.7.dfsg-13

Versions of packages gnupg recommends:
pn  gnupg-curl     <none>
ii  libldap-2.4-2  2.4.31-1+nmu2

Versions of packages gnupg suggests:
pn  gnupg-doc                       <none>
pn  libpcsclite1                    <none>
pn  xloadimage | imagemagick | eog  <none>

-- no debconf information
>From fbb50867f81d790c4bf819dcadcd14be6c3f957b Mon Sep 17 00:00:00 2001
From: Werner Koch <w...@gnupg.org>
Date: Fri, 14 Nov 2014 09:36:19 +0100
Subject: [PATCH 15/20] 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 <w...@gnupg.org>

(backported from commit 69384568f66a48eff3968bb1714aa13925580e9f)

Updated doc/gpg.texi.
---
 doc/gpg.texi    | 27 ++++++++++-------
 g10/main.h      |  1 +
 g10/mainproc.c  | 38 ++++++++++++++++++++++++
 g10/openfile.c  | 91 +++++++++++++++++++++++++++++++++++++--------------------
 g10/plaintext.c | 21 ++++++++-----
 5 files changed, 130 insertions(+), 48 deletions(-)

diff --git a/doc/gpg.texi b/doc/gpg.texi
index 728f314..7d08756 100644
--- a/doc/gpg.texi
+++ b/doc/gpg.texi
@@ -198,16 +198,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{gpg}
+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
@@ -217,6 +223,7 @@ out the actual signed data; but there are other pitfalls with this
 format as well.  It is suggested to avoid cleartext signatures in
 favor of detached signatures.
 
+
 @item --multifile
 @opindex multifile
 This modifies certain other commands to accept multiple files for
diff --git a/g10/main.h b/g10/main.h
index af35c77..05a4059 100644
--- a/g10/main.h
+++ b/g10/main.h
@@ -195,6 +195,7 @@ 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 *a );
+char *get_matching_datafile (const char *sigfilename);
 IOBUF open_sigfile( const char *iname, progress_filter_context_t *pfx );
 void try_make_homedir( const char *fname );
 
diff --git a/g10/mainproc.c b/g10/mainproc.c
index 1e140ed..d355a21 100644
--- a/g10/mainproc.c
+++ b/g10/mainproc.c
@@ -1940,6 +1940,44 @@ check_sig_and_print( CTX c, KBNODE node )
 		   sig->sig_class==0x01?_("textmode"):_("unknown"),
 		   digest_algo_to_string(sig->digest_algo));
 
+        if (!rc && !c->signed_data)
+          {
+            /* 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 )
diff --git a/g10/openfile.c b/g10/openfile.c
index 73fd5db..ac4596b 100644
--- a/g10/openfile.c
+++ b/g10/openfile.c
@@ -199,7 +199,7 @@ open_outfile( const char *iname, int mode, IOBUF *a )
   else {
     char *buf = NULL;
     const char *name;
-    
+
     if ( opt.dry_run )
       {
 #ifdef HAVE_W32_SYSTEM
@@ -224,12 +224,12 @@ open_outfile( const char *iname, int mode, IOBUF *a )
           char *dot;
           const char *newsfx = mode==1 ? ".asc" :
                                mode==2 ? ".sig" : ".gpg";
-          
+
           buf = xmalloc(strlen(iname)+4+1);
           strcpy(buf,iname);
           dot = strrchr(buf, '.' );
           if ( dot && dot > buf && dot[1] && strlen(dot) <= 4
-               && CMP_FILENAME(newsfx, dot) 
+               && CMP_FILENAME(newsfx, dot)
                && !(strchr (dot, '/') || strchr (dot, '\\')))
             {
               /* There is a dot, the dot is not the first character,
@@ -272,7 +272,7 @@ open_outfile( const char *iname, int mode, IOBUF *a )
         xfree (buf);
         name = buf = tmp;
       }
-    
+
     if( !rc )
       {
         if (is_secured_filename (name) )
@@ -300,41 +300,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 )
+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;
+          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.
  */
@@ -398,7 +427,7 @@ copy_options_file( const char *destdir )
                     ;
                 else if (c == '#')
                     esc = 2;
-                else 
+                else
                     any_option = 1;
             }
         }
diff --git a/g10/plaintext.c b/g10/plaintext.c
index 29324d9..4a5c3fb 100644
--- a/g10/plaintext.c
+++ b/g10/plaintext.c
@@ -538,13 +538,20 @@ hash_datafiles( MD_HANDLE md, MD_HANDLE md2, STRLIST files,
     STRLIST sl;
 
     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);
-	    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);
+	        return 0;
+            }
+        }
+
         log_error (_("no signed data\n"));
         return G10ERR_OPEN_FILE;
     }
-- 
2.1.3

Reply via email to