From da67a0ae3462f6c6447ed841a9ec514077244b02 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Tue, 5 Aug 2014 15:21:36 +0100 Subject: [PATCH] Revision of custom extension code. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Move custom extension structures from SSL_CTX to CERT structure. This change means the form can be revised in future without binary compatibility issues. Also since CERT is part of SSL structures so per-SSL custom extensions could be supported in future as well as per SSL_CTX. Reviewed-by: Rich Salz Reviewed-by: Emilia Käsper (cherry picked from commit b83294fe3022b9d5d525ccdcfeb53d39c25b05bd) Conflicts: ssl/ssl.h ssl/ssl_cert.c ssl/ssl_locl.h --- ssl/s23_clnt.c | 2 +- ssl/ssl.h | 20 ----------------- ssl/ssl_cert.c | 34 +++++++++++++++++++++++++++++ ssl/ssl_lib.c | 58 ++++++++++++++++++++++++++++---------------------- ssl/ssl_locl.h | 20 +++++++++++++++++ ssl/t1_lib.c | 22 +++++++++---------- 6 files changed, 98 insertions(+), 58 deletions(-) diff --git a/ssl/s23_clnt.c b/ssl/s23_clnt.c index 3953c564f5..e3f8a342d6 100644 --- a/ssl/s23_clnt.c +++ b/ssl/s23_clnt.c @@ -363,7 +363,7 @@ static int ssl23_client_hello(SSL *s) if (s->ctx->tlsext_opaque_prf_input_callback != 0 || s->tlsext_opaque_prf_input != NULL) ssl2_compat = 0; #endif - if (s->ctx->custom_cli_ext_records_count != 0) + if (s->cert->custom_cli_ext_records_count != 0) ssl2_compat = 0; } #endif diff --git a/ssl/ssl.h b/ssl/ssl.h index 3f5fc712e3..d113b43a39 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -421,20 +421,6 @@ typedef int (*custom_srv_ext_second_cb_fn)(SSL *s, unsigned short ext_type, const unsigned char **out, unsigned short *outlen, int *al, void *arg); -typedef struct { - unsigned short ext_type; - custom_cli_ext_first_cb_fn fn1; - custom_cli_ext_second_cb_fn fn2; - void *arg; -} custom_cli_ext_record; - -typedef struct { - unsigned short ext_type; - custom_srv_ext_first_cb_fn fn1; - custom_srv_ext_second_cb_fn fn2; - void *arg; -} custom_srv_ext_record; - #endif #ifndef OPENSSL_NO_SSL_INTERN @@ -1153,12 +1139,6 @@ struct ssl_ctx_st unsigned char *tlsext_ellipticcurvelist; # endif /* OPENSSL_NO_EC */ #endif - - /* Arrays containing the callbacks for custom TLS Extensions. */ - custom_cli_ext_record *custom_cli_ext_records; - size_t custom_cli_ext_records_count; - custom_srv_ext_record *custom_srv_ext_records; - size_t custom_srv_ext_records_count; }; #endif diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index fc63bdbf24..f0035d768d 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -434,6 +434,27 @@ CERT *ssl_cert_dup(CERT *cert) ret->ciphers_raw = NULL; +#ifndef OPENSSL_NO_TLSEXT + if (cert->custom_cli_ext_records_count) + { + ret->custom_cli_ext_records = BUF_memdup(cert->custom_cli_ext_records, sizeof(custom_cli_ext_record) * cert->custom_cli_ext_records_count); + if (ret->custom_cli_ext_records == NULL) + goto err; + ret->custom_cli_ext_records_count = + cert->custom_cli_ext_records_count; + } + + if (cert->custom_srv_ext_records_count) + { + ret->custom_srv_ext_records = BUF_memdup(cert->custom_srv_ext_records, sizeof(custom_srv_ext_record) * cert->custom_srv_ext_records_count); + if (ret->custom_srv_ext_records == NULL) + goto err; + ret->custom_srv_ext_records_count = + cert->custom_srv_ext_records_count; + } + +#endif + return(ret); #if !defined(OPENSSL_NO_DH) || !defined(OPENSSL_NO_ECDH) @@ -452,6 +473,13 @@ err: EC_KEY_free(ret->ecdh_tmp); #endif +#ifndef OPENSSL_NO_TLSEXT + if (ret->custom_cli_ext_records) + OPENSSL_free(ret->custom_cli_ext_records); + if (ret->custom_srv_ext_records) + OPENSSL_free(ret->custom_srv_ext_records); +#endif + ssl_cert_clear_certs(ret); return NULL; @@ -542,6 +570,12 @@ void ssl_cert_free(CERT *c) X509_STORE_free(c->chain_store); if (c->ciphers_raw) OPENSSL_free(c->ciphers_raw); +#ifndef OPENSSL_NO_TLSEXT + if (c->custom_cli_ext_records) + OPENSSL_free(c->custom_cli_ext_records); + if (c->custom_srv_ext_records) + OPENSSL_free(c->custom_srv_ext_records); +#endif OPENSSL_free(c); } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index f3a0edca5b..173d9b4904 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1726,7 +1726,7 @@ void SSL_CTX_set_next_proto_select_cb(SSL_CTX *ctx, int (*cb) (SSL *s, unsigned } # endif -int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned short ext_type, +static int cert_set_custom_cli_ext(CERT *cert, unsigned short ext_type, custom_cli_ext_first_cb_fn fn1, custom_cli_ext_second_cb_fn fn2, void* arg) { @@ -1734,19 +1734,19 @@ int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned short ext_type, custom_cli_ext_record* record; /* Check for duplicates */ - for (i=0; i < ctx->custom_cli_ext_records_count; i++) - if (ext_type == ctx->custom_cli_ext_records[i].ext_type) + for (i=0; i < cert->custom_cli_ext_records_count; i++) + if (ext_type == cert->custom_cli_ext_records[i].ext_type) return 0; - ctx->custom_cli_ext_records = OPENSSL_realloc(ctx->custom_cli_ext_records, - (ctx->custom_cli_ext_records_count + 1) * + cert->custom_cli_ext_records = OPENSSL_realloc(cert->custom_cli_ext_records, + (cert->custom_cli_ext_records_count + 1) * sizeof(custom_cli_ext_record)); - if (!ctx->custom_cli_ext_records) { - ctx->custom_cli_ext_records_count = 0; + if (!cert->custom_cli_ext_records) { + cert->custom_cli_ext_records_count = 0; return 0; } - ctx->custom_cli_ext_records_count++; - record = &ctx->custom_cli_ext_records[ctx->custom_cli_ext_records_count - 1]; + cert->custom_cli_ext_records_count++; + record = &cert->custom_cli_ext_records[cert->custom_cli_ext_records_count - 1]; record->ext_type = ext_type; record->fn1 = fn1; record->fn2 = fn2; @@ -1754,7 +1754,7 @@ int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned short ext_type, return 1; } -int SSL_CTX_set_custom_srv_ext(SSL_CTX *ctx, unsigned short ext_type, +static int cert_set_custom_srv_ext(CERT *cert, unsigned short ext_type, custom_srv_ext_first_cb_fn fn1, custom_srv_ext_second_cb_fn fn2, void* arg) { @@ -1762,25 +1762,39 @@ int SSL_CTX_set_custom_srv_ext(SSL_CTX *ctx, unsigned short ext_type, custom_srv_ext_record* record; /* Check for duplicates */ - for (i=0; i < ctx->custom_srv_ext_records_count; i++) - if (ext_type == ctx->custom_srv_ext_records[i].ext_type) + for (i=0; i < cert->custom_srv_ext_records_count; i++) + if (ext_type == cert->custom_srv_ext_records[i].ext_type) return 0; - ctx->custom_srv_ext_records = OPENSSL_realloc(ctx->custom_srv_ext_records, - (ctx->custom_srv_ext_records_count + 1) * + cert->custom_srv_ext_records = OPENSSL_realloc(cert->custom_srv_ext_records, + (cert->custom_srv_ext_records_count + 1) * sizeof(custom_srv_ext_record)); - if (!ctx->custom_srv_ext_records) { - ctx->custom_srv_ext_records_count = 0; + if (!cert->custom_srv_ext_records) { + cert->custom_srv_ext_records_count = 0; return 0; } - ctx->custom_srv_ext_records_count++; - record = &ctx->custom_srv_ext_records[ctx->custom_srv_ext_records_count - 1]; + cert->custom_srv_ext_records_count++; + record = &cert->custom_srv_ext_records[cert->custom_srv_ext_records_count - 1]; record->ext_type = ext_type; record->fn1 = fn1; record->fn2 = fn2; record->arg = arg; return 1; } + +int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned short ext_type, + custom_cli_ext_first_cb_fn fn1, + custom_cli_ext_second_cb_fn fn2, void *arg) + { + return cert_set_custom_cli_ext(ctx->cert, ext_type, fn1, fn2,arg); + } + +int SSL_CTX_set_custom_srv_ext(SSL_CTX *ctx, unsigned short ext_type, + custom_srv_ext_first_cb_fn fn1, + custom_srv_ext_second_cb_fn fn2, void *arg) + { + return cert_set_custom_srv_ext(ctx->cert, ext_type, fn1, fn2,arg); + } /* SSL_CTX_set_alpn_protos sets the ALPN protocol list on |ctx| to |protos|. * |protos| must be in wire-format (i.e. a series of non-empty, 8-bit @@ -2053,10 +2067,6 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) #ifndef OPENSSL_NO_SRP SSL_CTX_SRP_CTX_init(ret); #endif - ret->custom_cli_ext_records = NULL; - ret->custom_cli_ext_records_count = 0; - ret->custom_srv_ext_records = NULL; - ret->custom_srv_ext_records_count = 0; #ifndef OPENSSL_NO_BUF_FREELISTS ret->freelist_max_len = SSL_MAX_BUF_FREELIST_LEN_DEFAULT; ret->rbuf_freelist = OPENSSL_malloc(sizeof(SSL3_BUF_FREELIST)); @@ -2195,10 +2205,6 @@ void SSL_CTX_free(SSL_CTX *a) #ifndef OPENSSL_NO_SRP SSL_CTX_SRP_CTX_free(a); #endif -#ifndef OPENSSL_NO_TLSEXT - OPENSSL_free(a->custom_cli_ext_records); - OPENSSL_free(a->custom_srv_ext_records); -#endif #ifndef OPENSSL_NO_ENGINE if (a->client_cert_engine) ENGINE_finish(a->client_cert_engine); diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 91d55f82c3..655bca1231 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -526,6 +526,20 @@ typedef struct cert_pkey_st #define SSL_CERT_FLAGS_CHECK_TLS_STRICT \ (SSL_CERT_FLAG_SUITEB_128_LOS|SSL_CERT_FLAG_TLS_STRICT) +typedef struct { + unsigned short ext_type; + custom_cli_ext_first_cb_fn fn1; + custom_cli_ext_second_cb_fn fn2; + void *arg; +} custom_cli_ext_record; + +typedef struct { + unsigned short ext_type; + custom_srv_ext_first_cb_fn fn1; + custom_srv_ext_second_cb_fn fn2; + void *arg; +} custom_srv_ext_record; + typedef struct cert_st { /* Current active set */ @@ -621,6 +635,12 @@ typedef struct cert_st unsigned char *ciphers_raw; size_t ciphers_rawlen; + /* Arrays containing the callbacks for custom TLS Extensions. */ + custom_cli_ext_record *custom_cli_ext_records; + size_t custom_cli_ext_records_count; + custom_srv_ext_record *custom_srv_ext_records; + size_t custom_srv_ext_records_count; + int references; /* >1 only if SSL_copy_session_id is used */ } CERT; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index ba2d9ae8f0..78cdc52914 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1446,17 +1446,17 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c } /* Add custom TLS Extensions to ClientHello */ - if (s->ctx->custom_cli_ext_records_count) + if (s->cert->custom_cli_ext_records_count) { size_t i; custom_cli_ext_record* record; - for (i = 0; i < s->ctx->custom_cli_ext_records_count; i++) + for (i = 0; i < s->cert->custom_cli_ext_records_count; i++) { const unsigned char* out = NULL; unsigned short outlen = 0; - record = &s->ctx->custom_cli_ext_records[i]; + record = &s->cert->custom_cli_ext_records[i]; /* NULL callback sends empty extension */ /* -1 from callback omits extension */ if (record->fn1) @@ -1707,13 +1707,13 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned c } #endif - for (i = 0; i < s->ctx->custom_srv_ext_records_count; i++) + for (i = 0; i < s->cert->custom_srv_ext_records_count; i++) { const unsigned char *out = NULL; unsigned short outlen = 0; int cb_retval = 0; - record = &s->ctx->custom_srv_ext_records[i]; + record = &s->cert->custom_srv_ext_records[i]; /* NULL callback or -1 omits extension */ if (!record->fn2) @@ -2444,13 +2444,13 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char * so call the callback and record the extension number so that * an appropriate ServerHello may be later returned. */ - else if (!s->hit && s->ctx->custom_srv_ext_records_count) + else if (!s->hit && s->cert->custom_srv_ext_records_count) { custom_srv_ext_record *record; - for (i=0; i < s->ctx->custom_srv_ext_records_count; i++) + for (i=0; i < s->cert->custom_srv_ext_records_count; i++) { - record = &s->ctx->custom_srv_ext_records[i]; + record = &s->cert->custom_srv_ext_records[i]; if (type == record->ext_type) { if (record->fn1 && !record->fn1(s, type, data, size, al, record->arg)) @@ -2782,14 +2782,14 @@ static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char /* If this extension type was not otherwise handled, but * matches a custom_cli_ext_record, then send it to the c * callback */ - else if (s->ctx->custom_cli_ext_records_count) + else if (s->cert->custom_cli_ext_records_count) { size_t i; custom_cli_ext_record* record; - for (i = 0; i < s->ctx->custom_cli_ext_records_count; i++) + for (i = 0; i < s->cert->custom_cli_ext_records_count; i++) { - record = &s->ctx->custom_cli_ext_records[i]; + record = &s->cert->custom_cli_ext_records[i]; if (record->ext_type == type) { if (record->fn2 && !record->fn2(s, type, data, size, al, record->arg)) -- 2.25.1