#901 closed defect (fixed)
Changes in openssl master wrt SSL_shutdown()
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | nginx-core | Version: | 1.9.x |
Keywords: | Cc: | ||
uname -a: | |||
nginx -V: | 1.9.11 |
Description
ngx_ssl_shutdown() currently doesn't consider a -1 return value for openssl's SSL_shutdown() function. So far the comment
/* SSL_shutdown() never returns -1, on error it returns 0 */
was correct, but there's a recent change in openssl master which is also backported to the openssl 1.0.2 stable branch (not yet released), which changes that:
https://github.com/openssl/openssl/commit/64193c8218540499984cd63cda41f3cd491f3f59
Change History (8)
comment:1 by , 9 years ago
comment:2 by , 9 years ago
Thanks for fixing up the comment. The changes made to ssl/ssl_lib.c from
https://github.com/openssl/openssl/commit/64193c8218540499984cd63cda41f3cd491f3f59 also have an impact on nginx: On a busy server we're seeing a lot of log messages like this:
2016/02/12 14:31:45 [crit] 8106#8106: *82 SSL_shutdown() failed (SSL: error:140E0197:SSL routines:SSL_shutdown:shutdown while in init) while SSL handshaking, client: X.X.X.X, server: 0.0.0.0:443
I'm wondering whether these should really be logged with "critical" severity? After all nginx can't do anything about a misbehaving SSL client and the shutdown is handled gracefully on the openssl side.
comment:3 by , 9 years ago
This is logged at critical severity as this is not an error nginx expects to happen at this point. The error in question was just added in OpenSSL 1.0.2f (commit f73c737), so nginx knows nothing about this error and complains loudly.
Something like this should reduce logging severity as appropriate (untested):
# HG changeset patch # User Maxim Dounin <mdounin@mdounin.ru> # Date 1455631090 -10800 # Tue Feb 16 16:58:10 2016 +0300 # Node ID 7b5564208e67810689e5841a0f27584d732b1c97 # Parent 50fb3fd79f76c4d62e330650bb6047194c972352 SSL: logging level of "shutdown while in init". diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -1942,6 +1942,9 @@ ngx_ssl_connection_error(ngx_connection_ #ifdef SSL_R_INAPPROPRIATE_FALLBACK || n == SSL_R_INAPPROPRIATE_FALLBACK /* 373 */ #endif +#ifdef SSL_R_SHUTDOWN_WHILE_IN_INIT + || n == SSL_R_SHUTDOWN_WHILE_IN_INIT /* 407 */ +#endif || n == 1000 /* SSL_R_SSLV3_ALERT_CLOSE_NOTIFY */ || n == SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE /* 1010 */ || n == SSL_R_SSLV3_ALERT_BAD_RECORD_MAC /* 1020 */
comment:4 by , 9 years ago
Alternatively, something like this could be used to avoid calling SSL_shutdown() during SSL handshakes at all. This should also fix "calling a function you should not call" messages observed with OpenSSL 1.0.2f.
# HG changeset patch # User Maxim Dounin <mdounin@mdounin.ru> # Date 1455640522 -10800 # Tue Feb 16 19:35:22 2016 +0300 # Node ID fe3b7aa9c4bf62785d4cfe448efe818dbed1072a # Parent 27010543f3b619709a68a9272a5e3d90fbf8e468 SSL: avoid calling SSL_shutdown() during handshake (ticket #901). diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -1767,6 +1767,19 @@ ngx_ssl_shutdown(ngx_connection_t *c) int n, sslerr, mode; ngx_err_t err; + if (!c->ssl->handshaked) { + /* + * OpenSSL 1.0.2f complains if SSL_shutdown() is called during + * an SSL handshake, while previous versions always return 0. + * Avoid calling SSL_shutdown() if handshake wasn't completed. + */ + + SSL_free(c->ssl->connection); + c->ssl = NULL; + + return NGX_OK; + } + if (c->timedout) { mode = SSL_RECEIVED_SHUTDOWN|SSL_SENT_SHUTDOWN; SSL_set_quiet_shutdown(c->ssl->connection, 1);
This looks like a slightly better solution.
comment:7 by , 9 years ago
I used Nginx 1.8.1 stable version with OpenSSL 1.0.2g stable version. But I am getting error #942.
Do let me know which version should I use to resolve this issue ? Please help.
comment:8 by , 9 years ago
The patch in question is present in nginx 1.9.12 and up, quote from CHANGES:
*) Workaround: "called a function you should not call" and "shutdown while in init" messages might appear in logs when using OpenSSL 1.0.2f.
The commit in question actually reverts one case which returns -1. The comment was correct before OpenSSL 0.9.8m, when SSL_shutdown() was changed to return -1 (see this commit).
As it looks like people tend to complain a lot about this comment (this is the 3rd question I see in the last couple of months), here is a patch: