[Pkg-gnupg-commit] [gnupg2] 11/118: gpg: Fix false negatives in Ed25519 signature verification.
Daniel Kahn Gillmor
dkg at fifthhorseman.net
Thu Sep 15 18:24:58 UTC 2016
This is an automated email from the git hooks/post-receive script.
dkg pushed a commit to branch encoding-and-speling
in repository gnupg2.
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>
---
g10/pkglue.c | 58 ++++++++++++++++++++++++++++++++++++--
tests/openpgp/verify.scm | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 128 insertions(+), 3 deletions(-)
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))
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-gnupg/gnupg2.git
More information about the Pkg-gnupg-commit
mailing list