At the end of the testsuite in test_builtin() happens the following:
- a previously created signature gets modified at a random spot
- this signaure is compared against the data which was used to create the
  signature.

Now, in theory this should always fail in reality is passed sometimes. The
modifcation algorith did the following:
|       offset = sig[10] % 66;
|       dirt = sig[11];
|       dirt = dirt ? dirt : 1;
|       sig[offset] ^= dirt;

If sig[10] is 0xa7 and sig[11] is 0x9e the last line envolves to:
|       sig[35] ^= 0x9e;

The signature consists of to BIGNUMs encoded as ASN1 string. sig[34] and
sig[35] is the begin of the second and last number. sig[35] contains the
length of this number and its content is 0x1e. Now, 0x9e ^ 0x1e = 0x80
and this is a special value. It means that the length of the value is
"infinite" i.e. everything until the end of the stream. So the ASN1 parser
considers the remaining data as the last element. Since there is nothing
after it, it succeeds. This random modification was a zero change.

This change ensures that something like this does not happen again. If we
do a zero change by accident (R and S are unchanged) we make a third
change and hope that something will change now.

This bug has been reported as a Debian #440538 against 0.9.8e. I could
reproduce it on current 0.9 series, 1.0 looks affected as well.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
 crypto/ecdsa/ecdsatest.c |   87 +++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/crypto/ecdsa/ecdsatest.c b/crypto/ecdsa/ecdsatest.c
index aa4e148..67db141 100644
--- a/crypto/ecdsa/ecdsatest.c
+++ b/crypto/ecdsa/ecdsatest.c
@@ -281,6 +281,85 @@ x962_err:
        return ret;
        }
 
+static int compare_sig(unsigned char *osig, unsigned int sig_len, ECDSA_SIG 
*old_sig)
+{
+       unsigned char *signature = osig;
+       ECDSA_SIG *new_sig = NULL;
+       char *org_r = NULL, *org_s = NULL;
+       char *new_r = NULL, *new_s = NULL;
+       int ret = -1;
+
+       org_r = BN_bn2hex(old_sig->r);
+       org_s = BN_bn2hex(old_sig->s);
+       if (!org_r || !org_s)
+               goto out;
+
+       new_sig = ECDSA_SIG_new();
+       if (!new_sig)
+               goto out;
+       if (!d2i_ECDSA_SIG(&new_sig, (const unsigned char **)&signature, 
sig_len))
+               goto out;
+
+       new_r = BN_bn2hex(new_sig->r);
+       new_s = BN_bn2hex(new_sig->s);
+       if (!new_r || !new_s)
+               goto out;
+       if ((!strcmp(org_r, new_r)) &&
+                       !strcmp(org_s, new_s))
+               /* the signature did not change */
+               ret = 1;
+       else
+               ret = 0;
+out:
+       if (new_sig)
+               ECDSA_SIG_free(new_sig);
+       if (new_r)
+               OPENSSL_free(new_r);
+       if (new_s)
+               OPENSSL_free(new_s);
+       if (org_r)
+               OPENSSL_free(org_r);
+       if (org_s)
+               OPENSSL_free(org_s);
+       return ret;
+}
+
+static void modify_signature(unsigned char *osig, unsigned int sig_len, BIO 
*out)
+{
+       unsigned char dirt, offset;
+       unsigned char *signature = osig;
+       ECDSA_SIG *org_sig;
+       int ret;
+
+       org_sig = ECDSA_SIG_new();
+       if (!org_sig)
+               return;
+
+       if (!d2i_ECDSA_SIG(&org_sig, (const unsigned char **)&signature, 
sig_len))
+               goto out;
+
+       signature = osig;
+       offset = signature[10] % sig_len;
+       dirt = signature[11];
+       dirt = dirt ? dirt : 1;
+       signature[offset] ^= dirt;
+
+       ret = compare_sig(osig, sig_len, org_sig);
+       if (ret <= 0)
+               goto out;
+
+       signature[offset] = ~signature[offset];
+       ret = compare_sig(osig, sig_len, org_sig);
+       if (ret <= 0)
+               goto out;
+       BIO_printf(out, "Failed to modify signature. Tried: %02x => %02x => 
%02x\n",
+                       (unsigned char) (~osig[offset] ^ dirt),
+                       (unsigned char)~osig[offset], osig[offset]);
+       BIO_printf(out, "at offset 0x%02x and it was always equal.\n", offset);
+out:
+       ECDSA_SIG_free(org_sig);
+}
+
 int test_builtin(BIO *out)
        {
        EC_builtin_curve *curves = NULL;
@@ -325,8 +404,6 @@ int test_builtin(BIO *out)
        /* now create and verify a signature for every curve */
        for (n = 0; n < crv_len; n++)
                {
-               unsigned char dirt, offset;
-
                nid = curves[n].nid;
                if (nid == NID_ipsec4)
                        continue;
@@ -416,9 +493,9 @@ int test_builtin(BIO *out)
                BIO_printf(out, ".");
                (void)BIO_flush(out);
                /* modify a single byte of the signature */
-               offset = signature[10] % sig_len;
-               dirt   = signature[11];
-               signature[offset] ^= dirt ? dirt : 1; 
+
+               modify_signature(signature, sig_len, out);
+
                if (ECDSA_verify(0, digest, 20, signature, sig_len, eckey) == 1)
                        {
                        BIO_printf(out, " failed\n");
-- 
1.7.2.3

______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [email protected]
Automated List Manager                           [email protected]

Reply via email to