On 02/02/16 07:52, Jakob Bohm wrote:
> I am trying to upgrade an existing 3rd party multithreaded server
> from OpenSSL 1.0.2c to 1.0.2f . However when I do so, it starts
> mishandling the close_notify "alert".
>
> 1.0.2f seems to send the close_notify alert unencrypted followed
> by an encrypted decrypt_failed alert, where 1.0.2c correctly
> sends just an encrypted close_notify alert.
>
> I am unsure if this exposed a bug in the daemon or in OpenSSL
> itself.
I have a theory.
Previous versions of 1.0.2 handled an SSL_shutdown() call while in the
middle of a handshake by ignoring it and returning 1 immediately
(meaning everything shutdown successfully). Clearly everything did not
shutdown successfully so the return value is not correct.
1.0.2f "fixed" this to attempt to more appropriately shutdown the
connection. It now attempts to send a close_notify alert immediately
(and will return 0 - meaning more work to do), it will then fail with a
-1 return on a second call to SSL_shutdown (meaning we failed to
complete a bi-directional shutdown). The reason is that completing a
full bi-directional shutdown while mid-handshake is problematic (e.g. if
the peer has sent a CCS before it receives our close_notify).
My theory as to what is happening in this case is that SSL_shutdown is
being called mid-handshake. Could that be the case do you think? It
doesn't entirely match with the description you gave (i.e. 1.0.2c would
not send an encrypted close_notify...it wouldn't send anything in this
scenario)...but its the only change I can think of that might have a
bearing on this.
If so then I could see a situation where the server has not yet
processed a CCS from the client and SSL_shutdown is called. It therefore
sends its close_notify in the clear. In the meantime the client has sent
its CCS before processing the close_notify. The client then expects the
next message from the server to be encrypted...which it obviously isn't.
It's not entirely clear to me what the correct behaviour is in that
situation, but it is certainly unexpected and a change from what
happened before.
It looks to me that trying to send a close_notify mid-handshake was
probably a bad idea. Perhaps a better fix is just to have SSL_shutdown()
return -1 and do nothing if called mid-handshake.
I've attached a 1.0.2 patch that does this. Would you be able to try it,
and see if that corrects your problem?
Thanks
Matt
From d9a741175bfe83c36fcb6660f9052e58983eb9ff Mon Sep 17 00:00:00 2001
From: Matt Caswell <m...@openssl.org>
Date: Tue, 2 Feb 2016 10:05:43 +0000
Subject: [PATCH] Handle SSL_shutdown while in init more appropriately #2
Previous commit f73c737c7 attempted to "fix" a problem with the way
SSL_shutdown() behaved whilst in mid-handshake. The original behaviour had
SSL_shutdown() return immediately having taken no action if called mid-
handshake with a return value of 1 (meaning everything was shutdown
successfully). In fact the shutdown has not been successful.
Commit f73c737c7 changed that to send a close_notify anyway and then
return. This seems to be causing some problems for some applications so
perhaps a better (much simpler) approach is revert to the previous
behaviour (no attempt at a shutdown), but return -1 (meaning the shutdown
was not successful).
---
ssl/s3_lib.c | 15 ---------------
ssl/ssl.h | 1 -
ssl/ssl_err.c | 1 -
ssl/ssl_lib.c | 7 ++++++-
4 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index f846cb5..6a06625 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -4326,21 +4326,6 @@ int ssl3_shutdown(SSL *s)
}
#endif
} else if (!(s->shutdown & SSL_RECEIVED_SHUTDOWN)) {
- if (SSL_in_init(s)) {
- /*
- * We can't shutdown properly if we are in the middle of a
- * handshake. Doing so is problematic because the peer may send a
- * CCS before it acts on our close_notify. However we should not
- * continue to process received handshake messages or CCS once our
- * close_notify has been sent. Therefore any close_notify from
- * the peer will be unreadable because we have not moved to the next
- * cipher state. Its best just to avoid this can-of-worms. Return
- * an error if we are wanting to wait for a close_notify from the
- * peer and we are in init.
- */
- SSLerr(SSL_F_SSL3_SHUTDOWN, SSL_R_SHUTDOWN_WHILE_IN_INIT);
- return -1;
- }
/*
* If we are waiting for a close from our peer, we are closed
*/
diff --git a/ssl/ssl.h b/ssl/ssl.h
index ae8c925..04d4007 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2713,7 +2713,6 @@ void ERR_load_SSL_strings(void);
# define SSL_F_SSL3_SETUP_KEY_BLOCK 157
# define SSL_F_SSL3_SETUP_READ_BUFFER 156
# define SSL_F_SSL3_SETUP_WRITE_BUFFER 291
-# define SSL_F_SSL3_SHUTDOWN 396
# define SSL_F_SSL3_WRITE_BYTES 158
# define SSL_F_SSL3_WRITE_PENDING 159
# define SSL_F_SSL_ADD_CERT_CHAIN 318
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index dd3b2af..704088d 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -206,7 +206,6 @@ static ERR_STRING_DATA SSL_str_functs[] = {
{ERR_FUNC(SSL_F_SSL3_SETUP_KEY_BLOCK), "ssl3_setup_key_block"},
{ERR_FUNC(SSL_F_SSL3_SETUP_READ_BUFFER), "ssl3_setup_read_buffer"},
{ERR_FUNC(SSL_F_SSL3_SETUP_WRITE_BUFFER), "ssl3_setup_write_buffer"},
- {ERR_FUNC(SSL_F_SSL3_SHUTDOWN), "ssl3_shutdown"},
{ERR_FUNC(SSL_F_SSL3_WRITE_BYTES), "ssl3_write_bytes"},
{ERR_FUNC(SSL_F_SSL3_WRITE_PENDING), "ssl3_write_pending"},
{ERR_FUNC(SSL_F_SSL_ADD_CERT_CHAIN), "ssl_add_cert_chain"},
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 2744be8..7c23f9e 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1060,7 +1060,12 @@ int SSL_shutdown(SSL *s)
return -1;
}
- return s->method->ssl_shutdown(s);
+ if (!SSL_in_init(s)) {
+ return s->method->ssl_shutdown(s);
+ } else {
+ SSLerr(SSL_F_SSL_SHUTDOWN, SSL_R_SHUTDOWN_WHILE_IN_INIT);
+ return -1;
+ }
}
int SSL_renegotiate(SSL *s)
--
2.5.0
_______________________________________________
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
