Remove PACKET_(get|goto)_bookmark

The bookmark API results in a lot of boilerplate error checking that can
be much more easily achieved with a simple struct copy. It also lays the
path for removing the third PACKET field.

Reviewed-by: Rich Salz <rsalz@openssl.org>

4 files changed, 21 insertions, 59 deletions

diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h
index 3200c22846..7a72f8e658 100644
--- a/ssl/packet_locl.h
+++ b/ssl/packet_locl.h
@@ -421,25 +421,6 @@ __owur static inline int PACKET_forward(PACKET *pkt, size_t len)
     return 1;
 }
 
-/* Store a bookmark for the current reading position in |*bm| */
-__owur static inline int PACKET_get_bookmark(const PACKET *pkt, size_t *bm)
-{
-    *bm = pkt->curr - pkt->start;
-
-    return 1;
-}
-
-/* Set the current reading position to the bookmark |bm| */
-__owur static inline int PACKET_goto_bookmark(PACKET *pkt, size_t bm)
-{
-    if (bm > (size_t)(pkt->end - pkt->start))
-        return 0;
-
-    pkt->curr = pkt->start + bm;
-
-    return 1;
-}
-
 /*
  * Stores the total length of the packet we have in the underlying buffer in
  * |*len|
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 0195d09958..7cfff635f0 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1102,10 +1102,9 @@ int ssl3_get_server_hello(SSL *s)
     if (s->version >= TLS1_VERSION && s->tls_session_secret_cb &&
         s->session->tlsext_tick) {
         SSL_CIPHER *pref_cipher = NULL;
-        size_t bookm;
-        if (!PACKET_get_bookmark(&pkt, &bookm)
-                || !PACKET_forward(&pkt, j)
-                || !PACKET_get_bytes(&pkt, &cipherchars, ciphercharlen)) {
+        PACKET bookmark = pkt;
+        if (!PACKET_forward(&pkt, j)
+            || !PACKET_get_bytes(&pkt, &cipherchars, ciphercharlen)) {
             SSLerr(SSL_F_SSL3_GET_SERVER_HELLO, SSL_R_LENGTH_MISMATCH);
             al = SSL_AD_DECODE_ERROR;
             goto f_err;
@@ -1122,11 +1121,7 @@ int ssl3_get_server_hello(SSL *s)
             al = SSL_AD_INTERNAL_ERROR;
             goto f_err;
         }
-        if (!PACKET_goto_bookmark(&pkt, bookm)) {
-            SSLerr(SSL_F_SSL3_GET_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
-            al = SSL_AD_INTERNAL_ERROR;
-            goto f_err;
-        }
+        pkt = bookmark;
     }
 
     /* Get the session id */
@@ -1462,9 +1457,9 @@ int ssl3_get_key_exchange(SSL *s)
     int curve_nid = 0;
     unsigned int encoded_pt_len = 0;
 #endif
-    PACKET pkt;
+    PACKET pkt, save_param_start;
     unsigned char *data, *param;
-    size_t startparam, endparam;
+    size_t param_len;
 
     EVP_MD_CTX_init(&md_ctx);
 
@@ -1496,12 +1491,12 @@ int ssl3_get_key_exchange(SSL *s)
         return (1);
     }
 
-    if (!PACKET_buf_init(&pkt, s->init_msg, n)
-            || !PACKET_get_bookmark(&pkt, &startparam)) {
+    if (!PACKET_buf_init(&pkt, s->init_msg, n)) {
             SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             al = SSL_AD_INTERNAL_ERROR;
             goto f_err;
     }
+    save_param_start = pkt;
 
 #ifndef OPENSSL_NO_RSA
     RSA_free(s->s3->peer_rsa_tmp);
@@ -1894,10 +1889,11 @@ int ssl3_get_key_exchange(SSL *s)
     }
 #endif                          /* !OPENSSL_NO_EC */
 
-    if (!PACKET_get_bookmark(&pkt, &endparam)) {
-        SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
-        goto f_err;
-    }
+    /*
+     * |pkt| now points to the beginning of the signature, so the difference
+     * equals the length of the parameters.
+     */
+    param_len = PACKET_remaining(&save_param_start) - PACKET_remaining(&pkt);
 
     /* if it was signed, check the signature */
     if (pkey != NULL) {
@@ -1939,8 +1935,8 @@ int ssl3_get_key_exchange(SSL *s)
             SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_WRONG_SIGNATURE_LENGTH);
             goto f_err;
         }
-        if (!PACKET_goto_bookmark(&pkt, startparam)
-                || !PACKET_get_bytes(&pkt, &param, endparam - startparam)) {
+        pkt = save_param_start;
+        if (!PACKET_get_bytes(&pkt, &param, param_len)) {
             al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             goto f_err;
@@ -1960,7 +1956,7 @@ int ssl3_get_key_exchange(SSL *s)
                                  SSL3_RANDOM_SIZE);
                 EVP_DigestUpdate(&md_ctx, &(s->s3->server_random[0]),
                                  SSL3_RANDOM_SIZE);
-                EVP_DigestUpdate(&md_ctx, param, endparam - startparam);
+                EVP_DigestUpdate(&md_ctx, param, param_len);
                 EVP_DigestFinal_ex(&md_ctx, q, &size);
                 q += size;
                 j += size;
@@ -1986,7 +1982,7 @@ int ssl3_get_key_exchange(SSL *s)
                              SSL3_RANDOM_SIZE);
             EVP_VerifyUpdate(&md_ctx, &(s->s3->server_random[0]),
                              SSL3_RANDOM_SIZE);
-            EVP_VerifyUpdate(&md_ctx, param, endparam - startparam);
+            EVP_VerifyUpdate(&md_ctx, param, param_len);
             if (EVP_VerifyFinal(&md_ctx, data, (int)i, pkey) <= 0) {
                 /* bad signature */
                 al = SSL_AD_DECRYPT_ERROR;
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 18d3be1b2a..aea72794ef 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2481,14 +2481,9 @@ int ssl3_get_client_key_exchange(SSL *s)
     if (alg_k & (SSL_kDHE | SSL_kDHr | SSL_kDHd | SSL_kDHEPSK)) {
         int idx = -1;
         EVP_PKEY *skey = NULL;
-        size_t bookm;
+        PACKET bookmark = pkt;
         unsigned char shared[(OPENSSL_DH_MAX_MODULUS_BITS + 7) / 8];
 
-        if (!PACKET_get_bookmark(&pkt, &bookm)) {
-            al = SSL_AD_INTERNAL_ERROR;
-            SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
-            goto f_err;
-        }
         if (!PACKET_get_net_2(&pkt, &i)) {
             if (alg_k & (SSL_kDHE | SSL_kDHEPSK)) {
                 al = SSL_AD_HANDSHAKE_FAILURE;
@@ -2504,12 +2499,7 @@ int ssl3_get_client_key_exchange(SSL *s)
                        SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG);
                 goto err;
             } else {
-                if (!PACKET_goto_bookmark(&pkt, bookm)) {
-                    al = SSL_AD_INTERNAL_ERROR;
-                    SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE,
-                           ERR_R_INTERNAL_ERROR);
-                    goto f_err;
-                }
+                pkt = bookmark;
                 i = PACKET_remaining(&pkt);
             }
         }
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 95b4fb694d..463f34e687 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2934,7 +2934,7 @@ int tls1_process_ticket(SSL *s, PACKET *pkt,  unsigned char *session_id,
                         int len, SSL_SESSION **ret)
 {
     unsigned int i;
-    size_t bookmark = 0;
+    PACKET bookmark = *pkt;
     int retv = -1;
 
     *ret = NULL;
@@ -2949,10 +2949,6 @@ int tls1_process_ticket(SSL *s, PACKET *pkt,  unsigned char *session_id,
     if ((s->version <= SSL3_VERSION))
         return 0;
 
-    if (!PACKET_get_bookmark(pkt, &bookmark)) {
-        return -1;
-    }
-
     /* Skip past DTLS cookie */
     if (SSL_IS_DTLS(s)) {
         if (!PACKET_get_1(pkt, &i)
@@ -3043,8 +3039,7 @@ int tls1_process_ticket(SSL *s, PACKET *pkt,  unsigned char *session_id,
     }
     retv = 0;
 end:
-    if (!PACKET_goto_bookmark(pkt, bookmark))
-        return -1;
+    *pkt = bookmark;
     return retv;
 }