From fc7f190c732729c1d0eb9dcdb7ff05ed6b06056f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 3 May 2016 17:55:00 +0100 Subject: [PATCH] Handle no async jobs in libssl If the application has limited the size of the async pool using ASYNC_init_thread() then we could run out of jobs while trying to start a libssl io operation. However libssl was failing to handle this and treating it like a fatal error. It should not be fatal...we just need to retry when there are jobs available again. Reviewed-by: Richard Levitte --- apps/s_client.c | 5 ++ apps/s_server.c | 8 +++ doc/ssl/SSL_get_error.pod | 10 ++++ doc/ssl/SSL_want.pod | 23 +++++++-- include/openssl/ssl.h | 3 ++ ssl/ssl_lib.c | 102 ++++++++++++++++++++------------------ 6 files changed, 101 insertions(+), 50 deletions(-) diff --git a/apps/s_client.c b/apps/s_client.c index 5d575ad726..42ef049d24 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -238,6 +238,7 @@ static void do_ssl_shutdown(SSL *ssl) case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: case SSL_ERROR_WANT_ASYNC: + case SSL_ERROR_WANT_ASYNC_JOB: /* We just do busy waiting. Nothing clever */ continue; } @@ -2360,6 +2361,8 @@ int s_client_main(int argc, char **argv) write_ssl = 0; } break; + case SSL_ERROR_WANT_ASYNC_JOB: + /* This shouldn't ever happen in s_client - treat as an error */ case SSL_ERROR_SSL: ERR_print_errors(bio_err); goto shut; @@ -2446,6 +2449,8 @@ int s_client_main(int argc, char **argv) BIO_printf(bio_c_out, "closed\n"); ret = 0; goto shut; + case SSL_ERROR_WANT_ASYNC_JOB: + /* This shouldn't ever happen in s_client. Treat as an error */ case SSL_ERROR_SSL: ERR_print_errors(bio_err); goto shut; diff --git a/apps/s_server.c b/apps/s_server.c index f0b28fd288..9cbff09b85 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -2421,6 +2421,10 @@ static int sv_body(int s, int stype, unsigned char *context) case SSL_ERROR_WANT_X509_LOOKUP: BIO_printf(bio_s_out, "Write BLOCK\n"); break; + case SSL_ERROR_WANT_ASYNC_JOB: + /* + * This shouldn't ever happen in s_server. Treat as an error + */ case SSL_ERROR_SYSCALL: case SSL_ERROR_SSL: BIO_printf(bio_s_out, "ERROR\n"); @@ -2495,6 +2499,10 @@ static int sv_body(int s, int stype, unsigned char *context) case SSL_ERROR_WANT_READ: BIO_printf(bio_s_out, "Read BLOCK\n"); break; + case SSL_ERROR_WANT_ASYNC_JOB: + /* + * This shouldn't ever happen in s_server. Treat as an error + */ case SSL_ERROR_SYSCALL: case SSL_ERROR_SSL: BIO_printf(bio_s_out, "ERROR\n"); diff --git a/doc/ssl/SSL_get_error.pod b/doc/ssl/SSL_get_error.pod index 271f849666..dd7ac3c690 100644 --- a/doc/ssl/SSL_get_error.pod +++ b/doc/ssl/SSL_get_error.pod @@ -99,6 +99,16 @@ L. The TLS/SSL I/O function should be called again later. The function B be called from the same thread that the original call was made from. +=item SSL_ERROR_WANT_ASYNC_JOB + +The asynchronous job could not be started because there were no async jobs +available in the pool (see ASYNC_init_thread(3)). This will only occur if the +mode has been set to SSL_MODE_ASYNC using L or +L and a maximum limit has been set on the async job pool +through a call to L. The application should retry the +operation after a currently executing asynchronous operation for the current +thread has completed. + =item SSL_ERROR_SYSCALL Some I/O error occurred. The OpenSSL error queue may contain more diff --git a/doc/ssl/SSL_want.pod b/doc/ssl/SSL_want.pod index e8b426c7b5..d1c0fe0d5b 100644 --- a/doc/ssl/SSL_want.pod +++ b/doc/ssl/SSL_want.pod @@ -2,7 +2,9 @@ =head1 NAME -SSL_want, SSL_want_nothing, SSL_want_read, SSL_want_write, SSL_want_x509_lookup - obtain state information TLS/SSL I/O operation +SSL_want, SSL_want_nothing, SSL_want_read, SSL_want_write, SSL_want_x509_lookup, +SSL_want_async, SSL_want_async_job - obtain state information TLS/SSL I/O +operation =head1 SYNOPSIS @@ -13,6 +15,8 @@ SSL_want, SSL_want_nothing, SSL_want_read, SSL_want_write, SSL_want_x509_lookup int SSL_want_read(const SSL *ssl); int SSL_want_write(const SSL *ssl); int SSL_want_x509_lookup(const SSL *ssl); + int SSL_want_async(const SSL *ssl); + int SSL_want_async_job(const SSL *ssl); =head1 DESCRIPTION @@ -65,10 +69,23 @@ SSL_CTX_set_client_cert_cb() has asked to be called again. A call to L should return SSL_ERROR_WANT_X509_LOOKUP. +=item SSL_ASYNC_PAUSED + +An asynchronous operation partially completed and was then paused. See +L. A call to L should return +SSL_ERROR_WANT_ASYNC. + +=item SSL_ASYNC_NO_JOBS + +The asynchronous job could not be started because there were no async jobs +available in the pool (see ASYNC_init_thread(3)). A call to L +should return SSL_ERROR_WANT_ASYNC_JOB. + =back -SSL_want_nothing(), SSL_want_read(), SSL_want_write(), SSL_want_x509_lookup() -return 1, when the corresponding condition is true or 0 otherwise. +SSL_want_nothing(), SSL_want_read(), SSL_want_write(), SSL_want_x509_lookup(), +SSL_want_async() and SSL_want_async_job() return 1, when the corresponding +condition is true or 0 otherwise. =head1 SEE ALSO diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 3f732c8fd3..0ab0df2749 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -884,6 +884,7 @@ __owur int SSL_extension_supported(unsigned int ext_type); # define SSL_READING 3 # define SSL_X509_LOOKUP 4 # define SSL_ASYNC_PAUSED 5 +# define SSL_ASYNC_NO_JOBS 6 /* These will only be used when doing non-blocking IO */ # define SSL_want_nothing(s) (SSL_want(s) == SSL_NOTHING) @@ -891,6 +892,7 @@ __owur int SSL_extension_supported(unsigned int ext_type); # define SSL_want_write(s) (SSL_want(s) == SSL_WRITING) # define SSL_want_x509_lookup(s) (SSL_want(s) == SSL_X509_LOOKUP) # define SSL_want_async(s) (SSL_want(s) == SSL_ASYNC_PAUSED) +# define SSL_want_async_job(s) (SSL_want(s) == SSL_ASYNC_NO_JOBS) # define SSL_MAC_FLAG_READ_MAC_STREAM 1 # define SSL_MAC_FLAG_WRITE_MAC_STREAM 2 @@ -1122,6 +1124,7 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION) # define SSL_ERROR_WANT_CONNECT 7 # define SSL_ERROR_WANT_ACCEPT 8 # define SSL_ERROR_WANT_ASYNC 9 +# define SSL_ERROR_WANT_ASYNC_JOB 10 # define SSL_CTRL_SET_TMP_DH 3 # define SSL_CTRL_SET_TMP_ECDH 4 # define SSL_CTRL_SET_TMP_DH_CB 6 diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index e00c1191b5..e07fa07de7 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1516,6 +1516,9 @@ static int ssl_start_async_job(SSL *s, struct ssl_async_args *args, case ASYNC_PAUSE: s->rwstate = SSL_ASYNC_PAUSED; return -1; + case ASYNC_NO_JOBS: + s->rwstate = SSL_ASYNC_NO_JOBS; + return -1; case ASYNC_FINISH: s->job = NULL; return ret; @@ -2939,56 +2942,61 @@ int SSL_get_error(const SSL *s, int i) return (SSL_ERROR_SSL); } - if ((i < 0) && SSL_want_read(s)) { - bio = SSL_get_rbio(s); - if (BIO_should_read(bio)) - return (SSL_ERROR_WANT_READ); - else if (BIO_should_write(bio)) - /* - * This one doesn't make too much sense ... We never try to write - * to the rbio, and an application program where rbio and wbio - * are separate couldn't even know what it should wait for. - * However if we ever set s->rwstate incorrectly (so that we have - * SSL_want_read(s) instead of SSL_want_write(s)) and rbio and - * wbio *are* the same, this test works around that bug; so it - * might be safer to keep it. - */ - return (SSL_ERROR_WANT_WRITE); - else if (BIO_should_io_special(bio)) { - reason = BIO_get_retry_reason(bio); - if (reason == BIO_RR_CONNECT) - return (SSL_ERROR_WANT_CONNECT); - else if (reason == BIO_RR_ACCEPT) - return (SSL_ERROR_WANT_ACCEPT); - else - return (SSL_ERROR_SYSCALL); /* unknown */ + if (i < 0) { + if (SSL_want_read(s)) { + bio = SSL_get_rbio(s); + if (BIO_should_read(bio)) + return (SSL_ERROR_WANT_READ); + else if (BIO_should_write(bio)) + /* + * This one doesn't make too much sense ... We never try to write + * to the rbio, and an application program where rbio and wbio + * are separate couldn't even know what it should wait for. + * However if we ever set s->rwstate incorrectly (so that we have + * SSL_want_read(s) instead of SSL_want_write(s)) and rbio and + * wbio *are* the same, this test works around that bug; so it + * might be safer to keep it. + */ + return (SSL_ERROR_WANT_WRITE); + else if (BIO_should_io_special(bio)) { + reason = BIO_get_retry_reason(bio); + if (reason == BIO_RR_CONNECT) + return (SSL_ERROR_WANT_CONNECT); + else if (reason == BIO_RR_ACCEPT) + return (SSL_ERROR_WANT_ACCEPT); + else + return (SSL_ERROR_SYSCALL); /* unknown */ + } } - } - if ((i < 0) && SSL_want_write(s)) { - bio = SSL_get_wbio(s); - if (BIO_should_write(bio)) - return (SSL_ERROR_WANT_WRITE); - else if (BIO_should_read(bio)) - /* - * See above (SSL_want_read(s) with BIO_should_write(bio)) - */ - return (SSL_ERROR_WANT_READ); - else if (BIO_should_io_special(bio)) { - reason = BIO_get_retry_reason(bio); - if (reason == BIO_RR_CONNECT) - return (SSL_ERROR_WANT_CONNECT); - else if (reason == BIO_RR_ACCEPT) - return (SSL_ERROR_WANT_ACCEPT); - else - return (SSL_ERROR_SYSCALL); + if (SSL_want_write(s)) { + bio = SSL_get_wbio(s); + if (BIO_should_write(bio)) + return (SSL_ERROR_WANT_WRITE); + else if (BIO_should_read(bio)) + /* + * See above (SSL_want_read(s) with BIO_should_write(bio)) + */ + return (SSL_ERROR_WANT_READ); + else if (BIO_should_io_special(bio)) { + reason = BIO_get_retry_reason(bio); + if (reason == BIO_RR_CONNECT) + return (SSL_ERROR_WANT_CONNECT); + else if (reason == BIO_RR_ACCEPT) + return (SSL_ERROR_WANT_ACCEPT); + else + return (SSL_ERROR_SYSCALL); + } + } + if (SSL_want_x509_lookup(s)) { + return (SSL_ERROR_WANT_X509_LOOKUP); + } + if (SSL_want_async(s)) { + return SSL_ERROR_WANT_ASYNC; + } + if (SSL_want_async_job(s)) { + return SSL_ERROR_WANT_ASYNC_JOB; } - } - if ((i < 0) && SSL_want_x509_lookup(s)) { - return (SSL_ERROR_WANT_X509_LOOKUP); - } - if ((i < 0) && SSL_want_async(s)) { - return SSL_ERROR_WANT_ASYNC; } if (i == 0) { -- 2.34.1