From de2a9e38f39eacc2e052d694f5b5fa5b7e734abc Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Thu, 14 Aug 2014 13:25:50 +0100 Subject: [PATCH] Callback revision. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Use "parse" and "add" for function and callback names instead of "first" and "second". Change arguments to callback so the extension type is unsigned int and the buffer length is size_t. Note: this *will* break existing code. Reviewed-by: Emilia Käsper --- apps/s_client.c | 4 +- ssl/ssl.h | 41 +++++++++---------- ssl/ssl_locl.h | 4 +- ssl/ssl_rsa.c | 24 ++++++------ ssl/ssltest.c | 102 ++++++++++++++++++++++++------------------------ ssl/t1_ext.c | 32 +++++++++------ 6 files changed, 104 insertions(+), 103 deletions(-) diff --git a/apps/s_client.c b/apps/s_client.c index e1be6a908b..6a377743a1 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -547,8 +547,8 @@ static int next_proto_cb(SSL *s, unsigned char **out, unsigned char *outlen, con } # endif /* ndef OPENSSL_NO_NEXTPROTONEG */ -static int serverinfo_cli_cb(SSL* s, unsigned short ext_type, - const unsigned char* in, unsigned short inlen, +static int serverinfo_cli_cb(SSL* s, unsigned int ext_type, + const unsigned char* in, size_t inlen, int* al, void* arg) { char pem_name[100]; diff --git a/ssl/ssl.h b/ssl/ssl.h index 2498c3191d..8c59ed3520 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -390,10 +390,10 @@ typedef int (*tls_session_secret_cb_fn)(SSL *s, void *secret, int *secret_len, S #ifndef OPENSSL_NO_TLSEXT /* Callbacks and structures for handling custom TLS Extensions: - * cli_ext_first_cb - sends data for ClientHello TLS Extension - * cli_ext_second_cb - receives data from ServerHello TLS Extension - * srv_ext_first_cb - receives data from ClientHello TLS Extension - * srv_ext_second_cb - sends data for ServerHello TLS Extension + * cli_ext_add_cb - sends data for ClientHello TLS Extension + * cli_ext_parse_cb - receives data from ServerHello TLS Extension + * srv_ext_parse_cb - receives data from ClientHello TLS Extension + * srv_ext_add_cb - sends data for ServerHello TLS Extension * * All these functions return nonzero on success. Zero will terminate * the handshake (and return a specific TLS Fatal alert, if the function @@ -410,21 +410,16 @@ typedef int (*tls_session_secret_cb_fn)(SSL *s, void *secret, int *secret_len, S * fatal TLS alert, if the callback returns zero. */ -typedef int (*custom_ext_add_cb)(SSL *s, unsigned short ext_type, +typedef int (*custom_ext_add_cb)(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, + size_t *outlen, int *al, void *arg); -typedef int (*custom_ext_parse_cb)(SSL *s, unsigned short ext_type, +typedef int (*custom_ext_parse_cb)(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg); -typedef custom_ext_add_cb custom_cli_ext_first_cb_fn; -typedef custom_ext_parse_cb custom_cli_ext_second_cb_fn; -typedef custom_ext_add_cb custom_srv_ext_second_cb_fn; -typedef custom_ext_parse_cb custom_srv_ext_first_cb_fn; - #endif #ifndef OPENSSL_NO_SSL_INTERN @@ -1276,22 +1271,22 @@ const char *SSL_get_psk_identity(const SSL *s); * handled by OpenSSL will fail. * * NULL can be registered for any callback function. For the client - * functions, a NULL custom_cli_ext_first_cb_fn sends an empty ClientHello - * Extension, and a NULL custom_cli_ext_second_cb_fn ignores the ServerHello + * functions, a NULL custom_ext_add_cb sends an empty ClientHello + * Extension, and a NULL custom_ext_parse_cb ignores the ServerHello * response (if any). * - * For the server functions, a NULL custom_srv_ext_first_cb_fn means the + * For the server functions, a NULL custom_ext_parse means the * ClientHello extension's data will be ignored, but the extension will still - * be noted and custom_srv_ext_second_cb_fn will still be invoked. A NULL + * be noted and custom_ext_add_cb will still be invoked. A NULL * custom_srv_ext_second_cb doesn't send a ServerHello extension. */ -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); +int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned int ext_type, + custom_ext_add_cb add_cb, + custom_ext_parse_cb parse_cb, void *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); +int SSL_CTX_set_custom_srv_ext(SSL_CTX *ctx, unsigned int ext_type, + custom_ext_parse_cb parse_cb, + custom_ext_add_cb add_cb, void *arg); #endif diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 8e9110a4ce..d76269d608 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1428,9 +1428,9 @@ int srp_verify_server_param(SSL *s, int *al); void custom_ext_init(custom_ext_methods *meths); int custom_ext_parse(SSL *s, int server, - unsigned short ext_type, + unsigned int ext_type, const unsigned char *ext_data, - unsigned short ext_size, + size_t ext_size, int *al); int custom_ext_add(SSL *s, int server, unsigned char **pret, diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index e599533509..d4f0e28a16 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c @@ -811,9 +811,9 @@ end: #ifndef OPENSSL_NO_TLSEXT static int serverinfo_find_extension(const unsigned char *serverinfo, size_t serverinfo_length, - unsigned short extension_type, + unsigned int extension_type, const unsigned char **extension_data, - unsigned short *extension_length) + size_t *extension_length) { *extension_data = NULL; *extension_length = 0; @@ -821,8 +821,8 @@ static int serverinfo_find_extension(const unsigned char *serverinfo, return 0; for (;;) { - unsigned short type = 0; /* uint16 */ - unsigned short len = 0; /* uint16 */ + unsigned int type = 0; + size_t len = 0; /* end of serverinfo */ if (serverinfo_length == 0) @@ -858,9 +858,9 @@ static int serverinfo_find_extension(const unsigned char *serverinfo, return 0; /* Error */ } -static int serverinfo_srv_first_cb(SSL *s, unsigned short ext_type, +static int serverinfo_srv_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { @@ -873,8 +873,8 @@ static int serverinfo_srv_first_cb(SSL *s, unsigned short ext_type, return 1; } -static int serverinfo_srv_second_cb(SSL *s, unsigned short ext_type, - const unsigned char **out, unsigned short *outlen, +static int serverinfo_srv_add_cb(SSL *s, unsigned int ext_type, + const unsigned char **out, size_t *outlen, int *al, void *arg) { const unsigned char *serverinfo = NULL; @@ -906,8 +906,8 @@ static int serverinfo_process_buffer(const unsigned char *serverinfo, return 0; for (;;) { - unsigned short ext_type = 0; /* uint16 */ - unsigned short len = 0; /* uint16 */ + unsigned int ext_type = 0; + size_t len = 0; /* end of serverinfo */ if (serverinfo_length == 0) @@ -921,8 +921,8 @@ static int serverinfo_process_buffer(const unsigned char *serverinfo, /* Register callbacks for extensions */ ext_type = (serverinfo[0] << 8) + serverinfo[1]; if (ctx && !SSL_CTX_set_custom_srv_ext(ctx, ext_type, - serverinfo_srv_first_cb, - serverinfo_srv_second_cb, NULL)) + serverinfo_srv_parse_cb, + serverinfo_srv_add_cb, NULL)) return 0; serverinfo += 2; diff --git a/ssl/ssltest.c b/ssl/ssltest.c index f20553fc2f..5837abb24f 100644 --- a/ssl/ssltest.c +++ b/ssl/ssltest.c @@ -521,8 +521,8 @@ int custom_ext = 0; /* This set based on extension callbacks */ int custom_ext_error = 0; -static int serverinfo_cli_cb(SSL* s, unsigned short ext_type, - const unsigned char* in, unsigned short inlen, +static int serverinfo_cli_cb(SSL* s, unsigned int ext_type, + const unsigned char* in, size_t inlen, int* al, void* arg) { if (ext_type == SCT_EXT_TYPE) @@ -552,26 +552,26 @@ static int verify_serverinfo() * 3 - ClientHello with "abc", "defg" response */ -static int custom_ext_0_cli_first_cb(SSL *s, unsigned short ext_type, +static int custom_ext_0_cli_add_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, void *arg) + size_t *outlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_0) custom_ext_error = 1; return -1; /* Don't send an extension */ } -static int custom_ext_0_cli_second_cb(SSL *s, unsigned short ext_type, +static int custom_ext_0_cli_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { return 1; } -static int custom_ext_1_cli_first_cb(SSL *s, unsigned short ext_type, +static int custom_ext_1_cli_add_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, void *arg) + size_t *outlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_1) custom_ext_error = 1; @@ -580,17 +580,17 @@ static int custom_ext_1_cli_first_cb(SSL *s, unsigned short ext_type, return 1; /* Send "abc" */ } -static int custom_ext_1_cli_second_cb(SSL *s, unsigned short ext_type, +static int custom_ext_1_cli_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { return 1; } -static int custom_ext_2_cli_first_cb(SSL *s, unsigned short ext_type, +static int custom_ext_2_cli_add_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, void *arg) + size_t *outlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_2) custom_ext_error = 1; @@ -599,9 +599,9 @@ static int custom_ext_2_cli_first_cb(SSL *s, unsigned short ext_type, return 1; /* Send "abc" */ } -static int custom_ext_2_cli_second_cb(SSL *s, unsigned short ext_type, +static int custom_ext_2_cli_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_2) @@ -611,9 +611,9 @@ static int custom_ext_2_cli_second_cb(SSL *s, unsigned short ext_type, return 1; } -static int custom_ext_3_cli_first_cb(SSL *s, unsigned short ext_type, +static int custom_ext_3_cli_add_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, void *arg) + size_t *outlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_3) custom_ext_error = 1; @@ -622,9 +622,9 @@ static int custom_ext_3_cli_first_cb(SSL *s, unsigned short ext_type, return 1; /* Send "abc" */ } -static int custom_ext_3_cli_second_cb(SSL *s, unsigned short ext_type, +static int custom_ext_3_cli_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_3) @@ -636,26 +636,26 @@ static int custom_ext_3_cli_second_cb(SSL *s, unsigned short ext_type, return 1; } -/* custom_ext_0_cli_first_cb returns -1 - the server won't receive a callback for this extension */ -static int custom_ext_0_srv_first_cb(SSL *s, unsigned short ext_type, +/* custom_ext_0_cli_parse_cb returns -1 - the server won't receive a callback for this extension */ +static int custom_ext_0_srv_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { return 1; } /* 'generate' callbacks are always called, even if the 'receive' callback isn't called */ -static int custom_ext_0_srv_second_cb(SSL *s, unsigned short ext_type, +static int custom_ext_0_srv_add_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, void *arg) + size_t *outlen, int *al, void *arg) { return -1; /* Don't send an extension */ } -static int custom_ext_1_srv_first_cb(SSL *s, unsigned short ext_type, +static int custom_ext_1_srv_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_1) @@ -668,16 +668,16 @@ static int custom_ext_1_srv_first_cb(SSL *s, unsigned short ext_type, return 1; } -static int custom_ext_1_srv_second_cb(SSL *s, unsigned short ext_type, +static int custom_ext_1_srv_add_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, void *arg) + size_t *outlen, int *al, void *arg) { return -1; /* Don't send an extension */ } -static int custom_ext_2_srv_first_cb(SSL *s, unsigned short ext_type, +static int custom_ext_2_srv_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_2) @@ -690,18 +690,18 @@ static int custom_ext_2_srv_first_cb(SSL *s, unsigned short ext_type, return 1; } -static int custom_ext_2_srv_second_cb(SSL *s, unsigned short ext_type, +static int custom_ext_2_srv_add_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, void *arg) + size_t *outlen, int *al, void *arg) { *out = NULL; *outlen = 0; return 1; /* Send empty extension */ } -static int custom_ext_3_srv_first_cb(SSL *s, unsigned short ext_type, +static int custom_ext_3_srv_parse_cb(SSL *s, unsigned int ext_type, const unsigned char *in, - unsigned short inlen, int *al, + size_t inlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_3) @@ -714,9 +714,9 @@ static int custom_ext_3_srv_first_cb(SSL *s, unsigned short ext_type, return 1; } -static int custom_ext_3_srv_second_cb(SSL *s, unsigned short ext_type, +static int custom_ext_3_srv_add_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - unsigned short *outlen, int *al, void *arg) + size_t *outlen, int *al, void *arg) { *out = (const unsigned char*)custom_ext_srv_string; *outlen = strlen(custom_ext_srv_string); @@ -1600,31 +1600,31 @@ bad: if (custom_ext) { SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_0, - custom_ext_0_cli_first_cb, - custom_ext_0_cli_second_cb, NULL); + custom_ext_0_cli_add_cb, + custom_ext_0_cli_parse_cb, NULL); SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_1, - custom_ext_1_cli_first_cb, - custom_ext_1_cli_second_cb, NULL); + custom_ext_1_cli_add_cb, + custom_ext_1_cli_parse_cb, NULL); SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_2, - custom_ext_2_cli_first_cb, - custom_ext_2_cli_second_cb, NULL); + custom_ext_2_cli_add_cb, + custom_ext_2_cli_parse_cb, NULL); SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_3, - custom_ext_3_cli_first_cb, - custom_ext_3_cli_second_cb, NULL); + custom_ext_3_cli_add_cb, + custom_ext_3_cli_parse_cb, NULL); SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_0, - custom_ext_0_srv_first_cb, - custom_ext_0_srv_second_cb, NULL); + custom_ext_0_srv_parse_cb, + custom_ext_0_srv_add_cb, NULL); SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_1, - custom_ext_1_srv_first_cb, - custom_ext_1_srv_second_cb, NULL); + custom_ext_1_srv_parse_cb, + custom_ext_1_srv_add_cb, NULL); SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_2, - custom_ext_2_srv_first_cb, - custom_ext_2_srv_second_cb, NULL); + custom_ext_2_srv_parse_cb, + custom_ext_2_srv_add_cb, NULL); SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_3, - custom_ext_3_srv_first_cb, - custom_ext_3_srv_second_cb, NULL); + custom_ext_3_srv_parse_cb, + custom_ext_3_srv_add_cb, NULL); } if (alpn_server) diff --git a/ssl/t1_ext.c b/ssl/t1_ext.c index bd14806e6a..8b6c170ef6 100644 --- a/ssl/t1_ext.c +++ b/ssl/t1_ext.c @@ -87,9 +87,9 @@ void custom_ext_init(custom_ext_methods *exts) /* pass received custom extension data to the application for parsing */ int custom_ext_parse(SSL *s, int server, - unsigned short ext_type, + unsigned int ext_type, const unsigned char *ext_data, - unsigned short ext_size, + size_t ext_size, int *al) { custom_ext_methods *exts = server ? &s->cert->srv_ext : &s->cert->cli_ext; @@ -140,7 +140,7 @@ int custom_ext_add(SSL *s, int server, for (i = 0; i < exts->meths_count; i++) { const unsigned char *out = NULL; - unsigned short outlen = 0; + size_t outlen = 0; meth = exts->meths + i; if (server) @@ -165,7 +165,7 @@ int custom_ext_add(SSL *s, int server, if (cb_retval == -1) continue; /* skip this extension */ } - if (4 > limit - ret || outlen > limit - ret - 4) + if (4 > limit - ret || outlen > (size_t)(limit - ret - 4)) return 0; s2n(meth->ext_type, ret); s2n(outlen, ret); @@ -209,7 +209,7 @@ void custom_exts_free(custom_ext_methods *exts) /* Set callbacks for a custom extension */ static int custom_ext_set(custom_ext_methods *exts, - unsigned short ext_type, + unsigned int ext_type, custom_ext_parse_cb parse_cb, custom_ext_add_cb add_cb, void *arg) @@ -239,6 +239,9 @@ static int custom_ext_set(custom_ext_methods *exts, #endif return 0; } + /* Extension type must fit in 16 bits */ + if (ext_type > 0xffff) + return 0; /* Search for duplicate */ if (custom_ext_find(exts, ext_type)) return 0; @@ -263,17 +266,20 @@ static int custom_ext_set(custom_ext_methods *exts, /* Application level functions to add custom extension callbacks */ -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) +int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned int ext_type, + custom_ext_add_cb add_cb, + custom_ext_parse_cb parse_cb, void *arg) + { - return custom_ext_set(&ctx->cert->cli_ext, ext_type, fn2, fn1, arg); + return custom_ext_set(&ctx->cert->cli_ext, ext_type, parse_cb, add_cb, + 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) +int SSL_CTX_set_custom_srv_ext(SSL_CTX *ctx, unsigned int ext_type, + custom_ext_parse_cb parse_cb, + custom_ext_add_cb add_cb, void *arg) { - return custom_ext_set(&ctx->cert->srv_ext, ext_type, fn1, fn2, arg); + return custom_ext_set(&ctx->cert->srv_ext, ext_type, parse_cb, add_cb, + arg); } #endif -- 2.25.1