From f1b97da1fd90cf3935eafedc8df0d0165cb75f2f Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 7 Sep 2017 18:53:05 -0400 Subject: [PATCH] Introduce named constants for the ClientHello callback. It is otherwise unclear what all the magic numbers mean. Reviewed-by: Rich Salz Reviewed-by: Ben Kaduk (Merged from https://github.com/openssl/openssl/pull/4349) --- doc/man3/SSL_CTX_set_client_hello_cb.pod | 5 +++-- include/openssl/ssl.h | 5 +++++ ssl/statem/statem_srvr.c | 14 ++++++++------ test/handshake_helper.c | 18 ++++++++++-------- test/sslapitest.c | 10 +++++----- 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/doc/man3/SSL_CTX_set_client_hello_cb.pod b/doc/man3/SSL_CTX_set_client_hello_cb.pod index 18bbc2938d..6824b5b8d1 100644 --- 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 45d0083c58..9aac454c6c 100644 --- 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 360cd1c20b..81c8ee4f21 100644 --- 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 5e5c311cf3..3d59abc66b 100644 --- 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 7437020d58..5299d5794b 100644 --- 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) -- 2.25.1