Skip to content

Commit 0379aab

Browse files
rheniummatzbot
authored andcommitted
[ruby/openssl] ssl: fix errno display in exception messages
The errno reported in an OpenSSL::SSL::SSLError raised by SSLSocket#accept and #connect sometimes does not match what SSL_accept() or SSL_connect() actually encountered. Depending on the evaluation order of arguments passed to ossl_raise(), errno may be overwritten by peeraddr_ip_str(). While we could just fix peeraddr_ip_str(), we should avoid passing around errno since it is error-prone. Replace rb_sys_fail() and rb_io_{maybe_,}wait_{read,writ}able() with equivalents that do not read errno. ruby/openssl@bfc7df860f
1 parent 05b85fc commit 0379aab

1 file changed

Lines changed: 29 additions & 20 deletions

File tree

ext/openssl/ossl_ssl.c

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,11 +1714,15 @@ ossl_ssl_setup(VALUE self)
17141714
return Qtrue;
17151715
}
17161716

1717+
static int
1718+
errno_mapped(void)
1719+
{
17171720
#ifdef _WIN32
1718-
#define ssl_get_error(ssl, ret) (errno = rb_w32_map_errno(WSAGetLastError()), SSL_get_error((ssl), (ret)))
1721+
return rb_w32_map_errno(WSAGetLastError());
17191722
#else
1720-
#define ssl_get_error(ssl, ret) SSL_get_error((ssl), (ret))
1723+
return errno;
17211724
#endif
1725+
}
17221726

17231727
static void
17241728
write_would_block(int nonblock)
@@ -1759,35 +1763,34 @@ static void
17591763
io_wait_writable(VALUE io)
17601764
{
17611765
#ifdef HAVE_RB_IO_MAYBE_WAIT
1762-
if (!rb_io_maybe_wait_writable(errno, io, RUBY_IO_TIMEOUT_DEFAULT)) {
1766+
if (!rb_io_wait(io, INT2NUM(RUBY_IO_WRITABLE), RUBY_IO_TIMEOUT_DEFAULT)) {
17631767
rb_raise(IO_TIMEOUT_ERROR, "Timed out while waiting to become writable!");
17641768
}
17651769
#else
17661770
rb_io_t *fptr;
17671771
GetOpenFile(io, fptr);
1768-
rb_io_wait_writable(fptr->fd);
1772+
rb_thread_fd_writable(fptr->fd);
17691773
#endif
17701774
}
17711775

17721776
static void
17731777
io_wait_readable(VALUE io)
17741778
{
17751779
#ifdef HAVE_RB_IO_MAYBE_WAIT
1776-
if (!rb_io_maybe_wait_readable(errno, io, RUBY_IO_TIMEOUT_DEFAULT)) {
1780+
if (!rb_io_wait(io, INT2NUM(RUBY_IO_READABLE), RUBY_IO_TIMEOUT_DEFAULT)) {
17771781
rb_raise(IO_TIMEOUT_ERROR, "Timed out while waiting to become readable!");
17781782
}
17791783
#else
17801784
rb_io_t *fptr;
17811785
GetOpenFile(io, fptr);
1782-
rb_io_wait_readable(fptr->fd);
1786+
rb_thread_wait_fd(fptr->fd);
17831787
#endif
17841788
}
17851789

17861790
static VALUE
17871791
ossl_start_ssl(VALUE self, int (*func)(SSL *), const char *funcname, VALUE opts)
17881792
{
17891793
SSL *ssl;
1790-
int ret, ret2;
17911794
VALUE cb_state;
17921795
int nonblock = opts != Qfalse;
17931796

@@ -1797,7 +1800,8 @@ ossl_start_ssl(VALUE self, int (*func)(SSL *), const char *funcname, VALUE opts)
17971800

17981801
VALUE io = rb_attr_get(self, id_i_io);
17991802
for (;;) {
1800-
ret = func(ssl);
1803+
int ret = func(ssl);
1804+
int saved_errno = errno_mapped();
18011805

18021806
cb_state = rb_attr_get(self, ID_callback_state);
18031807
if (!NIL_P(cb_state)) {
@@ -1809,7 +1813,8 @@ ossl_start_ssl(VALUE self, int (*func)(SSL *), const char *funcname, VALUE opts)
18091813
if (ret > 0)
18101814
break;
18111815

1812-
switch ((ret2 = ssl_get_error(ssl, ret))) {
1816+
int code = SSL_get_error(ssl, ret);
1817+
switch (code) {
18131818
case SSL_ERROR_WANT_WRITE:
18141819
if (no_exception_p(opts)) { return sym_wait_writable; }
18151820
write_would_block(nonblock);
@@ -1823,10 +1828,11 @@ ossl_start_ssl(VALUE self, int (*func)(SSL *), const char *funcname, VALUE opts)
18231828
case SSL_ERROR_SYSCALL:
18241829
#ifdef __APPLE__
18251830
/* See ossl_ssl_write_internal() */
1826-
if (errno == EPROTOTYPE)
1831+
if (saved_errno == EPROTOTYPE)
18271832
continue;
18281833
#endif
1829-
if (errno) rb_sys_fail(funcname);
1834+
if (saved_errno)
1835+
rb_exc_raise(rb_syserr_new(saved_errno, funcname));
18301836
/* fallthrough */
18311837
default: {
18321838
VALUE error_append = Qnil;
@@ -1847,9 +1853,9 @@ ossl_start_ssl(VALUE self, int (*func)(SSL *), const char *funcname, VALUE opts)
18471853
ossl_raise(eSSLError,
18481854
"%s%s returned=%d errno=%d peeraddr=%"PRIsVALUE" state=%s%"PRIsVALUE,
18491855
funcname,
1850-
ret2 == SSL_ERROR_SYSCALL ? " SYSCALL" : "",
1851-
ret2,
1852-
errno,
1856+
code == SSL_ERROR_SYSCALL ? " SYSCALL" : "",
1857+
code,
1858+
saved_errno,
18531859
peeraddr_ip_str(self),
18541860
SSL_state_string_long(ssl),
18551861
error_append);
@@ -1992,6 +1998,7 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
19921998
for (;;) {
19931999
rb_str_locktmp(str);
19942000
int nread = SSL_read(ssl, RSTRING_PTR(str), ilen);
2001+
int saved_errno = errno_mapped();
19952002
rb_str_unlocktmp(str);
19962003

19972004
cb_state = rb_attr_get(self, ID_callback_state);
@@ -2001,7 +2008,7 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
20012008
rb_jump_tag(NUM2INT(cb_state));
20022009
}
20032010

2004-
switch (ssl_get_error(ssl, nread)) {
2011+
switch (SSL_get_error(ssl, nread)) {
20052012
case SSL_ERROR_NONE:
20062013
rb_str_set_len(str, nread);
20072014
return str;
@@ -2024,8 +2031,8 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
20242031
break;
20252032
case SSL_ERROR_SYSCALL:
20262033
if (!ERR_peek_error()) {
2027-
if (errno)
2028-
rb_sys_fail(0);
2034+
if (saved_errno)
2035+
rb_exc_raise(rb_syserr_new(saved_errno, "SSL_read"));
20292036
else {
20302037
/*
20312038
* The underlying BIO returned 0. This is actually a
@@ -2110,6 +2117,7 @@ ossl_ssl_write_internal_safe(VALUE _args)
21102117

21112118
for (;;) {
21122119
int nwritten = SSL_write(ssl, RSTRING_PTR(str), num);
2120+
int saved_errno = errno_mapped();
21132121

21142122
cb_state = rb_attr_get(self, ID_callback_state);
21152123
if (!NIL_P(cb_state)) {
@@ -2118,7 +2126,7 @@ ossl_ssl_write_internal_safe(VALUE _args)
21182126
rb_jump_tag(NUM2INT(cb_state));
21192127
}
21202128

2121-
switch (ssl_get_error(ssl, nwritten)) {
2129+
switch (SSL_get_error(ssl, nwritten)) {
21222130
case SSL_ERROR_NONE:
21232131
return INT2NUM(nwritten);
21242132
case SSL_ERROR_WANT_WRITE:
@@ -2139,10 +2147,11 @@ ossl_ssl_write_internal_safe(VALUE _args)
21392147
* make the error handling in line with the socket library.
21402148
* [Bug #14713] https://bugs.ruby-lang.org/issues/14713
21412149
*/
2142-
if (errno == EPROTOTYPE)
2150+
if (saved_errno == EPROTOTYPE)
21432151
continue;
21442152
#endif
2145-
if (errno) rb_sys_fail(0);
2153+
if (saved_errno)
2154+
rb_exc_raise(rb_syserr_new(saved_errno, "SSL_write"));
21462155
/* fallthrough */
21472156
default:
21482157
ossl_raise(eSSLError, "SSL_write");

0 commit comments

Comments
 (0)