Introduce named constants for the ClientHello callback.

It is otherwise unclear what all the magic numbers mean.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4349)

diff --git a/doc/man3/SSL_CTX_set_client_hello_cb.pod b/doc/man3/SSL_CTX_set_client_hello_cb.pod
index 18bbc2938d6a43e2589d42caf481cd8946a31466..6824b5b8d1a4c36b36d07b9dccda73c76c82bdd3 100644 (file)
--- a/doc/man3/SSL_CTX_set_client_hello_cb.pod
+++ b/doc/man3/SSL_CTX_set_client_hello_cb.pod
@@ -88,8 +88,9 @@ within a ClientHello callback.
 
 =head1 RETURN VALUES
 
-The application's supplied ClientHello callback returns 1 on success, 0 on failure,
-and a negative value to suspend processing.
+The application's supplied ClientHello callback returns
+SSL_CLIENT_HELLO_SUCCESS on success, SSL_CLIENT_HELLO_ERROR on failure, and
+SSL_CLIENT_HELLO_RETRY to suspend processing.
 
 SSL_client_hello_isv2() returns 1 for SSLv2-format ClientHellos and 0 otherwise.
 

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 45d0083c58f666607bb97b2ab54bb0ee1a723bf1..9aac454c6cd9e97a02458c88a97aadff9059f400 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1699,6 +1699,11 @@ __owur char *SSL_get_srp_userinfo(SSL *s);
 /*
  * ClientHello callback and helpers.
  */
+
+# define SSL_CLIENT_HELLO_SUCCESS 1
+# define SSL_CLIENT_HELLO_ERROR   0
+# define SSL_CLIENT_HELLO_RETRY   (-1)
+
 typedef int (*SSL_client_hello_cb_fn) (SSL *s, int *al, void *arg);
 void SSL_CTX_set_client_hello_cb(SSL_CTX *c, SSL_client_hello_cb_fn cb,
                                  void *arg);

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 360cd1c20b6d43906781dd298b2f46a1d07a553b..81c8ee4f21eb519cad1df50655d50a92cc2e9eb1 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1432,14 +1432,16 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal)
     /* Finished parsing the ClientHello, now we can start processing it */
     /* Give the ClientHello callback a crack at things */
     if (s->ctx->client_hello_cb != NULL) {
-        int code;
         /* A failure in the ClientHello callback terminates the connection. */
-        code = s->ctx->client_hello_cb(s, &al, s->ctx->client_hello_cb_arg);
-        if (code == 0)
-            goto err;
-        if (code < 0) {
+        switch (s->ctx->client_hello_cb(s, &al, s->ctx->client_hello_cb_arg)) {
+        case SSL_CLIENT_HELLO_SUCCESS:
+            break;
+        case SSL_CLIENT_HELLO_RETRY:
             s->rwstate = SSL_CLIENT_HELLO_CB;
-            return code;
+            return -1;
+        case SSL_CLIENT_HELLO_ERROR:
+        default:
+            goto err;
         }
     }
 

diff --git a/test/handshake_helper.c b/test/handshake_helper.c
index 5e5c311cf35c5e45aaeb64e7b542cd234e7decff..3d59abc66bb9160db236bcdd562ce6e32a15d490 100644 (file)
--- a/test/handshake_helper.c
+++ b/test/handshake_helper.c
@@ -224,18 +224,18 @@ static int client_hello_ignore_cb(SSL *s, int *al, void *arg)
 {
     if (!client_hello_select_server_ctx(s, arg, 1)) {
         *al = SSL_AD_UNRECOGNIZED_NAME;
-        return 0;
+        return SSL_CLIENT_HELLO_ERROR;
     }
-    return 1;
+    return SSL_CLIENT_HELLO_SUCCESS;
 }
 
 static int client_hello_reject_cb(SSL *s, int *al, void *arg)
 {
     if (!client_hello_select_server_ctx(s, arg, 0)) {
         *al = SSL_AD_UNRECOGNIZED_NAME;
-        return 0;
+        return SSL_CLIENT_HELLO_ERROR;
     }
-    return 1;
+    return SSL_CLIENT_HELLO_SUCCESS;
 }
 
 static int client_hello_nov12_cb(SSL *s, int *al, void *arg)
@@ -247,7 +247,7 @@ static int client_hello_nov12_cb(SSL *s, int *al, void *arg)
     v = SSL_client_hello_get0_legacy_version(s);
     if (v > TLS1_2_VERSION || v < SSL3_VERSION) {
         *al = SSL_AD_PROTOCOL_VERSION;
-        return 0;
+        return SSL_CLIENT_HELLO_ERROR;
     }
     (void)SSL_client_hello_get0_session_id(s, &p);
     if (p == NULL ||
@@ -255,13 +255,15 @@ static int client_hello_nov12_cb(SSL *s, int *al, void *arg)
         SSL_client_hello_get0_ciphers(s, &p) == 0 ||
         SSL_client_hello_get0_compression_methods(s, &p) == 0) {
         *al = SSL_AD_INTERNAL_ERROR;
-        return 0;
+        return SSL_CLIENT_HELLO_ERROR;
     }
     ret = client_hello_select_server_ctx(s, arg, 0);
     SSL_set_max_proto_version(s, TLS1_1_VERSION);
-    if (!ret)
+    if (!ret) {
         *al = SSL_AD_UNRECOGNIZED_NAME;
-    return ret;
+        return SSL_CLIENT_HELLO_ERROR;
+    }
+    return SSL_CLIENT_HELLO_SUCCESS;
 }
 
 static unsigned char dummy_ocsp_resp_good_val = 0xff;

diff --git a/test/sslapitest.c b/test/sslapitest.c
index 7437020d58dee13719c0a50dd1e4d3341cce5bb4..5299d5794bd056f43cd94c866970aeaa773ae032 100644 (file)
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -422,24 +422,24 @@ static int full_client_hello_callback(SSL *s, int *al, void *arg)
 
     /* Make sure we can defer processing and get called back. */
     if ((*ctr)++ == 0)
-        return -1;
+        return SSL_CLIENT_HELLO_RETRY;
 
     len = SSL_client_hello_get0_ciphers(s, &p);
     if (!TEST_mem_eq(p, len, expected_ciphers, sizeof(expected_ciphers))
             || !TEST_size_t_eq(
                        SSL_client_hello_get0_compression_methods(s, &p), 1)
             || !TEST_int_eq(*p, 0))
-        return 0;
+        return SSL_CLIENT_HELLO_ERROR;
     if (!SSL_client_hello_get1_extensions_present(s, &exts, &len))
-        return 0;
+        return SSL_CLIENT_HELLO_ERROR;
     if (len != OSSL_NELEM(expected_extensions) ||
         memcmp(exts, expected_extensions, len * sizeof(*exts)) != 0) {
         printf("ClientHello callback expected extensions mismatch\n");
         OPENSSL_free(exts);
-        return 0;
+        return SSL_CLIENT_HELLO_ERROR;
     }
     OPENSSL_free(exts);
-    return 1;
+    return SSL_CLIENT_HELLO_SUCCESS;
 }
 
 static int test_client_hello_cb(void)