ruby-changes:23746
From: emboss <ko1@a...>
Date: Sat, 26 May 2012 09:56:44 +0900 (JST)
Subject: [ruby-changes:23746] emboss:r35797 (trunk): * ext/openssl/ossl_ssl.c: Allow disabling client-side renegotiation.
emboss 2012-05-26 09:56:33 +0900 (Sat, 26 May 2012) New Revision: 35797 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=35797 Log: * ext/openssl/ossl_ssl.c: Allow disabling client-side renegotiation. * test/openssl/test_ssl.rb: Simple tests for this. Client-side renegotiation is still considered problematic, even when used in the context of secure renegotiation (RI, RFC 5746). The changes allow users to either completely disable client renegotiation on the server, or to specify a maximum number of handshakes allowed in total. The number of total handshakes is counted in a callback set as SSL_set_info_callback. If the maximum number of handshakes is exceeded an error will be raised We do not support renegotiation in the OpenSSL extension, therefore this feature can only be tested externally. The feature is opt-in, the default setting will be to allow unlimited client renegotiation, as was the case before. Modified files: trunk/ChangeLog trunk/ext/openssl/ossl_ssl.c trunk/test/openssl/test_ssl.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 35796) +++ ChangeLog (revision 35797) @@ -1,3 +1,20 @@ +Sat May 26 09:54:53 2012 Martin Bosslet <Martin.Bosslet@g...> + + * ext/openssl/ossl_ssl.c: Allow disabling client-side renegotiation. + * test/openssl/test_ssl.rb: Simple tests for this. + + Client-side renegotiation is still considered problematic, even + when used in the context of secure renegotiation (RI, RFC 5746). + The changes allow users to either completely disable client + renegotiation on the server, or to specify a maximum number of + handshakes allowed in total. The number of total handshakes is + counted in a callback set as SSL_set_info_callback. If the + maximum number of handshakes is exceeded an error will be raised + We do not support renegotiation in the OpenSSL extension, therefore + this feature can only be tested externally. + The feature is opt-in, the default setting will be to allow + unlimited client renegotiation, as was the case before. + Fri May 25 23:38:58 2012 Martin Bosslet <Martin.Bosslet@g...> * test/openssl/test_ssl.rb: Clarify the intention of errors to be Index: ext/openssl/ossl_ssl.c =================================================================== --- ext/openssl/ossl_ssl.c (revision 35796) +++ ext/openssl/ossl_ssl.c (revision 35797) @@ -29,41 +29,43 @@ VALUE cSSLContext; VALUE cSSLSocket; -#define ossl_sslctx_set_cert(o,v) rb_iv_set((o),"@cert",(v)) -#define ossl_sslctx_set_key(o,v) rb_iv_set((o),"@key",(v)) -#define ossl_sslctx_set_client_ca(o,v) rb_iv_set((o),"@client_ca",(v)) -#define ossl_sslctx_set_ca_file(o,v) rb_iv_set((o),"@ca_file",(v)) -#define ossl_sslctx_set_ca_path(o,v) rb_iv_set((o),"@ca_path",(v)) -#define ossl_sslctx_set_timeout(o,v) rb_iv_set((o),"@timeout",(v)) -#define ossl_sslctx_set_verify_mode(o,v) rb_iv_set((o),"@verify_mode",(v)) -#define ossl_sslctx_set_verify_dep(o,v) rb_iv_set((o),"@verify_depth",(v)) -#define ossl_sslctx_set_verify_cb(o,v) rb_iv_set((o),"@verify_callback",(v)) -#define ossl_sslctx_set_options(o,v) rb_iv_set((o),"@options",(v)) -#define ossl_sslctx_set_cert_store(o,v) rb_iv_set((o),"@cert_store",(v)) -#define ossl_sslctx_set_extra_cert(o,v) rb_iv_set((o),"@extra_chain_cert",(v)) -#define ossl_sslctx_set_client_cert_cb(o,v) rb_iv_set((o),"@client_cert_cb",(v)) -#define ossl_sslctx_set_tmp_dh_cb(o,v) rb_iv_set((o),"@tmp_dh_callback",(v)) -#define ossl_sslctx_set_sess_id_ctx(o, v) rb_iv_get((o),"@session_id_context"(v)) +#define ossl_sslctx_set_cert(o,v) rb_iv_set((o),"@cert",(v)) +#define ossl_sslctx_set_key(o,v) rb_iv_set((o),"@key",(v)) +#define ossl_sslctx_set_client_ca(o,v) rb_iv_set((o),"@client_ca",(v)) +#define ossl_sslctx_set_ca_file(o,v) rb_iv_set((o),"@ca_file",(v)) +#define ossl_sslctx_set_ca_path(o,v) rb_iv_set((o),"@ca_path",(v)) +#define ossl_sslctx_set_timeout(o,v) rb_iv_set((o),"@timeout",(v)) +#define ossl_sslctx_set_verify_mode(o,v) rb_iv_set((o),"@verify_mode",(v)) +#define ossl_sslctx_set_verify_dep(o,v) rb_iv_set((o),"@verify_depth",(v)) +#define ossl_sslctx_set_verify_cb(o,v) rb_iv_set((o),"@verify_callback",(v)) +#define ossl_sslctx_set_options(o,v) rb_iv_set((o),"@options",(v)) +#define ossl_sslctx_set_cert_store(o,v) rb_iv_set((o),"@cert_store",(v)) +#define ossl_sslctx_set_extra_cert(o,v) rb_iv_set((o),"@extra_chain_cert",(v)) +#define ossl_sslctx_set_client_cert_cb(o,v) rb_iv_set((o),"@client_cert_cb",(v)) +#define ossl_sslctx_set_tmp_dh_cb(o,v) rb_iv_set((o),"@tmp_dh_callback",(v)) +#define ossl_sslctx_set_sess_id_ctx(o, v) rb_iv_set((o),"@session_id_context",(v)) +#define ossl_sslctx_set_max_handshake(o, v) rb_iv_set((o),"@max_handshake",(v)) -#define ossl_sslctx_get_cert(o) rb_iv_get((o),"@cert") -#define ossl_sslctx_get_key(o) rb_iv_get((o),"@key") -#define ossl_sslctx_get_client_ca(o) rb_iv_get((o),"@client_ca") -#define ossl_sslctx_get_ca_file(o) rb_iv_get((o),"@ca_file") -#define ossl_sslctx_get_ca_path(o) rb_iv_get((o),"@ca_path") -#define ossl_sslctx_get_timeout(o) rb_iv_get((o),"@timeout") -#define ossl_sslctx_get_verify_mode(o) rb_iv_get((o),"@verify_mode") -#define ossl_sslctx_get_verify_dep(o) rb_iv_get((o),"@verify_depth") -#define ossl_sslctx_get_verify_cb(o) rb_iv_get((o),"@verify_callback") -#define ossl_sslctx_get_options(o) rb_iv_get((o),"@options") -#define ossl_sslctx_get_cert_store(o) rb_iv_get((o),"@cert_store") -#define ossl_sslctx_get_extra_cert(o) rb_iv_get((o),"@extra_chain_cert") -#define ossl_sslctx_get_client_cert_cb(o) rb_iv_get((o),"@client_cert_cb") -#define ossl_sslctx_get_tmp_dh_cb(o) rb_iv_get((o),"@tmp_dh_callback") -#define ossl_sslctx_get_sess_id_ctx(o) rb_iv_get((o),"@session_id_context") +#define ossl_sslctx_get_cert(o) rb_iv_get((o),"@cert") +#define ossl_sslctx_get_key(o) rb_iv_get((o),"@key") +#define ossl_sslctx_get_client_ca(o) rb_iv_get((o),"@client_ca") +#define ossl_sslctx_get_ca_file(o) rb_iv_get((o),"@ca_file") +#define ossl_sslctx_get_ca_path(o) rb_iv_get((o),"@ca_path") +#define ossl_sslctx_get_timeout(o) rb_iv_get((o),"@timeout") +#define ossl_sslctx_get_verify_mode(o) rb_iv_get((o),"@verify_mode") +#define ossl_sslctx_get_verify_dep(o) rb_iv_get((o),"@verify_depth") +#define ossl_sslctx_get_verify_cb(o) rb_iv_get((o),"@verify_callback") +#define ossl_sslctx_get_options(o) rb_iv_get((o),"@options") +#define ossl_sslctx_get_cert_store(o) rb_iv_get((o),"@cert_store") +#define ossl_sslctx_get_extra_cert(o) rb_iv_get((o),"@extra_chain_cert") +#define ossl_sslctx_get_client_cert_cb(o) rb_iv_get((o),"@client_cert_cb") +#define ossl_sslctx_get_tmp_dh_cb(o) rb_iv_get((o),"@tmp_dh_callback") +#define ossl_sslctx_get_sess_id_ctx(o) rb_iv_get((o),"@session_id_context") +#define ossl_sslctx_get_max_handshake(o) rb_iv_get((o),"@max_handshake") static const char *ossl_sslctx_attrs[] = { "cert", "key", "client_ca", "ca_file", "ca_path", - "timeout", "verify_mode", "verify_depth", + "timeout", "verify_mode", "verify_depth", "max_handshake", "verify_callback", "options", "cert_store", "extra_chain_cert", "client_cert_cb", "tmp_dh_callback", "session_id_context", "session_get_cb", "session_new_cb", "session_remove_cb", @@ -78,6 +80,7 @@ #define ossl_ssl_get_x509(o) rb_iv_get((o),"@x509") #define ossl_ssl_get_key(o) rb_iv_get((o),"@key") #define ossl_ssl_get_tmp_dh(o) rb_iv_get((o),"@tmp_dh") +#define ossl_ssl_get_handshake(o) rb_iv_get((o),"@handshake") #define ossl_ssl_set_io(o,v) rb_iv_set((o),"@io",(v)) #define ossl_ssl_set_ctx(o,v) rb_iv_set((o),"@context",(v)) @@ -85,6 +88,7 @@ #define ossl_ssl_set_x509(o,v) rb_iv_set((o),"@x509",(v)) #define ossl_ssl_set_key(o,v) rb_iv_set((o),"@key",(v)) #define ossl_ssl_set_tmp_dh(o,v) rb_iv_set((o),"@tmp_dh",(v)) +#define ossl_ssl_set_handshake(o,v) rb_iv_set((o),"@handshake",(v)) static const char *ossl_ssl_attr_readers[] = { "io", "context", }; static const char *ossl_ssl_attrs[] = { @@ -542,6 +546,33 @@ } #endif +static int +ssl_get_max_handshake(VALUE ssl) +{ + VALUE rb_ctx = ossl_ssl_get_ctx(ssl); + VALUE max_handshake = ossl_sslctx_get_max_handshake(rb_ctx); + return NIL_P(max_handshake) ? -1 : NUM2INT(max_handshake); +} + +static void +ssl_renegotiation_cb(const SSL *ssl, int where, int val) +{ + int state = SSL_state(ssl); + + /* Count handshakes on the server */ + if ((where & SSL_CB_HANDSHAKE_START) && + (state & SSL_ST_ACCEPT)) { + VALUE rb_ssl = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); + int max = ssl_get_max_handshake(rb_ssl); + int cur = NUM2INT(ossl_ssl_get_handshake(rb_ssl)); + + if (max != -1 && cur == max) + ossl_raise(eSSLError, "Client renegotations exceeded maximum"); + + ossl_ssl_set_handshake(rb_ssl, INT2NUM(cur + 1)); + } +} + /* * call-seq: * ctx.setup => Qtrue # first time @@ -792,7 +823,6 @@ return v; } - /* * call-seq: * ctx.session_add(session) -> true | false @@ -980,6 +1010,51 @@ } /* + * call-seq: + * ctx.disable_client_renegotiation -> self + * + * Completely disables client-side renegotiation. Will only affect the + * behavior of a server. A server with client renegotation disabled + * will reject any client-side attempts to renegotiate the session. + */ +static VALUE +ossl_sslctx_disable_client_renegotation(VALUE self) +{ + ossl_sslctx_set_max_handshake(self, INT2NUM(1)); + return self; +} + +/* + * call-seq: + * ctx.allow_client_renegotiation[(num_handshakes)] -> self + * + * Affects only server connections. If no argument is provided, there is no + * restriction on the number of client-side renegotiation attempts, which is + * also the default setting. If an Integer +num_handshakes+ is provided, this + * specifies the maximum number of total handshakes that are allowed before + * further attempts will be rejected. So to allow exactly one renegotiation, + * an argument of 2 would be needed (the initial handshake plus one + * renegotiation attempt). An ArgumentError will be raised for negative + * arguments or a value of 0. + */ +static VALUE +ossl_sslctx_allow_client_renegotiation(int argc, VALUE *argv, VALUE self) +{ + VALUE max = Qnil; + + rb_scan_args(argc, argv, "01", &max); + + if (NIL_P(max)) { + ossl_sslctx_set_max_handshake(self, INT2NUM(-1)); + } else { + if (NUM2INT(max) <= 0) + ossl_raise(rb_eArgError, "Maximum handshakes must be positive and non-zero"); + ossl_sslctx_set_max_handshake(self, max); + } + return self; +} + +/* * SSLSocket class */ static void @@ -998,8 +1073,8 @@ if (rc = SSL_shutdown(ssl)) break; } + SSL_clear(ssl); ERR_clear_error(); - SSL_clear(ssl); } } @@ -1045,6 +1120,7 @@ ossl_ssl_set_io(self, io); ossl_ssl_set_ctx(self, ctx); ossl_ssl_set_sync_close(self, Qfalse); + ossl_ssl_set_handshake(self, INT2NUM(0)); ossl_sslctx_setup(ctx); rb_iv_set(self, "@hostname", Qnil); @@ -1095,6 +1171,7 @@ SSL_set_ex_data(ssl, ossl_ssl_ex_client_cert_cb_idx, (void*)cb); cb = ossl_sslctx_get_tmp_dh_cb(v_ctx); SSL_set_ex_data(ssl, ossl_ssl_ex_tmp_dh_callback_idx, (void*)cb); + SSL_set_info_callback(ssl, ssl_renegotiation_cb); } return Qtrue; @@ -1305,7 +1382,6 @@ return str; } - /* * call-seq: * ssl.sysread(length) => string @@ -1415,6 +1491,7 @@ { SSL *ssl; + ossl_ssl_set_handshake(self, INT2NUM(0)); Data_Get_Struct(self, SSL, ssl); ossl_ssl_shutdown(ssl); if (RTEST(ossl_ssl_get_sync_close(self))) @@ -1894,6 +1971,8 @@ rb_define_method(cSSLContext, "ssl_version=", ossl_sslctx_set_ssl_version, 1); rb_define_method(cSSLContext, "ciphers", ossl_sslctx_get_ciphers, 0); rb_define_method(cSSLContext, "ciphers=", ossl_sslctx_set_ciphers, 1); + rb_define_method(cSSLContext, "disable_client_renegotiation", ossl_sslctx_disable_client_renegotation, 0); + rb_define_method(cSSLContext, "allow_client_renegotiation", ossl_sslctx_allow_client_renegotiation, -1); rb_define_method(cSSLContext, "setup", ossl_sslctx_setup, 0); Index: test/openssl/test_ssl.rb =================================================================== --- test/openssl/test_ssl.rb (revision 35796) +++ test/openssl/test_ssl.rb (revision 35797) @@ -505,14 +505,54 @@ end + def test_disable_client_renegotiation + ctx_proc = Proc.new { |ctx| ctx.disable_client_renegotiation } + start_server_version(:SSLv23, ctx_proc) { |server, port| + server_connect(port) { |ssl| + assert(ssl.ssl_version) + } + } + end + + def test_allow_client_renegotiation_args + ctx = OpenSSL::SSL::SSLContext.new + assert_raise(ArgumentError) { ctx.allow_client_renegotiation(0) } + assert_raise(ArgumentError) { ctx.allow_client_renegotiation(-1) } + end + + def test_allow_client_renegotiation_once + ctx_proc = Proc.new { |ctx| ctx.allow_client_renegotiation(2) } + start_server_version(:SSLv23, ctx_proc) { |server, port| + server_connect(port) { |ssl| + assert(ssl.ssl_version) + } + } + end + + def test_allow_arbitrary_client_renegotiation + ctx_proc = Proc.new { |ctx| ctx.allow_client_renegotiation } + start_server_version(:SSLv23, ctx_proc) { |server, port| + server_connect(port) { |ssl| + assert(ssl.ssl_version) + } + } + end + private - def start_server_version(version, ctx_proc=nil, &blk) + def start_server_version(version, ctx_proc=nil, server_proc=nil, &blk) ctx_wrap = Proc.new { |ctx| ctx.ssl_version = version ctx_proc.call(ctx) if ctx_proc } - start_server(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_wrap, &blk) + start_server( + PORT, + OpenSSL::SSL::VERIFY_NONE, + true, + :ctx_proc => ctx_wrap, + :server_proc => server_proc, + &blk + ) end def server_connect(port, ctx=nil) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/