0

While OpenSSL always assumes ECDSA signatures to be ASN.1/DER encoded, I also need to be able to verify P1363 encoded signatures. (For a comprehensive introduction to the 2 forms see e.g. the answer to this SO question.)

The idea is to patch ECDSA_verify() so that if ASN.1 parse fails, P1363 is assumed and transformed into a synthesized ECDSA_SIG that can then be fed into ECDSA_do_verify().

This is what I do:

#include <openssl/ecdsa.h>
#include <string.h>

using namespace std;
const unsigned char sigbuf[] =
{
    0x37, 0x25, 0x8a, 0x3c, 0xf0, 0x05, 0x6e, 0x23, 0x97, 0x83, 0xae, 0xf5, 0x84, 0x0a, 0x5e, 0x0a,
    0x1f, 0xc8, 0x8a, 0x54, 0x84, 0x05, 0x34, 0x1d, 0x82, 0x86, 0x47, 0x7c, 0x14, 0x51, 0x14, 0xf8,
    0x0a, 0xf4, 0xbc, 0xcf, 0x58, 0xef, 0xcd, 0x69, 0xbd, 0xc0, 0x23, 0xf1, 0xe2, 0x96, 0x6a, 0xa8,
    0x28, 0xcf, 0x35, 0x60, 0xe6, 0x75, 0x6d, 0x89, 0x4a, 0x60, 0x9b, 0x2b, 0x2a, 0x6d, 0x06, 0x51
};

const int sig_len = 64;

//int ECDSA_verify(int type, const unsigned char *dgst, int dgst_len,
//                 const unsigned char *sigbuf, int sig_len, EC_KEY *eckey)
int main(int argc, char *argv[])
{
    ECDSA_SIG *s;
    const unsigned char *p = sigbuf;
    unsigned char *der = NULL;
    int derlen = -1;
    int ret = -1;

    s = ECDSA_SIG_new();
    if (s == NULL)
        return (ret);
    if (d2i_ECDSA_SIG(&s, &p, sig_len) == NULL) {
        /*
         * ASN.1 decoding failed, see crypto/asn1/tasn_dec.c line 515ff.
         * Assume s is encoded as IEEE P1363. for a comprehensive description see
         * ttps://stackoverflow.com/questions/36542645/does-openssl-sign-for-ecdsa-apply-asn1-encoding-to-the-hash-before-signing
         * Fill the ECDSA_SIG from the P1363.
         */
        if ((sig_len % 2) != 0)
            return (ret);
        if (strlen((char *)sigbuf) != sig_len)
            return (ret);
        if (s == NULL)
            s = ECDSA_SIG_new();
        if (s == NULL)
            return (ret);
        /*
         * BN_hex2bn() stops immediately if the hex string starts with '\0', so we skip zeroes.
         * I /think/ only the s part of the P1363 may be padded, but it does no harm to skip them
         * for the r part, too.
         */
        const unsigned char *pr = sigbuf;
        while (*pr == '\0')
            pr++;
        /*
         * BN_hex2bn() is greedy, so we create null-terminated copies of both the r and s parts.
         * Also note that it looks like BN_hex2bn() takes care of the required leading zero padding
         * in case of negative bignums.
         */
        int hex_len = (sigbuf + sig_len / 2) - pr;
        char *hex = (char *)malloc(hex_len + 1);
        strncpy(hex, (const char *)pr, hex_len);
        hex[hex_len] = '\0';
        /*
         * Finally create the BIGNUM and put it in the r part of the ECDSA_SIG.
         */
        BN_hex2bn(&(s->r), hex);
        free(hex);

        /*
         * Now do the same for the s part...
         */
        unsigned char *ps = const_cast<unsigned char *>(sigbuf) + sig_len / 2;
        while (*ps == '\0')
            ps++;
        hex_len = (sigbuf + sig_len) - ps;
        hex = (char *)malloc(hex_len + 1);
        strncpy(hex, (const char *)ps, hex_len);
        hex[hex_len] = '\0';
        BN_hex2bn(&(s->s), hex);
        free(hex);
    }
    /* Ensure signature uses DER and doesn't have trailing garbage */
    derlen = i2d_ECDSA_SIG(s, &der);
//    if (derlen != sig_len || memcmp(sigbuf, der, derlen))
//        goto err;
//    ret = ECDSA_do_verify(dgst, dgst_len, s, eckey);
 err:
    if (derlen > 0) {
        OPENSSL_cleanse(der, derlen);
        OPENSSL_free(der);
    }
    ECDSA_SIG_free(s);
    return (ret);
}

sigbuf is a prime256v1 real-world example, fetched from asn1parse.

Now when I run the above program, derlen is 8 and der is "0\006\002\001\007\002\001". This is obviously not the ASN.1 I expect which should be:

#30 46 (SEQUENCE, 70 bytes)   <-- edit: wrong
30 44 (SEQUENCE, 68 bytes)
   02 20 (INTEGER, 32 bytes)
      (no padding)
      37 25 8A 3C F0 05 6E 23 97 83 AE F5 84 0A 5E 0A
      1F C8 8A 54 84 05 34 1D 82 86 47 7C 14 51 14 F8
   02 20 (INTEGER, 32 bytes)
      (no padding)
      0A F4 BC CF 58 EF CD 69 BD C0 23 F1 E2 96 6A A8
      28 CF 35 60 E6 75 6D 89 4A 60 9B 2B 2A 6D 06 51

shouldn't it?

Apparently I do something wrong with BIGNUM creation from the hex buffer, or ECDSA_SIG creation from them, or ASN.1 serialization. But for the life of me I don't see what it is. Any help appreciated!

jww
  • 97,681
  • 90
  • 411
  • 885
kzi
  • 61
  • 9
  • 2
    `if (strlen((char *)sigbuf) != sig_len)` - terrible idea, since `sigbuf` certainly isn't a terminated *string*. And no, adding a zero-octet on the end doesn't fix anything. It is certainly viable an embedded zero-octet can appear in that sequence *somewhere*. TL;DR: you shouldn't be treating that thing like a string, using any string-related operations, because it isn't a string; it's just octets. – WhozCraig Nov 12 '19 at 17:19
  • I wonder if I will ever be enlightened why my answer to WhozCraig's comment was deleted, being partly affirmative to the bad idea part, partly pointing out that it's unrelated to my question in that for the given sigbuf the test obviously passes? – kzi Nov 13 '19 at 10:38
  • The only people that can delete *anything* on SO are moderators and the original authors. If a moderator deleted a comment of yours, it's certainly within your right to ask why. In my 6+ years of participating in SO, I've never seen a comment, answer, or question deleted unless it was advertisement spam or crude/offensive content. I didn't see any of that in you comment, so I'm curious. Regarding your assessment that "the test obviously passes", not within the realm of defined behavior. `strlen` expects a terminated buffer; you're not providing that, and it's walking into uncharted memory. – WhozCraig Nov 13 '19 at 10:57
  • That said, I honestly don't see the need to do this with the ASN.1 library in the first place. Just manufacturing the actual ASN.1 encoding yourself should be sufficient (literally byte by byte; it's not like you don't what the lead sequence marker looks like, or what an ASN.1 integer `0x02` does). I've had to do this for similar encodings from yonder, including PGP signatures, and in the end, just building the octets into the ASN.1 yourself (well, myself) is easier than anything else. – WhozCraig Nov 13 '19 at 10:59
  • @WhozCraig I got your point, and you are certainly right _in general_. I blundered there. For the minimal example I provided however it just happens to be a non-issue. – kzi Nov 13 '19 at 11:08
  • I still think something is wrong with the BIGNUM creation; they looked crude in the debugger. The ASN.1 lib is not needed at that point. I get it that you advise to skip the ECDSA_SIG and jump right to the serialized ASN.1. I'll give it a go. Thx – kzi Nov 13 '19 at 11:13
  • 1
    I'm looking at your desired sequence, and it looks right , save for the sizing of the sequence. The sequence `0x30` is made up of two 34-byte integers (each consisting of 2-bytes for type and length, then the followup 32 bytes making up each integer's 256bit value.) Therefore, the sequence should be 0x30 0x44 0x02 0x20.... 0x02 0x20..... . As I said, unless you're really diving into odd ASN.1 artifacts (optionals are a good example) its usually easier to just code the bytes yourself. Sup to you, of course. – WhozCraig Nov 13 '19 at 11:16
  • Thx again, also for yet another correction. I copied from an example with padding, did the math, then removed the obsolete padding - but not in sequence size. – kzi Nov 13 '19 at 11:21

2 Answers2

1

Doh! This is so silly...

The "hex" in BN_hex2bn() does not mean an array of hex values. Instead it means a string representation of hex values!

So in my above example if I instead initialize like

const unsigned char sigbuf[] = "37258a3cf0056e239783aef5840a5e0a"
                               "1fc88a548405341d8286477c145114f8"
                               "0af4bccf58efcd69bdc023f1e2966aa8"
                               "28cf3560e6756d894a609b2b2a6d0651";
const int sig_len = 128;

I end up with a decent ECDSA_SIG and consequently decent der and derlen, too.

Hope this may help someone in the future. I really loathe OpenSSL documentation sometimes...

kzi
  • 61
  • 9
1

So the real answer is to use BN_bin2bn(), like in the above example

s->r = BN_bin2bn(pr, hex_len, NULL);

Note how this takes a length parameter. This underlines the fact pointed out by @WhozCraig that it's really just a sequence of octets that is dealt with here, not a string.

Also note that the doc states that

BN_bin2bn() converts the positive integer in big-endian form of length len at s into a BIGNUM and places it in ret. If ret is NULL, a new BIGNUM is created.

So since the integer must be positive, padding need be applied eventually.

jww
  • 97,681
  • 90
  • 411
  • 885
kzi
  • 61
  • 9