ssl: add verify_hostname option to SSLContext

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]

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;

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

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,16 +320,50 @@ 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);
-    X509_STORE_CTX_set_ex_data(ctx, ossl_store_ctx_ex_verify_cb_idx, (void *)cb);
-    return ossl_verify_cb(preverify_ok, ctx);
+    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);
 }
 
 static VALUE
@@ -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
      */

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 */

lib/openssl/ssl.rb

@@ -19,6 +19,7 @@ class SSLContext
       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 @@ class SSLContext
         "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 @@ def initialize(version = nil)
       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) }

test/test_ssl.rb

@@ -321,7 +321,7 @@ def test_verify_result
     start_server(OpenSSL::SSL::VERIFY_NONE, true, :ignore_listener_error => true){|server, port|
       sock = TCPSocket.new("127.0.0.1", port)
       ctx = OpenSSL::SSL::SSLContext.new
-      ctx.set_params
+      ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER
       ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx)
       ssl.sync_close = true
       begin
@@ -335,12 +335,11 @@ def test_verify_result
     start_server(OpenSSL::SSL::VERIFY_NONE, true){|server, port|
       sock = TCPSocket.new("127.0.0.1", port)
       ctx = OpenSSL::SSL::SSLContext.new
-      ctx.set_params(
-        :verify_callback => Proc.new do |preverify_ok, store_ctx|
-          store_ctx.error = OpenSSL::X509::V_OK
-          true
-        end
-      )
+      ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER
+      ctx.verify_callback = Proc.new do |preverify_ok, store_ctx|
+        store_ctx.error = OpenSSL::X509::V_OK
+        true
+      end
       ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx)
       ssl.sync_close = true
       begin
@@ -354,12 +353,11 @@ def test_verify_result
     start_server(OpenSSL::SSL::VERIFY_NONE, true, :ignore_listener_error => true){|server, port|
       sock = TCPSocket.new("127.0.0.1", port)
       ctx = OpenSSL::SSL::SSLContext.new
-      ctx.set_params(
-        :verify_callback => Proc.new do |preverify_ok, store_ctx|
-          store_ctx.error = OpenSSL::X509::V_ERR_APPLICATION_VERIFICATION
-          false
-        end
-      )
+      ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER
+      ctx.verify_callback = Proc.new do |preverify_ok, store_ctx|
+        store_ctx.error = OpenSSL::X509::V_ERR_APPLICATION_VERIFICATION
+        false
+      end
       ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx)
       ssl.sync_close = true
       begin
@@ -375,12 +373,11 @@ def test_exception_in_verify_callback_is_ignored
     start_server(OpenSSL::SSL::VERIFY_NONE, true, :ignore_listener_error => true){|server, port|
       sock = TCPSocket.new("127.0.0.1", port)
       ctx = OpenSSL::SSL::SSLContext.new
-      ctx.set_params(
-        :verify_callback => Proc.new do |preverify_ok, store_ctx|
-          store_ctx.error = OpenSSL::X509::V_OK
-          raise RuntimeError
-        end
-      )
+      ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER
+      ctx.verify_callback = Proc.new do |preverify_ok, store_ctx|
+        store_ctx.error = OpenSSL::X509::V_OK
+        raise RuntimeError
+      end
       ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx)
       ssl.sync_close = true
       begin
@@ -892,6 +889,53 @@ def test_tlsext_hostname
     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*')