From: Matt Caswell Date: Fri, 7 Apr 2017 09:56:59 +0000 (+0100) Subject: Various style tweaks based on feedback X-Git-Tag: OpenSSL_1_1_1-pre1~1843 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=64350ab5877aa30dc8b89cf3373dc28c8b013e19;p=oweals%2Fopenssl.git Various style tweaks based on feedback Style updates for the new custom extensions API Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3139) --- diff --git a/doc/man3/SSL_extension_supported.pod b/doc/man3/SSL_extension_supported.pod index b5e61b6cc9..994454be83 100644 --- a/doc/man3/SSL_extension_supported.pod +++ b/doc/man3/SSL_extension_supported.pod @@ -66,30 +66,31 @@ custom_ext_add_cb, custom_ext_free_cb, custom_ext_parse_cb =head1 DESCRIPTION -SSL_CTX_add_custom_ext() adds a custom extension for a (D)TLS client or server +SSL_CTX_add_custom_ext() adds a custom extension for a TLS/DTLS client or server for all supported protocol versions with extension type B and callbacks B, B and B (see the L section below). The B value determines which messages and under what conditions the extension will be added/parsed (see the L section below). -SSL_CTX_add_client_custom_ext() adds a custom extension for a (D)TLS client with -extension type B and callbacks B, B and B. -This function is similar to SSL_CTX_add_custom_ext() except it only applies -to clients, uses the older style of callbacks, and implicitly sets the +SSL_CTX_add_client_custom_ext() adds a custom extension for a TLS/DTLS client +with extension type B and callbacks B, B and +B. This function is similar to SSL_CTX_add_custom_ext() except it only +applies to clients, uses the older style of callbacks, and implicitly sets the B value to: SSL_EXT_TLS1_2_AND_BELOW_ONLY | SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO | SSL_EXT_IGNORE_ON_RESUMPTION -SSL_CTX_add_server_custom_ext() adds a custom extension for a (D)TLS server with -extension type B and callbacks B, B and +SSL_CTX_add_server_custom_ext() adds a custom extension for a TLS/DTLS server +with extension type B and callbacks B, B and B. This function is similar to SSL_CTX_add_custom_ext() except it only applies to servers, uses the older style of callbacks, and implicitly sets the B value to the same as for SSL_CTX_add_client_custom_ext() above. -In all cases the extension type must not be handled by OpenSSL internally -or an error occurs. +The B parameter corresponds to the B field of +RFC5246 et al. It is B a NID. In all cases the extension type must not be +handled by OpenSSL internally or an error occurs. SSL_extension_supported() returns 1 if the extension B is handled internally by OpenSSL and 0 otherwise. @@ -112,7 +113,7 @@ If the B does not wish to include the extension it must return 0. If B returns -1 a fatal handshake error occurs using the TLS alert value specified in B<*al>. -When constructing the ClientHello if B is set to NULL a zero length +When constructing the ClientHello, if B is set to NULL a zero length extension is added for B. For all other messages if B is set to NULL then no extension is added. @@ -120,7 +121,8 @@ When constructing a Certificate message the callback will be called for each certificate in the message. The B parameter will indicate the current certificate and the B parameter will indicate the position of the certificate in the message. The first certificate is always the end -entity certificate and has a B value of 0. +entity certificate and has a B value of 0. The certificates are in the +order that they were received in the Certificate message. For all messages except the ServerHello and EncryptedExtensions every registered B is always called to see if the application wishes to add an @@ -188,8 +190,9 @@ the extension in SSLv3. Applications will not typically need to use this. =item SSL_EXT_TLS1_2_AND_BELOW_ONLY -The extension is only defined for (D)TLSv1.2 and below. Servers will ignore this -extension if it is present in the ClientHello and TLSv1.3 is negotiated. +The extension is only defined for TLSv1.2/DTLSv1.2 and below. Servers will +ignore this extension if it is present in the ClientHello and TLSv1.3 is +negotiated. =item SSL_EXT_TLS1_3_ONLY @@ -247,9 +250,6 @@ which will be passed to the corresponding callbacks. They can, for example, be used to store the extension data received in a convenient structure or pass the extension data to be added or freed when adding extensions. -The B parameter corresponds to the B field of -RFC5246 et al. It is B a NID. - If the same custom extension type is received multiple times a fatal B alert is sent and the handshake aborts. If a custom extension is received in a ServerHello/EncryptedExtensions message which was not sent in diff --git a/ssl/statem/extensions_cust.c b/ssl/statem/extensions_cust.c index ec1ab6d539..6bd50a7297 100644 --- a/ssl/statem/extensions_cust.c +++ b/ssl/statem/extensions_cust.c @@ -79,8 +79,8 @@ custom_ext_method *custom_ext_find(const custom_ext_methods *exts, int server, unsigned int ext_type, size_t *idx) { size_t i; - custom_ext_method *meth = exts->meths; + for (i = 0; i < exts->meths_count; i++, meth++) { if (ext_type == meth->ext_type && (server == -1 || server == meth->server @@ -100,6 +100,7 @@ void custom_ext_init(custom_ext_methods *exts) { size_t i; custom_ext_method *meth = exts->meths; + for (i = 0; i < exts->meths_count; i++, meth++) meth->ext_flags = 0; } @@ -192,9 +193,10 @@ int custom_ext_add(SSL *s, int context, WPACKET *pkt, X509 *x, size_t chainidx, continue; if (meth->add_cb != NULL) { - int cb_retval = 0; - cb_retval = meth->add_cb(s, meth->ext_type, context, &out, &outlen, - x, chainidx, al, meth->add_arg); + int cb_retval = meth->add_cb(s, meth->ext_type, context, &out, + &outlen, x, chainidx, al, + meth->add_arg); + if (cb_retval < 0) return 0; /* error */ if (cb_retval == 0) @@ -212,7 +214,7 @@ int custom_ext_add(SSL *s, int context, WPACKET *pkt, X509 *x, size_t chainidx, /* * We can't send duplicates: code logic should prevent this. */ - assert(!(meth->ext_flags & SSL_EXT_FLAG_SENT)); + assert((meth->ext_flags & SSL_EXT_FLAG_SENT) == 0); /* * Indicate extension has been sent: this is both a sanity check to * ensure we don't send duplicate extensions and indicates that it @@ -220,7 +222,7 @@ int custom_ext_add(SSL *s, int context, WPACKET *pkt, X509 *x, size_t chainidx, */ meth->ext_flags |= SSL_EXT_FLAG_SENT; } - if (meth->free_cb) + if (meth->free_cb != NULL) meth->free_cb(s, meth->ext_type, context, out, meth->add_arg); } return 1; @@ -235,7 +237,7 @@ int custom_exts_copy(custom_ext_methods *dst, const custom_ext_methods *src) if (src->meths_count > 0) { dst->meths = OPENSSL_memdup(src->meths, - sizeof(custom_ext_method) * src->meths_count); + sizeof(*src->meths) * src->meths_count); if (dst->meths == NULL) return 0; dst->meths_count = src->meths_count; @@ -279,10 +281,9 @@ int custom_exts_copy(custom_ext_methods *dst, const custom_ext_methods *src) void custom_exts_free(custom_ext_methods *exts) { size_t i; + custom_ext_method *meth; - for (i = 0; i < exts->meths_count; i++) { - custom_ext_method *meth = exts->meths + i; - + for (i = 0, meth = exts->meths; i < exts->meths_count; i++, meth++) { if (meth->add_cb != custom_ext_add_old_cb_wrap) continue; @@ -315,7 +316,7 @@ static int add_custom_ext_intern(SSL_CTX *ctx, int server, * Check application error: if add_cb is not set free_cb will never be * called. */ - if (!add_cb && free_cb) + if (add_cb == NULL && free_cb != NULL) return 0; #ifndef OPENSSL_NO_CT @@ -346,7 +347,6 @@ static int add_custom_ext_intern(SSL_CTX *ctx, int server, return 0; tmp = OPENSSL_realloc(exts->meths, (exts->meths_count + 1) * sizeof(custom_ext_method)); - if (tmp == NULL) return 0; @@ -373,9 +373,9 @@ static int add_old_custom_ext(SSL_CTX *ctx, int server, unsigned int ext_type, custom_ext_parse_cb parse_cb, void *parse_arg) { custom_ext_add_cb_wrap *add_cb_wrap - = OPENSSL_malloc(sizeof(custom_ext_add_cb_wrap)); + = OPENSSL_malloc(sizeof(*add_cb_wrap)); custom_ext_parse_cb_wrap *parse_cb_wrap - = OPENSSL_malloc(sizeof(custom_ext_parse_cb_wrap)); + = OPENSSL_malloc(sizeof(*parse_cb_wrap)); int ret; if (add_cb_wrap == NULL || parse_cb_wrap == NULL) {