[git] GnuPG - branch, master, updated. gnupg-2.1.15-11-g0a5a854

by Werner Koch cvs at cvs.gnupg.org
Thu Aug 25 16:21:30 CEST 2016


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".

The branch, master has been updated
       via  0a5a854510fda6e6990938a3fca424df868fe676 (commit)
       via  74a082bc10960b2d65d4d1e31152f988a40a2225 (commit)
      from  19d12be3cea5b4ee8153287a2f2442913a5e07a1 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 0a5a854510fda6e6990938a3fca424df868fe676
Author: Werner Koch <wk at gnupg.org>
Date:   Thu Aug 25 15:18:51 2016 +0200

    gpg: Fix false negatives in Ed25519 signature verification.
    
    * g10/pkglue.c (pk_verify): Fix Ed25519 signatrue values.
    * tests/openpgp/verify.scm (msg_ed25519_rshort): New
    (msg_ed25519_sshort): New.
    ("Checking that a valid Ed25519 signature is verified as such"): New.
    --
    
    About one out of 256 signature won't verify due to stripped zero
    bytes.  See the source comment for details.
    
    Reported-by: Andre Heinecke
    Signed-off-by: Werner Koch <wk at gnupg.org>

diff --git a/g10/pkglue.c b/g10/pkglue.c
index 7de2fa7..50de347 100644
--- a/g10/pkglue.c
+++ b/g10/pkglue.c
@@ -58,6 +58,7 @@ pk_verify (pubkey_algo_t pkalgo, gcry_mpi_t hash,
 {
   gcry_sexp_t s_sig, s_hash, s_pkey;
   int rc;
+  unsigned int neededfixedlen = 0;
 
   /* Make a sexp from pkey.  */
   if (pkalgo == PUBKEY_ALGO_DSA)
@@ -103,6 +104,9 @@ pk_verify (pubkey_algo_t pkalgo, gcry_mpi_t hash,
                                 curve, pkey[1]);
           xfree (curve);
         }
+
+      if (openpgp_oid_is_ed25519 (pkey[0]))
+        neededfixedlen = 256 / 8;
     }
   else
     return GPG_ERR_PUBKEY_ALGO;
@@ -144,11 +148,59 @@ pk_verify (pubkey_algo_t pkalgo, gcry_mpi_t hash,
     }
   else if (pkalgo == PUBKEY_ALGO_EDDSA)
     {
-      if (!data[0] || !data[1])
+      gcry_mpi_t r = data[0];
+      gcry_mpi_t s = data[1];
+      size_t rlen, slen, n;  /* (bytes) */
+      char buf[64];
+
+      log_assert (neededfixedlen <= sizeof buf);
+
+      if (!r || !s)
+        rc = gpg_error (GPG_ERR_BAD_MPI);
+      else if ((rlen = (gcry_mpi_get_nbits (r)+7)/8) > neededfixedlen || !rlen)
+        rc = gpg_error (GPG_ERR_BAD_MPI);
+      else if ((slen = (gcry_mpi_get_nbits (s)+7)/8) > neededfixedlen || !slen)
         rc = gpg_error (GPG_ERR_BAD_MPI);
       else
-        rc = gcry_sexp_build (&s_sig, NULL,
-                              "(sig-val(eddsa(r%M)(s%M)))", data[0], data[1]);
+        {
+          /* We need to fixup the length in case of leading zeroes.
+           * OpenPGP does not allow leading zeroes and the parser for
+           * the signature packet has no information on the use curve,
+           * thus we need to do it here.  We won't do it for opaque
+           * MPIs under the assumption that they are known to be fine;
+           * we won't see them here anyway but the check is anyway
+           * required.  Fixme: A nifty feature for gcry_sexp_build
+           * would be a format to left pad the value (e.g. "%*M"). */
+          rc = 0;
+
+          if (rlen < neededfixedlen
+              && !gcry_mpi_get_flag (r, GCRYMPI_FLAG_OPAQUE)
+              && !(rc=gcry_mpi_print (GCRYMPI_FMT_USG, buf, sizeof buf, &n, r)))
+            {
+              log_assert (n < neededfixedlen);
+              memmove (buf + (neededfixedlen - n), buf, n);
+              memset (buf, 0, neededfixedlen - n);
+              r = gcry_mpi_set_opaque_copy (NULL, buf, neededfixedlen * 8);
+            }
+          if (slen < neededfixedlen
+              && !gcry_mpi_get_flag (s, GCRYMPI_FLAG_OPAQUE)
+              && !(rc=gcry_mpi_print (GCRYMPI_FMT_USG, buf, sizeof buf, &n, s)))
+            {
+              log_assert (n < neededfixedlen);
+              memmove (buf + (neededfixedlen - n), buf, n);
+              memset (buf, 0, neededfixedlen - n);
+              s = gcry_mpi_set_opaque_copy (NULL, buf, neededfixedlen * 8);
+            }
+
+          if (!rc)
+            rc = gcry_sexp_build (&s_sig, NULL,
+                                  "(sig-val(eddsa(r%M)(s%M)))", r, s);
+
+          if (r != data[0])
+            gcry_mpi_release (r);
+          if (s != data[1])
+            gcry_mpi_release (s);
+        }
     }
   else if (pkalgo == PUBKEY_ALGO_ELGAMAL || pkalgo == PUBKEY_ALGO_ELGAMAL_E)
     {
diff --git a/tests/openpgp/verify.scm b/tests/openpgp/verify.scm
index de03db5..2f03027 100755
--- a/tests/openpgp/verify.scm
+++ b/tests/openpgp/verify.scm
@@ -236,6 +236,67 @@ FWIAQUplk7JWbyRKAJ92ZJyJpWfzb0yc1s7MY65r2qEHrg==
 ;; Two clear text signatures in a row
 (define msg_clsclss_asc_multiple (string-append msg_cls_asc msg_clss_asc))
 
+
+;; An Ed25519 cleartext message with an R parameter of only 247 bits
+;; so that the code to re-insert the stripped zero byte kicks in.  The
+;; S parameter has 253 bits but that does not strip a full byte.
+(define msg_ed25519_rshort "
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA256
+
+Dear Emily:
+	I'm still confused as to what groups articles should be posted
+to.  How about an example?
+		-- Still Confused
+
+Dear Still:
+	Ok.  Let's say you want to report that Gretzky has been traded from
+the Oilers to the Kings.  Now right away you might think rec.sport.hockey
+would be enough.  WRONG.  Many more people might be interested.  This is a
+big trade!  Since it's a NEWS article, it belongs in the news.* hierarchy
+as well.  If you are a news admin, or there is one on your machine, try
+news.admin.  If not, use news.misc.
+	The Oilers are probably interested in geology, so try sci.physics.
+He is a big star, so post to sci.astro, and sci.space because they are also
+interested in stars.  Next, his name is Polish sounding.  So post to
+soc.culture.polish.  But that group doesn't exist, so cross-post to
+news.groups suggesting it should be created.  With this many groups of
+interest, your article will be quite bizarre, so post to talk.bizarre as
+well.  (And post to comp.std.mumps, since they hardly get any articles
+there, and a \"comp\" group will propagate your article further.)
+	You may also find it is more fun to post the article once in each
+group.  If you list all the newsgroups in the same article, some newsreaders
+will only show the the article to the reader once!  Don't tolerate this.
+		-- Emily Postnews Answers Your Questions on Netiquette
+-----BEGIN PGP SIGNATURE-----
+
+iJEEARYIADoWIQSyHeq0+HX7PaQvHR0TlWNoKgINCgUCV772DhwccGF0cmljZS5s
+dW11bWJhQGV4YW1wbGUubmV0AAoJEBOVY2gqAg0KMAIA90EtUwAja0iJGpO91wyz
+GLh9pS5v495V0r94yU6uUyUA/RT/StyPWe1wbnEZuacZnLbUV6Yy/aTXCVAlxf0r
+TusO
+=vQ3f
+-----END PGP SIGNATURE-----
+")
+
+;; An Ed25519 cleartext message with an S parameter of only 248 bits
+;; so that the code to re-insert the stripped zero byte kicks in.
+(define msg_ed25519_sshort "
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA256
+
+All articles that coruscate with resplendence are not truly auriferous.
+-----BEGIN PGP SIGNATURE-----
+
+iJEEARYIADoWIQSyHeq0+HX7PaQvHR0TlWNoKgINCgUCV771QhwccGF0cmljZS5s
+dW11bWJhQGV4YW1wbGUubmV0AAoJEBOVY2gqAg0KHVEBAI66OPDYXKWO3r6SaFT+
+uxmh8x4ZerW41vMA9gkJ4AEKAPjoe/Z7fDqo1lCptIFutFAGbfNxcm/53prfx2fT
+GisM
+=L7sk
+-----END PGP SIGNATURE-----
+")
+
+
+
 ;; Fixme:  We need more tests with manipulated cleartext signatures.
 
 ;;
@@ -272,3 +333,15 @@ FWIAQUplk7JWbyRKAJ92ZJyJpWfzb0yc1s7MY65r2qEHrg==
 	   (pipe:spawn `(, at GPG --verify)))
 	  (error "verification succeded but should not")))
  '(bad_ls_asc bad_fols_asc bad_olsf_asc bad_ools_asc))
+
+
+;;; Need to import the ed25519 sample key used for
+;;; the next two tests.
+(call-check `(, at GPG --quiet --yes --import ,(in-srcdir key-file2)))
+(for-each-p
+ "Checking that a valid Ed25519 signature is verified as such"
+ (lambda (armored-file)
+   (pipe:do
+    (pipe:echo (eval armored-file (current-environment)))
+    (pipe:spawn `(, at GPG --verify))))
+ '(msg_ed25519_rshort msg_ed25519_sshort))

commit 74a082bc10960b2d65d4d1e31152f988a40a2225
Author: Werner Koch <wk at gnupg.org>
Date:   Thu Aug 25 15:16:32 2016 +0200

    common: Rename an odd named function.
    
    * common/openpgp-oid.c (oid_crv25519): Rename to oid_cv25519.
    (openpgp_oid_is_crv25519): Rename to openpgp_oid_is_cv25519.  Change
    callers.
    
    --
    
    We use "cv25519" everywhere else and thus the test function should not
    have a surprising name.
    
    Signed-off-by: Werner Koch <wk at gnupg.org>

diff --git a/common/openpgp-oid.c b/common/openpgp-oid.c
index 7c93547..af3cbbe 100644
--- a/common/openpgp-oid.c
+++ b/common/openpgp-oid.c
@@ -68,7 +68,7 @@ static const char oid_ed25519[] =
   { 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x0f, 0x01 };
 
 /* The OID for Curve25519 in OpenPGP format.  */
-static const char oid_crv25519[] =
+static const char oid_cv25519[] =
   { 0x0a, 0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01 };
 
 
@@ -298,7 +298,7 @@ openpgp_oid_is_ed25519 (gcry_mpi_t a)
 
 
 int
-openpgp_oid_is_crv25519 (gcry_mpi_t a)
+openpgp_oid_is_cv25519 (gcry_mpi_t a)
 {
   const unsigned char *buf;
   unsigned int nbits;
@@ -309,8 +309,8 @@ openpgp_oid_is_crv25519 (gcry_mpi_t a)
 
   buf = gcry_mpi_get_opaque (a, &nbits);
   n = (nbits+7)/8;
-  return (n == DIM (oid_crv25519)
-          && !memcmp (buf, oid_crv25519, DIM (oid_crv25519)));
+  return (n == DIM (oid_cv25519)
+          && !memcmp (buf, oid_cv25519, DIM (oid_cv25519)));
 }
 
 
diff --git a/common/util.h b/common/util.h
index 1c3cce9..f293234 100644
--- a/common/util.h
+++ b/common/util.h
@@ -206,7 +206,7 @@ size_t percent_unescape_inplace (char *string, int nulrepl);
 gpg_error_t openpgp_oid_from_str (const char *string, gcry_mpi_t *r_mpi);
 char *openpgp_oid_to_str (gcry_mpi_t a);
 int openpgp_oid_is_ed25519 (gcry_mpi_t a);
-int openpgp_oid_is_crv25519 (gcry_mpi_t a);
+int openpgp_oid_is_cv25519 (gcry_mpi_t a);
 const char *openpgp_curve_to_oid (const char *name, unsigned int *r_nbits);
 const char *openpgp_oid_to_curve (const char *oid, int canon);
 const char *openpgp_enum_curves (int *idxp);
diff --git a/g10/keyid.c b/g10/keyid.c
index 84990a3..ea6ed5e 100644
--- a/g10/keyid.c
+++ b/g10/keyid.c
@@ -919,7 +919,7 @@ keygrip_from_pk (PKT_public_key *pk, unsigned char *array)
                                    pk->pubkey_algo == PUBKEY_ALGO_EDDSA?
                                    "(public-key(ecc(curve%s)(flags eddsa)(q%m)))":
                                    (pk->pubkey_algo == PUBKEY_ALGO_ECDH
-                                    && openpgp_oid_is_crv25519 (pk->pkey[0]))?
+                                    && openpgp_oid_is_cv25519 (pk->pkey[0]))?
                                    "(public-key(ecc(curve%s)(flags djb-tweak)(q%m)))":
                                    "(public-key(ecc(curve%s)(q%m)))",
                                    curve, pk->pkey[1]);
diff --git a/g10/pkglue.c b/g10/pkglue.c
index 232c489..7de2fa7 100644
--- a/g10/pkglue.c
+++ b/g10/pkglue.c
@@ -227,7 +227,7 @@ pk_encrypt (pubkey_algo_t algo, gcry_mpi_t *resarr, gcry_mpi_t data,
             rc = gpg_error_from_syserror ();
           else
             {
-              int with_djb_tweak_flag = openpgp_oid_is_crv25519 (pkey[0]);
+              int with_djb_tweak_flag = openpgp_oid_is_cv25519 (pkey[0]);
 
               /* Now use the ephemeral secret to compute the shared point.  */
               rc = gcry_sexp_build (&s_pkey, NULL,

-----------------------------------------------------------------------

Summary of changes:
 common/openpgp-oid.c     |  8 +++---
 common/util.h            |  2 +-
 g10/keyid.c              |  2 +-
 g10/pkglue.c             | 60 ++++++++++++++++++++++++++++++++++++---
 tests/openpgp/verify.scm | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 135 insertions(+), 10 deletions(-)


hooks/post-receive
-- 
The GNU Privacy Guard
http://git.gnupg.org




More information about the Gnupg-commits mailing list