Page MenuHome GnuPG

Regression in cipher/ecc.c
Closed, ResolvedPublic

Description

Commit 9933b9e5e1a3f5b1019c75f93bd265d4a1ecc270 introduced cofactor support into
gcrypt.

However, the commit had a bug: in cipher/ecc.c, around line 900, the call to
sexp_extract_param() duplicates the E.n parameter, which leads to E.h being
unset. The second &pk.E.n should be &pk.E.h.

This is the diff that contains the bug:

@@ -905,9 +912,9 @@ ecc_verify (gcry_sexp_t s_sig, gcry_sexp_t s_data,
gcry_sexp_t s_keyparms)

    • Extract the key. */ if ((ctx.flags & PUBKEY_FLAG_PARAM))
  • rc = sexp_extract_param (s_keyparms, NULL, "-p?a?b?g?n?/q",

+ rc = sexp_extract_param (s_keyparms, NULL, "-p?a?b?g?n?h?/q",

&pk.E.p, &pk.E.a, &pk.E.b, &mpi_g, &pk.E.n,
  • &mpi_q, NULL);

+ &pk.E.n, &mpi_q, NULL);

else
  rc = sexp_extract_param (s_keyparms, NULL, "/q",
                           &mpi_q, NULL);

Details

Version
1.7.0, 1.7.1

Event Timeline

I encountered this bug when I tried to use libgcrypt-HEAD with libaacs.
libaacs uses ECDSA, and when it calls gcry_pk_verify(), I get GPG_ERR_NO_OBJ
because the sexp was constructed without the h parameter.

As far as I can see, the public key does not specify a cofactor.
Which value would be appropriate in this case?
Or more precisely, which value was used before your patch?
1?

Cofactor is defined by the curve. I guess that the cofactor of the curve in
libaacs is 1. Although my patch includes the code which gets cofactor from key
parameters, the ECDSA computation itself doesn't use cofactor actually.

Well, I haven't expected there is a user who uses (flags param), because it was
mostly for our development. There must be more bugs around that...

gniibe added a project: Restricted Project.

Can we close this bug? (1.71 has been released)

Well, I still get a "Missing object" error in libaacs, and I'm not sure how to
fix it. But the patch is in, so I think the bug can be closed.

It seems the problem is around the code here:
http://git.videolan.org/?p=libaacs.git;a=blob;f=src/libaacs/crypto.c;h=4db7641ec8e9af3cdf056562de39a39e3fa0c09b;hb=HEAD#l308

And the error I get when I try to scan a disc with aacs_info is as follows:
crypto.c:587: _aacs_verify: gcry_pk_verify failed. error was: Missing item in object
aacs.c:1502: invalid signature in cached hrl

If you have any hints, I'd be very grateful. Thanks!

Sorry, my near sight. I only fixed cofactor support, in a case where "h" is
provided.
I should have fixed other parts, too. Now, I fixed in master:

http://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commitdiff;h=0f3a069211d8d24a61aa0dc2cc6c4ef04cc4fab7;hp=fa917d2e24b0c98143a079ab4889ad8f69bee446

It is backported to 1.7 branch too.

libaacs should work with this patch.

gniibe changed Version from 1.7.0 to 1.7.0, 1.7.1.Jun 16 2016, 4:16 AM

Awesome, that did the trick!
Many thanks.

werner removed a project: Restricted Project.

1.7.2 with the fix has been released