commit 4bd32a848308a6a1acd02f62db1710e5ad31e1c8
author David Benjamin <davidben@google.com> Fri May 14 17:06:29 2021 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jul 28 15:28:21 2022 +0000
tree db45c0e349706a2de168bb9135950eb7004de0a6
parent 8ce0e1c14e48109773f1e94e5f8b020aa1e24dc5

Convert more of the SSL write path to size_t and Spans.

We still have our <= 0 return values because anything with BIOs tries to
preserve BIO_write's error returns. (Maybe we can stop doing this?
BIO_read's error return is a little subtle with EOF vs error, but
BIO_write's is uninteresting.) But the rest of the logic is size_t-clean
and hopefully a little clearer. We still have to support SSL_write's
rather goofy calling convention, however.

I haven't pushed Spans down into the low-level record construction logic
yet. We should probably do that, but there are enough offsets tossed
around there that they warrant their own CL.

Bug: 507
Change-Id: Ia0c702d1a2d3713e71b0bbfa8d65649d3b20da9b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47544
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>

ssl/d1_pkt.cc

diff --git a/ssl/d1_pkt.cc b/ssl/d1_pkt.cc
index b9b0ef9..b866156 100644
--- a/ssl/d1_pkt.cc
+++ b/ssl/d1_pkt.cc
@@ -186,8 +186,8 @@
   return ssl_open_record_success;
 }
 
-int dtls1_write_app_data(SSL *ssl, bool *out_needs_handshake, const uint8_t *in,
-                         int len) {
+int dtls1_write_app_data(SSL *ssl, bool *out_needs_handshake,
+                         size_t *out_bytes_written, Span<const uint8_t> in) {
   assert(!SSL_in_init(ssl));
   *out_needs_handshake = false;
 
@@ -196,47 +196,46 @@
     return -1;
   }
 
-  if (len > SSL3_RT_MAX_PLAIN_LENGTH) {
+  // DTLS does not split the input across records.
+  if (in.size() > SSL3_RT_MAX_PLAIN_LENGTH) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_DTLS_MESSAGE_TOO_BIG);
     return -1;
   }
 
-  if (len < 0) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_BAD_LENGTH);
-    return -1;
+  if (in.empty()) {
+    *out_bytes_written = 0;
+    return 1;
   }
 
-  if (len == 0) {
-    return 0;
-  }
-
-  int ret = dtls1_write_record(ssl, SSL3_RT_APPLICATION_DATA, in, (size_t)len,
+  int ret = dtls1_write_record(ssl, SSL3_RT_APPLICATION_DATA, in,
                                dtls1_use_current_epoch);
   if (ret <= 0) {
     return ret;
   }
-  return len;
+  *out_bytes_written = in.size();
+  return 1;
 }
 
-int dtls1_write_record(SSL *ssl, int type, const uint8_t *in, size_t len,
+int dtls1_write_record(SSL *ssl, int type, Span<const uint8_t> in,
                        enum dtls1_use_epoch_t use_epoch) {
   SSLBuffer *buf = &ssl->s3->write_buffer;
-  assert(len <= SSL3_RT_MAX_PLAIN_LENGTH);
+  assert(in.size() <= SSL3_RT_MAX_PLAIN_LENGTH);
   // There should never be a pending write buffer in DTLS. One can't write half
   // a datagram, so the write buffer is always dropped in
   // |ssl_write_buffer_flush|.
   assert(buf->empty());
 
-  if (len > SSL3_RT_MAX_PLAIN_LENGTH) {
+  if (in.size() > SSL3_RT_MAX_PLAIN_LENGTH) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
     return -1;
   }
 
   size_t ciphertext_len;
   if (!buf->EnsureCap(ssl_seal_align_prefix_len(ssl),
-                      len + SSL_max_seal_overhead(ssl)) ||
+                      in.size() + SSL_max_seal_overhead(ssl)) ||
       !dtls_seal_record(ssl, buf->remaining().data(), &ciphertext_len,
-                        buf->remaining().size(), type, in, len, use_epoch)) {
+                        buf->remaining().size(), type, in.data(), in.size(),
+                        use_epoch)) {
     buf->Clear();
     return -1;
   }
@@ -250,7 +249,7 @@
 }
 
 int dtls1_dispatch_alert(SSL *ssl) {
-  int ret = dtls1_write_record(ssl, SSL3_RT_ALERT, &ssl->s3->send_alert[0], 2,
+  int ret = dtls1_write_record(ssl, SSL3_RT_ALERT, ssl->s3->send_alert,
                                dtls1_use_current_epoch);
   if (ret <= 0) {
     return ret;