From 3e00a2d7b5ba7d2117e375761f1428e57a8838e6 Mon Sep 17 00:00:00 2001 From: nagachika Date: Mon, 19 Mar 2018 16:22:34 +0000 Subject: [PATCH] Update openssl to 2.0.7. [Bug #13935] The patch is provided by Kazuki Yamaguchi. From: Kazuki Yamaguchi Date: Mon, 25 Sep 2017 01:32:02 +0900 Subject: [PATCH] openssl: import v2.0.7 Import Ruby/OpenSSL 2.0.7. This contains only bug fixes and test improvements. The full commit log since v2.0.5 (imported at r59567, to trunk) can be found at: https://github.com/ruby/openssl/compare/v2.0.5...v2.0.7 All the changes included in this changeset are already imported to trunk by r61235 or earlier revisions. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_4@62845 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ext/openssl/History.md | 17 +++++++++- ext/openssl/openssl.gemspec | 8 ++--- ext/openssl/ossl_asn1.c | 4 +-- ext/openssl/ossl_cipher.c | 2 ++ ext/openssl/ossl_ns_spki.c | 24 +++++++------ ext/openssl/ossl_pkey.c | 9 ++--- ext/openssl/ossl_pkey.h | 1 + ext/openssl/ossl_ssl.c | 13 +++---- ext/openssl/ossl_version.h | 2 +- ext/openssl/ossl_x509cert.c | 15 +++++---- ext/openssl/ossl_x509crl.c | 5 ++- ext/openssl/ossl_x509req.c | 12 +++---- test/openssl/test_cipher.rb | 7 ++++ test/openssl/test_ssl_session.rb | 58 ++++++++++++++++++++++---------- version.h | 2 +- 15 files changed, 115 insertions(+), 64 deletions(-) diff --git a/ext/openssl/History.md b/ext/openssl/History.md index d592bc6a1f0840..9cc60caaf8ba84 100644 --- a/ext/openssl/History.md +++ b/ext/openssl/History.md @@ -1,3 +1,18 @@ +Version 2.0.7 +============= + +Bug fixes +--------- + +* OpenSSL::Cipher#auth_data= could segfault if called against a non-AEAD cipher. + [[Bug #14024]](https://bugs.ruby-lang.org/issues/14024) +* OpenSSL::X509::Certificate#public_key= (and similar methods) could segfault + when an instance of OpenSSL::PKey::PKey with no public key components is + passed. + [[Bug #14087]](https://bugs.ruby-lang.org/issues/14087) + [[GitHub #168]](https://github.com/ruby/openssl/pull/168) + + Version 2.0.6 ============= @@ -170,7 +185,7 @@ Notable changes - A new option 'verify_hostname' is added to OpenSSL::SSL::SSLContext. When it is enabled, and the SNI hostname is also set, the hostname verification on the server certificate is automatically performed. It is now enabled by - OpenSSL::SSL::Context#set_params. + OpenSSL::SSL::SSLContext#set_params. [[GH ruby/openssl#60]](https://github.com/ruby/openssl/pull/60) Removals diff --git a/ext/openssl/openssl.gemspec b/ext/openssl/openssl.gemspec index 2af041ebde9b81..67041dc1f2c38f 100644 --- a/ext/openssl/openssl.gemspec +++ b/ext/openssl/openssl.gemspec @@ -1,16 +1,16 @@ # -*- encoding: utf-8 -*- -# stub: openssl 2.0.6 ruby lib +# stub: openssl 2.0.7 ruby lib # stub: ext/openssl/extconf.rb Gem::Specification.new do |s| s.name = "openssl".freeze - s.version = "2.0.6" + s.version = "2.0.7" s.required_rubygems_version = Gem::Requirement.new(">= 0".freeze) if s.respond_to? :required_rubygems_version= s.metadata = { "msys2_mingw_dependencies" => "openssl" } if s.respond_to? :metadata= s.require_paths = ["lib".freeze] s.authors = ["Martin Bosslet".freeze, "SHIBATA Hiroshi".freeze, "Zachary Scott".freeze, "Kazuki Yamaguchi".freeze] - s.date = "2017-09-24" + s.date = "2017-12-14" s.description = "It wraps the OpenSSL library.".freeze s.email = ["ruby-core@ruby-lang.org".freeze] s.extensions = ["ext/openssl/extconf.rb".freeze] @@ -20,7 +20,7 @@ Gem::Specification.new do |s| s.licenses = ["Ruby".freeze] s.rdoc_options = ["--main".freeze, "README.md".freeze] s.required_ruby_version = Gem::Requirement.new(">= 2.3.0".freeze) - s.rubygems_version = "2.6.13".freeze + s.rubygems_version = "2.7.3".freeze s.summary = "OpenSSL provides SSL, TLS and general purpose cryptography.".freeze if s.respond_to? :specification_version then diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c index 1d3ee4ac180215..0225597334fe27 100644 --- a/ext/openssl/ossl_asn1.c +++ b/ext/openssl/ossl_asn1.c @@ -1718,12 +1718,12 @@ Init_ossl_asn1(void) * == Primitive sub-classes and their mapping to Ruby classes * * OpenSSL::ASN1::EndOfContent <=> +value+ is always +nil+ * * OpenSSL::ASN1::Boolean <=> +value+ is a +Boolean+ - * * OpenSSL::ASN1::Integer <=> +value+ is a +Number+ + * * OpenSSL::ASN1::Integer <=> +value+ is an OpenSSL::BN * * OpenSSL::ASN1::BitString <=> +value+ is a +String+ * * OpenSSL::ASN1::OctetString <=> +value+ is a +String+ * * OpenSSL::ASN1::Null <=> +value+ is always +nil+ * * OpenSSL::ASN1::Object <=> +value+ is a +String+ - * * OpenSSL::ASN1::Enumerated <=> +value+ is a +Number+ + * * OpenSSL::ASN1::Enumerated <=> +value+ is an OpenSSL::BN * * OpenSSL::ASN1::UTF8String <=> +value+ is a +String+ * * OpenSSL::ASN1::NumericString <=> +value+ is a +String+ * * OpenSSL::ASN1::PrintableString <=> +value+ is a +String+ diff --git a/ext/openssl/ossl_cipher.c b/ext/openssl/ossl_cipher.c index 36e42ede8c92f7..740f04b27ea833 100644 --- a/ext/openssl/ossl_cipher.c +++ b/ext/openssl/ossl_cipher.c @@ -580,6 +580,8 @@ ossl_cipher_set_auth_data(VALUE self, VALUE data) in_len = RSTRING_LEN(data); GetCipher(self, ctx); + if (!(EVP_CIPHER_flags(EVP_CIPHER_CTX_cipher(ctx)) & EVP_CIPH_FLAG_AEAD_CIPHER)) + ossl_raise(eCipherError, "AEAD not supported by this cipher"); if (!ossl_cipher_update_long(ctx, NULL, &out_len, in, in_len)) ossl_raise(eCipherError, "couldn't set additional authenticated data"); diff --git a/ext/openssl/ossl_ns_spki.c b/ext/openssl/ossl_ns_spki.c index 4d978bd0096d57..5ac8ef55b54a13 100644 --- a/ext/openssl/ossl_ns_spki.c +++ b/ext/openssl/ossl_ns_spki.c @@ -208,12 +208,13 @@ static VALUE ossl_spki_set_public_key(VALUE self, VALUE key) { NETSCAPE_SPKI *spki; + EVP_PKEY *pkey; GetSPKI(self, spki); - if (!NETSCAPE_SPKI_set_pubkey(spki, GetPKeyPtr(key))) { /* NO NEED TO DUP */ - ossl_raise(eSPKIError, NULL); - } - + pkey = GetPKeyPtr(key); + ossl_pkey_check_public_key(pkey); + if (!NETSCAPE_SPKI_set_pubkey(spki, pkey)) + ossl_raise(eSPKIError, "NETSCAPE_SPKI_set_pubkey"); return key; } @@ -307,17 +308,20 @@ static VALUE ossl_spki_verify(VALUE self, VALUE key) { NETSCAPE_SPKI *spki; + EVP_PKEY *pkey; GetSPKI(self, spki); - switch (NETSCAPE_SPKI_verify(spki, GetPKeyPtr(key))) { /* NO NEED TO DUP */ - case 0: + pkey = GetPKeyPtr(key); + ossl_pkey_check_public_key(pkey); + switch (NETSCAPE_SPKI_verify(spki, pkey)) { + case 0: + ossl_clear_error(); return Qfalse; - case 1: + case 1: return Qtrue; - default: - ossl_raise(eSPKIError, NULL); + default: + ossl_raise(eSPKIError, "NETSCAPE_SPKI_verify"); } - return Qnil; /* dummy */ } /* Document-class: OpenSSL::Netscape::SPKI diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 314d1d94af0a1b..aad3e2e47c31d0 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -163,8 +163,8 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self) return ossl_pkey_new(pkey); } -static void -pkey_check_public_key(EVP_PKEY *pkey) +void +ossl_pkey_check_public_key(const EVP_PKEY *pkey) { void *ptr; const BIGNUM *n, *e, *pubkey; @@ -172,7 +172,8 @@ pkey_check_public_key(EVP_PKEY *pkey) if (EVP_PKEY_missing_parameters(pkey)) ossl_raise(ePKeyError, "parameters missing"); - ptr = EVP_PKEY_get0(pkey); + /* OpenSSL < 1.1.0 takes non-const pointer */ + ptr = EVP_PKEY_get0((EVP_PKEY *)pkey); switch (EVP_PKEY_base_id(pkey)) { case EVP_PKEY_RSA: RSA_get0_key(ptr, &n, &e, NULL); @@ -352,7 +353,7 @@ ossl_pkey_verify(VALUE self, VALUE digest, VALUE sig, VALUE data) int siglen, result; GetPKey(self, pkey); - pkey_check_public_key(pkey); + ossl_pkey_check_public_key(pkey); md = GetDigestPtr(digest); StringValue(sig); siglen = RSTRING_LENINT(sig); diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h index e3b723cd6883bc..a0b497440c1fae 100644 --- a/ext/openssl/ossl_pkey.h +++ b/ext/openssl/ossl_pkey.h @@ -48,6 +48,7 @@ int ossl_generate_cb_2(int p, int n, BN_GENCB *cb); void ossl_generate_cb_stop(void *ptr); VALUE ossl_pkey_new(EVP_PKEY *); +void ossl_pkey_check_public_key(const EVP_PKEY *); EVP_PKEY *GetPKeyPtr(VALUE); EVP_PKEY *DupPKeyPtr(VALUE); EVP_PKEY *GetPrivPKeyPtr(VALUE); diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index aa2dbbc8f748a5..c6bfb8312d9164 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -996,12 +996,7 @@ ossl_sslctx_get_ciphers(VALUE self) int i, num; GetSSLCTX(self, ctx); - if(!ctx){ - rb_warning("SSL_CTX is not initialized."); - return Qnil; - } ciphers = SSL_CTX_get_ciphers(ctx); - if (!ciphers) return rb_ary_new(); @@ -1049,10 +1044,6 @@ ossl_sslctx_set_ciphers(VALUE self, VALUE v) } GetSSLCTX(self, ctx); - if(!ctx){ - ossl_raise(eSSLError, "SSL_CTX is not initialized."); - return Qnil; - } if (!SSL_CTX_set_cipher_list(ctx, StringValueCStr(str))) { ossl_raise(eSSLError, "SSL_CTX_set_cipher_list"); } @@ -2446,6 +2437,10 @@ Init_ossl_ssl(void) * A callback invoked when a session is removed from the internal cache. * * The callback is invoked with an SSLContext and a Session. + * + * IMPORTANT NOTE: It is currently not possible to use this safely in a + * multi-threaded application. The callback is called inside a global lock + * and it can randomly cause deadlock on Ruby thread switching. */ rb_attr(cSSLContext, rb_intern("session_remove_cb"), 1, 1, Qfalse); diff --git a/ext/openssl/ossl_version.h b/ext/openssl/ossl_version.h index 7725bc05253e03..56dfe1d785f4bf 100644 --- a/ext/openssl/ossl_version.h +++ b/ext/openssl/ossl_version.h @@ -10,6 +10,6 @@ #if !defined(_OSSL_VERSION_H_) #define _OSSL_VERSION_H_ -#define OSSL_VERSION "2.0.6" +#define OSSL_VERSION "2.0.7" #endif /* _OSSL_VERSION_H_ */ diff --git a/ext/openssl/ossl_x509cert.c b/ext/openssl/ossl_x509cert.c index 87086a7c5943a5..91c25c4e0ff06e 100644 --- a/ext/openssl/ossl_x509cert.c +++ b/ext/openssl/ossl_x509cert.c @@ -546,18 +546,19 @@ ossl_x509_get_public_key(VALUE self) /* * call-seq: - * cert.public_key = key => key + * cert.public_key = key */ static VALUE ossl_x509_set_public_key(VALUE self, VALUE key) { X509 *x509; + EVP_PKEY *pkey; GetX509(self, x509); - if (!X509_set_pubkey(x509, GetPKeyPtr(key))) { /* DUPs pkey */ - ossl_raise(eX509CertError, NULL); - } - + pkey = GetPKeyPtr(key); + ossl_pkey_check_public_key(pkey); + if (!X509_set_pubkey(x509, pkey)) + ossl_raise(eX509CertError, "X509_set_pubkey"); return key; } @@ -594,9 +595,9 @@ ossl_x509_verify(VALUE self, VALUE key) X509 *x509; EVP_PKEY *pkey; - pkey = GetPKeyPtr(key); /* NO NEED TO DUP */ GetX509(self, x509); - + pkey = GetPKeyPtr(key); + ossl_pkey_check_public_key(pkey); switch (X509_verify(x509, pkey)) { case 1: return Qtrue; diff --git a/ext/openssl/ossl_x509crl.c b/ext/openssl/ossl_x509crl.c index 035025ab69047b..add72c6cce77c5 100644 --- a/ext/openssl/ossl_x509crl.c +++ b/ext/openssl/ossl_x509crl.c @@ -366,9 +366,12 @@ static VALUE ossl_x509crl_verify(VALUE self, VALUE key) { X509_CRL *crl; + EVP_PKEY *pkey; GetX509CRL(self, crl); - switch (X509_CRL_verify(crl, GetPKeyPtr(key))) { + pkey = GetPKeyPtr(key); + ossl_pkey_check_public_key(pkey); + switch (X509_CRL_verify(crl, pkey)) { case 1: return Qtrue; case 0: diff --git a/ext/openssl/ossl_x509req.c b/ext/openssl/ossl_x509req.c index 15bc7052d3fd86..0f7fecdc57620c 100644 --- a/ext/openssl/ossl_x509req.c +++ b/ext/openssl/ossl_x509req.c @@ -330,11 +330,10 @@ ossl_x509req_set_public_key(VALUE self, VALUE key) EVP_PKEY *pkey; GetX509Req(self, req); - pkey = GetPKeyPtr(key); /* NO NEED TO DUP */ - if (!X509_REQ_set_pubkey(req, pkey)) { - ossl_raise(eX509ReqError, NULL); - } - + pkey = GetPKeyPtr(key); + ossl_pkey_check_public_key(pkey); + if (!X509_REQ_set_pubkey(req, pkey)) + ossl_raise(eX509ReqError, "X509_REQ_set_pubkey"); return key; } @@ -365,7 +364,8 @@ ossl_x509req_verify(VALUE self, VALUE key) EVP_PKEY *pkey; GetX509Req(self, req); - pkey = GetPKeyPtr(key); /* NO NEED TO DUP */ + pkey = GetPKeyPtr(key); + ossl_pkey_check_public_key(pkey); switch (X509_REQ_verify(req, pkey)) { case 1: return Qtrue; diff --git a/test/openssl/test_cipher.rb b/test/openssl/test_cipher.rb index ad0e87b441ed29..48149d41786c17 100644 --- a/test/openssl/test_cipher.rb +++ b/test/openssl/test_cipher.rb @@ -297,6 +297,13 @@ def test_aes_gcm_key_iv_order_issue assert_equal tag1, tag2 end if has_cipher?("aes-128-gcm") + def test_non_aead_cipher_set_auth_data + assert_raise(OpenSSL::Cipher::CipherError) { + cipher = OpenSSL::Cipher.new("aes-128-cfb").encrypt + cipher.auth_data = "123" + } + end if has_cipher?("aes-128-gcm") + private def new_encryptor(algo, **kwargs) diff --git a/test/openssl/test_ssl_session.rb b/test/openssl/test_ssl_session.rb index 9d0e5a2db170ff..af8c65b17f456b 100644 --- a/test/openssl/test_ssl_session.rb +++ b/test/openssl/test_ssl_session.rb @@ -215,6 +215,10 @@ def test_server_session_cache end end + # Skipping tests that use session_remove_cb by default because it may cause + # deadlock. + TEST_SESSION_REMOVE_CB = ENV["OSSL_TEST_ALL"] == "1" + def test_ctx_client_session_cb pend "TLS 1.2 is not supported" unless tls12_supported? @@ -227,11 +231,13 @@ def test_ctx_client_session_cb sock, sess = ary called[:new] = [sock, sess] } - ctx.session_remove_cb = lambda { |ary| - ctx, sess = ary - called[:remove] = [ctx, sess] - # any resulting value is OK (ignored) - } + if TEST_SESSION_REMOVE_CB + ctx.session_remove_cb = lambda { |ary| + ctx, sess = ary + called[:remove] = [ctx, sess] + # any resulting value is OK (ignored) + } + end server_connect_with_session(port, ctx, nil) { |ssl| assert_equal(1, ctx.session_cache_stats[:cache_num]) @@ -239,7 +245,9 @@ def test_ctx_client_session_cb assert_equal([ssl, ssl.session], called[:new]) assert(ctx.session_remove(ssl.session)) assert(!ctx.session_remove(ssl.session)) - assert_equal([ctx, ssl.session], called[:remove]) + if TEST_SESSION_REMOVE_CB + assert_equal([ctx, ssl.session], called[:remove]) + end } end end @@ -275,10 +283,12 @@ def test_ctx_server_session_cb last_server_session = sess } - ctx.session_remove_cb = lambda { |ary| - _ctx, sess = ary - called[:remove] = sess - } + if TEST_SESSION_REMOVE_CB + ctx.session_remove_cb = lambda { |ary| + _ctx, sess = ary + called[:remove] = sess + } + end } start_server(ctx_proc: ctx_proc) do |port| connections = 0 @@ -290,7 +300,9 @@ def test_ctx_server_session_cb assert_nil called[:get] assert_not_nil called[:new] assert_equal sess0.id, called[:new].id - assert_nil called[:remove] + if TEST_SESSION_REMOVE_CB + assert_nil called[:remove] + end called.clear # Internal cache hit @@ -302,12 +314,16 @@ def test_ctx_server_session_cb } assert_nil called[:get] assert_nil called[:new] - assert_nil called[:remove] + if TEST_SESSION_REMOVE_CB + assert_nil called[:remove] + end called.clear sctx.flush_sessions(Time.now + 10000) - assert_not_nil called[:remove] - assert_equal sess0.id, called[:remove].id + if TEST_SESSION_REMOVE_CB + assert_not_nil called[:remove] + assert_equal sess0.id, called[:remove].id + end called.clear # External cache hit @@ -325,12 +341,16 @@ def test_ctx_server_session_cb assert_equal sess0.id, sess2.id assert_equal sess0.id, called[:get] assert_nil called[:new] - assert_nil called[:remove] + if TEST_SESSION_REMOVE_CB + assert_nil called[:remove] + end called.clear sctx.flush_sessions(Time.now + 10000) - assert_not_nil called[:remove] - assert_equal sess0.id, called[:remove].id + if TEST_SESSION_REMOVE_CB + assert_not_nil called[:remove] + assert_equal sess0.id, called[:remove].id + end called.clear # Cache miss @@ -344,7 +364,9 @@ def test_ctx_server_session_cb assert_equal sess0.id, called[:get] assert_not_nil called[:new] assert_equal sess3.id, called[:new].id - assert_nil called[:remove] + if TEST_SESSION_REMOVE_CB + assert_nil called[:remove] + end end end diff --git a/version.h b/version.h index 0caa0dc2731d82..55c8aaf00653d8 100644 --- a/version.h +++ b/version.h @@ -1,6 +1,6 @@ #define RUBY_VERSION "2.4.4" #define RUBY_RELEASE_DATE "2018-03-20" -#define RUBY_PATCHLEVEL 264 +#define RUBY_PATCHLEVEL 265 #define RUBY_RELEASE_YEAR 2018 #define RUBY_RELEASE_MONTH 3