From ff7831bdecc3efaab3a37354d5858939da354260 Mon Sep 17 00:00:00 2001 From: Joakim Antman Date: Wed, 27 Mar 2024 22:53:35 +0200 Subject: [PATCH] Use rb_protect and Ruby internals to generate strings --- ext/openssl/ossl_pkey.c | 53 +++++++++++++++++++++++---------------- test/openssl/test_pkey.rb | 2 +- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index b41b7bd36..d3604be9b 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -471,13 +471,14 @@ static const struct pkey_from_data_alias fcc_aliases[] = { }; struct pkey_from_data_arg { + VALUE options; OSSL_PARAM_BLD *param_bld; const OSSL_PARAM *settable_params; const struct pkey_from_data_alias *aliases; }; static int -add_parameter_to_builder(VALUE key, VALUE value, VALUE arg) { +add_data_to_builder(VALUE key, VALUE value, VALUE arg) { if(NIL_P(value)) return ST_CONTINUE; @@ -500,14 +501,12 @@ add_parameter_to_builder(VALUE key, VALUE value, VALUE arg) { case OSSL_PARAM_INTEGER: case OSSL_PARAM_UNSIGNED_INTEGER: if(!OSSL_PARAM_BLD_push_BN(params->param_bld, key_ptr, GetBNPtr(value))) { - OSSL_PARAM_BLD_free(params->param_bld); ossl_raise(ePKeyError, "OSSL_PARAM_BLD_push_BN"); } break; case OSSL_PARAM_UTF8_STRING: StringValue(value); if(!OSSL_PARAM_BLD_push_utf8_string(params->param_bld, key_ptr, RSTRING_PTR(value), RSTRING_LENINT(value))) { - OSSL_PARAM_BLD_free(params->param_bld); ossl_raise(ePKeyError, "OSSL_PARAM_BLD_push_utf8_string"); } break; @@ -515,13 +514,11 @@ add_parameter_to_builder(VALUE key, VALUE value, VALUE arg) { case OSSL_PARAM_OCTET_STRING: StringValue(value); if(!OSSL_PARAM_BLD_push_octet_string(params->param_bld, key_ptr, RSTRING_PTR(value), RSTRING_LENINT(value))) { - OSSL_PARAM_BLD_free(params->param_bld); ossl_raise(ePKeyError, "OSSL_PARAM_BLD_push_octet_string"); } break; case OSSL_PARAM_UTF8_PTR: case OSSL_PARAM_OCTET_PTR: - OSSL_PARAM_BLD_free(params->param_bld); ossl_raise(ePKeyError, "Unsupported parameter \"%s\", handling of OSSL_PARAM_UTF8_PTR and OSSL_PARAM_OCTET_PTR not implemented", key_ptr); break; } @@ -529,26 +526,28 @@ add_parameter_to_builder(VALUE key, VALUE value, VALUE arg) { return ST_CONTINUE; } } - OSSL_PARAM_BLD_free(params->param_bld); - char message_buffer[512] = { 0 }; - char *cur = message_buffer; - char *end = message_buffer + sizeof(message_buffer); + VALUE supported_parameters = rb_ary_new(); + for (const OSSL_PARAM *settable_params = params->settable_params; settable_params->key != NULL; settable_params++) { - const char *fmt = cur == message_buffer ? "%s" : ", %s"; - if (cur > end) - break; - cur += snprintf(cur, end-cur, fmt, settable_params->key); + rb_ary_push(supported_parameters, rb_str_new_cstr(settable_params->key)); } for(int i = 0; strlen(params->aliases[i].alias) > 0; i++) { - const char *fmt = cur == message_buffer ? "%s" : ", %s"; - if (cur > end) - break; - cur += snprintf(cur, end-cur, fmt, params->aliases[i].alias); + rb_ary_push(supported_parameters, rb_str_new_cstr(params->aliases[i].alias)); } - ossl_raise(ePKeyError, "Invalid parameter \"%s\". Supported parameters: \"%s\"", key_ptr, message_buffer); + ossl_raise(ePKeyError, "Invalid parameter \"%s\". Supported parameters: %"PRIsVALUE, key_ptr, rb_ary_join(supported_parameters, rb_str_new2(", "))); +} + +static VALUE +add_data_to_builder_protected(VALUE value) +{ + struct pkey_from_data_arg *args = (void *)value; + + rb_hash_foreach(args->options, &add_data_to_builder, (VALUE) args); + + return Qnil; } static VALUE @@ -567,6 +566,7 @@ pkey_from_data(int argc, VALUE *argv, VALUE self) struct pkey_from_data_arg from_data_args = { 0 }; from_data_args.param_bld = OSSL_PARAM_BLD_new(); + from_data_args.options = options; if (from_data_args.param_bld == NULL) { EVP_PKEY_CTX_free(ctx); @@ -577,6 +577,7 @@ pkey_from_data(int argc, VALUE *argv, VALUE self) if (from_data_args.settable_params == NULL) { EVP_PKEY_CTX_free(ctx); + OSSL_PARAM_BLD_free(from_data_args.param_bld); ossl_raise(ePKeyError, "EVP_PKEY_fromdata_settable"); } @@ -585,7 +586,14 @@ pkey_from_data(int argc, VALUE *argv, VALUE self) else from_data_args.aliases = fcc_aliases; - rb_hash_foreach(options, &add_parameter_to_builder, (VALUE) &from_data_args); + int state; + rb_protect(add_data_to_builder_protected, (VALUE) &from_data_args, &state); + + if(state) { + EVP_PKEY_CTX_free(ctx); + OSSL_PARAM_BLD_free(from_data_args.param_bld); + rb_jump_tag(state); + } OSSL_PARAM *params = OSSL_PARAM_BLD_to_param(from_data_args.param_bld); OSSL_PARAM_BLD_free(from_data_args.param_bld); @@ -612,6 +620,7 @@ pkey_from_data(int argc, VALUE *argv, VALUE self) return ossl_pkey_new(pkey); } + #endif /* @@ -686,11 +695,11 @@ ossl_pkey_s_generate_key(int argc, VALUE *argv, VALUE self) static VALUE ossl_pkey_s_from_data(int argc, VALUE *argv, VALUE self) { - #if OSSL_OPENSSL_PREREQ(3, 0, 0) +#if OSSL_OPENSSL_PREREQ(3, 0, 0) return pkey_from_data(argc, argv, self); - #else +#else rb_raise(ePKeyError, "OpenSSL::PKey.from_data requires OpenSSL 3.0 or later"); - #endif +#endif } /* diff --git a/test/openssl/test_pkey.rb b/test/openssl/test_pkey.rb index 3711833d6..c67a7c296 100644 --- a/test/openssl/test_pkey.rb +++ b/test/openssl/test_pkey.rb @@ -413,7 +413,7 @@ def test_s_from_data_dsa_with_gem_specific_keys end def test_s_from_data_dsa_with_invalid_parameter - assert_raise_with_message(OpenSSL::PKey::PKeyError, /Invalid parameter "invalid"/) do + assert_raise_with_message(OpenSSL::PKey::PKeyError, /Invalid parameter "invalid". Supported parameters: p, q, g, j/) do OpenSSL::PKey.from_data("DSA", invalid: 1234) end end