From 024f543c15e70acb57a80067c3b32227f87bfe5f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 22 Oct 2015 13:57:18 +0100 Subject: [PATCH] Move in_handshake into STATEM The SSL variable |in_handshake| seems misplaced. It would be better to have it in the STATEM structure. Reviewed-by: Tim Hudson Reviewed-by: Richard Levitte --- ssl/d1_lib.c | 2 +- ssl/d1_msg.c | 4 ++-- ssl/record/rec_layer_d1.c | 10 +++++----- ssl/record/rec_layer_s3.c | 11 ++++++----- ssl/record/ssl3_record.c | 2 +- ssl/s3_lib.c | 4 ++-- ssl/ssl_lib.c | 5 ++--- ssl/ssl_locl.h | 3 +-- ssl/statem/statem.c | 21 +++++++++++++++++---- ssl/statem/statem.h | 5 +++++ ssl/t1_lib.c | 2 +- 11 files changed, 43 insertions(+), 26 deletions(-) diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index a6f06329a2..f8a6a37362 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -1017,7 +1017,7 @@ int dtls1_heartbeat(SSL *s) } /* ...and no handshake in progress. */ - if (SSL_in_init(s) || s->in_handshake) { + if (SSL_in_init(s) || ossl_statem_get_in_handshake(s)) { SSLerr(SSL_F_DTLS1_HEARTBEAT, SSL_R_UNEXPECTED_MESSAGE); return -1; } diff --git a/ssl/d1_msg.c b/ssl/d1_msg.c index 40152c28d2..4a2f0dc980 100644 --- a/ssl/d1_msg.c +++ b/ssl/d1_msg.c @@ -125,11 +125,11 @@ int dtls1_write_app_data_bytes(SSL *s, int type, const void *buf_, int len) * Check if we have to continue an interrupted handshake for reading * belated app data with SCTP. */ - if ((SSL_in_init(s) && !s->in_handshake) || + if ((SSL_in_init(s) && !ossl_statem_get_in_handshake(s)) || (BIO_dgram_is_sctp(SSL_get_wbio(s)) && ossl_statem_in_sctp_read_sock(s))) #else - if (SSL_in_init(s) && !s->in_handshake) + if (SSL_in_init(s) && !ossl_statem_get_in_handshake(s)) #endif { i = s->handshake_func(s); diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c index 9992037a9f..0133ae3f18 100644 --- a/ssl/record/rec_layer_d1.c +++ b/ssl/record/rec_layer_d1.c @@ -439,12 +439,12 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, * Continue handshake if it had to be interrupted to read app data with * SCTP. */ - if ((!s->in_handshake && SSL_in_init(s)) || + if ((!ossl_statem_get_in_handshake(s) && SSL_in_init(s)) || (BIO_dgram_is_sctp(SSL_get_rbio(s)) && ossl_statem_in_sctp_read_sock(s) && s->s3->in_read_app_data != 2)) #else - if (!s->in_handshake && SSL_in_init(s)) + if (!ossl_statem_get_in_handshake(s) && SSL_in_init(s)) #endif { /* type == SSL3_RT_APPLICATION_DATA */ @@ -878,7 +878,7 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, * Unexpected handshake message (Client Hello, or protocol violation) */ if ((s->rlayer.d->handshake_fragment_len >= DTLS1_HM_HEADER_LENGTH) && - !s->in_handshake) { + !ossl_statem_get_in_handshake(s)) { struct hm_header_st msg_hdr; /* this may just be a stale retransmit */ @@ -950,8 +950,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, case SSL3_RT_HANDSHAKE: /* * we already handled all of these, with the possible exception of - * SSL3_RT_HANDSHAKE when s->in_handshake is set, but that should not - * happen when type != rr->type + * SSL3_RT_HANDSHAKE when ossl_statem_get_in_handshake(s) is true, but + * that should not happen when type != rr->type */ al = SSL_AD_UNEXPECTED_MESSAGE; SSLerr(SSL_F_DTLS1_READ_BYTES, ERR_R_INTERNAL_ERROR); diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 36df000725..e59c203366 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -459,7 +459,7 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len) tot = s->rlayer.wnum; s->rlayer.wnum = 0; - if (SSL_in_init(s) && !s->in_handshake) { + if (SSL_in_init(s) && !ossl_statem_get_in_handshake(s)) { i = s->handshake_func(s); if (i < 0) return (i); @@ -1025,7 +1025,7 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, * Now s->rlayer.handshake_fragment_len == 0 if type == SSL3_RT_HANDSHAKE. */ - if (!s->in_handshake && SSL_in_init(s)) { + if (!ossl_statem_get_in_handshake(s) && SSL_in_init(s)) { /* type == SSL3_RT_APPLICATION_DATA */ i = s->handshake_func(s); if (i < 0) @@ -1383,7 +1383,8 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, /* * Unexpected handshake message (Client Hello, or protocol violation) */ - if ((s->rlayer.handshake_fragment_len >= 4) && !s->in_handshake) { + if ((s->rlayer.handshake_fragment_len >= 4) + && !ossl_statem_get_in_handshake(s)) { if (SSL_is_init_finished(s) && !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS)) { ossl_statem_set_in_init(s, 1); @@ -1436,8 +1437,8 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, case SSL3_RT_HANDSHAKE: /* * we already handled all of these, with the possible exception of - * SSL3_RT_HANDSHAKE when s->in_handshake is set, but that should not - * happen when type != rr->type + * SSL3_RT_HANDSHAKE when ossl_statem_get_in_handshake(s) is true, but + * that should not happen when type != rr->type */ al = SSL_AD_UNEXPECTED_MESSAGE; SSLerr(SSL_F_SSL3_READ_BYTES, ERR_R_INTERNAL_ERROR); diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index 7383f137b7..86aaf4fcd8 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -1528,7 +1528,7 @@ int dtls1_get_record(SSL *s) * processed at this time. */ if (is_next_epoch) { - if ((SSL_in_init(s) || s->in_handshake)) { + if ((SSL_in_init(s) || ossl_statem_get_in_handshake(s))) { if (dtls1_buffer_record (s, &(DTLS_RECORD_LAYER_get_unprocessed_rcds(&s->rlayer)), rr->seq_num) < 0) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 211247504b..39d08a0fdb 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -5085,11 +5085,11 @@ static int ssl3_read_internal(SSL *s, void *buf, int len, int peek) * makes sense here; so disable handshake processing and try to read * application data again. */ - s->in_handshake++; + ossl_statem_set_in_handshake(s, 1); ret = s->method->ssl_read_bytes(s, SSL3_RT_APPLICATION_DATA, NULL, buf, len, peek); - s->in_handshake--; + ossl_statem_set_in_handshake(s, 0); } else s->s3->in_read_app_data = 0; diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 445907d617..7e30aba3c2 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -230,7 +230,7 @@ int SSL_clear(SSL *s) * Check to see if we were changed into a different method, if so, revert * back if we are not doing session-id reuse. */ - if (!s->in_handshake && (s->session == NULL) + if (!ossl_statem_get_in_handshake(s) && (s->session == NULL) && (s->method != s->ctx->method)) { s->method->ssl_free(s); s->method = s->ctx->method; @@ -1080,7 +1080,7 @@ long SSL_ctrl(SSL *s, int cmd, long larg, void *parg) return TLS_CIPHER_LEN; } case SSL_CTRL_GET_EXTMS_SUPPORT: - if (!s->session || SSL_in_init(s) || s->in_handshake) + if (!s->session || SSL_in_init(s) || ossl_statem_get_in_handshake(s)) return -1; if (s->session->flags & SSL_SESS_FLAG_EXTMS) return 1; @@ -2526,7 +2526,6 @@ SSL *SSL_dup(SSL *s) ret->wbio = ret->rbio; } ret->rwstate = s->rwstate; - ret->in_handshake = s->in_handshake; ret->handshake_func = s->handshake_func; ret->server = s->server; ret->renegotiate = s->renegotiate; diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 8d9f51a790..c04eaa58c1 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -986,8 +986,7 @@ struct ssl_st { * request needs re-doing when in SSL_accept or SSL_connect */ int rwstate; - /* true when we are actually in SSL_accept() or SSL_connect() */ - int in_handshake; + int (*handshake_func) (SSL *); /* * Imagine that here's a boolean member "init" that is switched as soon diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c index f22801987a..76e902c993 100644 --- a/ssl/statem/statem.c +++ b/ssl/statem/statem.c @@ -187,6 +187,19 @@ void ossl_statem_set_in_init(SSL *s, int init) s->statem.in_init = init; } +int ossl_statem_get_in_handshake(SSL *s) +{ + return s->statem.in_handshake; +} + +void ossl_statem_set_in_handshake(SSL *s, int inhand) +{ + if (inhand) + s->statem.in_handshake++; + else + s->statem.in_handshake--; +} + void ossl_statem_set_hello_verify_done(SSL *s) { s->statem.state = MSG_FLOW_UNINITED; @@ -267,7 +280,7 @@ static int state_machine(SSL *s, int server) { cb = get_callback(s); - s->in_handshake++; + st->in_handshake++; if (!SSL_in_init(s) || SSL_in_before(s)) { if (!SSL_clear(s)) return -1; @@ -280,7 +293,7 @@ static int state_machine(SSL *s, int server) { * identifier other than 0. Will be ignored if no SCTP is used. */ BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_SCTP_SET_IN_HANDSHAKE, - s->in_handshake, NULL); + st->in_handshake, NULL); } #endif @@ -447,7 +460,7 @@ static int state_machine(SSL *s, int server) { ret = 1; end: - s->in_handshake--; + st->in_handshake--; #ifndef OPENSSL_NO_SCTP if (SSL_IS_DTLS(s)) { @@ -456,7 +469,7 @@ static int state_machine(SSL *s, int server) { * identifier other than 0. Will be ignored if no SCTP is used. */ BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_SCTP_SET_IN_HANDSHAKE, - s->in_handshake, NULL); + st->in_handshake, NULL); } #endif diff --git a/ssl/statem/statem.h b/ssl/statem/statem.h index 2dc603ac05..2c5184dde5 100644 --- a/ssl/statem/statem.h +++ b/ssl/statem/statem.h @@ -136,6 +136,9 @@ struct ossl_statem_st { int in_init; int read_state_first_init; + /* true when we are actually in SSL_accept() or SSL_connect() */ + int in_handshake; + /* Should we skip the CertificateVerify message? */ unsigned int no_cert_verify; @@ -161,6 +164,8 @@ void ossl_statem_set_renegotiate(SSL *s); void ossl_statem_set_error(SSL *s); int ossl_statem_in_error(const SSL *s); void ossl_statem_set_in_init(SSL *s, int init); +int ossl_statem_get_in_handshake(SSL *s); +void ossl_statem_set_in_handshake(SSL *s, int inhand); void ossl_statem_set_hello_verify_done(SSL *s); __owur int ossl_statem_app_data_allowed(SSL *s); #ifndef OPENSSL_NO_SCTP diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 6446623920..f42fb64700 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -3680,7 +3680,7 @@ int tls1_heartbeat(SSL *s) } /* ...and no handshake in progress. */ - if (SSL_in_init(s) || s->in_handshake) { + if (SSL_in_init(s) || ossl_statem_get_in_handshake(s)) { SSLerr(SSL_F_TLS1_HEARTBEAT, SSL_R_UNEXPECTED_MESSAGE); return -1; } -- 2.34.1