Name |select_certificate_cb| return values
The |select_certificate_cb| return values are somewhat confusing due
to the fact that they don't match the |cert_cb| ones, despite the
similarities between the two callbacks (they both have "certificate" in
the name! well, sort of).
This also documents the error return value (-1) which was previously
undocumented, and it expands the |SSL_CTX_set_select_certificate_cb|
documentation regarding retrial (by shamelessly copying from
|SSL_CTX_set_ticket_aead_method|).
Also updates other scattered documentation that was missed by previous
changes.
Change-Id: Ib962b31d08e6475e09954cbc3c939988b0ba13f7
Reviewed-on: https://boringssl-review.googlesource.com/14245
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 11f8ef2..2b52ac3 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3144,6 +3144,20 @@
size_t extensions_len;
} SSL_CLIENT_HELLO;
+/* ssl_select_cert_result_t enumerates the possible results from selecting a
+ * certificate with |select_certificate_cb|. */
+enum ssl_select_cert_result_t {
+ /* ssl_select_cert_success indicates that the certificate selection was
+ * successful. */
+ ssl_select_cert_success = 1,
+ /* ssl_select_cert_retry indicates that the operation could not be
+ * immediately completed and must be reattempted at a later point. */
+ ssl_select_cert_retry = 0,
+ /* ssl_select_cert_error indicates that a fatal error occured and the
+ * handshake should be terminated. */
+ ssl_select_cert_error = -1,
+};
+
/* SSL_early_callback_ctx_extension_get searches the extensions in
* |client_hello| for an extension of the given type. If not found, it returns
* zero. Otherwise it sets |out_data| to point to the extension contents (not
@@ -3156,14 +3170,18 @@
/* SSL_CTX_set_select_certificate_cb sets a callback that is called before most
* ClientHello processing and before the decision whether to resume a session
* is made. The callback may inspect the ClientHello and configure the
- * connection. It may then return one to continue the handshake or zero to
- * pause the handshake to perform an asynchronous operation. If paused,
- * |SSL_get_error| will return |SSL_ERROR_PENDING_CERTIFICATE|.
+ * connection. See |ssl_select_cert_result_t| for details of the return values.
+ *
+ * In the case that a retry is indicated, |SSL_get_error| will return
+ * |SSL_ERROR_PENDING_CERTIFICATE| and the caller should arrange for the
+ * high-level operation on |ssl| to be retried at a later time, which will
+ * result in another call to |cb|.
*
* Note: The |SSL_CLIENT_HELLO| is only valid for the duration of the callback
* and is not valid while the handshake is paused. */
OPENSSL_EXPORT void SSL_CTX_set_select_certificate_cb(
- SSL_CTX *ctx, int (*cb)(const SSL_CLIENT_HELLO *));
+ SSL_CTX *ctx,
+ enum ssl_select_cert_result_t (*cb)(const SSL_CLIENT_HELLO *));
/* SSL_CTX_set_dos_protection_cb sets a callback that is called once the
* resumption decision for a ClientHello has been made. It can return one to
@@ -4112,12 +4130,10 @@
X509_VERIFY_PARAM *param;
/* select_certificate_cb is called before most ClientHello processing and
- * before the decision whether to resume a session is made. It may return one
- * to continue the handshake or zero to cause the handshake loop to return
- * with an error and cause SSL_get_error to return
- * SSL_ERROR_PENDING_CERTIFICATE. Note: when the handshake loop is resumed, it
- * will not call the callback a second time. */
- int (*select_certificate_cb)(const SSL_CLIENT_HELLO *);
+ * before the decision whether to resume a session is made. See
+ * |ssl_select_cert_result_t| for details of the return values. */
+ enum ssl_select_cert_result_t (*select_certificate_cb)(
+ const SSL_CLIENT_HELLO *);
/* dos_protection_cb is called once the resumption decision for a ClientHello
* has been made. It returns one to continue the handshake or zero to
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 81e45ef..fd6c8e9 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -812,11 +812,11 @@
/* Run the early callback. */
if (ssl->ctx->select_certificate_cb != NULL) {
switch (ssl->ctx->select_certificate_cb(&client_hello)) {
- case 0:
+ case ssl_select_cert_retry:
ssl->rwstate = SSL_CERTIFICATE_SELECTION_PENDING;
return -1;
- case -1:
+ case ssl_select_cert_error:
/* Connection rejected. */
OPENSSL_PUT_ERROR(SSL, SSL_R_CONNECTION_REJECTED);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index d16c952..e3bcb91 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2397,8 +2397,9 @@
int SSL_is_dtls(const SSL *ssl) { return ssl->method->is_dtls; }
-void SSL_CTX_set_select_certificate_cb(SSL_CTX *ctx,
- int (*cb)(const SSL_CLIENT_HELLO *)) {
+void SSL_CTX_set_select_certificate_cb(
+ SSL_CTX *ctx,
+ enum ssl_select_cert_result_t (*cb)(const SSL_CLIENT_HELLO *)) {
ctx->select_certificate_cb = cb;
}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 411ddb7..6678b57 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -2144,17 +2144,6 @@
return SSL_TLSEXT_ERR_OK;
}
-static int SwitchSessionIDContextEarly(const SSL_CLIENT_HELLO *client_hello) {
- static const uint8_t kContext[] = {3};
-
- if (!SSL_set_session_id_context(client_hello->ssl, kContext,
- sizeof(kContext))) {
- return -1;
- }
-
- return 1;
-}
-
static bool TestSessionIDContext(bool is_dtls, const SSL_METHOD *method,
uint16_t version) {
bssl::UniquePtr<X509> cert = GetTestCertificate();
@@ -2226,8 +2215,18 @@
// Switch the session ID context with the early callback instead.
SSL_CTX_set_tlsext_servername_callback(server_ctx.get(), nullptr);
- SSL_CTX_set_select_certificate_cb(server_ctx.get(),
- SwitchSessionIDContextEarly);
+ SSL_CTX_set_select_certificate_cb(
+ server_ctx.get(),
+ [](const SSL_CLIENT_HELLO *client_hello) -> ssl_select_cert_result_t {
+ static const uint8_t kContext[] = {3};
+
+ if (!SSL_set_session_id_context(client_hello->ssl, kContext,
+ sizeof(kContext))) {
+ return ssl_select_cert_error;
+ }
+
+ return ssl_select_cert_success;
+ });
if (!ExpectSessionReused(client_ctx.get(), server_ctx.get(), session.get(),
false /* expect session not reused */)) {
@@ -2614,12 +2613,13 @@
ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_3_VERSION));
SSL_CTX_set_select_certificate_cb(
- server_ctx.get(), [](const SSL_CLIENT_HELLO *client_hello) -> int {
+ server_ctx.get(),
+ [](const SSL_CLIENT_HELLO *client_hello) -> ssl_select_cert_result_t {
if (!SSL_set_max_proto_version(client_hello->ssl, TLS1_2_VERSION)) {
- return -1;
+ return ssl_select_cert_error;
}
- return 1;
+ return ssl_select_cert_success;
});
bssl::UniquePtr<SSL> client, server;
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 804cbbb..2c70361 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -533,7 +533,8 @@
return true;
}
-static int SelectCertificateCallback(const SSL_CLIENT_HELLO *client_hello) {
+static enum ssl_select_cert_result_t SelectCertificateCallback(
+ const SSL_CLIENT_HELLO *client_hello) {
const TestConfig *config = GetTestConfig(client_hello->ssl);
GetTestState(client_hello->ssl)->early_callback_called = true;
@@ -547,7 +548,7 @@
client_hello, TLSEXT_TYPE_server_name, &extension_data,
&extension_len)) {
fprintf(stderr, "Could not find server_name extension.\n");
- return -1;
+ return ssl_select_cert_error;
}
CBS_init(&extension, extension_data, extension_len);
@@ -558,7 +559,7 @@
!CBS_get_u16_length_prefixed(&server_name_list, &host_name) ||
CBS_len(&server_name_list) != 0) {
fprintf(stderr, "Could not decode server_name extension.\n");
- return -1;
+ return ssl_select_cert_error;
}
if (!CBS_mem_equal(&host_name,
@@ -569,7 +570,7 @@
}
if (config->fail_early_callback) {
- return -1;
+ return ssl_select_cert_error;
}
// Install the certificate in the early callback.
@@ -578,13 +579,13 @@
GetTestState(client_hello->ssl)->early_callback_ready;
if (config->async && !early_callback_ready) {
// Install the certificate asynchronously.
- return 0;
+ return ssl_select_cert_retry;
}
if (!InstallCertificate(client_hello->ssl)) {
- return -1;
+ return ssl_select_cert_error;
}
}
- return 1;
+ return ssl_select_cert_success;
}
static bool CheckCertificateRequest(SSL *ssl) {
