Implement SSL_clear with ssl_new and ssl_free.
State on s3 gets freed in both ssl3_clear and ssl3_free. Considate to just
ssl3_free. This replaces the (SSL,ssl,ssl3)_clear calls in (SSL,ssl,ssl3)_new
with the state that was initialized. This results in a little code duplication
between SSL_new and SSL_clear because state is on the wrong object. I've just
left TODOs for now; some of it will need disentangling.
We're far from it, but going forward, separate state between s and s->s3 as:
- s contains configuration state, DTLS or TLS. It is initialized from SSL_CTX,
configurable directly afterwards, and preserved across SSL_clear calls.
(Including when it's implicitly set as part of a handshake callback.)
- Connection state hangs off s->s3 (TLS) and s->d1 (DTLS). It is reset across
SSL_clear. This should happen naturally out of a ssl_free/ssl_new pair.
The goal is to avoid needing separate initialize and reset code for anything;
the point any particular state is reset is the point its owning context is
destroyed and recreated.
Change-Id: I5d779010778109f8c339c07433a0777feaf94d1f
Reviewed-on: https://boringssl-review.googlesource.com/2822
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 76f7408..0118aa5 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -190,6 +190,12 @@
assert(s->state == 0);
}
+ /* TODO(davidben): Some state on |s| is reset both in |SSL_new| and
+ * |SSL_clear| because it is per-connection state rather than configuration
+ * state. Per-connection state should be on |s->s3| and |s->d1| so it is
+ * naturally reset at the right points between |SSL_new|, |SSL_clear|, and
+ * |ssl3_new|. */
+
s->rwstate = SSL_NOTHING;
s->rstate = SSL_ST_READ_HEADER;
@@ -198,11 +204,39 @@
s->init_buf = NULL;
}
+ s->packet = NULL;
+ s->packet_length = 0;
+
ssl_clear_cipher_ctx(s);
ssl_clear_hash_ctx(&s->read_hash);
ssl_clear_hash_ctx(&s->write_hash);
- s->method->ssl_clear(s);
+ if (s->next_proto_negotiated) {
+ OPENSSL_free(s->next_proto_negotiated);
+ s->next_proto_negotiated = NULL;
+ s->next_proto_negotiated_len = 0;
+ }
+
+ /* The s->d1->mtu is simultaneously configuration (preserved across
+ * clear) and connection-specific state (gets reset).
+ *
+ * TODO(davidben): Avoid this. */
+ unsigned mtu = 0;
+ if (s->d1 != NULL) {
+ mtu = s->d1->mtu;
+ }
+
+ s->method->ssl_free(s);
+ if (!s->method->ssl_new(s)) {
+ return 0;
+ }
+ s->enc_method = ssl3_get_enc_method(s->version);
+ assert(s->enc_method != NULL);
+
+ if (SSL_IS_DTLS(s) && (SSL_get_options(s) & SSL_OP_NO_QUERY_MTU)) {
+ s->d1->mtu = mtu;
+ }
+
s->client_version = s->version;
return 1;
@@ -315,7 +349,8 @@
s->references = 1;
- SSL_clear(s);
+ s->rwstate = SSL_NOTHING;
+ s->rstate = SSL_ST_READ_HEADER;
CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL, s, &s->ex_data);