Add PACKET_copy_all

Reviewed-by: Matt Caswell <matt@openssl.org>

4 files changed, 47 insertions, 16 deletions

diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h
index b13aa5a5c0..e73eb3dba2 100644
--- a/ssl/packet_locl.h
+++ b/ssl/packet_locl.h
@@ -301,7 +301,7 @@ __owur static inline int PACKET_get_4(PACKET *pkt, unsigned long *data)
  * underlying buffer gets freed
  */
 __owur static inline int PACKET_peek_bytes(const PACKET *pkt, unsigned char **data,
-                                          size_t len)
+                                           size_t len)
 {
     if (PACKET_remaining(pkt) < len)
         return 0;
@@ -356,6 +356,24 @@ __owur static inline int PACKET_copy_bytes(PACKET *pkt, unsigned char *data,
 }
 
 /*
+ * Copy packet data to |dest|, and set |len| to the number of copied bytes.
+ * If the packet has more than |dest_len| bytes, nothing is copied.
+ * Returns 1 if the packet data fits in |dest_len| bytes, 0 otherwise.
+ * Does not forward PACKET position (because it is typically the last thing
+ * done with a given PACKET).
+ */
+__owur static inline int PACKET_copy_all(const PACKET *pkt, unsigned char *dest,
+                                         size_t dest_len, size_t *len) {
+    if (PACKET_remaining(pkt) > dest_len) {
+        *len = 0;
+        return 0;
+    }
+    *len = pkt->remaining;
+    memcpy(dest, pkt->curr, pkt->remaining);
+    return 1;
+}
+
+/*
  * Copy |pkt| bytes to a newly allocated buffer and store a pointer to the
  * result in |*data|, and the length in |len|.
  * If |*data| is not NULL, the old data is OPENSSL_free'd.
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index ef25202cbe..82162d8566 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -3457,15 +3457,6 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
     /* 3 = SSLV2_CIPHER_LEN > TLS_CIPHER_LEN = 2. */
     unsigned char cipher[SSLV2_CIPHER_LEN];
 
-    /*
-     * Can this ever happen?
-     * This method used to check for s->s3, but did so inconsistently.
-     */
-    if (s->s3 == NULL) {
-        *al = SSL_AD_INTERNAL_ERROR;
-        return NULL;
-    }
-
     s->s3->send_connection_binding = 0;
 
     n = sslv2format ? SSLV2_CIPHER_LEN : TLS_CIPHER_LEN;
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 41bc4e11a3..7660292196 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -564,11 +564,14 @@ int ssl_get_prev_session(SSL *s, const PACKET *ext, const PACKET *session_id)
         !(s->session_ctx->session_cache_mode &
           SSL_SESS_CACHE_NO_INTERNAL_LOOKUP)) {
         SSL_SESSION data;
+        size_t local_len;
         data.ssl_version = s->version;
-        data.session_id_length = len;
-        if (len == 0)
-            return 0;
-        memcpy(data.session_id, PACKET_data(session_id), len);
+        if (!PACKET_copy_all(session_id, data.session_id,
+                             sizeof(data.session_id),
+                             &local_len)) {
+            goto err;
+        }
+        data.session_id_length = local_len;
         CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX);
         ret = lh_SSL_SESSION_retrieve(s->session_ctx->sessions, &data);
         if (ret != NULL) {
diff --git a/test/packettest.c b/test/packettest.c
index acfc249885..915b42b129 100644
--- a/test/packettest.c
+++ b/test/packettest.c
@@ -240,6 +240,25 @@ static int test_PACKET_copy_bytes(unsigned char buf[BUF_LEN])
     return 1;
 }
 
+static int test_PACKET_copy_all(unsigned char buf[BUF_LEN])
+{
+    unsigned char dup[BUF_LEN];
+    PACKET pkt;
+    size_t len;
+
+    if (       !PACKET_buf_init(&pkt, buf, BUF_LEN)
+               || !PACKET_copy_all(&pkt, dup, BUF_LEN, &len)
+               || len != BUF_LEN
+               || memcmp(buf, dup, BUF_LEN) != 0
+               || PACKET_remaining(&pkt) != BUF_LEN
+               || PACKET_copy_all(&pkt, dup, BUF_LEN - 1, &len)) {
+        fprintf(stderr, "test_PACKET_copy_bytes() failed\n");
+        return 0;
+    }
+
+    return 1;
+}
+
 static int test_PACKET_memdup(unsigned char buf[BUF_LEN])
 {
     unsigned char *data = NULL;
@@ -314,7 +333,7 @@ static int test_PACKET_buf_init()
     unsigned char buf[BUF_LEN];
     PACKET pkt;
 
-    /* Also tests PACKET_get_len() */
+    /* Also tests PACKET_remaining() */
     if (       !PACKET_buf_init(&pkt, buf, 4)
             ||  PACKET_remaining(&pkt) != 4
             || !PACKET_buf_init(&pkt, buf, BUF_LEN)
@@ -332,7 +351,6 @@ static int test_PACKET_null_init()
     PACKET pkt;
 
     PACKET_null_init(&pkt);
-    /* Also tests PACKET_get_len() */
     if (       PACKET_remaining(&pkt) != 0
             || PACKET_forward(&pkt, 1)) {
         fprintf(stderr, "test_PACKET_null_init() failed\n");
@@ -442,6 +460,7 @@ int main(int argc, char **argv)
             || !test_PACKET_get_sub_packet(buf)
             || !test_PACKET_get_bytes(buf)
             || !test_PACKET_copy_bytes(buf)
+            || !test_PACKET_copy_all(buf)
             || !test_PACKET_memdup(buf)
             || !test_PACKET_strndup()
             || !test_PACKET_forward(buf)