diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index cad9750b2..828938dc2 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -508,12 +508,10 @@ OPENSSL_EXPORT int SSL_get_error(const SSL *ssl, int ret_code); // TODO(davidben): Remove this. It's used by accept BIOs which are bizarre. #define SSL_ERROR_WANT_ACCEPT 8 -// SSL_ERROR_WANT_CHANNEL_ID_LOOKUP indicates the operation failed looking up -// the Channel ID key. The caller may retry the operation when |channel_id_cb| -// is ready to return a key or one has been configured with -// |SSL_set1_tls_channel_id|. +// SSL_ERROR_WANT_CHANNEL_ID_LOOKUP is never used. // -// See also |SSL_CTX_set_channel_id_cb|. +// TODO(davidben): Remove this. Some callers reference it when stringifying +// errors. They should use |SSL_error_description| instead. #define SSL_ERROR_WANT_CHANNEL_ID_LOOKUP 9 // SSL_ERROR_PENDING_SESSION indicates the operation failed because the session @@ -2974,15 +2972,16 @@ OPENSSL_EXPORT int SSL_select_next_proto(uint8_t **out, uint8_t *out_len, // Channel ID. // -// See draft-balfanz-tls-channelid-01. +// See draft-balfanz-tls-channelid-01. This is an old, experimental mechanism +// and should not be used in new code. // SSL_CTX_set_tls_channel_id_enabled configures whether connections associated -// with |ctx| should enable Channel ID. +// with |ctx| should enable Channel ID as a server. OPENSSL_EXPORT void SSL_CTX_set_tls_channel_id_enabled(SSL_CTX *ctx, int enabled); // SSL_set_tls_channel_id_enabled configures whether |ssl| should enable Channel -// ID. +// ID as a server. OPENSSL_EXPORT void SSL_set_tls_channel_id_enabled(SSL *ssl, int enabled); // SSL_CTX_set1_tls_channel_id configures a TLS client to send a TLS Channel ID @@ -3005,20 +3004,6 @@ OPENSSL_EXPORT int SSL_set1_tls_channel_id(SSL *ssl, EVP_PKEY *private_key); OPENSSL_EXPORT size_t SSL_get_tls_channel_id(SSL *ssl, uint8_t *out, size_t max_out); -// SSL_CTX_set_channel_id_cb sets a callback to be called when a TLS Channel ID -// is requested. The callback may set |*out_pkey| to a key, passing a reference -// to the caller. If none is returned, the handshake will pause and -// |SSL_get_error| will return |SSL_ERROR_WANT_CHANNEL_ID_LOOKUP|. -// -// See also |SSL_ERROR_WANT_CHANNEL_ID_LOOKUP|. -OPENSSL_EXPORT void SSL_CTX_set_channel_id_cb( - SSL_CTX *ctx, void (*channel_id_cb)(SSL *ssl, EVP_PKEY **out_pkey)); - -// SSL_CTX_get_channel_id_cb returns the callback set by -// |SSL_CTX_set_channel_id_cb|. -OPENSSL_EXPORT void (*SSL_CTX_get_channel_id_cb(SSL_CTX *ctx))( - SSL *ssl, EVP_PKEY **out_pkey); - // DTLS-SRTP. // diff --git a/ssl/handshake.cc b/ssl/handshake.cc index ca2238f04..ebd5df005 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc @@ -684,10 +684,6 @@ int ssl_run_handshake(SSL_HANDSHAKE *hs, bool *out_early_return) { ssl->s3->rwstate = SSL_ERROR_WANT_X509_LOOKUP; hs->wait = ssl_hs_ok; return -1; - case ssl_hs_channel_id_lookup: - ssl->s3->rwstate = SSL_ERROR_WANT_CHANNEL_ID_LOOKUP; - hs->wait = ssl_hs_ok; - return -1; case ssl_hs_private_key_operation: ssl->s3->rwstate = SSL_ERROR_WANT_PRIVATE_KEY_OPERATION; hs->wait = ssl_hs_ok; diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index b7432ecce..fbf0ef5df 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc @@ -1480,18 +1480,6 @@ static enum ssl_hs_wait_t do_send_client_certificate_verify(SSL_HANDSHAKE *hs) { static enum ssl_hs_wait_t do_send_client_finished(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; hs->can_release_private_key = true; - // Resolve Channel ID first, before any non-idempotent operations. - if (hs->channel_id_negotiated) { - if (!ssl_do_channel_id_callback(hs)) { - return ssl_hs_error; - } - - if (hs->config->channel_id_private == NULL) { - hs->state = state_send_client_finished; - return ssl_hs_channel_id_lookup; - } - } - if (!ssl->method->add_change_cipher_spec(ssl) || !tls1_change_cipher_state(hs, evp_aead_seal)) { return ssl_hs_error; diff --git a/ssl/internal.h b/ssl/internal.h index 7be766d74..ad31e54d5 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1557,7 +1557,6 @@ enum ssl_hs_wait_t { ssl_hs_handoff, ssl_hs_handback, ssl_hs_x509_lookup, - ssl_hs_channel_id_lookup, ssl_hs_private_key_operation, ssl_hs_pending_session, ssl_hs_pending_ticket, @@ -2862,7 +2861,8 @@ struct SSL_CONFIG { Array supported_group_list; // our list - // The client's Channel ID private key. + // channel_id_private is the client's Channel ID private key, or null if + // Channel ID should not be offered on this connection. UniquePtr channel_id_private; // For a client, this contains the list of supported protocols in wire @@ -2901,9 +2901,8 @@ struct SSL_CONFIG { // whether OCSP stapling will be requested. bool ocsp_stapling_enabled : 1; - // channel_id_enabled is copied from the |SSL_CTX|. For a server, means that - // we'll accept Channel IDs from clients. For a client, means that we'll - // advertise support. + // channel_id_enabled is copied from the |SSL_CTX|. For a server, it means + // that we'll accept Channel IDs from clients. It is ignored on the client. bool channel_id_enabled : 1; // If enforce_rsa_key_usage is true, the handshake will fail if the @@ -3195,12 +3194,6 @@ bool tls1_channel_id_hash(SSL_HANDSHAKE *hs, uint8_t *out, size_t *out_len); // data. bool tls1_record_handshake_hashes_for_channel_id(SSL_HANDSHAKE *hs); -// ssl_do_channel_id_callback checks runs |hs->ssl->ctx->channel_id_cb| if -// necessary. It returns true on success and false on fatal error. Note that, on -// success, |hs->ssl->channel_id_private| may be unset, in which case the -// operation should be retried later. -bool ssl_do_channel_id_callback(SSL_HANDSHAKE *hs); - // ssl_can_write returns whether |ssl| is allowed to write. bool ssl_can_write(const SSL *ssl); @@ -3324,9 +3317,6 @@ struct ssl_ctx_st { int (*client_cert_cb)(SSL *ssl, X509 **out_x509, EVP_PKEY **out_pkey) = nullptr; - // get channel id callback - void (*channel_id_cb)(SSL *ssl, EVP_PKEY **out_pkey) = nullptr; - CRYPTO_EX_DATA ex_data; // Default values used when no per-SSL value is defined follow @@ -3454,7 +3444,8 @@ struct ssl_ctx_st { // Supported group values inherited by SSL structure bssl::Array supported_group_list; - // The client's Channel ID private key. + // channel_id_private is the client's Channel ID private key, or null if + // Channel ID should not be offered on this connection. bssl::UniquePtr channel_id_private; // ech_server_config_list contains the server's list of ECHConfig values and diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index a05b9b1f7..65dcfae19 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc @@ -1369,7 +1369,6 @@ int SSL_get_error(const SSL *ssl, int ret_code) { case SSL_ERROR_HANDOFF: case SSL_ERROR_HANDBACK: case SSL_ERROR_WANT_X509_LOOKUP: - case SSL_ERROR_WANT_CHANNEL_ID_LOOKUP: case SSL_ERROR_WANT_PRIVATE_KEY_OPERATION: case SSL_ERROR_PENDING_TICKET: case SSL_ERROR_EARLY_DATA_REJECTED: @@ -1443,8 +1442,6 @@ const char *SSL_error_description(int err) { return "WANT_CONNECT"; case SSL_ERROR_WANT_ACCEPT: return "WANT_ACCEPT"; - case SSL_ERROR_WANT_CHANNEL_ID_LOOKUP: - return "WANT_CHANNEL_ID_LOOKUP"; case SSL_ERROR_PENDING_SESSION: return "PENDING_SESSION"; case SSL_ERROR_PENDING_CERTIFICATE: @@ -2437,8 +2434,6 @@ int SSL_CTX_set1_tls_channel_id(SSL_CTX *ctx, EVP_PKEY *private_key) { } ctx->channel_id_private = UpRef(private_key); - ctx->channel_id_enabled = true; - return 1; } @@ -2452,8 +2447,6 @@ int SSL_set1_tls_channel_id(SSL *ssl, EVP_PKEY *private_key) { } ssl->config->channel_id_private = UpRef(private_key); - ssl->config->channel_id_enabled = true; - return 1; } diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc index a52ec3dd7..b0f08924f 100644 --- a/ssl/ssl_session.cc +++ b/ssl/ssl_session.cc @@ -1284,12 +1284,3 @@ void (*SSL_CTX_get_info_callback(SSL_CTX *ctx))(const SSL *ssl, int type, int value) { return ctx->info_callback; } - -void SSL_CTX_set_channel_id_cb(SSL_CTX *ctx, - void (*cb)(SSL *ssl, EVP_PKEY **pkey)) { - ctx->channel_id_cb = cb; -} - -void (*SSL_CTX_get_channel_id_cb(SSL_CTX *ctx))(SSL *ssl, EVP_PKEY **pkey) { - return ctx->channel_id_cb; -} diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index a63ca5dd0..44c96e881 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc @@ -1643,7 +1643,7 @@ static bool ext_alpn_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) { static bool ext_channel_id_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) { SSL *const ssl = hs->ssl; - if (!hs->config->channel_id_enabled || SSL_is_dtls(ssl)) { + if (!hs->config->channel_id_private || SSL_is_dtls(ssl)) { return true; } @@ -1663,7 +1663,7 @@ static bool ext_channel_id_parse_serverhello(SSL_HANDSHAKE *hs, } assert(!SSL_is_dtls(hs->ssl)); - assert(hs->config->channel_id_enabled); + assert(hs->config->channel_id_private); if (CBS_len(contents) != 0) { return false; @@ -4189,23 +4189,6 @@ bool tls1_record_handshake_hashes_for_channel_id(SSL_HANDSHAKE *hs) { return true; } -bool ssl_do_channel_id_callback(SSL_HANDSHAKE *hs) { - if (hs->config->channel_id_private != NULL || - hs->ssl->ctx->channel_id_cb == NULL) { - return true; - } - - EVP_PKEY *key = NULL; - hs->ssl->ctx->channel_id_cb(hs->ssl, &key); - if (key == NULL) { - // The caller should try again later. - return true; - } - - UniquePtr free_key(key); - return SSL_set1_tls_channel_id(hs->ssl, key); -} - bool ssl_is_sct_list_valid(const CBS *contents) { // Shallow parse the SCT list for sanity. By the RFC // (https://tools.ietf.org/html/rfc6962#section-3.3) neither the list nor any diff --git a/ssl/test/handshake_util.cc b/ssl/test/handshake_util.cc index 34cee4748..b9998317b 100644 --- a/ssl/test/handshake_util.cc +++ b/ssl/test/handshake_util.cc @@ -86,14 +86,6 @@ bool RetryAsync(SSL *ssl, int ret) { case SSL_ERROR_WANT_WRITE: AsyncBioAllowWrite(test_state->async_bio, 1); return true; - case SSL_ERROR_WANT_CHANNEL_ID_LOOKUP: { - UniquePtr pkey = LoadPrivateKey(config->send_channel_id); - if (!pkey) { - return false; - } - test_state->channel_id = std::move(pkey); - return true; - } case SSL_ERROR_WANT_X509_LOOKUP: test_state->cert_ready = true; return true; diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 47fac655c..c68df9f38 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -695,10 +695,6 @@ static void InfoCallback(const SSL *ssl, int type, int val) { } } -static void ChannelIdCallback(SSL *ssl, EVP_PKEY **out_pkey) { - *out_pkey = GetTestState(ssl)->channel_id.release(); -} - static SSL_SESSION *GetSessionCallback(SSL *ssl, const uint8_t *data, int len, int *copy) { TestState *async_state = GetTestState(ssl); @@ -1376,8 +1372,6 @@ bssl::UniquePtr TestConfig::SetupCtx(SSL_CTX *old_ctx) const { SSL_CTX_set_alpn_select_cb(ssl_ctx.get(), AlpnSelectCallback, NULL); } - SSL_CTX_set_channel_id_cb(ssl_ctx.get(), ChannelIdCallback); - SSL_CTX_set_current_time_cb(ssl_ctx.get(), CurrentTimeCallback); SSL_CTX_set_info_callback(ssl_ctx.get(), InfoCallback); @@ -1727,13 +1721,9 @@ bssl::UniquePtr TestConfig::NewSSL( } } if (!send_channel_id.empty()) { - SSL_set_tls_channel_id_enabled(ssl.get(), 1); - if (!async) { - // The async case will be supplied by |ChannelIdCallback|. - bssl::UniquePtr pkey = LoadPrivateKey(send_channel_id); - if (!pkey || !SSL_set1_tls_channel_id(ssl.get(), pkey.get())) { - return nullptr; - } + bssl::UniquePtr pkey = LoadPrivateKey(send_channel_id); + if (!pkey || !SSL_set1_tls_channel_id(ssl.get(), pkey.get())) { + return nullptr; } } if (!host_name.empty() && diff --git a/ssl/test/test_state.h b/ssl/test/test_state.h index d9fe945ac..4199d4ae0 100644 --- a/ssl/test/test_state.h +++ b/ssl/test/test_state.h @@ -41,7 +41,6 @@ struct TestState { // packeted_bio is the packeted BIO which simulates read timeouts. BIO *packeted_bio = nullptr; std::unique_ptr quic_transport; - bssl::UniquePtr channel_id; bool cert_ready = false; bssl::UniquePtr session; bssl::UniquePtr pending_session; diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index bac37ff07..37ca4b2af 100644 --- a/ssl/tls13_client.cc +++ b/ssl/tls13_client.cc @@ -821,15 +821,6 @@ static enum ssl_hs_wait_t do_complete_second_flight(SSL_HANDSHAKE *hs) { // Send a Channel ID assertion if necessary. if (hs->channel_id_negotiated) { - if (!ssl_do_channel_id_callback(hs)) { - hs->tls13_state = state_complete_second_flight; - return ssl_hs_error; - } - - if (hs->config->channel_id_private == NULL) { - return ssl_hs_channel_id_lookup; - } - ScopedCBB cbb; CBB body; if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CHANNEL_ID) ||