From 5d73437f13abe344123afc1dafcca9585284be05 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 29 Jun 2016 22:07:55 +0900 Subject: Refactor common verify callback code There is a function ossl_verify_cb() that fetches the custom callback Proc from X509_STORE/X509_STORE_CTX and calls it, but it was not very useful for SSL code. It's only used in ossl_x509store.c and ossl_ssl.c so move X509::Store specific code to ossl_x509store.c. Also make struct ossl_verify_cb_args and ossl_call_verify_cb_proc() local to ossl.c. --- ext/openssl/ossl.c | 70 ++++++++++++++++++++++---------------------- ext/openssl/ossl.h | 9 +----- ext/openssl/ossl_ssl.c | 4 +-- ext/openssl/ossl_x509store.c | 16 +++++++++- 4 files changed, 53 insertions(+), 46 deletions(-) (limited to 'ext') diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c index 1cd59e59..e77624a1 100644 --- a/ext/openssl/ossl.c +++ b/ext/openssl/ossl.c @@ -242,54 +242,54 @@ ossl_pem_passwd_cb(char *buf, int max_len, int flag, void *pwd_) int ossl_store_ctx_ex_verify_cb_idx; int ossl_store_ex_verify_cb_idx; -VALUE +struct ossl_verify_cb_args { + VALUE proc; + VALUE preverify_ok; + VALUE store_ctx; +}; + +static VALUE ossl_call_verify_cb_proc(struct ossl_verify_cb_args *args) { return rb_funcall(args->proc, rb_intern("call"), 2, - args->preverify_ok, args->store_ctx); + args->preverify_ok, args->store_ctx); } int -ossl_verify_cb(int ok, X509_STORE_CTX *ctx) +ossl_verify_cb_call(VALUE proc, int ok, X509_STORE_CTX *ctx) { - VALUE proc, rctx, ret; + VALUE rctx, ret; struct ossl_verify_cb_args args; - int state = 0; + int state; - proc = (VALUE)X509_STORE_CTX_get_ex_data(ctx, ossl_store_ctx_ex_verify_cb_idx); - if (!proc) - proc = (VALUE)X509_STORE_get_ex_data(X509_STORE_CTX_get0_store(ctx), ossl_store_ex_verify_cb_idx); - if (!proc) + if (NIL_P(proc)) return ok; - if (!NIL_P(proc)) { - ret = Qfalse; - rctx = rb_protect((VALUE(*)(VALUE))ossl_x509stctx_new, - (VALUE)ctx, &state); + + ret = Qfalse; + rctx = rb_protect((VALUE(*)(VALUE))ossl_x509stctx_new, (VALUE)ctx, &state); + if (state) { + rb_set_errinfo(Qnil); + rb_warn("StoreContext initialization failure"); + } + else { + args.proc = proc; + args.preverify_ok = ok ? Qtrue : Qfalse; + args.store_ctx = rctx; + ret = rb_protect((VALUE(*)(VALUE))ossl_call_verify_cb_proc, (VALUE)&args, &state); if (state) { rb_set_errinfo(Qnil); - rb_warn("StoreContext initialization failure"); - } - else { - args.proc = proc; - args.preverify_ok = ok ? Qtrue : Qfalse; - args.store_ctx = rctx; - ret = rb_protect((VALUE(*)(VALUE))ossl_call_verify_cb_proc, (VALUE)&args, &state); - if (state) { - rb_set_errinfo(Qnil); - rb_warn("exception in verify_callback is ignored"); - } - ossl_x509stctx_clear_ptr(rctx); - } - if (ret == Qtrue) { - X509_STORE_CTX_set_error(ctx, X509_V_OK); - ok = 1; - } - else{ - if (X509_STORE_CTX_get_error(ctx) == X509_V_OK) { - X509_STORE_CTX_set_error(ctx, X509_V_ERR_CERT_REJECTED); - } - ok = 0; + rb_warn("exception in verify_callback is ignored"); } + ossl_x509stctx_clear_ptr(rctx); + } + if (ret == Qtrue) { + X509_STORE_CTX_set_error(ctx, X509_V_OK); + ok = 1; + } + else { + if (X509_STORE_CTX_get_error(ctx) == X509_V_OK) + X509_STORE_CTX_set_error(ctx, X509_V_ERR_CERT_REJECTED); + ok = 0; } return ok; diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index 2317fa8b..c15a25bc 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -154,14 +154,7 @@ void ossl_clear_error(void); extern int ossl_store_ctx_ex_verify_cb_idx; extern int ossl_store_ex_verify_cb_idx; -struct ossl_verify_cb_args { - VALUE proc; - VALUE preverify_ok; - VALUE store_ctx; -}; - -VALUE ossl_call_verify_cb_proc(struct ossl_verify_cb_args *); -int ossl_verify_cb(int, X509_STORE_CTX *); +int ossl_verify_cb_call(VALUE, int, X509_STORE_CTX *); /* * String to DER String diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 5044c6d1..fd753254 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -327,8 +327,8 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); cb = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_vcb_idx); - X509_STORE_CTX_set_ex_data(ctx, ossl_store_ctx_ex_verify_cb_idx, (void *)cb); - return ossl_verify_cb(preverify_ok, ctx); + + return ossl_verify_cb_call(cb, preverify_ok, ctx); } static VALUE diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c index f3d6bf3b..ba34a056 100644 --- a/ext/openssl/ossl_x509store.c +++ b/ext/openssl/ossl_x509store.c @@ -106,6 +106,20 @@ DupX509StorePtr(VALUE obj) /* * Private functions */ +static int +x509store_verify_cb(int ok, X509_STORE_CTX *ctx) +{ + VALUE proc; + + proc = (VALUE)X509_STORE_CTX_get_ex_data(ctx, ossl_store_ctx_ex_verify_cb_idx); + if (!proc) + proc = (VALUE)X509_STORE_get_ex_data(X509_STORE_CTX_get0_store(ctx), ossl_store_ex_verify_cb_idx); + if (!proc) + return ok; + + return ossl_verify_cb_call(proc, ok, ctx); +} + static VALUE ossl_x509store_alloc(VALUE klass) { @@ -153,7 +167,7 @@ ossl_x509store_initialize(int argc, VALUE *argv, VALUE self) /* [Bug #405] [Bug #1678] [Bug #3000]; already fixed? */ store->ex_data.sk = NULL; #endif - X509_STORE_set_verify_cb(store, ossl_verify_cb); + X509_STORE_set_verify_cb(store, x509store_verify_cb); ossl_x509store_set_vfy_cb(self, Qnil); /* last verification status */ -- cgit v1.2.3 From 028e495734e9e6aa5dba1a2e130b08f66cf31a21 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Mon, 27 Jun 2016 21:41:05 +0900 Subject: ssl: add verify_hostname option to SSLContext If a client sets this to true and enables SNI with SSLSocket#hostname=, the hostname verification on the server certificate is performed automatically during the handshake using OpenSSL::SSL.verify_certificate_identity(). Currently an user who wants to do the hostname verification needs to call SSLSocket#post_connection_check explicitly after the TLS connection is established. This commit also enables the option in SSLContext::DEFAULT_PARAMS. Applications using SSLContext#set_params may be affected by this. [GH ruby/openssl#8] --- NEWS | 10 ++++++++++ ext/openssl/ossl_ssl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- lib/openssl/ssl.rb | 11 ++++++++--- test/test_ssl.rb | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 5 deletions(-) (limited to 'ext') diff --git a/NEWS b/NEWS index 1ff36e23..dfaf215c 100644 --- a/NEWS +++ b/NEWS @@ -39,6 +39,11 @@ Backward compatibility notes for consistency with OpenSSL::PKey::{DH,DSA,RSA,EC}#new. [Bug #11774] [GH ruby/openssl#55] +* OpenSSL::SSL::SSLContext#set_params enables verify_hostname option. With the + SNI hostname set by OpenSSL::SSL::SSLSocket#hostname=, the hostname + verification on the server certificate is automatically performed during the + handshake. [GH ruby/openssl#60] + Updates since Ruby 2.3 ---------------------- @@ -114,3 +119,8 @@ Updates since Ruby 2.3 - RC4 cipher suites are removed from OpenSSL::SSL::SSLContext::DEFAULT_PARAMS. RC4 is now considered to be weak. [GH ruby/openssl#50] + + - 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. [GH ruby/openssl#60] diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index fd753254..0e46a442 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -64,6 +64,7 @@ static VALUE eSSLErrorWaitWritable; #define ossl_sslctx_get_client_cert_cb(o) rb_iv_get((o),"@client_cert_cb") #define ossl_sslctx_get_tmp_ecdh_cb(o) rb_iv_get((o),"@tmp_ecdh_callback") #define ossl_sslctx_get_sess_id_ctx(o) rb_iv_get((o),"@session_id_context") +#define ossl_sslctx_get_verify_hostname(o) rb_iv_get((o),"@verify_hostname") #define ossl_ssl_get_io(o) rb_iv_get((o),"@io") #define ossl_ssl_get_ctx(o) rb_iv_get((o),"@context") @@ -319,14 +320,48 @@ ossl_tmp_ecdh_callback(SSL *ssl, int is_export, int keylength) } #endif +static VALUE +call_verify_certificate_identity(VALUE ctx_v) +{ + X509_STORE_CTX *ctx = (X509_STORE_CTX *)ctx_v; + SSL *ssl; + VALUE ssl_obj, hostname, cert_obj; + + ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); + ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); + hostname = rb_attr_get(ssl_obj, rb_intern("@hostname")); + + if (!RTEST(hostname)) { + rb_warning("verify_hostname requires hostname to be set"); + return Qtrue; + } + + cert_obj = ossl_x509_new(X509_STORE_CTX_get_current_cert(ctx)); + return rb_funcall(mSSL, rb_intern("verify_certificate_identity"), 2, + cert_obj, hostname); +} + static int ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { - VALUE cb; + VALUE cb, ssl_obj, verify_hostname, ret; SSL *ssl; + int status; ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); cb = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_vcb_idx); + ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); + verify_hostname = ossl_sslctx_get_verify_hostname(ossl_ssl_get_ctx(ssl_obj)); + + if (preverify_ok && RTEST(verify_hostname) && !SSL_is_server(ssl) && + !X509_STORE_CTX_get_error_depth(ctx)) { + ret = rb_protect(call_verify_certificate_identity, (VALUE)ctx, &status); + if (status) { + rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(status)); + return 0; + } + preverify_ok = ret == Qtrue; + } return ossl_verify_cb_call(cb, preverify_ok, ctx); } @@ -2278,10 +2313,19 @@ Init_ossl_ssl(void) * +store_context+ is an OpenSSL::X509::StoreContext containing the * context used for certificate verification. * - * If the callback returns false verification is stopped. + * If the callback returns false, the chain verification is immediately + * stopped and a bad_certificate alert is then sent. */ rb_attr(cSSLContext, rb_intern("verify_callback"), 1, 1, Qfalse); + /* + * Whether to check the server certificate is valid for the hostname. + * + * In order to make this work, verify_mode must be set to VERIFY_PEER and + * the server hostname must be given by OpenSSL::SSL::SSLSocket#hostname=. + */ + rb_attr(cSSLContext, rb_intern("verify_hostname"), 1, 1, Qfalse); + /* * An OpenSSL::X509::Store used for certificate verification */ diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index 9cac6925..a8059cba 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -19,6 +19,7 @@ module OpenSSL DEFAULT_PARAMS = { :ssl_version => "SSLv23", :verify_mode => OpenSSL::SSL::VERIFY_PEER, + :verify_hostname => true, :ciphers => %w{ ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-RSA-AES128-GCM-SHA256 @@ -71,7 +72,7 @@ module OpenSSL "session_get_cb", "session_new_cb", "session_remove_cb", "tmp_ecdh_callback", "servername_cb", "npn_protocols", "alpn_protocols", "alpn_select_cb", - "npn_select_cb"].map { |x| "@#{x}" } + "npn_select_cb", "verify_hostname"].map { |x| "@#{x}" } # A callback invoked when DH parameters are required. # @@ -107,13 +108,17 @@ module OpenSSL end ## - # Sets the parameters for this SSL context to the values in +params+. + # call-seq: + # ctx.set_params(params = {}) -> params + # + # Sets saner defaults optimized for the use with HTTP-like protocols. + # + # If a Hash +params+ is given, the parameters are overridden with it. # The keys in +params+ must be assignment methods on SSLContext. # # If the verify_mode is not VERIFY_NONE and ca_file, ca_path and # cert_store are not set then the system default certificate store is # used. - def set_params(params={}) params = DEFAULT_PARAMS.merge(params) params.each{|name, value| self.__send__("#{name}=", value) } diff --git a/test/test_ssl.rb b/test/test_ssl.rb index 9b8baf6f..7d27ff63 100644 --- a/test/test_ssl.rb +++ b/test/test_ssl.rb @@ -889,6 +889,53 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase end end + def test_verify_hostname_on_connect + ctx_proc = proc { |ctx| + now = Time.now + exts = [ + ["keyUsage", "keyEncipherment,digitalSignature", true], + ["subjectAltName", "DNS:a.example.com,DNS:*.b.example.com," \ + "DNS:c*.example.com,DNS:d.*.example.com"], + ] + ctx.cert = issue_cert(@svr, @svr_key, 4, now, now+1800, exts, + @ca_cert, @ca_key, OpenSSL::Digest::SHA1.new) + ctx.key = @svr_key + } + + start_server(OpenSSL::SSL::VERIFY_NONE, true, ctx_proc: ctx_proc, + ignore_listener_error: true) do |svr, port| + ctx = OpenSSL::SSL::SSLContext.new + ctx.verify_hostname = true + ctx.cert_store = OpenSSL::X509::Store.new + ctx.cert_store.add_cert(@ca_cert) + ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER + + [ + ["a.example.com", true], + ["A.Example.Com", true], + ["x.example.com", false], + ["b.example.com", false], + ["x.b.example.com", true], + ["cx.example.com", true], + ["d.x.example.com", false], + ].each do |name, expected_ok| + begin + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) + ssl.hostname = name + if expected_ok + assert_nothing_raised { ssl.connect } + else + assert_raise(OpenSSL::SSL::SSLError) { ssl.connect } + end + ensure + ssl.close if ssl + sock.close if sock + end + end + end + end + def test_multibyte_read_write #German a umlaut auml = [%w{ C3 A4 }.join('')].pack('H*') -- cgit v1.2.3