diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c index 578d0051b..8bb2e7dd5 100644 --- a/ext/openssl/ossl.c +++ b/ext/openssl/ossl.c @@ -145,6 +145,88 @@ ossl_bin2hex(const unsigned char *in, char *out, size_t inlen) } } +static ID id_cb_state; + +int +ossl_cb_state_get(void) +{ + VALUE val = rb_thread_local_aref(rb_thread_current(), id_cb_state); + return FIXNUM_P(val) ? FIX2INT(val) : 0; +} + +static VALUE +cb_state_set_i(VALUE vstate) +{ + rb_thread_local_aset(rb_thread_current(), id_cb_state, vstate); + return Qnil; +} + +void +ossl_cb_state_set(int state) +{ + int s; + RUBY_ASSERT(!state || !ossl_cb_state_get()); + rb_protect(cb_state_set_i, state ? INT2FIX(state) : Qnil, &s); + if (s) { + /* As a last resort */ + rb_set_errinfo(Qnil); + return; + } +} + +void +ossl_cb_state_begin(void) +{ + RUBY_ASSERT(!ossl_cb_state_get()); +} + +int +ossl_cb_state_end(void) +{ + int state = ossl_cb_state_get(); + if (state) + ossl_cb_state_set(0); + return state; +} + +struct suppress_args { + VALUE (*func)(VALUE args); + VALUE args; +}; + +static VALUE +do_nothing(VALUE unused) +{ + return Qnil; +} + +static VALUE +suppress_i(VALUE args_) +{ + struct suppress_args *p = (struct suppress_args *)args_; + int state; + rb_set_errinfo(Qnil); + rb_protect(p->func, p->args, &state); + if (state) { + fprintf(stderr, "ossl_suppress: exception suppressed"); + } + return Qnil; +} + +void +ossl_suppress(VALUE (*func)(VALUE args), VALUE args) +{ + struct suppress_args p = { func, args }; + if (ossl_cb_state_get()) { + int state = ossl_cb_state_end(); + rb_ensure(do_nothing, Qnil, suppress_i, (VALUE)&p); + ossl_cb_state_begin(); + ossl_cb_state_set(state); + return; + } + rb_ensure(do_nothing, Qnil, suppress_i, (VALUE)&p); +} + /* * our default PEM callback */ @@ -275,7 +357,7 @@ ossl_to_der_if_possible(VALUE obj) */ static ID id_i_errors; -static void collect_errors_into(VALUE ary); +static VALUE collect_errors_into(VALUE ary); VALUE ossl_make_error(VALUE exc, VALUE str) @@ -326,46 +408,44 @@ ossl_raise(VALUE exc, const char *fmt, ...) rb_exc_raise(ossl_make_error(exc, err)); } -static void +static VALUE collect_errors_into(VALUE ary) { - if (dOSSL == Qtrue || !NIL_P(ary)) { - unsigned long e; - const char *file, *data, *func, *lib, *reason; - int line, flags; + unsigned long e; + const char *file, *data, *func, *lib, *reason; + int line, flags; #ifdef HAVE_ERR_GET_ERROR_ALL - while ((e = ERR_get_error_all(&file, &line, &func, &data, &flags))) { + while ((e = ERR_get_error_all(&file, &line, &func, &data, &flags))) { #else - while ((e = ERR_get_error_line_data(&file, &line, &data, &flags))) { - func = ERR_func_error_string(e); + while ((e = ERR_get_error_line_data(&file, &line, &data, &flags))) { + func = ERR_func_error_string(e); #endif - lib = ERR_lib_error_string(e); - reason = ERR_reason_error_string(e); - - VALUE str = rb_sprintf("error:%08lX:%s:%s:%s", e, lib ? lib : "", - func ? func : "", reason ? reason : ""); - if (flags & ERR_TXT_STRING) { - if (!data) - data = "(null)"; - rb_str_catf(str, " (%s)", data); - } - - if (dOSSL == Qtrue) - rb_warn("error on stack: %"PRIsVALUE, str); - if (!NIL_P(ary)) - rb_ary_push(ary, str); + lib = ERR_lib_error_string(e); + reason = ERR_reason_error_string(e); + + VALUE str = rb_sprintf("error:%08lX:%s:%s:%s", e, lib ? lib : "", + func ? func : "", reason ? reason : ""); + if (flags & ERR_TXT_STRING) { + if (!data) + data = "(null)"; + rb_str_catf(str, " (%s)", data); } + + if (dOSSL == Qtrue) + rb_warn("error on stack: %"PRIsVALUE, str); + if (!NIL_P(ary)) + rb_ary_push(ary, str); } - else { - ERR_clear_error(); - } + return Qnil; } void ossl_clear_error(void) { - collect_errors_into(Qnil); + if (dOSSL == Qtrue) + ossl_suppress(collect_errors_into, Qnil); + ERR_clear_error(); } /* @@ -1155,6 +1235,7 @@ Init_openssl(void) */ ossl_s_to_der = rb_intern("to_der"); id_i_errors = rb_intern("@errors"); + id_cb_state = rb_intern("__openssl_cb_state__"); /* * Init components diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index 7762ad237..d7679b7f3 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -155,6 +155,23 @@ do{\ */ void ossl_bin2hex(const unsigned char *in, char *out, size_t len); +/* + * Ractor-local storage for storing rb_protect() jump tag. Use this only if + * the OpenSSL API doesn't let you pass arbitrary data to the callback. + * + * _begin() expects that there's no currently pending jump. + * No context switch is expected between _set() and _end(). + */ +void ossl_cb_state_begin(void); +int ossl_cb_state_end(void); +int ossl_cb_state_get(void); +void ossl_cb_state_set(int state); +/* + * Equivalent to rb_protect(func, args, NULL) except that this avoids + * clobbering the existing errinfo on the ec. + */ +void ossl_suppress(VALUE (*func)(VALUE args), VALUE args); + /* * Our default PEM callback */ diff --git a/ext/openssl/ossl_ocsp.c b/ext/openssl/ossl_ocsp.c index 2eaa3840f..b415c30c6 100644 --- a/ext/openssl/ossl_ocsp.c +++ b/ext/openssl/ossl_ocsp.c @@ -378,15 +378,21 @@ ossl_ocspreq_verify(int argc, VALUE *argv, VALUE self) OCSP_REQUEST *req; STACK_OF(X509) *x509s; X509_STORE *x509st; - int flg, result; + int flg, result, state; rb_scan_args(argc, argv, "21", &certs, &store, &flags); GetOCSPReq(self, req); x509st = GetX509StorePtr(store); flg = NIL_P(flags) ? 0 : NUM2INT(flags); x509s = ossl_x509_ary2sk(certs); + ossl_cb_state_begin(); result = OCSP_request_verify(req, x509s, x509st, flg); + state = ossl_cb_state_end(); sk_X509_pop_free(x509s, X509_free); + if (state) { + ossl_clear_error(); + rb_jump_tag(state); + } if (result <= 0) ossl_clear_error(); @@ -989,15 +995,21 @@ ossl_ocspbres_verify(int argc, VALUE *argv, VALUE self) OCSP_BASICRESP *bs; STACK_OF(X509) *x509s; X509_STORE *x509st; - int flg, result; + int flg, result, state; rb_scan_args(argc, argv, "21", &certs, &store, &flags); GetOCSPBasicRes(self, bs); x509st = GetX509StorePtr(store); flg = NIL_P(flags) ? 0 : NUM2INT(flags); x509s = ossl_x509_ary2sk(certs); + ossl_cb_state_begin(); result = OCSP_basic_verify(bs, x509s, x509st, flg); + state = ossl_cb_state_end(); sk_X509_pop_free(x509s, X509_free); + if (state) { + ossl_clear_error(); + rb_jump_tag(state); + } if (result <= 0) ossl_clear_error(); diff --git a/ext/openssl/ossl_pkcs7.c b/ext/openssl/ossl_pkcs7.c index e17a49410..b3c8baa5d 100644 --- a/ext/openssl/ossl_pkcs7.c +++ b/ext/openssl/ossl_pkcs7.c @@ -777,9 +777,16 @@ ossl_pkcs7_verify(int argc, VALUE *argv, VALUE self) sk_X509_pop_free(x509s, X509_free); ossl_raise(ePKCS7Error, NULL); } + ossl_cb_state_begin(); ok = PKCS7_verify(p7, x509s, x509st, in, out, flg); + status = ossl_cb_state_end(); BIO_free(in); sk_X509_pop_free(x509s, X509_free); + if (status) { + BIO_free(out); + ossl_clear_error(); + rb_jump_tag(status); + } data = ossl_membio2str(out); ossl_pkcs7_set_data(self, data); if (ok != 1) { diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index a3aff8009..3f8c447d5 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -36,8 +36,7 @@ VALUE cSSLSocket; static VALUE eSSLErrorWaitReadable; static VALUE eSSLErrorWaitWritable; -static ID id_call, ID_callback_state, id_npn_protocols_encoded, id_each, - id_original_context; +static ID id_call, id_npn_protocols_encoded, id_each, id_original_context; static VALUE sym_wait_readable, sym_wait_writable; static ID id_i_cert_store, id_i_ca_file, id_i_ca_path, id_i_verify_mode, @@ -111,38 +110,68 @@ ossl_sslctx_s_alloc(VALUE klass) return obj; } +struct client_cert_cb_args { + VALUE ssl_obj; + X509 **x509; + EVP_PKEY **pkey; +}; + static VALUE -ossl_call_client_cert_cb(VALUE obj) +ossl_call_client_cert_cb(VALUE args_) { + struct client_cert_cb_args *args = (struct client_cert_cb_args *)args_; VALUE ctx_obj, cb, ary, cert, key; + X509 *x509; + EVP_PKEY *pkey; - ctx_obj = rb_attr_get(obj, id_i_context); + ctx_obj = rb_attr_get(args->ssl_obj, id_i_context); cb = rb_attr_get(ctx_obj, id_i_client_cert_cb); if (NIL_P(cb)) return Qnil; - ary = rb_funcallv(cb, id_call, 1, &obj); - Check_Type(ary, T_ARRAY); - GetX509CertPtr(cert = rb_ary_entry(ary, 0)); - GetPrivPKeyPtr(key = rb_ary_entry(ary, 1)); + ary = rb_funcall(cb, id_call, 1, args->ssl_obj); + if (NIL_P(ary)) + return Qnil; + ary = rb_check_array_type(ary); + if (NIL_P(ary) || RARRAY_LEN(ary) != 2) { + rb_raise(rb_eTypeError, + "client_cert_cb must return either nil or [cert, key]"); + } - return rb_ary_new3(2, cert, key); + x509 = GetX509CertPtr(cert = rb_ary_entry(ary, 0)); + pkey = GetPrivPKeyPtr(key = rb_ary_entry(ary, 1)); + if (!X509_check_private_key(x509, pkey)) + ossl_raise(eSSLError, "X509_check_private_key"); + if (!X509_up_ref(x509)) + ossl_raise(eSSLError, "X509_up_ref"); + if (!EVP_PKEY_up_ref(pkey)) { + X509_free(x509); + ossl_raise(eSSLError, "EVP_PKEY_up_ref"); + } + RB_GC_GUARD(cert); + RB_GC_GUARD(key); + + *args->x509 = x509; + *args->pkey = pkey; + return Qtrue; } static int ossl_client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) { - VALUE obj, ret; - - obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - ret = rb_protect(ossl_call_client_cert_cb, obj, NULL); - if (NIL_P(ret)) - return 0; - - *x509 = DupX509CertPtr(RARRAY_AREF(ret, 0)); - *pkey = DupPKeyPtr(RARRAY_AREF(ret, 1)); + VALUE obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); + struct client_cert_cb_args args = { obj, x509, pkey }; + VALUE ret; + int state; - return 1; + if (ossl_cb_state_get()) + return -1; + ret = rb_protect(ossl_call_client_cert_cb, (VALUE)&args, &state); + if (state) { + ossl_cb_state_set(state); + return -1; + } + return RTEST(ret); } #if !defined(OPENSSL_NO_DH) @@ -180,9 +209,13 @@ ossl_tmp_dh_callback(SSL *ssl, int is_export, int keylength) int state; VALUE rb_ssl = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); struct tmp_dh_callback_args args = {rb_ssl, is_export, keylength}; - VALUE ret = rb_protect(ossl_call_tmp_dh_callback, (VALUE)&args, &state); + VALUE ret; + + if (ossl_cb_state_get()) + return NULL; + ret = rb_protect(ossl_call_tmp_dh_callback, (VALUE)&args, &state); if (state) { - rb_ivar_set(rb_ssl, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); return NULL; } return (DH *)ret; @@ -215,8 +248,10 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { VALUE cb, ssl_obj, sslctx_obj, verify_hostname, ret; SSL *ssl; - int status; + int state; + if (ossl_cb_state_get()) + return 0; 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); sslctx_obj = rb_attr_get(ssl_obj, id_i_context); @@ -225,9 +260,9 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) 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)); + ret = rb_protect(call_verify_certificate_identity, (VALUE)ctx, &state); + if (state) { + ossl_cb_state_set(state); return 0; } if (ret != Qtrue) { @@ -239,89 +274,99 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) return ossl_verify_cb_call(cb, preverify_ok, ctx); } +struct sess_get_cb_args { + SSL *ssl; + const unsigned char *data; + int len; + SSL_SESSION *ret; +}; + static VALUE -ossl_call_session_get_cb(VALUE ary) +ossl_call_session_get_cb(VALUE args_) { - VALUE ssl_obj, cb; + struct sess_get_cb_args *args = (struct sess_get_cb_args *)args_; + VALUE ssl_obj, sslctx_obj, cb, session_id, ret; - Check_Type(ary, T_ARRAY); - ssl_obj = rb_ary_entry(ary, 0); - - cb = rb_funcall(ssl_obj, rb_intern("session_get_cb"), 0); + ssl_obj = (VALUE)SSL_get_ex_data(args->ssl, ossl_ssl_ex_ptr_idx); + sslctx_obj = rb_attr_get(ssl_obj, id_i_context); + cb = rb_attr_get(sslctx_obj, id_i_session_get_cb); if (NIL_P(cb)) return Qnil; - return rb_funcallv(cb, id_call, 1, &ary); + session_id = rb_str_new((const char *)args->data, args->len); + ret = rb_funcall(cb, id_call, 1, rb_assoc_new(ssl_obj, session_id)); + if (NIL_P(ret)) + return Qnil; + // XXX: Should we raise if ret is not a Session? + if (!rb_obj_is_instance_of(ret, cSSLSession)) + return Qnil; + + GetSSLSession(ret, args->ret); + return Qnil; } static SSL_SESSION * ossl_sslctx_session_get_cb(SSL *ssl, const unsigned char *buf, int len, int *copy) { - VALUE ary, ssl_obj, ret_obj; - SSL_SESSION *sess; - int state = 0; + struct sess_get_cb_args args = { ssl, buf, len, NULL }; + int state; + if (ossl_cb_state_get()) + return NULL; OSSL_Debug("SSL SESSION get callback entered"); - ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - ary = rb_ary_new2(2); - rb_ary_push(ary, ssl_obj); - rb_ary_push(ary, rb_str_new((const char *)buf, len)); - - ret_obj = rb_protect(ossl_call_session_get_cb, ary, &state); + rb_protect(ossl_call_session_get_cb, (VALUE)&args, &state); if (state) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); return NULL; } - if (!rb_obj_is_instance_of(ret_obj, cSSLSession)) - return NULL; - - GetSSLSession(ret_obj, sess); *copy = 1; - - return sess; + return args.ret; } +struct sess_new_cb_args { + SSL *ssl; + SSL_SESSION *sess; +}; + static VALUE -ossl_call_session_new_cb(VALUE ary) +ossl_call_session_new_cb(VALUE args_) { - VALUE ssl_obj, cb; - - Check_Type(ary, T_ARRAY); - ssl_obj = rb_ary_entry(ary, 0); + struct sess_new_cb_args *args = (struct sess_new_cb_args *)args_; + VALUE ssl_obj, sslctx_obj, cb, sess_obj; - cb = rb_funcall(ssl_obj, rb_intern("session_new_cb"), 0); + ssl_obj = (VALUE)SSL_get_ex_data(args->ssl, ossl_ssl_ex_ptr_idx); + sslctx_obj = rb_attr_get(ssl_obj, id_i_context); + cb = rb_attr_get(sslctx_obj, id_i_session_new_cb); if (NIL_P(cb)) return Qnil; - return rb_funcallv(cb, id_call, 1, &ary); + sess_obj = rb_obj_alloc(cSSLSession); + if (!SSL_SESSION_up_ref(args->sess)) + ossl_raise(eSSLError, "SSL_SESSION_up_ref"); + RTYPEDDATA_DATA(sess_obj) = args->sess; + + rb_funcall(cb, id_call, 1, rb_assoc_new(ssl_obj, sess_obj)); + return Qnil; } /* return 1 normal. return 0 removes the session */ static int ossl_sslctx_session_new_cb(SSL *ssl, SSL_SESSION *sess) { - VALUE ary, ssl_obj, sess_obj; - int state = 0; + struct sess_new_cb_args args = { ssl, sess }; + int state; + if (ossl_cb_state_get()) + return 0; OSSL_Debug("SSL SESSION new callback entered"); - - ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - sess_obj = rb_obj_alloc(cSSLSession); - SSL_SESSION_up_ref(sess); - DATA_PTR(sess_obj) = sess; - - ary = rb_ary_new2(2); - rb_ary_push(ary, ssl_obj); - rb_ary_push(ary, sess_obj); - - rb_protect(ossl_call_session_new_cb, ary, &state); + rb_protect(ossl_call_session_new_cb, (VALUE)&args, &state); if (state) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); } /* * return 0 which means to OpenSSL that the session is still * valid (since we created Ruby Session object) and was not freed by us - * with SSL_SESSION_free(). Call SSLContext#remove_session(sess) in - * session_get_cb block if you don't want OpenSSL to cache the session + * with SSL_SESSION_free(). Call SSLContext#session_remove(sess) in + * session_new_cb block if you don't want OpenSSL to cache the session * internally. */ return 0; @@ -362,6 +407,8 @@ ossl_sslctx_keylog_cb(const SSL *ssl, const char *line) struct ossl_call_keylog_cb_args args; int state = 0; + if (ossl_cb_state_get()) + return; OSSL_Debug("SSL keylog callback entered"); ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); @@ -370,30 +417,39 @@ ossl_sslctx_keylog_cb(const SSL *ssl, const char *line) rb_protect(ossl_call_keylog_cb, (VALUE)&args, &state); if (state) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); } } #endif +struct sess_remove_cb_args { + SSL_CTX *ctx; + SSL_SESSION *sess; +}; + static VALUE -ossl_call_session_remove_cb(VALUE ary) +ossl_call_session_remove_cb(VALUE args_) { - VALUE sslctx_obj, cb; - - Check_Type(ary, T_ARRAY); - sslctx_obj = rb_ary_entry(ary, 0); + struct sess_remove_cb_args *args = (struct sess_remove_cb_args *)args_; + VALUE sslctx_obj, cb, sess_obj; + sslctx_obj = (VALUE)SSL_CTX_get_ex_data(args->ctx, ossl_sslctx_ex_ptr_idx); cb = rb_attr_get(sslctx_obj, id_i_session_remove_cb); if (NIL_P(cb)) return Qnil; - return rb_funcallv(cb, id_call, 1, &ary); + sess_obj = rb_obj_alloc(cSSLSession); + if (!SSL_SESSION_up_ref(args->sess)) + ossl_raise(eSSLError, "SSL_SESSION_up_ref"); + RTYPEDDATA_DATA(sess_obj) = args->sess; + + rb_funcall(cb, id_call, 1, rb_assoc_new(sslctx_obj, sess_obj)); + return Qnil; } static void ossl_sslctx_session_remove_cb(SSL_CTX *ctx, SSL_SESSION *sess) { - VALUE ary, sslctx_obj, sess_obj; - int state = 0; + struct sess_remove_cb_args args = { ctx, sess }; /* * This callback is also called for all sessions in the internal store @@ -403,24 +459,7 @@ ossl_sslctx_session_remove_cb(SSL_CTX *ctx, SSL_SESSION *sess) return; OSSL_Debug("SSL SESSION remove callback entered"); - - sslctx_obj = (VALUE)SSL_CTX_get_ex_data(ctx, ossl_sslctx_ex_ptr_idx); - sess_obj = rb_obj_alloc(cSSLSession); - SSL_SESSION_up_ref(sess); - DATA_PTR(sess_obj) = sess; - - ary = rb_ary_new2(2); - rb_ary_push(ary, sslctx_obj); - rb_ary_push(ary, sess_obj); - - rb_protect(ossl_call_session_remove_cb, ary, &state); - if (state) { -/* - the SSL_CTX is frozen, nowhere to save state. - there is no common accessor method to check it either. - rb_ivar_set(sslctx_obj, ID_callback_state, INT2NUM(state)); -*/ - } + ossl_suppress(ossl_call_session_remove_cb, (VALUE)&args); } static VALUE @@ -475,27 +514,42 @@ ssl_servername_cb(SSL *ssl, int *ad, void *arg) { int state; + if (ossl_cb_state_get()) + return SSL_TLSEXT_ERR_ALERT_FATAL; rb_protect(ossl_call_servername_cb, (VALUE)ssl, &state); if (state) { - VALUE ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); return SSL_TLSEXT_ERR_ALERT_FATAL; } return SSL_TLSEXT_ERR_OK; } -static void -ssl_renegotiation_cb(const SSL *ssl) +static VALUE +call_renegotiation_cb(VALUE args_) { - VALUE ssl_obj, sslctx_obj, cb; - - ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - sslctx_obj = rb_attr_get(ssl_obj, id_i_context); - cb = rb_attr_get(sslctx_obj, id_i_renegotiation_cb); - if (NIL_P(cb)) return; + VALUE *args = (VALUE *)args_; + return rb_funcall(args[0], id_call, 1, args[1]); +} - rb_funcallv(cb, id_call, 1, &ssl_obj); +/* This function may serve as the entry point to support further callbacks. */ +static void +ssl_info_cb(const SSL *ssl, int where, int val) +{ + if (ossl_cb_state_get()) + return; + if (where & SSL_CB_HANDSHAKE_START && SSL_is_server(ssl)) { + VALUE ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); + VALUE sslctx_obj = rb_attr_get(ssl_obj, id_i_context); + VALUE cb = rb_attr_get(sslctx_obj, id_i_renegotiation_cb); + if (!NIL_P(cb)) { + VALUE args[] = { cb, ssl_obj }; + int state; + rb_protect(call_renegotiation_cb, (VALUE)args, &state); + if (state) + ossl_cb_state_set(state); + } + } } static VALUE @@ -559,18 +613,19 @@ ssl_npn_select_cb_common(SSL *ssl, VALUE cb, const unsigned char **out, unsigned int inlen) { VALUE selected; - int status; + int state; struct npn_select_cb_common_args args; + if (ossl_cb_state_get()) + return SSL_TLSEXT_ERR_ALERT_FATAL; + args.cb = cb; args.in = in; args.inlen = inlen; - selected = rb_protect(npn_select_cb_common_i, (VALUE)&args, &status); - if (status) { - VALUE ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(status)); + selected = rb_protect(npn_select_cb_common_i, (VALUE)&args, &state); + if (state) { + ossl_cb_state_set(state); return SSL_TLSEXT_ERR_ALERT_FATAL; } @@ -628,17 +683,6 @@ ssl_alpn_select_cb(SSL *ssl, const unsigned char **out, unsigned char *outlen, return ssl_npn_select_cb_common(ssl, cb, out, outlen, in, inlen); } -/* This function may serve as the entry point to support further callbacks. */ -static void -ssl_info_cb(const SSL *ssl, int where, int val) -{ - int is_server = SSL_is_server((SSL *)ssl); - - if (is_server && where & SSL_CB_HANDSHAKE_START) { - ssl_renegotiation_cb(ssl); - } -} - /* * call-seq: * ctx.options -> integer @@ -812,6 +856,9 @@ ossl_sslctx_setup(VALUE self) val = rb_attr_get(self, id_i_verify_depth); if(!NIL_P(val)) SSL_CTX_set_verify_depth(ctx, NUM2INT(val)); + if (!NIL_P(rb_attr_get(self, id_i_renegotiation_cb))) + SSL_CTX_set_info_callback(ctx, ssl_info_cb); + #ifdef OSSL_USE_NEXTPROTONEG val = rb_attr_get(self, id_i_npn_protocols); if (!NIL_P(val)) { @@ -1780,7 +1827,6 @@ ossl_ssl_initialize(int argc, VALUE *argv, VALUE self) if (!SSL_set_ex_data(ssl, ossl_ssl_ex_ptr_idx, (void *)self)) ossl_raise(eSSLError, "SSL_set_ex_data"); - SSL_set_info_callback(ssl, ssl_info_cb); rb_call_super(0, NULL); @@ -1820,38 +1866,44 @@ ossl_ssl_setup(VALUE self) return Qtrue; } +static void +ssl_call_begin(void) +{ + ossl_cb_state_begin(); +} + static int -errno_mapped(void) +ssl_call_end(void) { #ifdef _WIN32 - return rb_w32_map_errno(WSAGetLastError()); + int saved_errno = rb_w32_map_errno(WSAGetLastError()); #else - return errno; + int saved_errno = errno; #endif + + int state = ossl_cb_state_end(); + if (state) { + /* must cleanup OpenSSL error stack before re-raising */ + ossl_clear_error(); + rb_jump_tag(state); + } + + return saved_errno; } static VALUE ossl_start_ssl(VALUE self, int (*func)(SSL *), const char *funcname) { SSL *ssl; - VALUE cb_state; - - rb_ivar_set(self, ID_callback_state, Qnil); GetSSL(self, ssl); #ifdef __APPLE__ retry:; #endif + ssl_call_begin(); int ret = func(ssl); - int saved_errno = errno_mapped(); - - cb_state = rb_attr_get(self, ID_callback_state); - if (!NIL_P(cb_state)) { - /* must cleanup OpenSSL error stack before re-raising */ - ossl_clear_error(); - rb_jump_tag(NUM2INT(cb_state)); - } + int saved_errno = ssl_call_end(); int code = SSL_get_error(ssl, ret); switch (code) { @@ -1945,7 +1997,6 @@ ossl_ssl_read(VALUE self, VALUE len, VALUE str) { SSL *ssl; int ilen; - VALUE cb_state; GetSSL(self, ssl); if (!ssl_started(ssl)) @@ -1965,17 +2016,11 @@ ossl_ssl_read(VALUE self, VALUE len, VALUE str) return str; } + ssl_call_begin(); rb_str_locktmp(str); int nread = SSL_read(ssl, RSTRING_PTR(str), ilen); rb_str_unlocktmp(str); - int saved_errno = errno_mapped(); - - cb_state = rb_attr_get(self, ID_callback_state); - if (!NIL_P(cb_state)) { - rb_ivar_set(self, ID_callback_state, Qnil); - ossl_clear_error(); - rb_jump_tag(NUM2INT(cb_state)); - } + int saved_errno = ssl_call_end(); switch (SSL_get_error(ssl, nread)) { case SSL_ERROR_NONE: @@ -2016,7 +2061,6 @@ static VALUE ossl_ssl_write(VALUE self, VALUE str) { SSL *ssl; - VALUE cb_state; GetSSL(self, ssl); if (!ssl_started(ssl)) @@ -2031,6 +2075,7 @@ ossl_ssl_write(VALUE self, VALUE str) #ifdef __APPLE__ retry:; #endif + ssl_call_begin(); int frozen = RB_OBJ_FROZEN(str); if (!frozen) { rb_str_locktmp(str); @@ -2039,14 +2084,7 @@ ossl_ssl_write(VALUE self, VALUE str) if (!frozen) { rb_str_unlocktmp(str); } - int saved_errno = errno_mapped(); - - cb_state = rb_attr_get(self, ID_callback_state); - if (!NIL_P(cb_state)) { - rb_ivar_set(self, ID_callback_state, Qnil); - ossl_clear_error(); - rb_jump_tag(NUM2INT(cb_state)); - } + int saved_errno = ssl_call_end(); switch (SSL_get_error(ssl, nwritten)) { case SSL_ERROR_NONE: @@ -2608,9 +2646,6 @@ Init_ossl_ssl(void) #endif #ifndef OPENSSL_NO_SOCK - id_call = rb_intern_const("call"); - ID_callback_state = rb_intern_const("callback_state"); - ossl_ssl_ex_ptr_idx = SSL_get_ex_new_index(0, (void *)"ossl_ssl_ex_ptr_idx", 0, 0, 0); if (ossl_ssl_ex_ptr_idx < 0) ossl_raise(rb_eRuntimeError, "SSL_get_ex_new_index"); @@ -2746,11 +2781,11 @@ Init_ossl_ssl(void) /* * A callback invoked when a client certificate is requested by a server - * and no certificate has been set. + * and no certificate has been set (see #add_certificate). * - * The callback is invoked with a Session and must return an Array - * containing an OpenSSL::X509::Certificate and an OpenSSL::PKey. If any - * other value is returned the handshake is suspended. + * The callback is invoked with the SSLSocket, and must return a + * two-element Array of an OpenSSL::X509::Certificate and the corresponding + * private key OpenSSL::PKey::PKey, or nil to send no certificate. */ rb_attr(cSSLContext, rb_intern_const("client_cert_cb"), 1, 1, Qfalse); @@ -3191,6 +3226,7 @@ Init_ossl_ssl(void) sym_wait_readable = ID2SYM(rb_intern_const("wait_readable")); sym_wait_writable = ID2SYM(rb_intern_const("wait_writable")); + id_call = rb_intern_const("call"); id_npn_protocols_encoded = rb_intern_const("npn_protocols_encoded"); id_each = rb_intern_const("each"); id_original_context = rb_intern_const("original_context"); diff --git a/ext/openssl/ossl_ts.c b/ext/openssl/ossl_ts.c index 697d740f0..158a2424c 100644 --- a/ext/openssl/ossl_ts.c +++ b/ext/openssl/ossl_ts.c @@ -810,9 +810,15 @@ ossl_ts_resp_verify(int argc, VALUE *argv, VALUE self) #endif TS_VERIFY_CTX_add_flags(ctx, TS_VFY_SIGNATURE); + ossl_cb_state_begin(); ok = TS_RESP_verify_response(ctx, resp); + status = ossl_cb_state_end(); TS_VERIFY_CTX_free(ctx); + if (status) { + ossl_clear_error(); + rb_jump_tag(status); + } if (!ok) ossl_raise(eTimestampError, "TS_RESP_verify_response"); diff --git a/ext/openssl/ossl_x509.h b/ext/openssl/ossl_x509.h index 71932ef1a..3bb952c25 100644 --- a/ext/openssl/ossl_x509.h +++ b/ext/openssl/ossl_x509.h @@ -90,6 +90,8 @@ void Init_ossl_x509store(void); /* * Calls the verify callback Proc (the first parameter) with given pre-verify * result and the X509_STORE_CTX. + * + * Assumes ossl_cb_state_*() is active. */ int ossl_verify_cb_call(VALUE, int, X509_STORE_CTX *); diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c index 037a03660..9266be8b8 100644 --- a/ext/openssl/ossl_x509store.c +++ b/ext/openssl/ossl_x509store.c @@ -56,37 +56,34 @@ ossl_verify_cb_call(VALUE proc, int ok, X509_STORE_CTX *ctx) struct ossl_verify_cb_args args; int state; + if (ossl_cb_state_get()) + goto err; if (NIL_P(proc)) return ok; - ret = Qfalse; rctx = rb_protect(ossl_x509stctx_new_i, (VALUE)ctx, &state); if (state) { - rb_set_errinfo(Qnil); - rb_warn("StoreContext initialization failure"); + ossl_cb_state_set(state); + goto err; } - else { - args.proc = proc; - args.preverify_ok = ok ? Qtrue : Qfalse; - args.store_ctx = rctx; - ret = rb_protect(call_verify_cb_proc, (VALUE)&args, &state); - RTYPEDDATA_DATA(rctx) = NULL; - if (state) { - rb_set_errinfo(Qnil); - rb_warn("exception in verify_callback is ignored"); - } + args.proc = proc; + args.preverify_ok = ok ? Qtrue : Qfalse; + args.store_ctx = rctx; + ret = rb_protect(call_verify_cb_proc, (VALUE)&args, &state); + RTYPEDDATA_DATA(rctx) = NULL; + if (state) { + ossl_cb_state_set(state); + goto err; } 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 1; } - return ok; + err: + if (X509_STORE_CTX_get_error(ctx) == X509_V_OK) + X509_STORE_CTX_set_error(ctx, X509_V_ERR_CERT_REJECTED); + return 0; } /* @@ -673,6 +670,7 @@ static VALUE ossl_x509stctx_verify(VALUE self) { X509_STORE_CTX *ctx; + int ret, state; GetX509StCtx(self, ctx); VALUE cb = rb_iv_get(self, "@verify_callback"); @@ -680,7 +678,14 @@ ossl_x509stctx_verify(VALUE self) ossl_raise(eX509StoreError, "X509_STORE_CTX_set_ex_data"); RB_OBJ_WRITTEN(self, Qundef, cb); - switch (X509_verify_cert(ctx)) { + ossl_cb_state_begin(); + ret = X509_verify_cert(ctx); + state = ossl_cb_state_end(); + if (state) { + ossl_clear_error(); + rb_jump_tag(state); + } + switch (ret) { case 1: return Qtrue; case 0: @@ -1026,7 +1031,8 @@ Init_ossl_x509store(void) * The callback can use StoreContext#error= to change the error code as * needed. The callback must return either true or false. * - * NOTE: any exception raised within the callback will be ignored. + * Exceptions raised within the callback propagate to the caller of the + * verification method. * * See also the man page X509_STORE_CTX_set_verify_cb(3). */ diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index e50be00ff..e68c8c14b 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -609,18 +609,6 @@ def using_anon_cipher? ctx.ciphers.include?(cipher) end - def client_cert_cb - @context.client_cert_cb - end - - def session_new_cb - @context.session_new_cb - end - - def session_get_cb - @context.session_get_cb - end - class << self # call-seq: diff --git a/test/openssl/test_ocsp.rb b/test/openssl/test_ocsp.rb index c43ff5cb5..44dbe6c83 100644 --- a/test/openssl/test_ocsp.rb +++ b/test/openssl/test_ocsp.rb @@ -116,6 +116,13 @@ def test_request_sign_verify req.sign(@cert, @cert_key, []) assert_equal true, req.verify([], store) + # Exceptions in verify_callback are re-raised + s = OpenSSL::X509::Store.new.tap { |s| + s.add_cert(@ca_cert) + s.verify_callback = proc { raise "verify_callback" } + } + assert_raise(RuntimeError) { req.verify([], s) } + # without signer cert req = OpenSSL::OCSP::Request.new.add_certid(cid) req.sign(@cert, @cert_key, nil) @@ -195,6 +202,13 @@ def test_basic_response_sign_verify bres.sign(@ca_cert, @ca_key, [], 0, "SHA256") assert_equal true, bres.verify([], store) + # Exceptions in verify_callback are re-raised + s = OpenSSL::X509::Store.new.tap { |s| + s.add_cert(@ca_cert) + s.verify_callback = proc { raise "verify_callback" } + } + assert_raise(RuntimeError) { bres.verify([], s) } + # signed by OCSP signer bres = OpenSSL::OCSP::BasicResponse.new cid = OpenSSL::OCSP::CertificateId.new(@cert2, @cert) diff --git a/test/openssl/test_pkcs7.rb b/test/openssl/test_pkcs7.rb index c677d8223..8beae9662 100644 --- a/test/openssl/test_pkcs7.rb +++ b/test/openssl/test_pkcs7.rb @@ -64,6 +64,18 @@ def test_signed assert_false(p7.verify([@ca_cert], OpenSSL::X509::Store.new)) end + def test_signed_verify_callback + data = "aaaaa\r\nbbbbb\r\nccccc\r\n" + tmp = OpenSSL::PKCS7.sign(@ee1_cert, @ee1_key, data) + + p7 = OpenSSL::PKCS7.new(tmp) + store = OpenSSL::X509::Store.new + store.verify_callback = proc { raise "verify_callback" } + assert_raise_with_message(RuntimeError, "verify_callback") { + p7.verify([], store) + } + end + def test_signed_flags store = OpenSSL::X509::Store.new store.add_cert(@ca_cert) diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index c1ab7a07c..02fcb2416 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -565,20 +565,34 @@ def test_client_auth_success } end - def test_client_cert_cb_ignore_error + def test_client_cert_cb_error + omit "LibreSSL doesn't support client_cert_cb in TLS 1.3" if libressl? + sctx = make_server_context sctx.verify_mode = OpenSSL::SSL::VERIFY_PEER|OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT - start_server(sctx, ignore_listener_error: true) do |port| + # Bad return value ctx = OpenSSL::SSL::SSLContext.new ctx.client_cert_cb = -> ssl { - raise "exception in client_cert_cb must be suppressed" + assert_kind_of(OpenSSL::SSL::SSLSocket, ssl) + [@cli_cert, OpenSSL::PKey.read(@cli_key.public_to_der)] } - # 1. Exception in client_cert_cb is suppressed - # 2. No client certificate will be sent to the server - # 3. SSL_VERIFY_FAIL_IF_NO_PEER_CERT causes the handshake to fail - assert_raise(OpenSSL::SSL::SSLError) { + assert_raise_with_message(ArgumentError, /private key/) { + server_connect(port, ctx) { raise "unreachable" } + } + + # Exception in callback + ctx = OpenSSL::SSL::SSLContext.new + ctx.client_cert_cb = -> ssl { raise "in client_cert_cb" } + assert_raise_with_message(RuntimeError, /in client_cert_cb/) { + server_connect(port, ctx) { raise "unreachable" } + } + + # nil return, rejected due to OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT + ctx = OpenSSL::SSL::SSLContext.new + ctx.client_cert_cb = -> ssl { nil } + assert_raise_with_message(OpenSSL::SSL::SSLError, /alert number 116/) { server_connect(port, ctx) { |ssl| ssl.puts("abc"); ssl.gets } } end @@ -695,22 +709,24 @@ def test_verify_result } end - def test_exception_in_verify_callback_is_ignored + def test_exception_in_verify_callback start_server(ignore_listener_error: true) { |port| sock = TCPSocket.new("127.0.0.1", port) ctx = OpenSSL::SSL::SSLContext.new - ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER + ctx.cert_store = OpenSSL::X509::Store.new.add_cert(@ca_cert) + called = [] ctx.verify_callback = Proc.new do |preverify_ok, store_ctx| store_ctx.error = OpenSSL::X509::V_OK - raise RuntimeError + called << store_ctx.current_cert + raise "in verify_callback" end ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) ssl.sync_close = true begin - EnvUtil.suppress_warning do - # SSLError, not RuntimeError - assert_raise(OpenSSL::SSL::SSLError) { ssl.connect } - end + assert_raise_with_message(RuntimeError, "in verify_callback") { + ssl.connect + } + assert_equal([@ca_cert], called) assert_equal(OpenSSL::X509::V_ERR_CERT_REJECTED, ssl.verify_result) ensure ssl.close @@ -1725,6 +1741,26 @@ def test_renegotiation_cb ssl.puts "abc"; assert_equal "abc\n", ssl.gets } } + + client_can_close = Thread::Queue.new + server_proc = proc do |sock| + sctx = make_server_context + sctx.renegotiation_cb = -> ssl { raise "in renegotiation_cb" } + ssl = OpenSSL::SSL::SSLSocket.new(sock, sctx) + assert_raise_with_message(RuntimeError, "in renegotiation_cb") { + ssl.accept + } + ensure + client_can_close.close + end + start_server_proc(server_proc) do |port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.connect_nonblock(exception: false) + client_can_close.pop + ensure + sock.close + end end def test_alpn_protocol_selection_ary diff --git a/test/openssl/test_ts.rb b/test/openssl/test_ts.rb index 69780a657..4cc7e3d6d 100644 --- a/test/openssl/test_ts.rb +++ b/test/openssl/test_ts.rb @@ -407,6 +407,13 @@ def test_verify_ee_intermediate_type_error assert_raise(TypeError) { ts.verify(req, [ca_cert], 123) } end + def test_verify_with_verify_callback + ts, req = timestamp_ee + store = ca_store + store.verify_callback = proc { raise "verify callback" } + assert_raise(RuntimeError) { ts.verify(req, store) } + end + def test_verify_ee_def_policy req = OpenSSL::Timestamp::Request.new req.algorithm = "SHA1" diff --git a/test/openssl/test_x509store.rb b/test/openssl/test_x509store.rb index 7132af9f9..b2559b705 100644 --- a/test/openssl/test_x509store.rb +++ b/test/openssl/test_x509store.rb @@ -150,17 +150,23 @@ def test_verify_callback assert_equal(false, store.verify(ee1_cert, [ca2_cert])) assert_equal(OpenSSL::X509::V_ERR_APPLICATION_VERIFICATION, store.error) - # Exception raised by verify_callback is currently suppressed, and is - # treated as a non-truthy return (with warning) + # Exceptions raised in verify_callback are re-raised store = OpenSSL::X509::Store.new store.verify_callback = -> (preverify_ok, sctx) { - raise "suppressed" + raise "in verify_callback" } store.add_cert(ca1_cert) - assert_warning(/exception in verify_callback/) { - assert_equal(false, store.verify(ee1_cert, [ca2_cert])) + assert_raise_with_message(RuntimeError, "in verify_callback") { + store.verify(ee1_cert, [ca2_cert]) } + # verify_callback is nested + ret = store.verify(ca1_cert) do |preverify_ok, sctx| + assert_raise(RuntimeError) { store.verify(ca1_cert) } + preverify_ok + end + assert_true(ret) + # The block given to X509::Store#verify replaces it called = nil store = OpenSSL::X509::Store.new