mpeylo / cmpossl

An OpenSSL-based implementation of the Certificate Management Protocol (CMP), defined in IETF RFCs 4210, 4211, and 6712. It is being extended according to the emerging RFCs 'CMP Updates' (CMPv3), 'CMP Algorithms', and 'Lightweight CMP Profile'.
https://github.com/mpeylo/cmpossl/wiki
Other
35 stars 13 forks source link

PoP is broken without --debug #165

Closed mpeylo closed 5 years ago

mpeylo commented 5 years ago

After 795706a0a1c PoP for EC and RSA keys is not working anymore when building without --debug flag

$ make distclean; ./config shared && make depend && make update && make
$ openssl ecparam -name prime256v1 -genkey -out eckey.pem
$ LD_LIBRARY_PATH=. ./apps/openssl cmp \
        -server example.com:80 \
         -path /pkix/ \
        -ref 50123 \
        -secret 'pass:dasfasdfas' \
        -cmd ir \
        -recipient '/CN=blah' \
        -certout newcert.pem \
        -newkey eckey.pem \
        -subject "/CN=Martin/O=Test" \
        -digest sha256
140240990331136:error:100DA064:elliptic curve routines:pkey_ec_sign:buffer too small:crypto/ec/ec_pmeth.c:119:
140240990331136:error:36064067:CRMF routines:CRMF_poposigningkey_init:error:crypto/crmf/crmf_lib.c:439:
140240990331136:error:3707907B:CMP routines:OSSL_CMP_certreq_new:error creating ir:crypto/cmp/cmp_msg.c:351:
$ openssl genrsa -out rsakey.pem 2048
Generating RSA private key, 2048 bit long modulus (2 primes)
......+++++
..........................+++++
e is 65537 (0x010001)
$ LD_LIBRARY_PATH=. ./apps/openssl cmp \
        -server example.com:80 \
         -path /pkix/ \
        -ref 50123 \
        -secret 'pass:dasfasdfas' \
        -cmd ir \
        -recipient '/CN=blah' \
        -certout newcert.pem \
        -newkey rsakey.pem \
        -subject "/CN=Martin/O=Test" \
        -digest sha256
139917169185024:error:0608C09B:digital envelope routines:EVP_PKEY_sign:buffer too small:crypto/evp/pmeth_fn.c:65:
139917169185024:error:37064067:CRMF routines:CRMF_poposigningkey_init:error:crypto/crmf/crmf_lib.c:439:
139917169185024:error:3807907B:CMP routines:OSSL_CMP_certreq_new:error creating ir:crypto/cmp/cmp_msg.c:351:

Validated with rebase at d616bb6611a4e3ddeb6dabb563813b384c429ee5 that nothing related to this issue would have been fixed by upstream within a month.

mpeylo commented 5 years ago

fixed in 84b6bb67ed0ab73f633e87e3b1c233a3c330bcf7

DDvO commented 5 years ago

Glad that you've been able to fix this already!

In retrospect, very strange that EVP_PKEY_size(pkey) apparently was sufficient when compilation was done using --debug, since the signature size for EC is not the same as the key size, but double of that.

It turns out that the API of EVP_DigestSign* is even more awkward than what its documentation suggests :-( And even the knowledgeable reviewers of our branch didn't notice this pitfall.

mpeylo commented 5 years ago

Actually, that EVP_PKEY_size(pkey) result was completely irrelevant.

Note the following hidden in EVP_DigestSignInit.pod - a bit misleading with the "should" I guess.

If sig is not NULL then before the call the siglen parameter should contain the length of the sig buffer.

DDvO commented 5 years ago

Ah, so sig_len was used without initialization, and it was just luck that most times it worked.

DavidZemon commented 5 years ago

Thank you @mpeylo - this thread is one of the few places on the Internet where this bug was talked about, and it pointed me to the exact problem. I assumed I should pass a size of 0 in, and that it would be ignored and reset to the output parameter. But no - once I set that to my max buffer size, life was good.