Support EVP_AEAD in DTLS.
This CL removes the last of the EVP_CIPHER codepath in ssl/. The dead code is
intentionally not pruned for ease of review, except in DTLS-only code where
adding new logic to support both, only to remove half, would be cumbersome.
Fixes made:
- dtls1_retransmit_state is taught to retain aead_write_ctx rather than
enc_write_ctx.
- d1_pkt.c reserves space for the variable-length nonce when echoed into the
packet.
- dtls1_do_write sizes the MTU based on EVP_AEAD max overhead.
- tls1_change_cipher_state_cipher should not free AEAD write contexts in DTLS.
This matches the (rather confused) ownership for the EVP_CIPHER contexts.
I've added a TODO to resolve this craziness.
A follow-up CL will remove all the resultant dead code.
Change-Id: I644557f4db53bbfb182950823ab96d5e4c908866
Reviewed-on: https://boringssl-review.googlesource.com/2699
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/dtls1.h b/include/openssl/dtls1.h
index 57337f6..0fc3ae6 100644
--- a/include/openssl/dtls1.h
+++ b/include/openssl/dtls1.h
@@ -111,8 +111,7 @@
struct dtls1_retransmit_state
{
- EVP_CIPHER_CTX *enc_write_ctx; /* cryptographic state */
- EVP_MD_CTX *write_hash; /* used for mac generation */
+ SSL_AEAD_CTX *aead_write_ctx;
SSL_SESSION *session;
unsigned short epoch;
};
diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 9081e4b..8d1dbf9 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -214,8 +214,14 @@
void dtls1_hm_fragment_free(hm_fragment *frag) {
if (frag->msg_header.is_ccs) {
- EVP_CIPHER_CTX_free(frag->msg_header.saved_retransmit_state.enc_write_ctx);
- EVP_MD_CTX_destroy(frag->msg_header.saved_retransmit_state.write_hash);
+ /* TODO(davidben): Simplify aead_write_ctx ownership, probably by just
+ * forbidding DTLS renego. */
+ SSL_AEAD_CTX *aead_write_ctx =
+ frag->msg_header.saved_retransmit_state.aead_write_ctx;
+ if (aead_write_ctx) {
+ EVP_AEAD_CTX_cleanup(&aead_write_ctx->ctx);
+ OPENSSL_free(aead_write_ctx);
+ }
}
if (frag->fragment) {
OPENSSL_free(frag->fragment);
@@ -231,7 +237,8 @@
int dtls1_do_write(SSL *s, int type) {
int ret;
int curr_mtu;
- unsigned int len, frag_off, mac_size = 0, blocksize = 0;
+ unsigned int len, frag_off;
+ size_t max_overhead = 0;
/* AHA! Figure out the MTU, and stick to the right size */
if (s->d1->mtu < dtls1_min_mtu() &&
@@ -255,21 +262,18 @@
(int)s->d1->w_msg_hdr.msg_len + DTLS1_HM_HEADER_LENGTH);
}
- if (s->write_hash &&
- (s->enc_write_ctx == NULL ||
- EVP_CIPHER_CTX_mode(s->enc_write_ctx) != EVP_CIPH_GCM_MODE)) {
- mac_size = EVP_MD_CTX_size(s->write_hash);
- }
-
- if (s->enc_write_ctx &&
- (EVP_CIPHER_CTX_mode(s->enc_write_ctx) == EVP_CIPH_CBC_MODE)) {
- blocksize = 2 * EVP_CIPHER_block_size(s->enc_write_ctx->cipher);
+ /* Determine the maximum overhead of the current cipher. */
+ if (s->aead_write_ctx != NULL) {
+ max_overhead = EVP_AEAD_max_overhead(s->aead_write_ctx->ctx.aead);
+ if (s->aead_write_ctx->variable_nonce_included_in_record) {
+ max_overhead += s->aead_write_ctx->variable_nonce_len;
+ }
}
frag_off = 0;
while (s->init_num) {
curr_mtu = s->d1->mtu - BIO_wpending(SSL_get_wbio(s)) -
- DTLS1_RT_HEADER_LENGTH - mac_size - blocksize;
+ DTLS1_RT_HEADER_LENGTH - max_overhead;
if (curr_mtu <= DTLS1_HM_HEADER_LENGTH) {
/* grr.. we could get an error if MTU picked was wrong */
@@ -277,7 +281,7 @@
if (ret <= 0) {
return ret;
}
- curr_mtu = s->d1->mtu - DTLS1_RT_HEADER_LENGTH - mac_size - blocksize;
+ curr_mtu = s->d1->mtu - DTLS1_RT_HEADER_LENGTH - max_overhead;
}
if (s->init_num > curr_mtu) {
@@ -978,8 +982,7 @@
frag->msg_header.is_ccs = is_ccs;
/* save current state*/
- frag->msg_header.saved_retransmit_state.enc_write_ctx = s->enc_write_ctx;
- frag->msg_header.saved_retransmit_state.write_hash = s->write_hash;
+ frag->msg_header.saved_retransmit_state.aead_write_ctx = s->aead_write_ctx;
frag->msg_header.saved_retransmit_state.session = s->session;
frag->msg_header.saved_retransmit_state.epoch = s->d1->w_epoch;
@@ -1044,14 +1047,12 @@
0, frag->msg_header.frag_len);
/* save current state */
- saved_state.enc_write_ctx = s->enc_write_ctx;
- saved_state.write_hash = s->write_hash;
+ saved_state.aead_write_ctx = s->aead_write_ctx;
saved_state.session = s->session;
saved_state.epoch = s->d1->w_epoch;
/* restore state in which the message was originally sent */
- s->enc_write_ctx = frag->msg_header.saved_retransmit_state.enc_write_ctx;
- s->write_hash = frag->msg_header.saved_retransmit_state.write_hash;
+ s->aead_write_ctx = frag->msg_header.saved_retransmit_state.aead_write_ctx;
s->session = frag->msg_header.saved_retransmit_state.session;
s->d1->w_epoch = frag->msg_header.saved_retransmit_state.epoch;
@@ -1066,8 +1067,7 @@
: SSL3_RT_HANDSHAKE);
/* restore current state */
- s->enc_write_ctx = saved_state.enc_write_ctx;
- s->write_hash = saved_state.write_hash;
+ s->aead_write_ctx = saved_state.aead_write_ctx;
s->session = saved_state.session;
s->d1->w_epoch = saved_state.epoch;
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index 7e76905..e4928a5 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -256,22 +256,11 @@
return ret;
}
-/* As it's impossible to use stream ciphers in "datagram" mode, this
- * simple filter is designed to disengage them in DTLS. Unfortunately
- * there is no universal way to identify stream SSL_CIPHER, so we have
- * to explicitly list their SSL_* codes. Currently RC4 is the only one
- * available, but if new ones emerge, they will have to be added... */
const SSL_CIPHER *dtls1_get_cipher(unsigned int u) {
const SSL_CIPHER *ciph = ssl3_get_cipher(u);
-
- if (ciph != NULL) {
- if (ciph->algorithm_enc == SSL_RC4) {
- return NULL;
- }
- /* TODO(davidben): EVP_AEAD does not work in DTLS yet. */
- if (ciph->algorithm2 & SSL_CIPHER_ALGORITHM2_AEAD) {
- return NULL;
- }
+ /* DTLS does not support stream ciphers. */
+ if (ciph == NULL || ciph->algorithm_enc == SSL_RC4) {
+ return NULL;
}
return ciph;
diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 67b06a5..85bd305 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -1213,17 +1213,15 @@
p += 10;
/* Explicit IV length, block ciphers appropriate version flag */
- if (s->enc_write_ctx) {
- int mode = EVP_CIPHER_CTX_mode(s->enc_write_ctx);
- if (mode == EVP_CIPH_CBC_MODE) {
- eivlen = EVP_CIPHER_CTX_iv_length(s->enc_write_ctx);
- if (eivlen <= 1) {
- eivlen = 0;
- }
- } else if (mode == EVP_CIPH_GCM_MODE) {
- /* Need explicit part of IV for GCM mode */
- eivlen = EVP_GCM_TLS_EXPLICIT_IV_LEN;
+ if (s->enc_write_ctx && SSL_USE_EXPLICIT_IV(s) &&
+ EVP_CIPHER_CTX_mode(s->enc_write_ctx) == EVP_CIPH_CBC_MODE) {
+ eivlen = EVP_CIPHER_CTX_iv_length(s->enc_write_ctx);
+ if (eivlen <= 1) {
+ eivlen = 0;
}
+ } else if (s->aead_write_ctx != NULL &&
+ s->aead_write_ctx->variable_nonce_included_in_record) {
+ eivlen = s->aead_write_ctx->variable_nonce_len;
}
/* lets setup the record stuff. */
diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index 6f2e844..e4c57c3 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -294,9 +294,7 @@
case SSL3_ST_SW_CERT_A:
case SSL3_ST_SW_CERT_B:
- /* Check if it is anon DH or normal PSK */
- if (!(s->s3->tmp.new_cipher->algorithm_auth & SSL_aNULL) &&
- !(s->s3->tmp.new_cipher->algorithm_mkey & SSL_kPSK)) {
+ if (ssl_cipher_has_server_public_key(s->s3->tmp.new_cipher)) {
dtls1_start_timer(s);
ret = ssl3_send_server_certificate(s);
if (ret <= 0) {
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 48079ff..e4e9907 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -362,6 +362,15 @@
}
aead_ctx = s->aead_read_ctx;
} else {
+ /* When updating the cipher state for DTLS, we do not wish to overwrite the
+ * old ones because DTLS stores pointers to them in order to implement
+ * retransmission. See dtls1_hm_fragment_free.
+ *
+ * TODO(davidben): Simplify aead_write_ctx ownership, probably by just
+ * forbidding DTLS renego. */
+ if (SSL_IS_DTLS(s)) {
+ s->aead_write_ctx = NULL;
+ }
if (!tls1_aead_ctx_init(&s->aead_write_ctx)) {
return 0;
}
@@ -440,7 +449,17 @@
if (is_read) {
tls1_cleanup_aead_ctx(&s->aead_read_ctx);
} else {
- tls1_cleanup_aead_ctx(&s->aead_write_ctx);
+ /* When updating the cipher state for DTLS, we do not wish to free the old
+ * ones because DTLS stores pointers to them in order to implement
+ * retransmission. See dtls1_hm_fragment_free.
+ *
+ * TODO(davidben): Simplify aead_write_ctx ownership, probably by just
+ * forbidding DTLS renego. */
+ if (!SSL_IS_DTLS(s)) {
+ tls1_cleanup_aead_ctx(&s->aead_write_ctx);
+ } else {
+ s->aead_write_ctx = NULL;
+ }
}
if (is_read) {
@@ -622,8 +641,8 @@
goto cipher_unavailable_err;
}
- /* TODO(davidben): Make DTLS record-layer code EVP_AEAD-aware. */
- if (!SSL_IS_DTLS(s)) {
+ /* TODO(davidben): Prune away dead code. To be done in follow-up commit. */
+ if (1) {
if (!ssl_cipher_get_evp_aead(&aead, &mac_secret_len, &fixed_iv_len,
s->session->cipher,
ssl3_version_from_wire(s, s->version))) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index d76bdf7..2342682 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -998,10 +998,7 @@
}
func isDTLSCipher(suiteName string) bool {
- // TODO(davidben): AES-GCM exists in DTLS 1.2 but is currently
- // broken because DTLS is not EVP_AEAD-aware.
- return !hasComponent(suiteName, "RC4") &&
- !hasComponent(suiteName, "GCM")
+ return !hasComponent(suiteName, "RC4")
}
func addCipherSuiteTests() {
