From f2cf3afc6fa1e13e960f732c0bc658ad408ee219 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 12 Jun 2020 14:12:59 +0900 Subject: [PATCH 1/3] pkey: fix potential memory leak in PKey#sign Fix potential leak of EVP_MD_CTX object in an error path. This path is normally unreachable, since the size of a signature generated by any supported algorithms would not be larger than LONG_MAX. --- ext/openssl/ossl_pkey.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index df8b425a0f..7488190e0e 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -777,8 +777,10 @@ ossl_pkey_sign(VALUE self, VALUE digest, VALUE data) EVP_MD_CTX_free(ctx); ossl_raise(ePKeyError, "EVP_DigestSign"); } - if (siglen > LONG_MAX) + if (siglen > LONG_MAX) { + EVP_MD_CTX_free(ctx); rb_raise(ePKeyError, "signature would be too large"); + } sig = ossl_str_new(NULL, (long)siglen, &state); if (state) { EVP_MD_CTX_free(ctx); @@ -799,8 +801,10 @@ ossl_pkey_sign(VALUE self, VALUE digest, VALUE data) EVP_MD_CTX_free(ctx); ossl_raise(ePKeyError, "EVP_DigestSignFinal"); } - if (siglen > LONG_MAX) + if (siglen > LONG_MAX) { + EVP_MD_CTX_free(ctx); rb_raise(ePKeyError, "signature would be too large"); + } sig = ossl_str_new(NULL, (long)siglen, &state); if (state) { EVP_MD_CTX_free(ctx); -- 2.32.0 From 8b30ce20eb9e03180c28288e29a96308e594f860 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 2 Apr 2021 23:58:48 +0900 Subject: [PATCH 2/3] pkey: prepare pkey_ctx_apply_options() for usage by other operations The routine to apply Hash to EVP_PKEY_CTX_ctrl_str() is currently used by key generation, but it is useful for other operations too. Let's change it to a slightly more generic name. --- ext/openssl/ossl_pkey.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 7488190e0e..fed4a2b81f 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -198,7 +198,7 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self) } static VALUE -pkey_gen_apply_options_i(RB_BLOCK_CALL_FUNC_ARGLIST(i, ctx_v)) +pkey_ctx_apply_options_i(RB_BLOCK_CALL_FUNC_ARGLIST(i, ctx_v)) { VALUE key = rb_ary_entry(i, 0), value = rb_ary_entry(i, 1); EVP_PKEY_CTX *ctx = (EVP_PKEY_CTX *)ctx_v; @@ -214,15 +214,25 @@ pkey_gen_apply_options_i(RB_BLOCK_CALL_FUNC_ARGLIST(i, ctx_v)) } static VALUE -pkey_gen_apply_options0(VALUE args_v) +pkey_ctx_apply_options0(VALUE args_v) { VALUE *args = (VALUE *)args_v; rb_block_call(args[1], rb_intern("each"), 0, NULL, - pkey_gen_apply_options_i, args[0]); + pkey_ctx_apply_options_i, args[0]); return Qnil; } +static void +pkey_ctx_apply_options(EVP_PKEY_CTX *ctx, VALUE options, int *state) +{ + VALUE args[2]; + args[0] = (VALUE)ctx; + args[1] = options; + + rb_protect(pkey_ctx_apply_options0, (VALUE)args, state); +} + struct pkey_blocking_generate_arg { EVP_PKEY_CTX *ctx; EVP_PKEY *pkey; @@ -330,11 +340,7 @@ pkey_generate(int argc, VALUE *argv, VALUE self, int genparam) } if (!NIL_P(options)) { - VALUE args[2]; - - args[0] = (VALUE)ctx; - args[1] = options; - rb_protect(pkey_gen_apply_options0, (VALUE)args, &state); + pkey_ctx_apply_options(ctx, options, &state); if (state) { EVP_PKEY_CTX_free(ctx); rb_jump_tag(state); -- 2.32.0 From 4c7b0f91da666961d11908b94520db4e09ce4e67 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sat, 18 Jul 2020 20:40:39 +0900 Subject: [PATCH 3/3] pkey: allow setting algorithm-specific options in #sign and #verify Similarly to OpenSSL::PKey.generate_key and .generate_parameters, let OpenSSL::PKey::PKey#sign and #verify take an optional parameter for specifying control strings for EVP_PKEY_CTX_ctrl_str(). --- ext/openssl/ossl_pkey.c | 113 ++++++++++++++++++++++------------ test/openssl/test_pkey_rsa.rb | 34 +++++----- 2 files changed, 89 insertions(+), 58 deletions(-) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index fed4a2b81f..22e9f19982 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -739,33 +739,51 @@ ossl_pkey_public_to_pem(VALUE self) } /* - * call-seq: - * pkey.sign(digest, data) -> String + * call-seq: + * pkey.sign(digest, data [, options]) -> string * - * To sign the String _data_, _digest_, an instance of OpenSSL::Digest, must - * be provided. The return value is again a String containing the signature. - * A PKeyError is raised should errors occur. - * Any previous state of the Digest instance is irrelevant to the signature - * outcome, the digest instance is reset to its initial state during the - * operation. + * Hashes and signs the +data+ using a message digest algorithm +digest+ and + * a private key +pkey+. * - * == Example - * data = 'Sign me!' - * digest = OpenSSL::Digest.new('SHA256') - * pkey = OpenSSL::PKey::RSA.new(2048) - * signature = pkey.sign(digest, data) + * See #verify for the verification operation. + * + * See also the man page EVP_DigestSign(3). + * + * +digest+:: + * A String that represents the message digest algorithm name, or +nil+ + * if the PKey type requires no digest algorithm. + * For backwards compatibility, this can be an instance of OpenSSL::Digest. + * Its state will not affect the signature. + * +data+:: + * A String. The data to be hashed and signed. + * +options+:: + * A Hash that contains algorithm specific control operations to \OpenSSL. + * See OpenSSL's man page EVP_PKEY_CTX_ctrl_str(3) for details. + * +options+ parameter was added in version 2.3. + * + * Example: + * data = "Sign me!" + * pkey = OpenSSL::PKey.generate_key("RSA", rsa_keygen_bits: 2048) + * signopts = { rsa_padding_mode: "pss" } + * signature = pkey.sign("SHA256", data, signopts) + * + * # Creates a copy of the RSA key pkey, but without the private components + * pub_key = pkey.public_key + * puts pub_key.verify("SHA256", signature, data, signopts) # => true */ static VALUE -ossl_pkey_sign(VALUE self, VALUE digest, VALUE data) +ossl_pkey_sign(int argc, VALUE *argv, VALUE self) { EVP_PKEY *pkey; + VALUE digest, data, options, sig; const EVP_MD *md = NULL; EVP_MD_CTX *ctx; + EVP_PKEY_CTX *pctx; size_t siglen; int state; - VALUE sig; pkey = GetPrivPKeyPtr(self); + rb_scan_args(argc, argv, "21", &digest, &data, &options); if (!NIL_P(digest)) md = ossl_evp_get_digestbyname(digest); StringValue(data); @@ -773,10 +791,17 @@ ossl_pkey_sign(VALUE self, VALUE digest, VALUE data) ctx = EVP_MD_CTX_new(); if (!ctx) ossl_raise(ePKeyError, "EVP_MD_CTX_new"); - if (EVP_DigestSignInit(ctx, NULL, md, /* engine */NULL, pkey) < 1) { + if (EVP_DigestSignInit(ctx, &pctx, md, /* engine */NULL, pkey) < 1) { EVP_MD_CTX_free(ctx); ossl_raise(ePKeyError, "EVP_DigestSignInit"); } + if (!NIL_P(options)) { + pkey_ctx_apply_options(pctx, options, &state); + if (state) { + EVP_MD_CTX_free(ctx); + rb_jump_tag(state); + } + } #if OPENSSL_VERSION_NUMBER >= 0x10101000 && !defined(LIBRESSL_VERSION_NUMBER) if (EVP_DigestSign(ctx, NULL, &siglen, (unsigned char *)RSTRING_PTR(data), RSTRING_LEN(data)) < 1) { @@ -828,35 +853,40 @@ ossl_pkey_sign(VALUE self, VALUE digest, VALUE data) } /* - * call-seq: - * pkey.verify(digest, signature, data) -> String + * call-seq: + * pkey.verify(digest, signature, data [, options]) -> true or false * - * To verify the String _signature_, _digest_, an instance of - * OpenSSL::Digest, must be provided to re-compute the message digest of the - * original _data_, also a String. The return value is +true+ if the - * signature is valid, +false+ otherwise. A PKeyError is raised should errors - * occur. - * Any previous state of the Digest instance is irrelevant to the validation - * outcome, the digest instance is reset to its initial state during the - * operation. + * Verifies the +signature+ for the +data+ using a message digest algorithm + * +digest+ and a public key +pkey+. * - * == Example - * data = 'Sign me!' - * digest = OpenSSL::Digest.new('SHA256') - * pkey = OpenSSL::PKey::RSA.new(2048) - * signature = pkey.sign(digest, data) - * pub_key = pkey.public_key - * puts pub_key.verify(digest, signature, data) # => true + * Returns +true+ if the signature is successfully verified, +false+ otherwise. + * The caller must check the return value. + * + * See #sign for the signing operation and an example. + * + * See also the man page EVP_DigestVerify(3). + * + * +digest+:: + * See #sign. + * +signature+:: + * A String containing the signature to be verified. + * +data+:: + * See #sign. + * +options+:: + * See #sign. +options+ parameter was added in version 2.3. */ static VALUE -ossl_pkey_verify(VALUE self, VALUE digest, VALUE sig, VALUE data) +ossl_pkey_verify(int argc, VALUE *argv, VALUE self) { EVP_PKEY *pkey; + VALUE digest, sig, data, options; const EVP_MD *md = NULL; EVP_MD_CTX *ctx; - int ret; + EVP_PKEY_CTX *pctx; + int state, ret; GetPKey(self, pkey); + rb_scan_args(argc, argv, "31", &digest, &sig, &data, &options); ossl_pkey_check_public_key(pkey); if (!NIL_P(digest)) md = ossl_evp_get_digestbyname(digest); @@ -866,10 +896,17 @@ ossl_pkey_verify(VALUE self, VALUE digest, VALUE sig, VALUE data) ctx = EVP_MD_CTX_new(); if (!ctx) ossl_raise(ePKeyError, "EVP_MD_CTX_new"); - if (EVP_DigestVerifyInit(ctx, NULL, md, /* engine */NULL, pkey) < 1) { + if (EVP_DigestVerifyInit(ctx, &pctx, md, /* engine */NULL, pkey) < 1) { EVP_MD_CTX_free(ctx); ossl_raise(ePKeyError, "EVP_DigestVerifyInit"); } + if (!NIL_P(options)) { + pkey_ctx_apply_options(pctx, options, &state); + if (state) { + EVP_MD_CTX_free(ctx); + rb_jump_tag(state); + } + } #if OPENSSL_VERSION_NUMBER >= 0x10101000 && !defined(LIBRESSL_VERSION_NUMBER) ret = EVP_DigestVerify(ctx, (unsigned char *)RSTRING_PTR(sig), RSTRING_LEN(sig), (unsigned char *)RSTRING_PTR(data), @@ -1042,8 +1079,8 @@ Init_ossl_pkey(void) rb_define_method(cPKey, "public_to_der", ossl_pkey_public_to_der, 0); rb_define_method(cPKey, "public_to_pem", ossl_pkey_public_to_pem, 0); - rb_define_method(cPKey, "sign", ossl_pkey_sign, 2); - rb_define_method(cPKey, "verify", ossl_pkey_verify, 3); + rb_define_method(cPKey, "sign", ossl_pkey_sign, -1); + rb_define_method(cPKey, "verify", ossl_pkey_verify, -1); rb_define_method(cPKey, "derive", ossl_pkey_derive, -1); id_private_q = rb_intern("private?"); diff --git a/test/openssl/test_pkey_rsa.rb b/test/openssl/test_pkey_rsa.rb index 88164c3b52..d1e68dbc9f 100644 --- a/test/openssl/test_pkey_rsa.rb +++ b/test/openssl/test_pkey_rsa.rb @@ -117,27 +117,21 @@ def test_sign_verify assert_equal false, rsa1024.verify("SHA256", signature1, data) end - def test_digest_state_irrelevant_sign + def test_sign_verify_options key = Fixtures.pkey("rsa1024") - digest1 = OpenSSL::Digest.new('SHA1') - digest2 = OpenSSL::Digest.new('SHA1') - data = 'Sign me!' - digest1 << 'Change state of digest1' - sig1 = key.sign(digest1, data) - sig2 = key.sign(digest2, data) - assert_equal(sig1, sig2) - end - - def test_digest_state_irrelevant_verify - key = Fixtures.pkey("rsa1024") - digest1 = OpenSSL::Digest.new('SHA1') - digest2 = OpenSSL::Digest.new('SHA1') - data = 'Sign me!' - sig = key.sign(digest1, data) - digest1.reset - digest1 << 'Change state of digest1' - assert(key.verify(digest1, sig, data)) - assert(key.verify(digest2, sig, data)) + data = "Sign me!" + pssopts = { + "rsa_padding_mode" => "pss", + "rsa_pss_saltlen" => 20, + "rsa_mgf1_md" => "SHA1" + } + sig_pss = key.sign("SHA256", data, pssopts) + assert_equal 128, sig_pss.bytesize + assert_equal true, key.verify("SHA256", sig_pss, data, pssopts) + assert_equal true, key.verify_pss("SHA256", sig_pss, data, + salt_length: 20, mgf1_hash: "SHA1") + # Defaults to PKCS #1 v1.5 padding => verification failure + assert_equal false, key.verify("SHA256", sig_pss, data) end def test_verify_empty_rsa -- 2.32.0