Don't assign handshake_func in the handshake functions.
It should already be assigned, as of upstream's
b31b04d951e9b65bde29657e1ae057b76f0f0a73. I believe these assignments are part
of the reason it used to appear to work. Replace them with assertions. So the
assertions are actually valid, check in SSL_connect / SSL_accept that they are
never called if the socket had been placed in the opposite state. (Or we'd be
in another place where it would have appeared to work with the handshake
functions fixing things afterwards.)
Now the only places handshake_func is set are in SSL_set_{connect,accept}_state
and the method switches.
Change-Id: Ib249212bf4aa889b94c35965a62ca06bdbcf52e1
Reviewed-on: https://boringssl-review.googlesource.com/2432
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index cc47bde..42f0b2d 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2405,6 +2405,8 @@
#define SSL_F_d2i_SSL_SESSION_get_string 290
#define SSL_F_ssl3_send_new_session_ticket 291
#define SSL_F_SSL_SESSION_to_bytes_full 292
+#define SSL_F_SSL_accept 293
+#define SSL_F_SSL_connect 294
#define SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS 100
#define SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC 101
#define SSL_R_INVALID_NULL_CMD_NAME 102
diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
index 60bb12a..9a8b361 100644
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -112,6 +112,7 @@
* [including the GNU Public Licence.]
*/
+#include <assert.h>
#include <stdio.h>
#include <openssl/bn.h>
@@ -152,6 +153,7 @@
int ret= -1;
int new_state,state,skip=0;
+ assert(s->handshake_func == dtls1_connect);
ERR_clear_error();
ERR_clear_system_error();
@@ -521,7 +523,6 @@
ret=1;
/* s->server=0; */
- s->handshake_func=dtls1_connect;
s->ctx->stats.sess_connect_good++;
if (cb != NULL) cb(s,SSL_CB_HANDSHAKE_DONE,1);
diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index 6d6c46e..43cfefa 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -112,6 +112,7 @@
* [including the GNU Public Licence.]
*/
+#include <assert.h>
#include <stdio.h>
#include <openssl/bn.h>
@@ -153,6 +154,7 @@
int ret= -1;
int new_state,state,skip=0;
+ assert(s->handshake_func == dtls1_accept);
ERR_clear_error();
ERR_clear_system_error();
@@ -576,7 +578,6 @@
s->ctx->stats.sess_accept_good++;
/* s->server=1; */
- s->handshake_func=dtls1_accept;
if (cb != NULL) cb(s,SSL_CB_HANDSHAKE_DONE,1);
}
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index cc034c6..b16e163 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -193,6 +193,7 @@
int ret= -1;
int new_state,state,skip=0;
+ assert(s->handshake_func == ssl3_connect);
ERR_clear_error();
ERR_clear_system_error();
@@ -574,7 +575,6 @@
ret=1;
/* s->server=0; */
- s->handshake_func=ssl3_connect;
s->ctx->stats.sess_connect_good++;
if (cb != NULL) cb(s,SSL_CB_HANDSHAKE_DONE,1);
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index fa4a591..c64cff6 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -200,6 +200,7 @@
int ret= -1;
int new_state,state,skip=0;
+ assert(s->handshake_func == ssl3_accept);
ERR_clear_error();
ERR_clear_system_error();
@@ -663,7 +664,6 @@
s->ctx->stats.sess_accept_good++;
/* s->server=1; */
- s->handshake_func=ssl3_accept;
if (cb != NULL) cb(s,SSL_CB_HANDSHAKE_DONE,1);
}
diff --git a/ssl/ssl_error.c b/ssl/ssl_error.c
index 1737b1f..4d09ee4 100644
--- a/ssl/ssl_error.c
+++ b/ssl/ssl_error.c
@@ -40,10 +40,12 @@
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_print_fp, 0), "SSL_SESSION_print_fp"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_set1_id_context, 0), "SSL_SESSION_set1_id_context"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_to_bytes_full, 0), "SSL_SESSION_to_bytes_full"},
+ {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_accept, 0), "SSL_accept"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_add_dir_cert_subjects_to_stack, 0), "SSL_add_dir_cert_subjects_to_stack"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_add_file_cert_subjects_to_stack, 0), "SSL_add_file_cert_subjects_to_stack"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_check_private_key, 0), "SSL_check_private_key"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_clear, 0), "SSL_clear"},
+ {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_connect, 0), "SSL_connect"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_do_handshake, 0), "SSL_do_handshake"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_load_client_CA_file, 0), "SSL_load_client_CA_file"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_new, 0), "SSL_new"},
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 6d3ee40..483608b 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -977,7 +977,13 @@
/* Not properly initialized yet */
SSL_set_accept_state(s);
- return(s->method->ssl_accept(s));
+ if (s->handshake_func != s->method->ssl_accept)
+ {
+ OPENSSL_PUT_ERROR(SSL, SSL_connect, ERR_R_INTERNAL_ERROR);
+ return -1;
+ }
+
+ return s->handshake_func(s);
}
int SSL_connect(SSL *s)
@@ -986,7 +992,13 @@
/* Not properly initialized yet */
SSL_set_connect_state(s);
- return(s->method->ssl_connect(s));
+ if (s->handshake_func != s->method->ssl_connect)
+ {
+ OPENSSL_PUT_ERROR(SSL, SSL_connect, ERR_R_INTERNAL_ERROR);
+ return -1;
+ }
+
+ return s->handshake_func(s);
}
long SSL_get_default_timeout(const SSL *s)
