Tidy up shutdown state.
The existing logic gets confused in a number of cases around close_notify vs.
fatal alert. SSL_shutdown, while still pushing to the error queue, will fail to
notice alerts. We also get confused if we try to send a fatal alert when we've
already sent something else.
Change-Id: I9b1d217fbf1ee8a9c59efbebba60165b7de9689e
Reviewed-on: https://boringssl-review.googlesource.com/7952
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index ade7120..25e1349 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -516,14 +516,12 @@
void SSL_set_connect_state(SSL *ssl) {
ssl->server = 0;
- ssl->shutdown = 0;
ssl->state = SSL_ST_CONNECT;
ssl->handshake_func = ssl->method->ssl_connect;
}
void SSL_set_accept_state(SSL *ssl) {
ssl->server = 1;
- ssl->shutdown = 0;
ssl->state = SSL_ST_ACCEPT;
ssl->handshake_func = ssl->method->ssl_accept;
}
@@ -637,7 +635,7 @@
return -1;
}
- if (ssl->shutdown & SSL_SENT_SHUTDOWN) {
+ if (ssl->s3->send_shutdown != ssl_shutdown_none) {
OPENSSL_PUT_ERROR(SSL, SSL_R_PROTOCOL_IS_SHUTDOWN);
return -1;
}
@@ -662,11 +660,6 @@
/* Functions which use SSL_get_error must clear the error queue on entry. */
ERR_clear_error();
- /* Note that this function behaves differently from what one might expect.
- * Return values are 0 for no success (yet), 1 for success; but calling it
- * once is usually not enough, even if blocking I/O is used (see
- * ssl3_shutdown). */
-
if (ssl->handshake_func == NULL) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNINITIALIZED);
return -1;
@@ -678,44 +671,37 @@
return -1;
}
- /* Do nothing if configured not to send a close_notify. */
if (ssl->quiet_shutdown) {
- ssl->shutdown = SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN;
+ /* Do nothing if configured not to send a close_notify. */
+ ssl->s3->send_shutdown = ssl_shutdown_close_notify;
+ ssl->s3->recv_shutdown = ssl_shutdown_close_notify;
return 1;
}
- if (!(ssl->shutdown & SSL_SENT_SHUTDOWN)) {
- ssl->shutdown |= SSL_SENT_SHUTDOWN;
- ssl3_send_alert(ssl, SSL3_AL_WARNING, SSL_AD_CLOSE_NOTIFY);
+ /* This function completes in two stages. It sends a close_notify and then it
+ * waits for a close_notify to come in. Perform exactly one action and return
+ * whether or not it succeeds. */
- /* our shutdown alert has been sent now, and if it still needs to be
- * written, ssl->s3->alert_dispatch will be true */
- if (ssl->s3->alert_dispatch) {
- return -1; /* return WANT_WRITE */
+ if (ssl->s3->send_shutdown != ssl_shutdown_close_notify) {
+ /* Send a close_notify. */
+ if (ssl3_send_alert(ssl, SSL3_AL_WARNING, SSL_AD_CLOSE_NOTIFY) <= 0) {
+ return -1;
}
} else if (ssl->s3->alert_dispatch) {
- /* resend it if not sent */
- int ret = ssl->method->ssl_dispatch_alert(ssl);
- if (ret == -1) {
- /* we only get to return -1 here the 2nd/Nth invocation, we must have
- * already signalled return 0 upon a previous invoation, return
- * WANT_WRITE */
- return ret;
+ /* Finish sending the close_notify. */
+ if (ssl->method->ssl_dispatch_alert(ssl) <= 0) {
+ return -1;
}
- } else if (!(ssl->shutdown & SSL_RECEIVED_SHUTDOWN)) {
- /* If we are waiting for a close from our peer, we are closed */
+ } else if (ssl->s3->recv_shutdown != ssl_shutdown_close_notify) {
+ /* Wait for the peer's close_notify. */
ssl->method->ssl_read_close_notify(ssl);
- if (!(ssl->shutdown & SSL_RECEIVED_SHUTDOWN)) {
- return -1; /* return WANT_READ */
+ if (ssl->s3->recv_shutdown != ssl_shutdown_close_notify) {
+ return -1;
}
}
- if (ssl->shutdown == (SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN) &&
- !ssl->s3->alert_dispatch) {
- return 1;
- } else {
- return 0;
- }
+ /* Return 0 for unidirectional shutdown and 1 for bidirectional shutdown. */
+ return ssl->s3->recv_shutdown == ssl_shutdown_close_notify;
}
int SSL_get_error(const SSL *ssl, int ret_code) {
@@ -738,8 +724,7 @@
}
if (ret_code == 0) {
- if ((ssl->shutdown & SSL_RECEIVED_SHUTDOWN) && ssl->s3->clean_shutdown) {
- /* The socket was cleanly shut down with a close_notify. */
+ if (ssl->s3->recv_shutdown == ssl_shutdown_close_notify) {
return SSL_ERROR_ZERO_RETURN;
}
/* An EOF was observed which violates the protocol, and the underlying
@@ -1929,12 +1914,32 @@
void SSL_set_shutdown(SSL *ssl, int mode) {
/* It is an error to clear any bits that have already been set. (We can't try
* to get a second close_notify or send two.) */
- assert((ssl->shutdown & mode) == ssl->shutdown);
+ assert((SSL_get_shutdown(ssl) & mode) == SSL_get_shutdown(ssl));
- ssl->shutdown |= mode;
+ if (mode & SSL_RECEIVED_SHUTDOWN &&
+ ssl->s3->recv_shutdown == ssl_shutdown_none) {
+ ssl->s3->recv_shutdown = ssl_shutdown_close_notify;
+ }
+
+ if (mode & SSL_SENT_SHUTDOWN &&
+ ssl->s3->send_shutdown == ssl_shutdown_none) {
+ ssl->s3->send_shutdown = ssl_shutdown_close_notify;
+ }
}
-int SSL_get_shutdown(const SSL *ssl) { return ssl->shutdown; }
+int SSL_get_shutdown(const SSL *ssl) {
+ int ret = 0;
+ if (ssl->s3->recv_shutdown != ssl_shutdown_none) {
+ /* Historically, OpenSSL set |SSL_RECEIVED_SHUTDOWN| on both close_notify
+ * and fatal alert. */
+ ret |= SSL_RECEIVED_SHUTDOWN;
+ }
+ if (ssl->s3->send_shutdown == ssl_shutdown_close_notify) {
+ /* Historically, OpenSSL set |SSL_SENT_SHUTDOWN| on only close_notify. */
+ ret |= SSL_SENT_SHUTDOWN;
+ }
+ return ret;
+}
int SSL_version(const SSL *ssl) { return ssl->version; }
@@ -2642,7 +2647,6 @@
}
ssl->hit = 0;
- ssl->shutdown = 0;
/* SSL_clear may be called before or after the |ssl| is initialized in either
* accept or connect state. In the latter case, SSL_clear should preserve the