On 02/02/16 11:24, Jakob Bohm wrote:
> On 02/02/2016 11:40, Matt Caswell wrote:
>> 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.
> No, actual application data (just a few bytes) was sent in each
> direction.
>
> Specifically, some bytes were sent client to server, then a reply
> was sent server to client and the connection was closed.
>
> Also, the s_client output showed a completed handshake, with
> ChangeCipherSpec in both directions and dump of certificate
> chain before the first application data was sent (client to
> server, then server to client).
>
> The s_client command line was
>
> cat data | openssl s_client -connect xx.xx.xx.xx:xxxx -msg -tls1
> -ign_eof -debug
>
> However I cannot rule out that the changes to either SSL_shutdown()
> or the rearranged error checking code triggered the issue.
Hmmm. Perhaps try reverting the SSL_shutdown() change to rule that out
as related in some way? Patch attached to revert that change back to the
previous implementation.
Matt
>From bc1db8044d293989e674250eda632728aa96f0de Mon Sep 17 00:00:00 2001
From: Matt Caswell <m...@openssl.org>
Date: Tue, 2 Feb 2016 11:31:59 +0000
Subject: [PATCH] Revert "Handle SSL_shutdown while in init more appropriately"
This reverts commit f73c737c7ac908c5d6407c419769123392a3b0a9.
---
ssl/s3_lib.c | 15 ---------------
ssl/ssl.h | 2 --
ssl/ssl_err.c | 2 --
ssl/ssl_lib.c | 5 ++++-
4 files changed, 4 insertions(+), 20 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..fb5112c 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
@@ -3057,7 +3056,6 @@ void ERR_load_SSL_strings(void);
# define SSL_R_SERVERHELLO_TLSEXT 275
# define SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED 277
# define SSL_R_SHORT_READ 219
-# define SSL_R_SHUTDOWN_WHILE_IN_INIT 407
# define SSL_R_SIGNATURE_ALGORITHMS_ERROR 360
# define SSL_R_SIGNATURE_FOR_NON_SIGNING_CERTIFICATE 220
# define SSL_R_SRP_A_CALC 361
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index dd3b2af..6d1366f 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"},
@@ -648,7 +647,6 @@ static ERR_STRING_DATA SSL_str_reasons[] = {
{ERR_REASON(SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED),
"session id context uninitialized"},
{ERR_REASON(SSL_R_SHORT_READ), "short read"},
- {ERR_REASON(SSL_R_SHUTDOWN_WHILE_IN_INIT), "shutdown while in init"},
{ERR_REASON(SSL_R_SIGNATURE_ALGORITHMS_ERROR),
"signature algorithms error"},
{ERR_REASON(SSL_R_SIGNATURE_FOR_NON_SIGNING_CERTIFICATE),
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 2744be8..f2071db 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1060,7 +1060,10 @@ int SSL_shutdown(SSL *s)
return -1;
}
- return s->method->ssl_shutdown(s);
+ if ((s != NULL) && !SSL_in_init(s))
+ return (s->method->ssl_shutdown(s));
+ else
+ return (1);
}
int SSL_renegotiate(SSL *s)
--
2.5.0
_______________________________________________
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
