Give DTLS1_STATE a destructor.
Change-Id: I3fb797bad91caf7d2aff09313734edfb58fb9f26
Reviewed-on: https://boringssl-review.googlesource.com/22066
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/d1_both.cc b/ssl/d1_both.cc
index 357d171..c219f5a 100644
--- a/ssl/d1_both.cc
+++ b/ssl/d1_both.cc
@@ -253,8 +253,8 @@
// dtls1_is_current_message_complete returns whether the current handshake
// message is complete.
static bool dtls1_is_current_message_complete(const SSL *ssl) {
- hm_fragment *frag = ssl->d1->incoming_messages[ssl->d1->handshake_read_seq %
- SSL_MAX_HANDSHAKE_FLIGHT];
+ size_t idx = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
+ hm_fragment *frag = ssl->d1->incoming_messages[idx].get();
return frag != NULL && frag->reassembly == NULL;
}
@@ -271,7 +271,7 @@
}
size_t idx = msg_hdr->seq % SSL_MAX_HANDSHAKE_FLIGHT;
- hm_fragment *frag = ssl->d1->incoming_messages[idx];
+ hm_fragment *frag = ssl->d1->incoming_messages[idx].get();
if (frag != NULL) {
assert(frag->seq == msg_hdr->seq);
// The new fragment must be compatible with the previous fragments from this
@@ -286,13 +286,12 @@
}
// This is the first fragment from this message.
- frag = dtls1_hm_fragment_new(msg_hdr).release();
- if (frag == NULL) {
+ ssl->d1->incoming_messages[idx] = dtls1_hm_fragment_new(msg_hdr);
+ if (!ssl->d1->incoming_messages[idx]) {
*out_alert = SSL_AD_INTERNAL_ERROR;
return NULL;
}
- ssl->d1->incoming_messages[idx] = frag;
- return frag;
+ return ssl->d1->incoming_messages[idx].get();
}
ssl_open_record_t dtls1_open_handshake(SSL *ssl, size_t *out_consumed,
@@ -411,8 +410,8 @@
return false;
}
- hm_fragment *frag = ssl->d1->incoming_messages[ssl->d1->handshake_read_seq %
- SSL_MAX_HANDSHAKE_FLIGHT];
+ size_t idx = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
+ hm_fragment *frag = ssl->d1->incoming_messages[idx].get();
out->type = frag->type;
CBS_init(&out->body, frag->data + DTLS1_HM_HEADER_LENGTH, frag->msg_len);
CBS_init(&out->raw, frag->data, DTLS1_HM_HEADER_LENGTH + frag->msg_len);
@@ -428,8 +427,7 @@
assert(ssl->s3->has_message);
assert(dtls1_is_current_message_complete(ssl));
size_t index = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
- Delete(ssl->d1->incoming_messages[index]);
- ssl->d1->incoming_messages[index] = NULL;
+ ssl->d1->incoming_messages[index].reset();
ssl->d1->handshake_read_seq++;
ssl->s3->has_message = false;
// If we previously sent a flight, mark it as having a reply, so
@@ -439,13 +437,6 @@
}
}
-void dtls_clear_incoming_messages(SSL *ssl) {
- for (size_t i = 0; i < SSL_MAX_HANDSHAKE_FLIGHT; i++) {
- Delete(ssl->d1->incoming_messages[i]);
- ssl->d1->incoming_messages[i] = NULL;
- }
-}
-
bool dtls_has_unprocessed_handshake_data(const SSL *ssl) {
if (ssl->d1->has_change_cipher_spec) {
return true;
@@ -458,7 +449,7 @@
assert(dtls1_is_current_message_complete(ssl));
continue;
}
- if (ssl->d1->incoming_messages[i] != NULL) {
+ if (ssl->d1->incoming_messages[i] != nullptr) {
return true;
}
}
diff --git a/ssl/d1_lib.cc b/ssl/d1_lib.cc
index 38bc5be..eff06ee 100644
--- a/ssl/d1_lib.cc
+++ b/ssl/d1_lib.cc
@@ -78,18 +78,24 @@
// before failing the DTLS handshake.
#define DTLS1_MAX_TIMEOUTS 12
+DTLS1_STATE::DTLS1_STATE()
+ : has_change_cipher_spec(false),
+ outgoing_messages_complete(false),
+ flight_has_reply(false) {}
+
+DTLS1_STATE::~DTLS1_STATE() {}
+
bool dtls1_new(SSL *ssl) {
if (!ssl3_new(ssl)) {
return false;
}
- DTLS1_STATE *d1 = (DTLS1_STATE *)OPENSSL_malloc(sizeof *d1);
- if (d1 == NULL) {
+ UniquePtr<DTLS1_STATE> d1 = MakeUnique<DTLS1_STATE>();
+ if (!d1) {
ssl3_free(ssl);
return false;
}
- OPENSSL_memset(d1, 0, sizeof *d1);
- ssl->d1 = d1;
+ ssl->d1 = d1.release();
// Set the version to the highest supported version.
//
@@ -103,15 +109,11 @@
void dtls1_free(SSL *ssl) {
ssl3_free(ssl);
- if (ssl == NULL || ssl->d1 == NULL) {
+ if (ssl == NULL) {
return;
}
- dtls_clear_incoming_messages(ssl);
- dtls_clear_outgoing_messages(ssl);
- Delete(ssl->d1->last_aead_write_ctx);
-
- OPENSSL_free(ssl->d1);
+ Delete(ssl->d1);
ssl->d1 = NULL;
}
diff --git a/ssl/dtls_method.cc b/ssl/dtls_method.cc
index c64c75e..91a955f 100644
--- a/ssl/dtls_method.cc
+++ b/ssl/dtls_method.cc
@@ -105,8 +105,7 @@
sizeof(ssl->s3->write_sequence));
OPENSSL_memset(ssl->s3->write_sequence, 0, sizeof(ssl->s3->write_sequence));
- Delete(ssl->d1->last_aead_write_ctx);
- ssl->d1->last_aead_write_ctx = ssl->s3->aead_write_ctx.release();
+ ssl->d1->last_aead_write_ctx = std::move(ssl->s3->aead_write_ctx);
ssl->s3->aead_write_ctx = std::move(aead_ctx);
return true;
}
diff --git a/ssl/dtls_record.cc b/ssl/dtls_record.cc
index 3a5e50f..5e795fa 100644
--- a/ssl/dtls_record.cc
+++ b/ssl/dtls_record.cc
@@ -275,7 +275,7 @@
enum dtls1_use_epoch_t use_epoch) {
if (use_epoch == dtls1_use_previous_epoch) {
assert(ssl->d1->w_epoch >= 1);
- return ssl->d1->last_aead_write_ctx;
+ return ssl->d1->last_aead_write_ctx.get();
}
return ssl->s3->aead_write_ctx.get();
@@ -308,7 +308,7 @@
if (use_epoch == dtls1_use_previous_epoch) {
assert(ssl->d1->w_epoch >= 1);
epoch = ssl->d1->w_epoch - 1;
- aead = ssl->d1->last_aead_write_ctx;
+ aead = ssl->d1->last_aead_write_ctx.get();
seq = ssl->d1->last_write_sequence;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index 46d231f..23bd8a1 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -746,10 +746,10 @@
struct DTLS1_BITMAP {
// map is a bit mask of the last 64 sequence numbers. Bit
// |1<<i| corresponds to |max_seq_num - i|.
- uint64_t map;
+ uint64_t map = 0;
// max_seq_num is the largest sequence number seen so far as a 64-bit
// integer.
- uint64_t max_seq_num;
+ uint64_t max_seq_num = 0;
};
@@ -998,9 +998,6 @@
// in a handshake message for |ssl|.
size_t ssl_max_handshake_message_len(const SSL *ssl);
-// dtls_clear_incoming_messages releases all buffered incoming messages.
-void dtls_clear_incoming_messages(SSL *ssl);
-
// tls_can_accept_handshake_data returns whether |ssl| is able to accept more
// data into handshake buffer.
bool tls_can_accept_handshake_data(const SSL *ssl, uint8_t *out_alert);
@@ -2391,6 +2388,11 @@
};
struct DTLS1_STATE {
+ static constexpr bool kAllowUniquePtr = true;
+
+ DTLS1_STATE();
+ ~DTLS1_STATE();
+
// has_change_cipher_spec is true if we have received a ChangeCipherSpec from
// the peer in this epoch.
bool has_change_cipher_spec:1;
@@ -2405,54 +2407,54 @@
// peer sent the final flight.
bool flight_has_reply:1;
- uint8_t cookie[DTLS1_COOKIE_LENGTH];
- size_t cookie_len;
+ uint8_t cookie[DTLS1_COOKIE_LENGTH] = {0};
+ size_t cookie_len = 0;
// The current data and handshake epoch. This is initially undefined, and
// starts at zero once the initial handshake is completed.
- uint16_t r_epoch;
- uint16_t w_epoch;
+ uint16_t r_epoch = 0;
+ uint16_t w_epoch = 0;
// records being received in the current epoch
DTLS1_BITMAP bitmap;
- uint16_t handshake_write_seq;
- uint16_t handshake_read_seq;
+ uint16_t handshake_write_seq = 0;
+ uint16_t handshake_read_seq = 0;
// save last sequence number for retransmissions
- uint8_t last_write_sequence[8];
- SSLAEADContext *last_aead_write_ctx;
+ uint8_t last_write_sequence[8] = {0};
+ UniquePtr<SSLAEADContext> last_aead_write_ctx;
// incoming_messages is a ring buffer of incoming handshake messages that have
// yet to be processed. The front of the ring buffer is message number
// |handshake_read_seq|, at position |handshake_read_seq| %
// |SSL_MAX_HANDSHAKE_FLIGHT|.
- hm_fragment *incoming_messages[SSL_MAX_HANDSHAKE_FLIGHT];
+ UniquePtr<hm_fragment> incoming_messages[SSL_MAX_HANDSHAKE_FLIGHT];
// outgoing_messages is the queue of outgoing messages from the last handshake
// flight.
DTLS_OUTGOING_MESSAGE outgoing_messages[SSL_MAX_HANDSHAKE_FLIGHT];
- uint8_t outgoing_messages_len;
+ uint8_t outgoing_messages_len = 0;
// outgoing_written is the number of outgoing messages that have been
// written.
- uint8_t outgoing_written;
+ uint8_t outgoing_written = 0;
// outgoing_offset is the number of bytes of the next outgoing message have
// been written.
- uint32_t outgoing_offset;
+ uint32_t outgoing_offset = 0;
- unsigned int mtu; // max DTLS packet size
+ unsigned mtu = 0; // max DTLS packet size
// num_timeouts is the number of times the retransmit timer has fired since
// the last time it was reset.
- unsigned int num_timeouts;
+ unsigned num_timeouts = 0;
// Indicates when the last handshake msg or heartbeat sent will
// timeout.
- struct OPENSSL_timeval next_timeout;
+ struct OPENSSL_timeval next_timeout = {0, 0};
// timeout_duration_ms is the timeout duration in milliseconds.
- unsigned timeout_duration_ms;
+ unsigned timeout_duration_ms = 0;
};
// SSLConnection backs the public |SSL| type. Due to compatibility constraints,
