Use tls_buffer for alert and handshake fragments in the legacy stack.

This avoids a bunch of pointer munging and a handrolled memmove.

ok tb@

5 files changed, 94 insertions, 49 deletions

diff --git a/src/lib/libssl/s3_lib.c b/src/lib/libssl/s3_lib.c
index 52ad16a697..68c6fc6324 100644
--- a/src/lib/libssl/s3_lib.c
+++ b/src/lib/libssl/s3_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.239 2022/10/02 16:36:41 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.240 2022/11/10 18:06:37 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1560,6 +1560,9 @@ ssl3_free(SSL *s)
         ssl3_release_read_buffer(s);
         ssl3_release_write_buffer(s);
 
+        tls_buffer_free(s->s3->alert_fragment);
+        tls_buffer_free(s->s3->handshake_fragment);
+
         freezero(s->s3->hs.sigalgs, s->s3->hs.sigalgs_len);
         sk_X509_pop_free(s->s3->hs.peer_certs, X509_free);
         sk_X509_pop_free(s->s3->hs.peer_certs_no_leaf, X509_free);
@@ -1598,6 +1601,11 @@ ssl3_clear(SSL *s)
         sk_X509_pop_free(s->verified_chain, X509_free);
         s->verified_chain = NULL;
 
+        tls_buffer_free(s->s3->alert_fragment);
+        s->s3->alert_fragment = NULL;
+        tls_buffer_free(s->s3->handshake_fragment);
+        s->s3->handshake_fragment = NULL;
+
         freezero(s->s3->hs.sigalgs, s->s3->hs.sigalgs_len);
         s->s3->hs.sigalgs = NULL;
         s->s3->hs.sigalgs_len = 0;
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index 64cf6a60f9..69546c0962 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.430 2022/11/07 11:58:45 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.431 2022/11/10 18:06:37 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1168,12 +1168,9 @@ typedef struct ssl3_state_st {
 
         SSL3_RECORD_INTERNAL rrec;      /* each decoded record goes in here */
 
-        /* storage for Alert/Handshake protocol data received but not
-         * yet processed by ssl3_read_bytes: */
-        unsigned char alert_fragment[2];
-        unsigned int alert_fragment_len;
-        unsigned char handshake_fragment[4];
-        unsigned int handshake_fragment_len;
+        /* Unprocessed Alert/Handshake protocol data. */
+        struct tls_buffer *alert_fragment;
+        struct tls_buffer *handshake_fragment;
 
         /* partial write - check the numbers match */
         unsigned int wnum;      /* number of bytes sent so far */
diff --git a/src/lib/libssl/ssl_pkt.c b/src/lib/libssl/ssl_pkt.c
index ddb2ce0935..58d3ee1db2 100644
--- a/src/lib/libssl/ssl_pkt.c
+++ b/src/lib/libssl/ssl_pkt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_pkt.c,v 1.62 2022/10/21 15:48:14 tb Exp $ */
+/* $OpenBSD: ssl_pkt.c,v 1.63 2022/11/10 18:06:37 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -688,11 +688,32 @@ ssl3_write_pending(SSL *s, int type, const unsigned char *buf, unsigned int len)
         }
 }
 
+static ssize_t
+ssl3_read_cb(void *buf, size_t n, void *cb_arg)
+{
+        SSL3_RECORD_INTERNAL *rr;
+        SSL *s = cb_arg;
+
+        rr = &s->s3->rrec;
+
+        if (n > rr->length)
+                n = rr->length;
+
+        memcpy(buf, &rr->data[rr->off], n);
+
+        rr->off += n;
+        rr->length -= n;
+
+        return n;
+}
+
+#define SSL3_ALERT_LENGTH       2
+
 int
 ssl3_read_alert(SSL *s)
 {
-        SSL3_RECORD_INTERNAL *rr = &s->s3->rrec;
         uint8_t alert_level, alert_descr;
+        ssize_t ret;
         CBS cbs;
 
         /*
@@ -702,13 +723,17 @@ ssl3_read_alert(SSL *s)
          * fragmented across multiple records, hence a full alert must be
          * available in the record.
          */
-        while (rr->length > 0 &&
-            s->s3->alert_fragment_len < sizeof(s->s3->alert_fragment)) {
-                s->s3->alert_fragment[s->s3->alert_fragment_len++] =
-                    rr->data[rr->off++];
-                rr->length--;
+        if (s->s3->alert_fragment == NULL) {
+                if ((s->s3->alert_fragment = tls_buffer_new(0)) == NULL)
+                        return -1;
+                tls_buffer_set_capacity_limit(s->s3->alert_fragment,
+                    SSL3_ALERT_LENGTH);
         }
-        if (s->s3->alert_fragment_len < sizeof(s->s3->alert_fragment)) {
+        ret = tls_buffer_extend(s->s3->alert_fragment, SSL3_ALERT_LENGTH,
+            ssl3_read_cb, s);
+        if (ret <= 0 && ret != TLS_IO_WANT_POLLIN)
+                return -1;
+        if (ret != SSL3_ALERT_LENGTH) {
                 if (SSL_is_dtls(s)) {
                         SSLerror(s, SSL_R_BAD_LENGTH);
                         ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
@@ -717,7 +742,8 @@ ssl3_read_alert(SSL *s)
                 return 1;
         }
 
-        CBS_init(&cbs, s->s3->alert_fragment, sizeof(s->s3->alert_fragment));
+        if (!tls_buffer_data(s->s3->alert_fragment, &cbs))
+                return -1;
 
         ssl_msg_callback_cbs(s, 0, SSL3_RT_ALERT, &cbs);
 
@@ -726,7 +752,8 @@ ssl3_read_alert(SSL *s)
         if (!CBS_get_u8(&cbs, &alert_descr))
                 return -1;
 
-        s->s3->alert_fragment_len = 0;
+        tls_buffer_free(s->s3->alert_fragment);
+        s->s3->alert_fragment = NULL;
 
         ssl_info_callback(s, SSL_CB_READ_ALERT,
             (alert_level << 8) | alert_descr);
@@ -829,9 +856,9 @@ ssl3_read_change_cipher_spec(SSL *s)
 static int
 ssl3_read_handshake_unexpected(SSL *s)
 {
-        SSL3_RECORD_INTERNAL *rr = &s->s3->rrec;
         uint32_t hs_msg_length;
         uint8_t hs_msg_type;
+        ssize_t ssret;
         CBS cbs;
         int ret;
 
@@ -840,14 +867,17 @@ ssl3_read_handshake_unexpected(SSL *s)
          * header - this may be in the same record or fragmented across multiple
          * records.
          */
-        while (rr->length > 0 &&
-            s->s3->handshake_fragment_len < sizeof(s->s3->handshake_fragment)) {
-                s->s3->handshake_fragment[s->s3->handshake_fragment_len++] =
-                    rr->data[rr->off++];
-                rr->length--;
+        if (s->s3->handshake_fragment == NULL) {
+                if ((s->s3->handshake_fragment = tls_buffer_new(0)) == NULL)
+                        return -1;
+                tls_buffer_set_capacity_limit(s->s3->handshake_fragment,
+                    SSL3_HM_HEADER_LENGTH);
         }
-
-        if (s->s3->handshake_fragment_len < sizeof(s->s3->handshake_fragment))
+        ssret = tls_buffer_extend(s->s3->handshake_fragment, SSL3_HM_HEADER_LENGTH,
+            ssl3_read_cb, s);
+        if (ssret <= 0 && ssret != TLS_IO_WANT_POLLIN)
+                return -1;
+        if (ssret != SSL3_HM_HEADER_LENGTH)
                 return 1;
 
         if (s->in_handshake) {
@@ -862,7 +892,8 @@ ssl3_read_handshake_unexpected(SSL *s)
          */
 
         /* Parse handshake message header. */
-        CBS_init(&cbs, s->s3->handshake_fragment, s->s3->handshake_fragment_len);
+        if (!tls_buffer_data(s->s3->handshake_fragment, &cbs))
+                return -1;
         if (!CBS_get_u8(&cbs, &hs_msg_type))
                 return -1;
         if (!CBS_get_u24(&cbs, &hs_msg_length))
@@ -888,10 +919,12 @@ ssl3_read_handshake_unexpected(SSL *s)
                         return -1;
                 }
 
-                ssl_msg_callback(s, 0, SSL3_RT_HANDSHAKE,
-                    s->s3->handshake_fragment, s->s3->handshake_fragment_len);
+                if (!tls_buffer_data(s->s3->handshake_fragment, &cbs))
+                        return -1;
+                ssl_msg_callback_cbs(s, 0, SSL3_RT_HANDSHAKE, &cbs);
 
-                s->s3->handshake_fragment_len = 0;
+                tls_buffer_free(s->s3->handshake_fragment);
+                s->s3->handshake_fragment = NULL;
 
                 /*
                  * It should be impossible to hit this, but keep the safety
@@ -1045,24 +1078,21 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
                 return -1;
         }
 
-        if (type == SSL3_RT_HANDSHAKE && s->s3->handshake_fragment_len > 0) {
-                /* Partially satisfy request from fragment storage. */
-                unsigned char *src = s->s3->handshake_fragment;
-                unsigned char *dst = buf;
-                unsigned int k;
-
-                /* peek == 0 */
-                n = 0;
-                while (len > 0 && s->s3->handshake_fragment_len > 0) {
-                        *dst++ = *src++;
-                        len--;
-                        s->s3->handshake_fragment_len--;
-                        n++;
+        if (type == SSL3_RT_HANDSHAKE &&
+            s->s3->handshake_fragment != NULL &&
+            tls_buffer_remaining(s->s3->handshake_fragment) > 0) {
+                ssize_t ssn;
+
+                if ((ssn = tls_buffer_read(s->s3->handshake_fragment, buf,
+                    len)) <= 0)
+                        return -1;
+
+                if (tls_buffer_remaining(s->s3->handshake_fragment) == 0) {
+                        tls_buffer_free(s->s3->handshake_fragment);
+                        s->s3->handshake_fragment = NULL;
                 }
-                /* move any remaining fragment bytes: */
-                for (k = 0; k < s->s3->handshake_fragment_len; k++)
-                        s->s3->handshake_fragment[k] = *src++;
-                return n;
+
+                return (int)ssn;
         }
 
         if (SSL_in_init(s) && !s->in_handshake) {
diff --git a/src/lib/libssl/tls_buffer.c b/src/lib/libssl/tls_buffer.c
index f70cfbc1a0..517d66d685 100644
--- a/src/lib/libssl/tls_buffer.c
+++ b/src/lib/libssl/tls_buffer.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_buffer.c,v 1.3 2022/07/22 19:33:53 jsing Exp $ */
+/* $OpenBSD: tls_buffer.c,v 1.4 2022/11/10 18:06:37 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019, 2022 Joel Sing <jsing@openbsd.org>
  *
@@ -155,6 +155,15 @@ tls_buffer_extend(struct tls_buffer *buf, size_t len,
         }
 }
 
+size_t
+tls_buffer_remaining(struct tls_buffer *buf)
+{
+        if (buf->offset > buf->len)
+                return 0;
+
+        return buf->len - buf->offset;
+}
+
 ssize_t
 tls_buffer_read(struct tls_buffer *buf, uint8_t *rbuf, size_t n)
 {
diff --git a/src/lib/libssl/tls_internal.h b/src/lib/libssl/tls_internal.h
index 1d3a8133cd..84edde8474 100644
--- a/src/lib/libssl/tls_internal.h
+++ b/src/lib/libssl/tls_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_internal.h,v 1.9 2022/07/24 14:28:16 jsing Exp $ */
+/* $OpenBSD: tls_internal.h,v 1.10 2022/11/10 18:06:37 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019, 2021 Joel Sing <jsing@openbsd.org>
  *
@@ -64,6 +64,7 @@ void tls_buffer_free(struct tls_buffer *buf);
 void tls_buffer_set_capacity_limit(struct tls_buffer *buf, size_t limit);
 ssize_t tls_buffer_extend(struct tls_buffer *buf, size_t len,
     tls_read_cb read_cb, void *cb_arg);
+size_t tls_buffer_remaining(struct tls_buffer *buf);
 ssize_t tls_buffer_read(struct tls_buffer *buf, uint8_t *rbuf, size_t n);
 ssize_t tls_buffer_write(struct tls_buffer *buf, const uint8_t *wbuf, size_t n);
 int tls_buffer_append(struct tls_buffer *buf, const uint8_t *wbuf, size_t n);