From 7ca0267ddf8718f0f3d3f887372dc4e368e0ebcb Mon Sep 17 00:00:00 2001 From: Trevor Bentley Date: Tue, 21 Nov 2017 17:08:38 +0100 Subject: [PATCH] Fix OpenSSL 1.1 compat layer - Changes for latest ykpiv_util refactor - Passes hw tests with openssl 1.0 and 1.1 - Passes valgrind --- configure.ac | 2 +- lib/internal.c | 4 +-- lib/tests/Makefile.am | 1 + lib/tests/api.c | 64 ++++++++++++++++++++++--------------- lib/ykpiv.c | 7 ++-- tool/util.c | 10 +++--- tool/yubico-piv-tool.c | 10 +++--- ykcs11/tests/Makefile.am | 1 + ykcs11/tests/ykcs11_tests.c | 1 + 9 files changed, 60 insertions(+), 40 deletions(-) diff --git a/configure.ac b/configure.ac index 7797bd1..8817e0d 100644 --- a/configure.ac +++ b/configure.ac @@ -37,7 +37,7 @@ AC_SUBST([LT_CURRENT], 5) AC_SUBST([LT_REVISION], 0) AC_SUBST([LT_AGE], 4) -AM_INIT_AUTOMAKE([-Wall -Werror foreign]) +AM_INIT_AUTOMAKE([-Wall -Werror foreign subdir-objects]) AM_SILENT_RULES([yes]) AC_PROG_CC m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) diff --git a/lib/internal.c b/lib/internal.c index e8942b3..2846045 100644 --- a/lib/internal.c +++ b/lib/internal.c @@ -21,7 +21,7 @@ #ifdef _WINDOWS -#define STATUS_SUCCESS ((NTSTATUS)0x00000000L) +#define STATUS_SUCCESS ((NTSTATUS)0x00000000L) struct des_key { HCRYPTPROV hProv; @@ -399,7 +399,7 @@ prng_rc _ykpiv_prng_generate(unsigned char *buffer, const size_t cb_req) { } #else - if (-1 == RAND_pseudo_bytes(buffer, cb_req)) { + if (-1 == RAND_bytes(buffer, cb_req)) { rc = PRNG_GENERAL_ERROR; } diff --git a/lib/tests/Makefile.am b/lib/tests/Makefile.am index 46264f4..d0900f0 100644 --- a/lib/tests/Makefile.am +++ b/lib/tests/Makefile.am @@ -38,6 +38,7 @@ endif LDADD = ../libykpiv.la $(OPENSSL_LIBS) +api_SOURCES = api.c ../../tool/openssl-compat.c ../../tool/openssl-compat.h check_PROGRAMS = basic parse_key api TESTS = $(check_PROGRAMS) diff --git a/lib/tests/api.c b/lib/tests/api.c index b36f0ff..7ac783d 100644 --- a/lib/tests/api.c +++ b/lib/tests/api.c @@ -28,6 +28,7 @@ * */ +#include "../../tool/openssl-compat.h" #include "ykpiv.h" #include "internal.h" @@ -268,23 +269,21 @@ static bool set_component(unsigned char *in_ptr, const BIGNUM *bn, int element_l } static bool prepare_rsa_signature(const unsigned char *in, unsigned int in_len, unsigned char *out, unsigned int *out_len, int nid) { - X509_SIG digestInfo; - X509_ALGOR algor; + X509_SIG *digestInfo; + X509_ALGOR *algor; ASN1_TYPE parameter; - ASN1_OCTET_STRING digest; + ASN1_OCTET_STRING *digest; unsigned char data[1024]; memcpy(data, in, in_len); - digestInfo.algor = &algor; - digestInfo.algor->algorithm = OBJ_nid2obj(nid); - digestInfo.algor->parameter = ¶meter; - digestInfo.algor->parameter->type = V_ASN1_NULL; - digestInfo.algor->parameter->value.ptr = NULL; - digestInfo.digest = &digest; - digestInfo.digest->data = data; - digestInfo.digest->length = (int)in_len; - *out_len = (unsigned int)i2d_X509_SIG(&digestInfo, &out); + digestInfo = X509_SIG_new(); + X509_SIG_getm(digestInfo, &algor, &digest); + algor->algorithm = OBJ_nid2obj(nid); + X509_ALGOR_set0(algor, OBJ_nid2obj(nid), V_ASN1_NULL, NULL); + ASN1_STRING_set(digest, data, in_len); + *out_len = (unsigned int)i2d_X509_SIG(digestInfo, &out); + X509_SIG_free(digestInfo); return true; } @@ -303,6 +302,7 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { unsigned char dmq1[128]; unsigned char iqmp[128]; int element_len = 128; + const BIGNUM *bn_e, *bn_p, *bn_q, *bn_dmp1, *bn_dmq1, *bn_iqmp; bio = BIO_new_mem_buf(private_key_pem, strlen(private_key_pem)); private_key = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); @@ -310,12 +310,15 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { BIO_free(bio); rsa_private_key = EVP_PKEY_get1_RSA(private_key); ck_assert_ptr_nonnull(rsa_private_key); - ck_assert(set_component(e, rsa_private_key->e, 3)); - ck_assert(set_component(p, rsa_private_key->p, element_len)); - ck_assert(set_component(q, rsa_private_key->q, element_len)); - ck_assert(set_component(dmp1, rsa_private_key->dmp1, element_len)); - ck_assert(set_component(dmq1, rsa_private_key->dmq1, element_len)); - ck_assert(set_component(iqmp, rsa_private_key->iqmp, element_len)); + RSA_get0_key(rsa_private_key, NULL, &bn_e, NULL); + RSA_get0_factors(rsa_private_key, &bn_p, &bn_q); + RSA_get0_crt_params(rsa_private_key, &bn_dmp1, &bn_dmq1, &bn_iqmp); + ck_assert(set_component(e, bn_e, 3)); + ck_assert(set_component(p, bn_p, element_len)); + ck_assert(set_component(q, bn_q, element_len)); + ck_assert(set_component(dmp1, bn_dmp1, element_len)); + ck_assert(set_component(dmq1, bn_dmq1, element_len)); + ck_assert(set_component(iqmp, bn_iqmp, element_len)); // Try wrong algorithm, fail. res = ykpiv_import_private_key(g_state, @@ -342,6 +345,7 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { NULL, 0, pp, tp); ck_assert_int_eq(res, YKPIV_OK); + RSA_free(rsa_private_key); EVP_PKEY_free(private_key); } @@ -364,7 +368,9 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { ck_assert_ptr_nonnull(pub_key); rsa = EVP_PKEY_get1_RSA(pub_key); ck_assert_ptr_nonnull(rsa); - ck_assert_int_gt(RAND_pseudo_bytes(secret, sizeof(secret)), 0); + EVP_PKEY_free(pub_key); + + ck_assert_int_gt(RAND_bytes(secret, sizeof(secret)), 0); len = RSA_public_encrypt(sizeof(secret), secret, data, rsa, RSA_PKCS1_PADDING); ck_assert_int_ge(len, 0); res = ykpiv_verify(g_state, "123456", NULL); @@ -374,6 +380,7 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { len = RSA_padding_check_PKCS1_type_2(secret2, sizeof(secret2), data + 1, len2 - 1, RSA_size(rsa)); ck_assert_int_eq(len, sizeof(secret)); ck_assert_int_eq(memcmp(secret, secret2, sizeof(secret)), 0); + RSA_free(rsa); X509_free(cert); } } @@ -411,8 +418,9 @@ START_TEST(test_import_key) { ck_assert_ptr_nonnull(pub_key); rsa = EVP_PKEY_get1_RSA(pub_key); ck_assert_ptr_nonnull(rsa); + EVP_PKEY_free(pub_key); - ck_assert_int_gt(RAND_pseudo_bytes(rand, 128), 0); + ck_assert_int_gt(RAND_bytes(rand, 128), 0); mdctx = EVP_MD_CTX_create(); EVP_DigestInit_ex(mdctx, md, NULL); EVP_DigestUpdate(mdctx, rand, 128); @@ -425,7 +433,9 @@ START_TEST(test_import_key) { ck_assert_int_eq(RSA_verify(EVP_MD_type(md), data, data_len, signature, sig_len, rsa), 1); + RSA_free(rsa); X509_free(cert); + EVP_MD_CTX_destroy(mdctx); } // Verify that imported key can not be attested @@ -488,8 +498,9 @@ START_TEST(test_pin_policy_always) { ck_assert_ptr_nonnull(pub_key); rsa = EVP_PKEY_get1_RSA(pub_key); ck_assert_ptr_nonnull(rsa); + EVP_PKEY_free(pub_key); - ck_assert_int_gt(RAND_pseudo_bytes(rand, 128), 0); + ck_assert_int_gt(RAND_bytes(rand, 128), 0); mdctx = EVP_MD_CTX_create(); EVP_DigestInit_ex(mdctx, md, NULL); EVP_DigestUpdate(mdctx, rand, 128); @@ -520,7 +531,9 @@ START_TEST(test_pin_policy_always) { ck_assert_int_eq(RSA_verify(EVP_MD_type(md), data, data_len, signature, sig_len, rsa), 1); + RSA_free(rsa); X509_free(cert); + EVP_MD_CTX_destroy(mdctx); } } END_TEST @@ -860,6 +873,7 @@ START_TEST(test_pin_cache) { ykpiv_rc res; ykpiv_state *local_state; unsigned char data[256]; + unsigned char data_in[256] = {0}; int len = sizeof(data); size_t len2 = sizeof(data); @@ -877,17 +891,17 @@ START_TEST(test_pin_cache) { ck_assert_int_eq(res, YKPIV_OK); // Verify decryption does not work without auth - res = ykpiv_decipher_data(g_state, data, (size_t)len, data, &len2, YKPIV_ALGO_RSA2048, 0x9a); + res = ykpiv_decipher_data(g_state, data_in, (size_t)len, data, &len2, YKPIV_ALGO_RSA2048, 0x9a); ck_assert_int_eq(res, YKPIV_AUTHENTICATION_ERROR); // Verify decryption does work when authed res = ykpiv_verify_select(g_state, "123456", 6, NULL, true); ck_assert_int_eq(res, YKPIV_OK); - res = ykpiv_decipher_data(g_state, data, (size_t)len, data, &len2, YKPIV_ALGO_RSA2048, 0x9a); + res = ykpiv_decipher_data(g_state, data_in, (size_t)len, data, &len2, YKPIV_ALGO_RSA2048, 0x9a); ck_assert_int_eq(res, YKPIV_OK); // Verify PIN policy allows continuing to decrypt without re-verifying - res = ykpiv_decipher_data(g_state, data, (size_t)len, data, &len2, YKPIV_ALGO_RSA2048, 0x9a); + res = ykpiv_decipher_data(g_state, data_in, (size_t)len, data, &len2, YKPIV_ALGO_RSA2048, 0x9a); ck_assert_int_eq(res, YKPIV_OK); // Create a new ykpiv state, connect, and close it. @@ -908,7 +922,7 @@ START_TEST(test_pin_cache) { // // Note that you can verify that this fails by rebuilding with // DISABLE_PIN_CACHE set to 1. - res = ykpiv_decipher_data(g_state, data, (size_t)len, data, &len2, YKPIV_ALGO_RSA2048, 0x9a); + res = ykpiv_decipher_data(g_state, data_in, (size_t)len, data, &len2, YKPIV_ALGO_RSA2048, 0x9a); ck_assert_int_eq(res, YKPIV_OK); } END_TEST diff --git a/lib/ykpiv.c b/lib/ykpiv.c index f556c87..e9d5a92 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -399,7 +399,7 @@ ykpiv_rc ykpiv_connect(ykpiv_state *state, const char *wanted) { } static ykpiv_rc reconnect(ykpiv_state *state) { - pcsc_word active_protocol; + pcsc_word active_protocol = 0; long rc; ykpiv_rc res; int tries; @@ -813,7 +813,7 @@ static ykpiv_rc _general_authenticate(ykpiv_state *state, unsigned char templ[] = {0, YKPIV_INS_AUTHENTICATE, algorithm, key}; unsigned long recv_len = sizeof(data); size_t key_len = 0; - int sw; + int sw = 0; size_t bytes; size_t len = 0; ykpiv_rc res; @@ -982,6 +982,9 @@ static ykpiv_rc _cache_pin(ykpiv_state *state, const char *pin, size_t len) { #else if (!state) return YKPIV_ARGUMENT_ERROR; + if (pin && state->pin == pin) { + return YKPIV_OK; + } if (state->pin) { _ykpiv_free(state, state->pin); state->pin = NULL; diff --git a/tool/util.c b/tool/util.c index e845cfc..de6b071 100644 --- a/tool/util.c +++ b/tool/util.c @@ -344,13 +344,11 @@ bool prepare_rsa_signature(const unsigned char *in, unsigned int in_len, unsigne digestInfo = X509_SIG_new(); X509_SIG_getm(digestInfo, &algor, &digest); - algor = X509_ALGOR_new(); - X509_ALGOR_set0(algor, OBJ_nid2obj(nid), V_ASN1_NULL, ¶meter); - parameter.type = V_ASN1_NULL; - parameter.value.ptr = NULL; - digest->data = data; - digest->length = (int)in_len; + algor->algorithm = OBJ_nid2obj(nid); + X509_ALGOR_set0(algor, OBJ_nid2obj(nid), V_ASN1_NULL, NULL); + ASN1_STRING_set(digest, data, in_len); *out_len = (unsigned int)i2d_X509_SIG(digestInfo, &out); + X509_SIG_free(digestInfo); return true; } diff --git a/tool/yubico-piv-tool.c b/tool/yubico-piv-tool.c index 0186e6f..89daa79 100644 --- a/tool/yubico-piv-tool.c +++ b/tool/yubico-piv-tool.c @@ -221,14 +221,16 @@ static bool generate_key(ykpiv_state *state, enum enum_slot slot, if(key_format == key_format_arg_PEM) { public_key = EVP_PKEY_new(); if(algorithm == algorithm_arg_RSA1024 || algorithm == algorithm_arg_RSA2048) { + BIGNUM *bignum_n = NULL; + BIGNUM *bignum_e = NULL; rsa = RSA_new(); - rsa->n = BN_bin2bn(mod, mod_len, NULL); - if (rsa->n == NULL) { + bignum_n = BN_bin2bn(mod, mod_len, NULL); + if (bignum_n == NULL) { fprintf(stderr, "Failed to parse public key modulus.\n"); goto generate_out; } - rsa->e = BN_bin2bn(exp, exp_len, NULL); - if(rsa->e == NULL) { + bignum_e = BN_bin2bn(exp, exp_len, NULL); + if(bignum_e == NULL) { fprintf(stderr, "Failed to parse public key exponent.\n"); goto generate_out; } diff --git a/ykcs11/tests/Makefile.am b/ykcs11/tests/Makefile.am index 800b610..456abbb 100644 --- a/ykcs11/tests/Makefile.am +++ b/ykcs11/tests/Makefile.am @@ -44,6 +44,7 @@ endif ykcs11_tests_LDADD = ../libykcs11.la $(OPENSSL_LIBS) +ykcs11_tests_SOURCES = ykcs11_tests.c ../../tool/openssl-compat.c ../../tool/openssl-compat.h check_PROGRAMS = ykcs11_tests TESTS = reset.sh $(check_PROGRAMS) diff --git a/ykcs11/tests/ykcs11_tests.c b/ykcs11/tests/ykcs11_tests.c index 28c7d23..9fb51da 100644 --- a/ykcs11/tests/ykcs11_tests.c +++ b/ykcs11/tests/ykcs11_tests.c @@ -28,6 +28,7 @@ * */ +#include "../../tool/openssl-compat.h" #include #include