From 3b3e1e2458eeca2855d6bac7851470dfa82ca2fb Mon Sep 17 00:00:00 2001 From: Per Nilsson Date: Tue, 27 Aug 2024 14:26:18 +0200 Subject: [PATCH] misc: Refactor set_component --- common/util.c | 19 +++++++++++-------- common/util.h | 2 +- lib/tests/api.c | 41 ++++++++++++++++++++++++----------------- tool/yubico-piv-tool.c | 33 ++++++++++++++++++++------------- 4 files changed, 56 insertions(+), 39 deletions(-) diff --git a/common/util.c b/common/util.c index 8701ae24..f0a9d5c9 100644 --- a/common/util.c +++ b/common/util.c @@ -382,8 +382,10 @@ int get_slot_hex(enum enum_slot slot_enum) { return slot; } -bool set_component(unsigned char *in_ptr, const BIGNUM *bn, int element_len) { - return BN_bn2binpad(bn, in_ptr, element_len) == element_len; +bool set_component(unsigned char *in_ptr, const BIGNUM *bn, int *element_len) { + if(BN_num_bytes(bn) > *element_len) return false; + *element_len = BN_bn2bin(bn, in_ptr); + return true; } bool prepare_rsa_signature(const unsigned char *in, unsigned int in_len, unsigned char *out, unsigned int *out_len, int nid) { @@ -612,26 +614,27 @@ int SSH_write_X509(FILE *fp, X509 *x) { switch (EVP_PKEY_base_id(pkey)) { case EVP_PKEY_RSA: { - RSA *rsa; + const RSA *rsa; unsigned char n[256] = {0}; const BIGNUM *bn_n; char rsa_id[] = "\x00\x00\x00\x07ssh-rsa"; char rsa_f4[] = "\x00\x00\x00\x03\x01\x00\x01"; - rsa = EVP_PKEY_get1_RSA(pkey); + rsa = EVP_PKEY_get0_RSA(pkey); if(rsa == NULL) { break; } RSA_get0_key(rsa, &bn_n, NULL, NULL); - if (!set_component(n, bn_n, RSA_size(rsa))) { + int len = RSA_size(rsa); + if (!set_component(n, bn_n, &len)) { break; } - uint32_t bytes = BN_num_bytes(bn_n); + uint32_t bytes = len; char len_buf[5] = {0}; - int len = 4; + len = 4; len_buf[0] = (bytes >> 24) & 0x000000ff; len_buf[1] = (bytes << 16) & 0x000000ff; @@ -668,7 +671,7 @@ int SSH_write_X509(FILE *fp, X509 *x) { BIO_free_all(b64); break; } - if(BIO_write(b64, n, RSA_size(rsa)) <= 0) { + if(BIO_write(b64, n, bytes) <= 0) { fprintf(stderr, "Failed to write RSA n component\n"); BIO_free_all(b64); break; diff --git a/common/util.h b/common/util.h index 6d59bd0d..cfe97821 100644 --- a/common/util.h +++ b/common/util.h @@ -54,7 +54,7 @@ X509_NAME *parse_name(const char*); unsigned char get_algorithm(EVP_PKEY*); FILE *open_file(const char *file_name, enum file_mode mode); int get_slot_hex(enum enum_slot slot_enum); -bool set_component(unsigned char *in_ptr, const BIGNUM *bn, int element_len); +bool set_component(unsigned char *in_ptr, const BIGNUM *bn, int *element_len); bool prepare_rsa_signature(const unsigned char*, unsigned int, unsigned char*, unsigned int*, int); bool read_pw(const char*, char*, size_t, int, int); diff --git a/lib/tests/api.c b/lib/tests/api.c index 8919f4f9..9e6373e7 100644 --- a/lib/tests/api.c +++ b/lib/tests/api.c @@ -324,7 +324,7 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { EVP_PKEY *private_key = NULL; BIO *bio = NULL; RSA *rsa_private_key = NULL; - unsigned char e[4] = {0}; + unsigned char e[3] = {0}; unsigned char p[256] = {0}; unsigned char q[256] = {0}; unsigned char dmp1[256] = {0}; @@ -332,6 +332,7 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { unsigned char iqmp[256] = {0}; int element_len = 256; const BIGNUM *bn_e, *bn_p, *bn_q, *bn_dmp1, *bn_dmq1, *bn_iqmp; + int e_len, p_len, q_len, dmp1_len, dmq1_len, iqmp_len; bio = BIO_new_mem_buf(private_key_pem, strlen(private_key_pem)); private_key = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); @@ -342,22 +343,28 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { 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)); + e_len = sizeof(e); + ck_assert(set_component(e, bn_e, &e_len)); + p_len = element_len; + ck_assert(set_component(p, bn_p, &p_len)); + q_len = element_len; + ck_assert(set_component(q, bn_q, &q_len)); + dmp1_len = element_len; + ck_assert(set_component(dmp1, bn_dmp1, &dmp1_len)); + dmq1_len = element_len; + ck_assert(set_component(dmq1, bn_dmq1, &dmq1_len)); + iqmp_len = element_len; + ck_assert(set_component(iqmp, bn_iqmp, &iqmp_len)); // Try wrong algorithm, fail. res = ykpiv_import_private_key(g_state, slot, YKPIV_ALGO_RSA1024, - p, element_len, - q, element_len, - dmp1, element_len, - dmq1, element_len, - iqmp, element_len, + p, p_len, + q, q_len, + dmp1, dmp1_len, + dmq1, dmq1_len, + iqmp, iqmp_len, NULL, 0, pp, tp); ck_assert_int_eq(res, YKPIV_ALGORITHM_ERROR); @@ -366,11 +373,11 @@ static void import_key(unsigned char slot, unsigned char pin_policy) { res = ykpiv_import_private_key(g_state, slot, YKPIV_ALGO_RSA4096, - p, element_len, - q, element_len, - dmp1, element_len, - dmq1, element_len, - iqmp, element_len, + p, p_len, + q, q_len, + dmp1, dmp1_len, + dmq1, dmq1_len, + iqmp, iqmp_len, NULL, 0, pp, tp); ck_assert_int_eq(res, YKPIV_OK); diff --git a/tool/yubico-piv-tool.c b/tool/yubico-piv-tool.c index d7e99a90..9fc0d650 100644 --- a/tool/yubico-piv-tool.c +++ b/tool/yubico-piv-tool.c @@ -515,13 +515,14 @@ static bool import_key(ykpiv_state *state, enum enum_key_format key_format, if(YKPIV_IS_RSA(algorithm)) { RSA *rsa_private_key = EVP_PKEY_get1_RSA(private_key); - unsigned char e[4] = {0}; + unsigned char e[3] = {0}; unsigned char p[256] = {0}; unsigned char q[256] = {0}; unsigned char dmp1[256] = {0}; unsigned char dmq1[256] = {0}; unsigned char iqmp[256] = {0}; const BIGNUM *bn_e, *bn_p, *bn_q, *bn_dmp1, *bn_dmq1, *bn_iqmp; + int len_e, len_p, len_q, len_dmp1, len_dmq1, len_iqmp; int element_len = 0; switch(algorithm) { @@ -545,43 +546,49 @@ static bool import_key(ykpiv_state *state, enum enum_key_format key_format, 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); - if((set_component(e, bn_e, 3) == false) || + len_e = sizeof(e); + if((set_component(e, bn_e, &len_e) == false) || !(e[0] == 0x01 && e[1] == 0x00 && e[2] == 0x01)) { fprintf(stderr, "Invalid public exponent for import (only 0x10001 supported)\n"); goto import_out; } - if(set_component(p, bn_p, element_len) == false) { + len_p = element_len; + if(set_component(p, bn_p, &len_p) == false) { fprintf(stderr, "Failed setting p component.\n"); goto import_out; } - if(set_component(q, bn_q, element_len) == false) { + len_q = element_len; + if(set_component(q, bn_q, &len_q) == false) { fprintf(stderr, "Failed setting q component.\n"); goto import_out; } - if(set_component(dmp1, bn_dmp1, element_len) == false) { + len_dmp1 = element_len; + if(set_component(dmp1, bn_dmp1, &len_dmp1) == false) { fprintf(stderr, "Failed setting dmp1 component.\n"); goto import_out; } - if(set_component(dmq1, bn_dmq1, element_len) == false) { + len_dmq1 = element_len; + if(set_component(dmq1, bn_dmq1, &len_dmq1) == false) { fprintf(stderr, "Failed setting dmq1 component.\n"); goto import_out; } - if(set_component(iqmp, bn_iqmp, element_len) == false) { + len_iqmp = element_len; + if(set_component(iqmp, bn_iqmp, &len_iqmp) == false) { fprintf(stderr, "Failed setting iqmp component.\n"); goto import_out; } rc = ykpiv_import_private_key(state, key, algorithm, - p, element_len, - q, element_len, - dmp1, element_len, - dmq1, element_len, - iqmp, element_len, + p, len_p, + q, len_q, + dmp1, len_dmp1, + dmq1, len_dmq1, + iqmp, len_iqmp, NULL, 0, pp, tp); } @@ -595,7 +602,7 @@ static bool import_key(ykpiv_state *state, enum enum_key_format key_format, element_len = 48; } - if(set_component(s_ptr, s, element_len) == false) { + if(set_component(s_ptr, s, &element_len) == false) { fprintf(stderr, "Failed setting ec private key.\n"); goto import_out; }