From b0210e0710a4b0c0614bf359e7dc171c80b329b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Myr=C3=A9en?= Date: Sat, 30 Dec 2017 22:08:09 +0200 Subject: [PATCH] Fixed some bugs in the port to Openssl-1.1: - wrap_public_key() passed the address of the local stack variable internal_key to RSA_meth_set0_data(), which was used long after wrap_public_key() had returned. Changed to static. - The callback functions yk_rsa_meth_sign and yk_ec_meth_sign 'siglen' parameter has type (unisgned int *), which was cast to (size_t *) before it was used to write a value in the caller's memory space. This caused stack corruption on machines where size_t is bigger than unsigned int. - The callback function's 'siglen' parameter is output-only, not in-out. The input value was assumed to contain the maximum size of the output buffer as input, and a bogus value was compared to the amount of data received from the token in function _general_authenticate(). Changed to pass in the values returned by RSA_size(rsa) and ECDSA_size(ec), which Openssl specifies as minimum buffer sizes. - The callback functions' return values were swapped; fixed to return 1 on success, 0 on failure. --- tool/yubico-piv-tool.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tool/yubico-piv-tool.c b/tool/yubico-piv-tool.c index 89daa79..64c678d 100644 --- a/tool/yubico-piv-tool.c +++ b/tool/yubico-piv-tool.c @@ -135,22 +135,28 @@ struct internal_key { int yk_rsa_meth_sign(int dtype, const unsigned char *m, unsigned int m_length, unsigned char *sigret, unsigned int *siglen, const RSA *rsa) { + size_t yk_siglen = RSA_size(rsa); const RSA_METHOD *meth = RSA_get_method(rsa); const struct internal_key *key = RSA_meth_get0_app_data(meth); - if (sign_data(key->state, m, m_length, sigret, (size_t *)siglen, key->algorithm, key->key)) - return 0; + if (sign_data(key->state, m, m_length, sigret, &yk_siglen, key->algorithm, key->key)) { + *siglen = (unsigned int)yk_siglen; + return 1; + } - return 1; + return 0; } int yk_ec_meth_sign(int type, const unsigned char *dgst, int dlen, unsigned char *sig, unsigned int *siglen, const BIGNUM *kinv, const BIGNUM *r, EC_KEY *ec) { + size_t yk_siglen = ECDSA_size(ec); const struct internal_key *key = EC_KEY_get_ex_data(ec, ec_key_ex_data_idx); - if (sign_data(key->state, dgst, dlen, sig, (size_t *)siglen, key->algorithm, key->key)) - return 0; + if (sign_data(key->state, dgst, dlen, sig, &yk_siglen, key->algorithm, key->key)) { + *siglen = (unsigned int)yk_siglen; + return 1; + } - return 1; + return 0; } static int wrap_public_key(ykpiv_state *state, int algorithm, EVP_PKEY *public_key, @@ -158,7 +164,10 @@ static int wrap_public_key(ykpiv_state *state, int algorithm, EVP_PKEY *public_k if(YKPIV_IS_RSA(algorithm)) { RSA_METHOD *meth = RSA_meth_dup(RSA_get_default_method()); RSA *rsa = EVP_PKEY_get0_RSA(public_key); - struct internal_key int_key = {state, algorithm, key}; + static struct internal_key int_key; + int_key.state = state; + int_key.algorithm = algorithm; + int_key.key = key; RSA_meth_set0_app_data(meth, &int_key); RSA_meth_set_sign(meth, yk_rsa_meth_sign); RSA_set_method(rsa, meth);