From a05d427b41c21e35c57740951e9b78a8333a488e Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 6 Oct 2017 19:34:55 -0400 Subject: [PATCH] Align dtls_open_record and tls_open_record more closely. Ultimately the ssl_buffer_* code will be above SSL_PROTOCOL_METHOD, so having the processing be analogous is simpler. This also means that DTLS can surface errors out of dtls_open_record without the caller reading an extra record. Bug: 206 Change-Id: Ic1cb3a884763c8e875e1129b1cda226f72bc95b7 Reviewed-on: https://boringssl-review.googlesource.com/21364 Commit-Queue: Steven Valdez CQ-Verified: CQ bot account: commit-bot@chromium.org Reviewed-by: Steven Valdez --- ssl/d1_pkt.cc | 24 +++++++++++------------- ssl/dtls_record.cc | 3 +++ ssl/internal.h | 5 +++-- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/ssl/d1_pkt.cc b/ssl/d1_pkt.cc index 0f1c4075..c2cbd7ea 100644 --- a/ssl/d1_pkt.cc +++ b/ssl/d1_pkt.cc @@ -140,25 +140,23 @@ again: return 0; } - // Read a new packet if there is no unconsumed one. - if (ssl_read_buffer(ssl).empty()) { - int read_ret = ssl_read_buffer_extend_to(ssl, 0 /* unused */); - if (read_ret <= 0) { - return read_ret; - } - } - assert(!ssl_read_buffer(ssl).empty()); - Span body; uint8_t type, alert; size_t consumed; enum ssl_open_record_t open_ret = dtls_open_record( ssl, &type, &body, &consumed, &alert, ssl_read_buffer(ssl)); - ssl_read_buffer_consume(ssl, consumed); + if (open_ret != ssl_open_record_partial) { + ssl_read_buffer_consume(ssl, consumed); + } switch (open_ret) { - case ssl_open_record_partial: - // Impossible in DTLS. - break; + case ssl_open_record_partial: { + assert(ssl_read_buffer(ssl).empty()); + int read_ret = ssl_read_buffer_extend_to(ssl, 0 /* unused */); + if (read_ret <= 0) { + return read_ret; + } + goto again; + } case ssl_open_record_success: { if (body.size() > 0xffff) { diff --git a/ssl/dtls_record.cc b/ssl/dtls_record.cc index 71b5e3b1..c06a62db 100644 --- a/ssl/dtls_record.cc +++ b/ssl/dtls_record.cc @@ -179,6 +179,9 @@ enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type, size_t *out_consumed, uint8_t *out_alert, Span in) { *out_consumed = 0; + if (in.empty()) { + return ssl_open_record_partial; + } CBS cbs = CBS(in); diff --git a/ssl/internal.h b/ssl/internal.h index f141923b..edbf4eb5 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -801,8 +801,9 @@ enum ssl_open_record_t tls_open_record(SSL *ssl, uint8_t *out_type, Span *out, size_t *out_consumed, uint8_t *out_alert, Span in); -// dtls_open_record implements |tls_open_record| for DTLS. It never returns -// |ssl_open_record_partial| but otherwise behaves analogously. +// dtls_open_record implements |tls_open_record| for DTLS. It only returns +// |ssl_open_record_partial| if |in| was empty and sets |*out_consumed| to +// zero. The caller should read one packet and try again. enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type, Span *out, size_t *out_consumed,