Merge pull request ruby#640 from rhenium/ky/ssl-connect-verify-error-…

…ssl-error-syscall ssl: adjust "certificate verify failed" error on SSL_ERROR_SYSCALL
junaruga · Aug 16, 2023 · bff0606 · bff0606
2 parents 6588fad + c309745
commit bff0606
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 41 deletions.
diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
@@ -1756,71 +1756,71 @@ ossl_start_ssl(VALUE self, int (*func)(SSL *), const char *funcname, VALUE opts)
     int ret, ret2;
     VALUE cb_state;
     int nonblock = opts != Qfalse;
-#if defined(SSL_R_CERTIFICATE_VERIFY_FAILED)
-    unsigned long err;
-#endif
 
     rb_ivar_set(self, ID_callback_state, Qnil);
 
     GetSSL(self, ssl);
 
     VALUE io = rb_attr_get(self, id_i_io);
-    for(;;){
-	ret = func(ssl);
+    for (;;) {
+        ret = func(ssl);
 
-	cb_state = rb_attr_get(self, ID_callback_state);
+        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));
-	}
+            /* must cleanup OpenSSL error stack before re-raising */
+            ossl_clear_error();
+            rb_jump_tag(NUM2INT(cb_state));
+        }
 
-	if (ret > 0)
-	    break;
+        if (ret > 0)
+            break;
 
-	switch((ret2 = ssl_get_error(ssl, ret))){
-	case SSL_ERROR_WANT_WRITE:
+        switch ((ret2 = ssl_get_error(ssl, ret))) {
+          case SSL_ERROR_WANT_WRITE:
             if (no_exception_p(opts)) { return sym_wait_writable; }
             write_would_block(nonblock);
             io_wait_writable(io);
             continue;
-	case SSL_ERROR_WANT_READ:
+          case SSL_ERROR_WANT_READ:
             if (no_exception_p(opts)) { return sym_wait_readable; }
             read_would_block(nonblock);
             io_wait_readable(io);
             continue;
-	case SSL_ERROR_SYSCALL:
+          case SSL_ERROR_SYSCALL:
 #ifdef __APPLE__
             /* See ossl_ssl_write_internal() */
             if (errno == EPROTOTYPE)
                 continue;
 #endif
-	    if (errno) rb_sys_fail(funcname);
-	    ossl_raise(eSSLError, "%s SYSCALL returned=%d errno=%d peeraddr=%"PRIsVALUE" state=%s",
-                funcname, ret2, errno, peeraddr_ip_str(self), SSL_state_string_long(ssl));
-
+            if (errno) rb_sys_fail(funcname);
+            /* fallthrough */
+          default: {
+              VALUE error_append = Qnil;
 #if defined(SSL_R_CERTIFICATE_VERIFY_FAILED)
-	case SSL_ERROR_SSL:
-	    err = ERR_peek_last_error();
-	    if (ERR_GET_LIB(err) == ERR_LIB_SSL &&
-		ERR_GET_REASON(err) == SSL_R_CERTIFICATE_VERIFY_FAILED) {
-		const char *err_msg = ERR_reason_error_string(err),
-		      *verify_msg = X509_verify_cert_error_string(SSL_get_verify_result(ssl));
-		if (!err_msg)
-		    err_msg = "(null)";
-		if (!verify_msg)
-		    verify_msg = "(null)";
-		ossl_clear_error(); /* let ossl_raise() not append message */
-		ossl_raise(eSSLError, "%s returned=%d errno=%d peeraddr=%"PRIsVALUE" state=%s: %s (%s)",
-			   funcname, ret2, errno, peeraddr_ip_str(self), SSL_state_string_long(ssl),
-			   err_msg, verify_msg);
-	    }
+              unsigned long err = ERR_peek_last_error();
+              if (ERR_GET_LIB(err) == ERR_LIB_SSL &&
+                  ERR_GET_REASON(err) == SSL_R_CERTIFICATE_VERIFY_FAILED) {
+                  const char *err_msg = ERR_reason_error_string(err),
+                        *verify_msg = X509_verify_cert_error_string(SSL_get_verify_result(ssl));
+                  if (!err_msg)
+                      err_msg = "(null)";
+                  if (!verify_msg)
+                      verify_msg = "(null)";
+                  ossl_clear_error(); /* let ossl_raise() not append message */
+                  error_append = rb_sprintf(": %s (%s)", err_msg, verify_msg);
+              }
 #endif
-	    /* fallthrough */
-	default:
-	    ossl_raise(eSSLError, "%s returned=%d errno=%d peeraddr=%"PRIsVALUE" state=%s",
-                funcname, ret2, errno, peeraddr_ip_str(self), SSL_state_string_long(ssl));
-	}
+              ossl_raise(eSSLError,
+                         "%s%s returned=%d errno=%d peeraddr=%"PRIsVALUE" state=%s%"PRIsVALUE,
+                         funcname,
+                         ret2 == SSL_ERROR_SYSCALL ? " SYSCALL" : "",
+                         ret2,
+                         errno,
+                         peeraddr_ip_str(self),
+                         SSL_state_string_long(ssl),
+                         error_append);
+          }
+        }
     }
 
     return self;

diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb
@@ -1080,7 +1080,9 @@ def test_connect_certificate_verify_failed_exception_message
     start_server(ignore_listener_error: true) { |port|
       ctx = OpenSSL::SSL::SSLContext.new
       ctx.set_params
-      assert_raise_with_message(OpenSSL::SSL::SSLError, /certificate/) {
+      # OpenSSL <= 1.1.0: "self signed certificate in certificate chain"
+      # OpenSSL >= 3.0.0: "self-signed certificate in certificate chain"
+      assert_raise_with_message(OpenSSL::SSL::SSLError, /self.signed/) {
         server_connect(port, ctx)
       }
     }