From b69817449315f3818a8472468b3328ea755819db Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Mon, 1 Feb 2016 15:26:18 +0100 Subject: constify PACKET PACKET contents should be read-only. To achieve this, also - constify two user callbacks - constify BUF_reverse. Reviewed-by: Rich Salz --- ssl/d1_lib.c | 3 ++- ssl/packet_locl.h | 19 +++++++++---------- ssl/s3_lib.c | 2 +- ssl/ssl_locl.h | 5 +++-- ssl/ssl_sess.c | 16 ++++++---------- ssl/statem/statem_clnt.c | 12 ++++++------ ssl/statem/statem_srvr.c | 24 +++++++++++++++++------- ssl/t1_lib.c | 12 ++++++------ ssl/t1_reneg.c | 4 ++-- 9 files changed, 52 insertions(+), 45 deletions(-) (limited to 'ssl') diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index fed1340377..65e30f7132 100644 --- a/ssl/d1_lib.c +++ b/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; diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h index b4337e4682..48767b6728 100644 --- a/ssl/packet_locl.h +++ b/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)) { diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index f998f72370..5252d04cb5 100644 --- a/ssl/s3_lib.c +++ b/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: diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 1f8b0ea88c..9a30598104 100644 --- a/ssl/ssl_locl.h +++ b/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; /*- diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 1ca0f964a2..85f6c7ff67 100644 --- a/ssl/ssl_sess.c +++ b/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, ©); - OPENSSL_free(sid); + ret = s->session_ctx->get_session_cb(s, PACKET_data(session_id), + PACKET_remaining(session_id), + ©); 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; } diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 456bbd8a7b..2da16fd664 100644 --- a/ssl/statem/statem_clnt.c +++ b/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) { diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 2f5fdb658a..bc651c76ee 100644 --- a/ssl/statem/statem_srvr.c +++ b/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; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 0131dc833c..446a678dee 100644 --- a/ssl/t1_lib.c +++ b/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) { diff --git a/ssl/t1_reneg.c b/ssl/t1_reneg.c index 0aeaa55263..0c090a282d 100644 --- a/ssl/t1_reneg.c +++ b/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); -- cgit v1.2.3