constify PACKET

PACKET contents should be read-only. To achieve this, also
- constify two user callbacks
- constify BUF_reverse.
Reviewed-by: NRich Salz <rsalz@openssl.org>

CHANGES

@@ -4,6 +4,12 @@
 
  Changes between 1.0.2f and 1.1.0  [xx XXX xxxx]
 
+  *) The signature of the session callback configured with
+     SSL_CTX_sess_set_get_cb was changed. The read-only input buffer
+     was explicitly marked as 'const unsigned char*' instead of
+     'unsigned char*'.
+     [Emilia Käsper]
+
   *) Always DPURIFY. Remove the use of uninitialized memory in the
      RNG, and other conditional uses of DPURIFY. This makes -DPURIFY a no-op.
      [Emilia Käsper]

apps/s_apps.h

@@ -188,7 +188,7 @@ long bio_dump_callback(BIO *bio, int cmd, const char *argp,
 void apps_ssl_info_callback(const SSL *s, int where, int ret);
 void msg_cb(int write_p, int version, int content_type, const void *buf,
             size_t len, SSL *ssl, void *arg);
-void tlsext_cb(SSL *s, int client_server, int type, unsigned char *data,
+void tlsext_cb(SSL *s, int client_server, int type, const unsigned char *data,
                int len, void *arg);
 #endif
 

apps/s_cb.c

@@ -722,14 +722,14 @@ static STRINT_PAIR tlsext_types[] = {
 };
 
 void tlsext_cb(SSL *s, int client_server, int type,
-               unsigned char *data, int len, void *arg)
+               const unsigned char *data, int len, void *arg)
 {
     BIO *bio = arg;
     const char *extname = lookup(type, tlsext_types, "unknown");
 
     BIO_printf(bio, "TLS %s extension \"%s\" (id=%d), len=%d\n",
                client_server ? "server" : "client", extname, type, len);
-    BIO_dump(bio, (char *)data, len);
+    BIO_dump(bio, (const char *)data, len);
     (void)BIO_flush(bio);
 }
 

apps/s_server.c

@@ -3216,7 +3216,7 @@ static int add_session(SSL *ssl, SSL_SESSION *session)
     return 0;
 }
 
-static SSL_SESSION *get_session(SSL *ssl, unsigned char *id, int idlen,
+static SSL_SESSION *get_session(SSL *ssl, const unsigned char *id, int idlen,
                                 int *do_copy)
 {
     simple_ssl_session *sess;

crypto/buffer/buffer.c

@@ -191,7 +191,7 @@ size_t BUF_MEM_grow_clean(BUF_MEM *str, size_t len)
     return (len);
 }
 
-void BUF_reverse(unsigned char *out, unsigned char *in, size_t size)
+void BUF_reverse(unsigned char *out, const unsigned char *in, size_t size)
 {
     size_t i;
     if (in) {

doc/ssl/SSL_CTX_sess_set_get_cb.pod

@@ -13,11 +13,11 @@ SSL_CTX_sess_set_new_cb, SSL_CTX_sess_set_remove_cb, SSL_CTX_sess_set_get_cb, SS
  void SSL_CTX_sess_set_remove_cb(SSL_CTX *ctx,
 	   void (*remove_session_cb)(SSL_CTX *ctx, SSL_SESSION *));
  void SSL_CTX_sess_set_get_cb(SSL_CTX *ctx,
-	   SSL_SESSION (*get_session_cb)(SSL *, unsigned char *, int, int *));
+	   SSL_SESSION (*get_session_cb)(SSL *, const unsigned char *, int, int *));
 
  int (*SSL_CTX_sess_get_new_cb(SSL_CTX *ctx))(struct ssl_st *ssl, SSL_SESSION *sess);
  void (*SSL_CTX_sess_get_remove_cb(SSL_CTX *ctx))(struct ssl_ctx_st *ctx, SSL_SESSION *sess);
- SSL_SESSION *(*SSL_CTX_sess_get_get_cb(SSL_CTX *ctx))(struct ssl_st *ssl, unsigned char *data, int len, int *copy);
+ SSL_SESSION *(*SSL_CTX_sess_get_get_cb(SSL_CTX *ctx))(struct ssl_st *ssl, const unsigned char *data, int len, int *copy);
 
  int (*new_session_cb)(struct ssl_st *ssl, SSL_SESSION *sess);
  void (*remove_session_cb)(struct ssl_ctx_st *ctx, SSL_SESSION *sess);

include/openssl/buffer.h

@@ -99,7 +99,7 @@ BUF_MEM *BUF_MEM_new_ex(unsigned long flags);
 void BUF_MEM_free(BUF_MEM *a);
 size_t BUF_MEM_grow(BUF_MEM *str, size_t len);
 size_t BUF_MEM_grow_clean(BUF_MEM *str, size_t len);
-void BUF_reverse(unsigned char *out, unsigned char *in, size_t siz);
+void BUF_reverse(unsigned char *out, const unsigned char *in, size_t siz);
 
 /* BEGIN ERROR CODES */
 /*

include/openssl/ssl.h

@@ -716,11 +716,11 @@ void (*SSL_CTX_sess_get_remove_cb(SSL_CTX *ctx)) (struct ssl_ctx_st *ctx,
 void SSL_CTX_sess_set_get_cb(SSL_CTX *ctx,
                              SSL_SESSION *(*get_session_cb) (struct ssl_st
                                                              *ssl,
-                                                             unsigned char
+                                                             const unsigned char
                                                              *data, int len,
                                                              int *copy));
 SSL_SESSION *(*SSL_CTX_sess_get_get_cb(SSL_CTX *ctx)) (struct ssl_st *ssl,
-                                                       unsigned char *Data,
+                                                       const unsigned char *data,
                                                        int len, int *copy);
 void SSL_CTX_set_info_callback(SSL_CTX *ctx,
                                void (*cb) (const SSL *ssl, int type,

ssl/d1_lib.c

@@ -489,7 +489,8 @@ int dtls1_listen(SSL *s, struct sockaddr *client)
     int next, n, ret = 0, clearpkt = 0;
     unsigned char cookie[DTLS1_COOKIE_LENGTH];
     unsigned char seq[SEQ_NUM_SIZE];
-    unsigned char *data, *p, *buf;
+    const unsigned char *data;
+    unsigned char *p, *buf;
     unsigned long reclen, fragoff, fraglen, msglen;
     unsigned int rectype, versmajor, msgseq, msgtype, clientvers, cookielen;
     BIO *rbio, *wbio;

ssl/packet_locl.h

@@ -72,7 +72,7 @@ extern "C" {
 
 typedef struct {
     /* Pointer to where we are currently reading from */
-    unsigned char *curr;
+    const unsigned char *curr;
     /* Number of bytes remaining */
     size_t remaining;
 } PACKET;
@@ -95,10 +95,8 @@ static ossl_inline size_t PACKET_remaining(const PACKET *pkt)
 /*
  * Returns a pointer to the PACKET's current position.
  * For use in non-PACKETized APIs.
- * TODO(openssl-team): this should return 'const unsigned char*' but can't
- * currently because legacy code passes 'unsigned char*'s around.
  */
-static ossl_inline unsigned char *PACKET_data(const PACKET *pkt)
+static ossl_inline const unsigned char *PACKET_data(const PACKET *pkt)
 {
     return pkt->curr;
 }
@@ -108,7 +106,8 @@ static ossl_inline unsigned char *PACKET_data(const PACKET *pkt)
  * copy of the data so |buf| must be present for the whole time that the PACKET
  * is being used.
  */
-__owur static ossl_inline int PACKET_buf_init(PACKET *pkt, unsigned char *buf,
+__owur static ossl_inline int PACKET_buf_init(PACKET *pkt,
+                                              const unsigned char *buf,
                                               size_t len)
 {
     /* Sanity check for negative values. */
@@ -325,7 +324,7 @@ __owur static ossl_inline int PACKET_get_4(PACKET *pkt, unsigned long *data)
  * underlying buffer gets freed
  */
 __owur static ossl_inline int PACKET_peek_bytes(const PACKET *pkt,
-                                                unsigned char **data,
+                                                const unsigned char **data,
                                                 size_t len)
 {
     if (PACKET_remaining(pkt) < len)
@@ -343,7 +342,7 @@ __owur static ossl_inline int PACKET_peek_bytes(const PACKET *pkt,
  * freed
  */
 __owur static ossl_inline int PACKET_get_bytes(PACKET *pkt,
-                                               unsigned char **data,
+                                               const unsigned char **data,
                                                size_t len)
 {
     if (!PACKET_peek_bytes(pkt, data, len))
@@ -475,7 +474,7 @@ __owur static ossl_inline int PACKET_get_length_prefixed_1(PACKET *pkt,
                                                            PACKET *subpkt)
 {
     unsigned int length;
-    unsigned char *data;
+    const unsigned char *data;
     PACKET tmp = *pkt;
     if (!PACKET_get_1(&tmp, &length) ||
         !PACKET_get_bytes(&tmp, &data, (size_t)length)) {
@@ -500,7 +499,7 @@ __owur static ossl_inline int PACKET_get_length_prefixed_2(PACKET *pkt,
                                                            PACKET *subpkt)
 {
     unsigned int length;
-    unsigned char *data;
+    const unsigned char *data;
     PACKET tmp = *pkt;
     if (!PACKET_get_net_2(&tmp, &length) ||
         !PACKET_get_bytes(&tmp, &data, (size_t)length)) {
@@ -525,7 +524,7 @@ __owur static ossl_inline int PACKET_get_length_prefixed_3(PACKET *pkt,
                                                            PACKET *subpkt)
 {
     unsigned long length;
-    unsigned char *data;
+    const unsigned char *data;
     PACKET tmp = *pkt;
     if (!PACKET_get_net_3(&tmp, &length) ||
         !PACKET_get_bytes(&tmp, &data, (size_t)length)) {

ssl/s3_lib.c

@@ -3799,7 +3799,7 @@ long ssl3_callback_ctrl(SSL *s, int cmd, void (*fp) (void))
 #endif
     case SSL_CTRL_SET_TLSEXT_DEBUG_CB:
         s->tlsext_debug_cb = (void (*)(SSL *, int, int,
-                                       unsigned char *, int, void *))fp;
+                                       const unsigned char *, int, void *))fp;
         break;
 
     case SSL_CTRL_SET_NOT_RESUMABLE_SESS_CB:

ssl/ssl_locl.h

@@ -724,7 +724,8 @@ struct ssl_ctx_st {
     int (*new_session_cb) (struct ssl_st *ssl, SSL_SESSION *sess);
     void (*remove_session_cb) (struct ssl_ctx_st *ctx, SSL_SESSION *sess);
     SSL_SESSION *(*get_session_cb) (struct ssl_st *ssl,
-                                    unsigned char *data, int len, int *copy);
+                                    const unsigned char *data, int len,
+                                    int *copy);
     struct {
         int sess_connect;       /* SSL new conn - started */
         int sess_connect_renegotiate; /* SSL reneg - requested */
@@ -1077,7 +1078,7 @@ struct ssl_st {
 
     /* TLS extension debug callback */
     void (*tlsext_debug_cb) (SSL *s, int client_server, int type,
-                             unsigned char *data, int len, void *arg);
+                             const unsigned char *data, int len, void *arg);
     void *tlsext_debug_arg;
     char *tlsext_hostname;
     /*-

ssl/ssl_sess.c

@@ -576,13 +576,9 @@ int ssl_get_prev_session(SSL *s, const PACKET *ext, const PACKET *session_id)
     if (try_session_cache &&
         ret == NULL && s->session_ctx->get_session_cb != NULL) {
         int copy = 1;
-        /* The user callback takes a non-const pointer, so grab a local copy. */
-        unsigned char *sid = NULL;
-        size_t sid_len;
-        if (!PACKET_memdup(session_id, &sid, &sid_len))
-            goto err;
-        ret = s->session_ctx->get_session_cb(s, sid, sid_len, &copy);
-        OPENSSL_free(sid);
+        ret = s->session_ctx->get_session_cb(s, PACKET_data(session_id),
+                                             PACKET_remaining(session_id),
+                                             &copy);
 
         if (ret != NULL) {
             s->session_ctx->stats.sess_cb_hit++;
@@ -1158,14 +1154,14 @@ void (*SSL_CTX_sess_get_remove_cb(SSL_CTX *ctx)) (SSL_CTX *ctx,
 
 void SSL_CTX_sess_set_get_cb(SSL_CTX *ctx,
                              SSL_SESSION *(*cb) (struct ssl_st *ssl,
-                                                 unsigned char *data, int len,
-                                                 int *copy))
+                                                 const unsigned char *data,
+                                                 int len, int *copy))
 {
     ctx->get_session_cb = cb;
 }
 
 SSL_SESSION *(*SSL_CTX_sess_get_get_cb(SSL_CTX *ctx)) (SSL *ssl,
-                                                       unsigned char *data,
+                                                       const unsigned char *data,
                                                        int len, int *copy) {
     return ctx->get_session_cb;
 }

ssl/statem/statem_clnt.c

@@ -1023,7 +1023,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     const SSL_CIPHER *c;
     PACKET session_id;
     size_t session_id_len;
-    unsigned char *cipherchars;
+    const unsigned char *cipherchars;
     int i, al = SSL_AD_INTERNAL_ERROR;
     unsigned int compression;
     unsigned int sversion;
@@ -1284,7 +1284,7 @@ MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt)
     int al, i, ret = MSG_PROCESS_ERROR, exp_idx;
     unsigned long cert_list_len, cert_len;
     X509 *x = NULL;
-    unsigned char *certstart, *certbytes;
+    const unsigned char *certstart, *certbytes;
     STACK_OF(X509) *sk = NULL;
     EVP_PKEY *pkey = NULL;
 
@@ -1575,7 +1575,7 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt)
 #ifndef OPENSSL_NO_EC
     else if (alg_k & (SSL_kECDHE | SSL_kECDHEPSK)) {
         PACKET encoded_pt;
-        unsigned char *ecparams;
+        const unsigned char *ecparams;
         int curve_nid;
 
         /*
@@ -1667,7 +1667,7 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt)
         }
 
         if (SSL_USE_SIGALGS(s)) {
-            unsigned char *sigalgs;
+            const unsigned char *sigalgs;
             int rv;
             if (!PACKET_get_bytes(pkt, &sigalgs, 2)) {
                 SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_LENGTH_TOO_SHORT);
@@ -1761,8 +1761,8 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt)
     int ret = MSG_PROCESS_ERROR;
     unsigned int list_len, ctype_num, i, name_len;
     X509_NAME *xn = NULL;
-    unsigned char *data;
-    unsigned char *namestart, *namebytes;
+    const unsigned char *data;
+    const unsigned char *namestart, *namebytes;
     STACK_OF(X509_NAME) *ca_sk = NULL;
 
     if ((ca_sk = sk_X509_NAME_new(ca_dn_cmp)) == NULL) {

ssl/statem/statem_srvr.c

@@ -2084,7 +2084,8 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
     EVP_PKEY *ckey = NULL;
 #endif
     PACKET enc_premaster;
-    unsigned char *data, *rsa_decrypt = NULL;
+    const unsigned char *data;
+    unsigned char *rsa_decrypt = NULL;
 
     alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
 
@@ -2463,7 +2464,8 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
     if (alg_k & SSL_kGOST) {
         EVP_PKEY_CTX *pkey_ctx;
         EVP_PKEY *client_pub_pkey = NULL, *pk = NULL;
-        unsigned char premaster_secret[32], *start;
+        unsigned char premaster_secret[32];
+        const unsigned char *start;
         size_t outlen = 32, inlen;
         unsigned long alg_a;
         int Ttag, Tclass;
@@ -2656,7 +2658,8 @@ WORK_STATE tls_post_process_client_key_exchange(SSL *s, WORK_STATE wst)
 MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
 {
     EVP_PKEY *pkey = NULL;
-    unsigned char *sig, *data;
+    const unsigned char *sig, *data;
+    unsigned char *gost_data = NULL;
     int al, ret = MSG_PROCESS_ERROR;
     int type = 0, j;
     unsigned int len;
@@ -2765,8 +2768,15 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
         int pktype = EVP_PKEY_id(pkey);
         if (pktype == NID_id_GostR3410_2001
             || pktype == NID_id_GostR3410_2012_256
-            || pktype == NID_id_GostR3410_2012_512)
-            BUF_reverse(data, NULL, len);
+            || pktype == NID_id_GostR3410_2012_512) {
+            if ((gost_data = OPENSSL_malloc(len)) == NULL) {
+                SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_MALLOC_FAILURE);
+                al = SSL_AD_INTERNAL_ERROR;
+                goto f_err;
+            }
+            BUF_reverse(gost_data, data, len);
+            data = gost_data;
+        }
     }
 #endif
 
@@ -2794,6 +2804,7 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
     BIO_free(s->s3->handshake_buffer);
     s->s3->handshake_buffer = NULL;
     EVP_MD_CTX_free(mctx);
+    OPENSSL_free(gost_data);
     return ret;
 }
 
@@ -2802,8 +2813,7 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
     int i, al = SSL_AD_INTERNAL_ERROR, ret = MSG_PROCESS_ERROR;
     X509 *x = NULL;
     unsigned long l, llen;
-    const unsigned char *certstart;
-    unsigned char *certbytes;
+    const unsigned char *certstart, *certbytes;
     STACK_OF(X509) *sk = NULL;
     PACKET spkt;
 

ssl/t1_lib.c

@@ -1736,7 +1736,7 @@ static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al)
     unsigned int data_len;
     unsigned int proto_len;
     const unsigned char *selected;
-    unsigned char *data;
+    const unsigned char *data;
     unsigned char selected_len;
     int r;
 
@@ -1795,7 +1795,7 @@ static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al)
 static void ssl_check_for_safari(SSL *s, const PACKET *pkt)
 {
     unsigned int type, size;
-    unsigned char *eblock1, *eblock2;
+    const unsigned char *eblock1, *eblock2;
     PACKET tmppkt;
 
     static const unsigned char kSafariExtensionsBlock[] = {
@@ -1866,7 +1866,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
     unsigned int type;
     unsigned int size;
     unsigned int len;
-    unsigned char *data;
+    const unsigned char *data;
     int renegotiate_seen = 0;
 
     s->servername_done = 0;
@@ -1954,7 +1954,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
  */
 
         else if (type == TLSEXT_TYPE_server_name) {
-            unsigned char *sdata;
+            const unsigned char *sdata;
             unsigned int servname_type;
             unsigned int dsize;
             PACKET ssubpkt;
@@ -2375,7 +2375,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
     }
 
     while (PACKET_get_net_2(pkt, &type) && PACKET_get_net_2(pkt, &size)) {
-        unsigned char *data;
+        const unsigned char *data;
         PACKET spkt;
 
         if (!PACKET_get_sub_packet(pkt, &spkt, size)
@@ -2965,7 +2965,7 @@ int tls_check_serverhello_tlsext_early(SSL *s, const PACKET *ext,
         }
         if (type == TLSEXT_TYPE_session_ticket && use_ticket) {
             int r;
-            unsigned char *etick;
+            const unsigned char *etick;
 
             /* Duplicate extension */
             if (have_ticket != 0) {

ssl/t1_reneg.c

@@ -145,7 +145,7 @@ int ssl_add_clienthello_renegotiate_ext(SSL *s, unsigned char *p, int *len,
 int ssl_parse_clienthello_renegotiate_ext(SSL *s, PACKET *pkt, int *al)
 {
     unsigned int ilen;
-    unsigned char *d;
+    const unsigned char *d;
 
     /* Parse the length byte */
     if (!PACKET_get_1(pkt, &ilen)
@@ -224,7 +224,7 @@ int ssl_parse_serverhello_renegotiate_ext(SSL *s, PACKET *pkt, int *al)
     unsigned int expected_len = s->s3->previous_client_finished_len
         + s->s3->previous_server_finished_len;
     unsigned int ilen;
-    unsigned char *data;
+    const unsigned char *data;
 
     /* Check for logic errors */
     OPENSSL_assert(!expected_len || s->s3->previous_client_finished_len);

test/packettest.c

@@ -197,7 +197,7 @@ static int test_PACKET_get_sub_packet(unsigned char buf[BUF_LEN])
 
 static int test_PACKET_get_bytes(unsigned char buf[BUF_LEN])
 {
-    unsigned char *bytes;
+    const unsigned char *bytes;
     PACKET pkt;
 
     if (       !PACKET_buf_init(&pkt, buf, BUF_LEN)
@@ -310,7 +310,7 @@ static int test_PACKET_strndup()
 
 static int test_PACKET_forward(unsigned char buf[BUF_LEN])
 {
-    unsigned char *byte;
+    const unsigned char *byte;
     PACKET pkt;
 
     if (       !PACKET_buf_init(&pkt, buf, BUF_LEN)