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@

diff --git a/lib/libssl/s3_lib.c b/lib/libssl/s3_lib.c
index 52ad16a..68c6fc6 100644 (file)
--- a/lib/libssl/s3_lib.c
+++ b/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/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 64cf6a6..69546c0 100644 (file)
--- a/lib/libssl/ssl_locl.h
+++ b/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/lib/libssl/ssl_pkt.c b/lib/libssl/ssl_pkt.c
index ddb2ce0..58d3ee1 100644 (file)
--- a/lib/libssl/ssl_pkt.c
+++ b/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/lib/libssl/tls_buffer.c b/lib/libssl/tls_buffer.c
index f70cfbc..517d66d 100644 (file)
--- a/lib/libssl/tls_buffer.c
+++ b/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/lib/libssl/tls_internal.h b/lib/libssl/tls_internal.h
index 1d3a813..84edde8 100644 (file)
--- a/lib/libssl/tls_internal.h
+++ b/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);