Various style tweaks based on feedback

Style updates for the new custom extensions API

Reviewed-by: Rich Salz <rsalz@openssl.org>
(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 b5e61b6cc97ea928120db27b29a724dccccbfda2..994454be83886885f3c96ce465f8a2885eb7bea3 100644 (file)
--- 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<ext_type> and
 callbacks B<add_cb>, B<free_cb> and B<parse_cb> (see the
 L</EXTENSION CALLBACKS> section below). The B<context> value determines
 which messages and under what conditions the extension will be added/parsed (see
 the L</EXTENSION CONTEXTS> section below).
 
-SSL_CTX_add_client_custom_ext() adds a custom extension for a (D)TLS client with
-extension type B<ext_type> and callbacks B<add_cb>, B<free_cb> and B<parse_cb>.
-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<ext_type> and callbacks B<add_cb>, B<free_cb> and
+B<parse_cb>. 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<context> 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<ext_type> and callbacks B<add_cb>, B<free_cb> and
+SSL_CTX_add_server_custom_ext() adds a custom extension for a TLS/DTLS server
+with extension type B<ext_type> and callbacks B<add_cb>, B<free_cb> and
 B<parse_cb>. 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<context> 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<ext_type> parameter corresponds to the B<extension_type> field of
+RFC5246 et al. It is B<not> 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<ext_type> is handled
 internally by OpenSSL and 0 otherwise.
@@ -112,7 +113,7 @@ If the B<add_cb> does not wish to include the extension it must return 0.
 If B<add_cb> returns -1 a fatal handshake error occurs using the TLS
 alert value specified in B<*al>.
 
-When constructing the ClientHello if B<add_cb> is set to NULL a zero length
+When constructing the ClientHello, if B<add_cb> is set to NULL a zero length
 extension is added for B<ext_type>. For all other messages if B<add_cb> 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<x> parameter will indicate the
 current certificate and the B<chainidx> parameter will indicate the position
 of the certificate in the message. The first certificate is always the end
-entity certificate and has a B<chainidx> value of 0.
+entity certificate and has a B<chainidx> 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<add_cb> 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<ext_type> parameter corresponds to the B<extension_type> field of
-RFC5246 et al. It is B<not> a NID.
-
 If the same custom extension type is received multiple times a fatal
 B<decode_error> 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 ec1ab6d539080e97554d4d9ee3f304674b236df6..6bd50a7297d9bbe9e8f653eedb340fb82d067c62 100644 (file)
--- 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) {