Only allow SSL_set_session before the handshake.
Otherwise things break horribly. Explicitly abort to help catch bugs.
Change-Id: I66e2bf8808199b3331b3adde68d73758a601eb8c
Reviewed-on: https://boringssl-review.googlesource.com/10761
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index ce2ba41..4ff25ce 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1683,7 +1683,9 @@
/* SSL_set_session, for a client, configures |ssl| to offer to resume |session|
* in the initial handshake and returns one. The caller retains ownership of
- * |session|. */
+ * |session|.
+ *
+ * It is an error to call this function after the handshake has begun. */
OPENSSL_EXPORT int SSL_set_session(SSL *ssl, SSL_SESSION *session);
/* SSL_get_session returns a non-owning pointer to |ssl|'s session. For
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 9a5cd02..c8177b4 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -728,7 +728,7 @@
ssl->session->not_resumable ||
!ssl_session_is_time_valid(ssl, ssl->session) ||
session_version < min_version || session_version > max_version) {
- SSL_set_session(ssl, NULL);
+ ssl_set_session(ssl, NULL);
}
}
@@ -896,7 +896,7 @@
} else {
/* The session wasn't resumed. Create a fresh SSL_SESSION to
* fill out. */
- SSL_set_session(ssl, NULL);
+ ssl_set_session(ssl, NULL);
if (!ssl_get_new_session(ssl, 0 /* client */)) {
goto f_err;
}
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index c5bd4be..f7c127d 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -719,7 +719,7 @@
session = NULL;
ssl->s3->session_reused = 1;
} else {
- SSL_set_session(ssl, NULL);
+ ssl_set_session(ssl, NULL);
if (!ssl_get_new_session(ssl, 1 /* server */)) {
goto err;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index 660ba79..f285682 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1262,6 +1262,8 @@
* it has expired. */
int ssl_session_is_time_valid(const SSL *ssl, const SSL_SESSION *session);
+void ssl_set_session(SSL *ssl, SSL_SESSION *session);
+
enum ssl_session_result_t {
ssl_session_success,
ssl_session_error,
diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c
index 3a56dcd..78dfeab 100644
--- a/ssl/ssl_session.c
+++ b/ssl/ssl_session.c
@@ -136,6 +136,7 @@
#include <openssl/ssl.h>
#include <assert.h>
+#include <stdlib.h>
#include <string.h>
#include <openssl/err.h>
@@ -482,7 +483,7 @@
SSL_SESSION_free(ssl->s3->new_session);
ssl->s3->new_session = session;
- SSL_set_session(ssl, NULL);
+ ssl_set_session(ssl, NULL);
return 1;
err:
@@ -792,8 +793,18 @@
}
int SSL_set_session(SSL *ssl, SSL_SESSION *session) {
+ /* SSL_set_session may only be called before the handshake has started. */
+ if (ssl->state != SSL_ST_INIT || ssl->s3->initial_handshake_complete) {
+ abort();
+ }
+
+ ssl_set_session(ssl, session);
+ return 1;
+}
+
+void ssl_set_session(SSL *ssl, SSL_SESSION *session) {
if (ssl->session == session) {
- return 1;
+ return;
}
SSL_SESSION_free(ssl->session);
@@ -801,8 +812,6 @@
if (session != NULL) {
SSL_SESSION_up_ref(session);
}
-
- return 1;
}
long SSL_CTX_set_timeout(SSL_CTX *ctx, long timeout) {
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index d2d99a7..8dfb5da 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -228,7 +228,7 @@
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return ssl_hs_error;
}
- SSL_set_session(ssl, NULL);
+ ssl_set_session(ssl, NULL);
} else {
if (!ssl_get_new_session(ssl, 0)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
