Implement ChannelID for TLS 1.3.

Channel ID for TLS 1.3 uses the same digest construction as
CertificateVerify. This message is signed with the Channel ID key and
put in the same handshake message (with the same format) as in TLS 1.2.

BUG=103

Change-Id: Ia5b2dffe5a39c39db0cecb0aa6bdc328e53accc2
Reviewed-on: https://boringssl-review.googlesource.com/11420
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>

ssl/handshake_client.c

@@ -1831,16 +1831,8 @@ static int ssl3_send_channel_id(SSL *ssl) {
 
   assert(ssl->state == SSL3_ST_CW_CHANNEL_ID_A);
 
-  if (ssl->tlsext_channel_id_private == NULL &&
-      ssl->ctx->channel_id_cb != NULL) {
-    EVP_PKEY *key = NULL;
-    ssl->ctx->channel_id_cb(ssl, &key);
-    if (key != NULL &&
-        !SSL_set1_tls_channel_id(ssl, key)) {
-      EVP_PKEY_free(key);
-      return -1;
-    }
-    EVP_PKEY_free(key);
+  if (!ssl_do_channel_id_callback(ssl)) {
+    return -1;
   }
 
   if (ssl->tlsext_channel_id_private == NULL) {
@@ -1848,55 +1840,17 @@ static int ssl3_send_channel_id(SSL *ssl) {
     return -1;
   }
 
-  EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(ssl->tlsext_channel_id_private);
-  if (ec_key == NULL) {
-    OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
-    return -1;
-  }
-
-  int ret = -1;
-  BIGNUM *x = BN_new();
-  BIGNUM *y = BN_new();
-  ECDSA_SIG *sig = NULL;
-  if (x == NULL || y == NULL ||
-      !EC_POINT_get_affine_coordinates_GFp(EC_KEY_get0_group(ec_key),
-                                           EC_KEY_get0_public_key(ec_key),
-                                           x, y, NULL)) {
-    goto err;
-  }
-
-  uint8_t digest[EVP_MAX_MD_SIZE];
-  size_t digest_len;
-  if (!tls1_channel_id_hash(ssl, digest, &digest_len)) {
-    goto err;
-  }
-
-  sig = ECDSA_do_sign(digest, digest_len, ec_key);
-  if (sig == NULL) {
-    goto err;
-  }
-
-  CBB cbb, body, child;
+  CBB cbb, body;
   if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CHANNEL_ID) ||
-      !CBB_add_u16(&body, TLSEXT_TYPE_channel_id) ||
-      !CBB_add_u16_length_prefixed(&body, &child) ||
-      !BN_bn2cbb_padded(&child, 32, x) || !BN_bn2cbb_padded(&child, 32, y) ||
-      !BN_bn2cbb_padded(&child, 32, sig->r) ||
-      !BN_bn2cbb_padded(&child, 32, sig->s) ||
+      !tls1_write_channel_id(ssl, &body) ||
       !ssl->method->finish_message(ssl, &cbb)) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
     CBB_cleanup(&cbb);
-    goto err;
+    return -1;
   }
 
   ssl->state = SSL3_ST_CW_CHANNEL_ID_B;
-  ret = ssl->method->write_message(ssl);
-
-err:
-  BN_free(x);
-  BN_free(y);
-  ECDSA_SIG_free(sig);
-  return ret;
+  return ssl->method->write_message(ssl);
 }
 
 static int ssl3_get_new_session_ticket(SSL *ssl) {

ssl/handshake_server.c

@@ -1796,109 +1796,17 @@ static int ssl3_get_next_proto(SSL *ssl) {
 
 /* ssl3_get_channel_id reads and verifies a ClientID handshake message. */
 static int ssl3_get_channel_id(SSL *ssl) {
-  int ret = -1;
-  uint8_t channel_id_hash[EVP_MAX_MD_SIZE];
-  size_t channel_id_hash_len;
-  const uint8_t *p;
-  uint16_t extension_type;
-  EC_GROUP *p256 = NULL;
-  EC_KEY *key = NULL;
-  EC_POINT *point = NULL;
-  ECDSA_SIG sig;
-  BIGNUM x, y;
-  CBS encrypted_extensions, extension;
-
   int msg_ret = ssl->method->ssl_get_message(ssl, SSL3_MT_CHANNEL_ID,
                                              ssl_dont_hash_message);
   if (msg_ret <= 0) {
     return msg_ret;
   }
 
-  /* Before incorporating the EncryptedExtensions message to the handshake
-   * hash, compute the hash that should have been signed. */
-  if (!tls1_channel_id_hash(ssl, channel_id_hash, &channel_id_hash_len)) {
-    return -1;
-  }
-  assert(channel_id_hash_len == SHA256_DIGEST_LENGTH);
-
-  if (!ssl->method->hash_current_message(ssl)) {
+  if (!tls1_verify_channel_id(ssl) ||
+      !ssl->method->hash_current_message(ssl)) {
     return -1;
   }
-
-  CBS_init(&encrypted_extensions, ssl->init_msg, ssl->init_num);
-
-  /* EncryptedExtensions could include multiple extensions, but the only
-   * extension that could be negotiated is Channel ID, so there can only be one
-   * entry. */
-  if (!CBS_get_u16(&encrypted_extensions, &extension_type) ||
-      !CBS_get_u16_length_prefixed(&encrypted_extensions, &extension) ||
-      CBS_len(&encrypted_extensions) != 0 ||
-      extension_type != TLSEXT_TYPE_channel_id ||
-      CBS_len(&extension) != TLSEXT_CHANNEL_ID_SIZE) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
-    ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-    return -1;
-  }
-
-  p256 = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
-  if (!p256) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_NO_P256_SUPPORT);
-    return -1;
-  }
-
-  BN_init(&x);
-  BN_init(&y);
-  sig.r = BN_new();
-  sig.s = BN_new();
-  if (sig.r == NULL || sig.s == NULL) {
-    goto err;
-  }
-
-  p = CBS_data(&extension);
-  if (BN_bin2bn(p + 0, 32, &x) == NULL ||
-      BN_bin2bn(p + 32, 32, &y) == NULL ||
-      BN_bin2bn(p + 64, 32, sig.r) == NULL ||
-      BN_bin2bn(p + 96, 32, sig.s) == NULL) {
-    goto err;
-  }
-
-  point = EC_POINT_new(p256);
-  if (!point ||
-      !EC_POINT_set_affine_coordinates_GFp(p256, point, &x, &y, NULL)) {
-    goto err;
-  }
-
-  key = EC_KEY_new();
-  if (!key || !EC_KEY_set_group(key, p256) ||
-      !EC_KEY_set_public_key(key, point)) {
-    goto err;
-  }
-
-  /* We stored the handshake hash in |tlsext_channel_id| the first time that we
-   * were called. */
-  int sig_ok = ECDSA_do_verify(channel_id_hash, channel_id_hash_len, &sig, key);
-#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
-  sig_ok = 1;
-#endif
-  if (!sig_ok) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_CHANNEL_ID_SIGNATURE_INVALID);
-    ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECRYPT_ERROR);
-    ssl->s3->tlsext_channel_id_valid = 0;
-    goto err;
-  }
-
-  memcpy(ssl->s3->tlsext_channel_id, p, 64);
-  ret = 1;
-
-err:
-  BN_free(&x);
-  BN_free(&y);
-  BN_free(sig.r);
-  BN_free(sig.s);
-  EC_KEY_free(key);
-  EC_POINT_free(point);
-  EC_GROUP_free(p256);
-  return ret;
+  return 1;
 }
 
 static int ssl3_send_new_session_ticket(SSL *ssl) {

ssl/internal.h

@@ -878,6 +878,7 @@ enum ssl_hs_wait_t {
   ssl_hs_flush,
   ssl_hs_flush_and_read_message,
   ssl_hs_x509_lookup,
+  ssl_hs_channel_id_lookup,
   ssl_hs_private_key_operation,
 };
 
@@ -1060,6 +1061,21 @@ int ssl_add_client_hello_body(SSL *ssl, CBB *body);
  * should be called once the version is known to be TLS 1.2 or earlier. */
 void ssl_clear_tls13_state(SSL *ssl);
 
+enum ssl_cert_verify_context_t {
+  ssl_cert_verify_server,
+  ssl_cert_verify_client,
+  ssl_cert_verify_channel_id,
+};
+
+/* tls13_get_cert_verify_signature_input generates the message to be signed for
+ * TLS 1.3's CertificateVerify message. |cert_verify_context| determines the
+ * type of signature. It sets |*out| and |*out_len| to a newly allocated buffer
+ * containing the result. The caller must free it with |OPENSSL_free| to release
+ * it. This function returns one on success and zero on failure. */
+int tls13_get_cert_verify_signature_input(
+    SSL *ssl, uint8_t **out, size_t *out_len,
+    enum ssl_cert_verify_context_t cert_verify_context);
+
 
 /* SSLKEYLOGFILE functions. */
 
@@ -1808,6 +1824,16 @@ int tls_process_ticket(SSL *ssl, SSL_SESSION **out_session,
                        size_t ticket_len, const uint8_t *session_id,
                        size_t session_id_len);
 
+/* tls1_verify_channel_id processes the current message as a Channel ID message,
+ * and verifies the signature. If the key is valid, it saves the Channel ID and
+ * returns one. Otherwise, it returns zero. */
+int tls1_verify_channel_id(SSL *ssl);
+
+/* tls1_write_channel_id generates a Channel ID message and puts the output in
+ * |cbb|. |ssl->tlsext_channel_id_private| must already be set before calling.
+ * This function returns one on success and zero on error. */
+int tls1_write_channel_id(SSL *ssl, CBB *cbb);
+
 /* tls1_channel_id_hash computes the hash to be signed by Channel ID and writes
  * it to |out|, which must contain at least |EVP_MAX_MD_SIZE| bytes. It returns
  * one on success and zero on failure. */
@@ -1815,6 +1841,12 @@ int tls1_channel_id_hash(SSL *ssl, uint8_t *out, size_t *out_len);
 
 int tls1_record_handshake_hashes_for_channel_id(SSL *ssl);
 
+/* ssl_do_channel_id_callback checks runs |ssl->ctx->channel_id_cb| if
+ * necessary. It returns one on success and zero on fatal error. Note that, on
+ * success, |ssl->tlsext_channel_id_private| may be unset, in which case the
+ * operation should be retried later. */
+int ssl_do_channel_id_callback(SSL *ssl);
+
 /* ssl3_can_false_start returns one if |ssl| is allowed to False Start and zero
  * otherwise. */
 int ssl3_can_false_start(const SSL *ssl);

ssl/t1_lib.c

@@ -1633,10 +1633,6 @@ static int ext_channel_id_parse_serverhello(SSL *ssl, uint8_t *out_alert,
     return 1;
   }
 
-  if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) {
-    return 0;
-  }
-
   assert(!SSL_is_dtls(ssl));
   assert(ssl->tlsext_channel_id_enabled);
 
@@ -1665,10 +1661,6 @@ static int ext_channel_id_parse_clienthello(SSL *ssl, uint8_t *out_alert,
 }
 
 static int ext_channel_id_add_serverhello(SSL *ssl, CBB *out) {
-  if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) {
-    return 1;
-  }
-
   if (!ssl->s3->tlsext_channel_id_valid) {
     return 1;
   }
@@ -3207,7 +3199,157 @@ int tls1_choose_signature_algorithm(SSL *ssl, uint16_t *out) {
   return 0;
 }
 
+int tls1_verify_channel_id(SSL *ssl) {
+  int ret = 0;
+  uint16_t extension_type;
+  CBS extension, channel_id;
+
+  /* A Channel ID handshake message is structured to contain multiple
+   * extensions, but the only one that can be present is Channel ID. */
+  CBS_init(&channel_id, ssl->init_msg, ssl->init_num);
+  if (!CBS_get_u16(&channel_id, &extension_type) ||
+      !CBS_get_u16_length_prefixed(&channel_id, &extension) ||
+      CBS_len(&channel_id) != 0 ||
+      extension_type != TLSEXT_TYPE_channel_id ||
+      CBS_len(&extension) != TLSEXT_CHANNEL_ID_SIZE) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+    ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+    return 0;
+  }
+
+  EC_GROUP *p256 = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
+  if (!p256) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_NO_P256_SUPPORT);
+    return 0;
+  }
+
+  EC_KEY *key = NULL;
+  EC_POINT *point = NULL;
+  BIGNUM x, y;
+  ECDSA_SIG sig;
+  BN_init(&x);
+  BN_init(&y);
+  sig.r = BN_new();
+  sig.s = BN_new();
+  if (sig.r == NULL || sig.s == NULL) {
+    goto err;
+  }
+
+  const uint8_t *p = CBS_data(&extension);
+  if (BN_bin2bn(p + 0, 32, &x) == NULL ||
+      BN_bin2bn(p + 32, 32, &y) == NULL ||
+      BN_bin2bn(p + 64, 32, sig.r) == NULL ||
+      BN_bin2bn(p + 96, 32, sig.s) == NULL) {
+    goto err;
+  }
+
+  point = EC_POINT_new(p256);
+  if (point == NULL ||
+      !EC_POINT_set_affine_coordinates_GFp(p256, point, &x, &y, NULL)) {
+    goto err;
+  }
+
+  key = EC_KEY_new();
+  if (key == NULL ||
+      !EC_KEY_set_group(key, p256) ||
+      !EC_KEY_set_public_key(key, point)) {
+    goto err;
+  }
+
+  uint8_t digest[EVP_MAX_MD_SIZE];
+  size_t digest_len;
+  if (!tls1_channel_id_hash(ssl, digest, &digest_len)) {
+    goto err;
+  }
+
+  int sig_ok = ECDSA_do_verify(digest, digest_len, &sig, key);
+#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
+  sig_ok = 1;
+#endif
+  if (!sig_ok) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_CHANNEL_ID_SIGNATURE_INVALID);
+    ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECRYPT_ERROR);
+    ssl->s3->tlsext_channel_id_valid = 0;
+    goto err;
+  }
+
+  memcpy(ssl->s3->tlsext_channel_id, p, 64);
+  ret = 1;
+
+err:
+  BN_free(&x);
+  BN_free(&y);
+  BN_free(sig.r);
+  BN_free(sig.s);
+  EC_KEY_free(key);
+  EC_POINT_free(point);
+  EC_GROUP_free(p256);
+  return ret;
+}
+
+int tls1_write_channel_id(SSL *ssl, CBB *cbb) {
+  uint8_t digest[EVP_MAX_MD_SIZE];
+  size_t digest_len;
+  if (!tls1_channel_id_hash(ssl, digest, &digest_len)) {
+    return 0;
+  }
+
+  EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(ssl->tlsext_channel_id_private);
+  if (ec_key == NULL) {
+    OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+    return 0;
+  }
+
+  int ret = 0;
+  BIGNUM *x = BN_new();
+  BIGNUM *y = BN_new();
+  ECDSA_SIG *sig = NULL;
+  if (x == NULL || y == NULL ||
+      !EC_POINT_get_affine_coordinates_GFp(EC_KEY_get0_group(ec_key),
+                                           EC_KEY_get0_public_key(ec_key),
+                                           x, y, NULL)) {
+    goto err;
+  }
+
+  sig = ECDSA_do_sign(digest, digest_len, ec_key);
+  if (sig == NULL) {
+    goto err;
+  }
+
+  CBB child;
+  if (!CBB_add_u16(cbb, TLSEXT_TYPE_channel_id) ||
+      !CBB_add_u16_length_prefixed(cbb, &child) ||
+      !BN_bn2cbb_padded(&child, 32, x) ||
+      !BN_bn2cbb_padded(&child, 32, y) ||
+      !BN_bn2cbb_padded(&child, 32, sig->r) ||
+      !BN_bn2cbb_padded(&child, 32, sig->s) ||
+      !CBB_flush(cbb)) {
+    goto err;
+  }
+
+  ret = 1;
+
+err:
+  BN_free(x);
+  BN_free(y);
+  ECDSA_SIG_free(sig);
+  return ret;
+}
+
 int tls1_channel_id_hash(SSL *ssl, uint8_t *out, size_t *out_len) {
+  if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) {
+    uint8_t *msg;
+    size_t msg_len;
+    if (!tls13_get_cert_verify_signature_input(ssl, &msg, &msg_len,
+                                               ssl_cert_verify_channel_id)) {
+      return 0;
+    }
+    SHA256(msg, msg_len, out);
+    *out_len = SHA256_DIGEST_LENGTH;
+    OPENSSL_free(msg);
+    return 1;
+  }
+
   int ret = 0;
   EVP_MD_CTX ctx;
 
@@ -3272,3 +3414,21 @@ int tls1_record_handshake_hashes_for_channel_id(SSL *ssl) {
 
   return 1;
 }
+
+int ssl_do_channel_id_callback(SSL *ssl) {
+  if (ssl->tlsext_channel_id_private != NULL ||
+      ssl->ctx->channel_id_cb == NULL) {
+    return 1;
+  }
+
+  EVP_PKEY *key = NULL;
+  ssl->ctx->channel_id_cb(ssl, &key);
+  if (key == NULL) {
+    /* The caller should try again later. */
+    return 1;
+  }
+
+  int ret = SSL_set1_tls_channel_id(ssl, key);
+  EVP_PKEY_free(key);
+  return ret;
+}

ssl/test/runner/common.go

@@ -1028,10 +1028,6 @@ type ProtocolBugs struct {
 	// Renegotiation Info to be negotiated at all versions.
 	NegotiateRenegotiationInfoAtAllVersions bool
 
-	// NegotiateChannelIDAtAllVersions, if true, causes Channel ID to be
-	// negotiated at all versions.
-	NegotiateChannelIDAtAllVersions bool
-
 	// NegotiateNPNAtAllVersions, if true, causes NPN to be negotiated at
 	// all versions.
 	NegotiateNPNAtAllVersions bool

ssl/test/runner/handshake_client.go

@@ -820,6 +820,17 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 		}
 	}
 
+	if encryptedExtensions.extensions.channelIDRequested {
+		channelIDHash := crypto.SHA256.New()
+		channelIDHash.Write(hs.finishedHash.certificateVerifyInput(channelIDContextTLS13))
+		channelIDMsgBytes, err := hs.writeChannelIDMessage(channelIDHash.Sum(nil))
+		if err != nil {
+			return err
+		}
+		hs.writeClientHash(channelIDMsgBytes)
+		c.writeRecord(recordTypeHandshake, channelIDMsgBytes)
+	}
+
 	// Send a client Finished message.
 	finished := new(finishedMsg)
 	finished.verifyData = hs.finishedHash.clientSum(clientHandshakeTrafficSecret)
@@ -1169,11 +1180,6 @@ func (hs *clientHandshakeState) processServerExtensions(serverExtensions *server
 		return errors.New("server advertised unrequested Channel ID extension")
 	}
 
-	if serverExtensions.channelIDRequested && c.vers >= VersionTLS13 {
-		c.sendAlert(alertHandshakeFailure)
-		return errors.New("server advertised Channel ID over TLS 1.3")
-	}
-
 	if serverExtensions.extendedMasterSecret && c.vers >= VersionTLS13 {
 		return errors.New("tls: server advertised extended master secret over TLS 1.3")
 	}
@@ -1346,31 +1352,14 @@ func (hs *clientHandshakeState) sendFinished(out []byte, isResume bool) error {
 	}
 
 	if hs.serverHello.extensions.channelIDRequested {
-		channelIDMsg := new(channelIDMsg)
-		if c.config.ChannelID.Curve != elliptic.P256() {
-			return fmt.Errorf("tls: Channel ID is not on P-256.")
-		}
 		var resumeHash []byte
 		if isResume {
 			resumeHash = hs.session.handshakeHash
 		}
-		r, s, err := ecdsa.Sign(c.config.rand(), c.config.ChannelID, hs.finishedHash.hashForChannelID(resumeHash))
+		channelIDMsgBytes, err := hs.writeChannelIDMessage(hs.finishedHash.hashForChannelID(resumeHash))
 		if err != nil {
 			return err
 		}
-		channelID := make([]byte, 128)
-		writeIntPadded(channelID[0:32], c.config.ChannelID.X)
-		writeIntPadded(channelID[32:64], c.config.ChannelID.Y)
-		writeIntPadded(channelID[64:96], r)
-		writeIntPadded(channelID[96:128], s)
-		if c.config.Bugs.InvalidChannelIDSignature {
-			channelID[64] ^= 1
-		}
-		channelIDMsg.channelID = channelID
-
-		c.channelID = &c.config.ChannelID.PublicKey
-
-		channelIDMsgBytes := channelIDMsg.marshal()
 		hs.writeHash(channelIDMsgBytes, seqno)
 		seqno++
 		postCCSMsgs = append(postCCSMsgs, channelIDMsgBytes)
@@ -1431,6 +1420,31 @@ func (hs *clientHandshakeState) sendFinished(out []byte, isResume bool) error {
 	return nil
 }
 
+func (hs *clientHandshakeState) writeChannelIDMessage(channelIDHash []byte) ([]byte, error) {
+	c := hs.c
+	channelIDMsg := new(channelIDMsg)
+	if c.config.ChannelID.Curve != elliptic.P256() {
+		return nil, fmt.Errorf("tls: Channel ID is not on P-256.")
+	}
+	r, s, err := ecdsa.Sign(c.config.rand(), c.config.ChannelID, channelIDHash)
+	if err != nil {
+		return nil, err
+	}
+	channelID := make([]byte, 128)
+	writeIntPadded(channelID[0:32], c.config.ChannelID.X)
+	writeIntPadded(channelID[32:64], c.config.ChannelID.Y)
+	writeIntPadded(channelID[64:96], r)
+	writeIntPadded(channelID[96:128], s)
+	if c.config.Bugs.InvalidChannelIDSignature {
+		channelID[64] ^= 1
+	}
+	channelIDMsg.channelID = channelID
+
+	c.channelID = &c.config.ChannelID.PublicKey
+
+	return channelIDMsg.marshal(), nil
+}
+
 func (hs *clientHandshakeState) writeClientHash(msg []byte) {
 	// writeClientHash is called before writeRecord.
 	hs.writeHash(msg, hs.c.sendHandshakeSeq)

ssl/test/runner/handshake_server.go

@@ -900,6 +900,27 @@ ResendHelloRetryRequest:
 		}
 	}
 
+	if encryptedExtensions.extensions.channelIDRequested {
+		msg, err := c.readHandshake()
+		if err != nil {
+			return err
+		}
+		channelIDMsg, ok := msg.(*channelIDMsg)
+		if !ok {
+			c.sendAlert(alertUnexpectedMessage)
+			return unexpectedMessageError(channelIDMsg, msg)
+		}
+		channelIDHash := crypto.SHA256.New()
+		channelIDHash.Write(hs.finishedHash.certificateVerifyInput(channelIDContextTLS13))
+		channelID, err := verifyChannelIDMessage(channelIDMsg, channelIDHash.Sum(nil))
+		if err != nil {
+			return err
+		}
+		c.channelID = channelID
+
+		hs.writeClientHash(channelIDMsg.marshal())
+	}
+
 	// Read the client Finished message.
 	msg, err := c.readHandshake()
 	if err != nil {
@@ -1126,10 +1147,8 @@ func (hs *serverHandshakeState) processClientExtensions(serverExtensions *server
 		serverExtensions.extendedMasterSecret = c.vers >= VersionTLS10 && hs.clientHello.extendedMasterSecret && !disableEMS
 	}
 
-	if c.vers < VersionTLS13 || config.Bugs.NegotiateChannelIDAtAllVersions {
-		if hs.clientHello.channelIDSupported && config.RequestChannelID {
-			serverExtensions.channelIDRequested = true
-		}
+	if hs.clientHello.channelIDSupported && config.RequestChannelID {
+		serverExtensions.channelIDRequested = true
 	}
 
 	if hs.clientHello.srtpProtectionProfiles != nil {
@@ -1568,20 +1587,13 @@ func (hs *serverHandshakeState) readFinished(out []byte, isResume bool) error {
 			c.sendAlert(alertUnexpectedMessage)
 			return unexpectedMessageError(channelIDMsg, msg)
 		}
-		x := new(big.Int).SetBytes(channelIDMsg.channelID[0:32])
-		y := new(big.Int).SetBytes(channelIDMsg.channelID[32:64])
-		r := new(big.Int).SetBytes(channelIDMsg.channelID[64:96])
-		s := new(big.Int).SetBytes(channelIDMsg.channelID[96:128])
-		if !elliptic.P256().IsOnCurve(x, y) {
-			return errors.New("tls: invalid channel ID public key")
-		}
-		channelID := &ecdsa.PublicKey{elliptic.P256(), x, y}
 		var resumeHash []byte
 		if isResume {
 			resumeHash = hs.sessionState.handshakeHash
 		}
-		if !ecdsa.Verify(channelID, hs.finishedHash.hashForChannelID(resumeHash), r, s) {
-			return errors.New("tls: invalid channel ID signature")
+		channelID, err := verifyChannelIDMessage(channelIDMsg, hs.finishedHash.hashForChannelID(resumeHash))
+		if err != nil {
+			return err
 		}
 		c.channelID = channelID
 
@@ -1765,6 +1777,21 @@ func (hs *serverHandshakeState) processCertsFromClient(certificates [][]byte) (c
 	return nil, nil
 }
 
+func verifyChannelIDMessage(channelIDMsg *channelIDMsg, channelIDHash []byte) (*ecdsa.PublicKey, error) {
+	x := new(big.Int).SetBytes(channelIDMsg.channelID[0:32])
+	y := new(big.Int).SetBytes(channelIDMsg.channelID[32:64])
+	r := new(big.Int).SetBytes(channelIDMsg.channelID[64:96])
+	s := new(big.Int).SetBytes(channelIDMsg.channelID[96:128])
+	if !elliptic.P256().IsOnCurve(x, y) {
+		return nil, errors.New("tls: invalid channel ID public key")
+	}
+	channelID := &ecdsa.PublicKey{elliptic.P256(), x, y}
+	if !ecdsa.Verify(channelID, channelIDHash, r, s) {
+		return nil, errors.New("tls: invalid channel ID signature")
+	}
+	return channelID, nil
+}
+
 func (hs *serverHandshakeState) writeServerHash(msg []byte) {
 	// writeServerHash is called before writeRecord.
 	hs.writeHash(msg, hs.c.sendHandshakeSeq)

ssl/test/runner/prf.go

@@ -442,6 +442,7 @@ func (h *finishedHash) deriveSecret(secret, label []byte) []byte {
 var (
 	clientCertificateVerifyContextTLS13 = []byte("TLS 1.3, client CertificateVerify")
 	serverCertificateVerifyContextTLS13 = []byte("TLS 1.3, server CertificateVerify")
+	channelIDContextTLS13               = []byte("TLS 1.3, Channel ID")
 )
 
 // certificateVerifyMessage returns the input to be signed for CertificateVerify

ssl/test/runner/runner.go

@@ -3940,33 +3940,54 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) {
 			},
 		})
 
-		// Client sends a Channel ID.
-		tests = append(tests, testCase{
-			name: "ChannelID-Client",
-			config: Config{
-				MaxVersion:       VersionTLS12,
-				RequestChannelID: true,
-			},
-			flags:           []string{"-send-channel-id", path.Join(*resourceDir, channelIDKeyFile)},
-			resumeSession:   true,
-			expectChannelID: true,
-		})
+		// Test Channel ID
+		for _, ver := range tlsVersions {
+			if ver.version < VersionTLS12 {
+				continue
+			}
+			// Client sends a Channel ID.
+			tests = append(tests, testCase{
+				name: "ChannelID-Client-" + ver.name,
+				config: Config{
+					MaxVersion:       ver.version,
+					RequestChannelID: true,
+				},
+				flags:           []string{"-send-channel-id", path.Join(*resourceDir, channelIDKeyFile)},
+				resumeSession:   true,
+				expectChannelID: true,
+			})
 
-		// Server accepts a Channel ID.
-		tests = append(tests, testCase{
-			testType: serverTest,
-			name:     "ChannelID-Server",
-			config: Config{
-				MaxVersion: VersionTLS12,
-				ChannelID:  channelIDKey,
-			},
-			flags: []string{
-				"-expect-channel-id",
-				base64.StdEncoding.EncodeToString(channelIDBytes),
-			},
-			resumeSession:   true,
-			expectChannelID: true,
-		})
+			// Server accepts a Channel ID.
+			tests = append(tests, testCase{
+				testType: serverTest,
+				name:     "ChannelID-Server-" + ver.name,
+				config: Config{
+					MaxVersion: ver.version,
+					ChannelID:  channelIDKey,
+				},
+				flags: []string{
+					"-expect-channel-id",
+					base64.StdEncoding.EncodeToString(channelIDBytes),
+				},
+				resumeSession:   true,
+				expectChannelID: true,
+			})
+
+			tests = append(tests, testCase{
+				testType: serverTest,
+				name:     "InvalidChannelIDSignature-" + ver.name,
+				config: Config{
+					MaxVersion: ver.version,
+					ChannelID:  channelIDKey,
+					Bugs: ProtocolBugs{
+						InvalidChannelIDSignature: true,
+					},
+				},
+				flags:         []string{"-enable-channel-id"},
+				shouldFail:    true,
+				expectedError: ":CHANNEL_ID_SIGNATURE_INVALID:",
+			})
+		}
 
 		// Channel ID and NPN at the same time, to ensure their relative
 		// ordering is correct.
@@ -5170,19 +5191,6 @@ func addExtensionTests() {
 		shouldFail:    true,
 		expectedError: ":ERROR_PARSING_EXTENSION:",
 	})
-	testCases = append(testCases, testCase{
-		name: "ChannelID-Forbidden-TLS13",
-		config: Config{
-			MaxVersion:       VersionTLS13,
-			RequestChannelID: true,
-			Bugs: ProtocolBugs{
-				NegotiateChannelIDAtAllVersions: true,
-			},
-		},
-		flags:         []string{"-send-channel-id", path.Join(*resourceDir, channelIDKeyFile)},
-		shouldFail:    true,
-		expectedError: ":ERROR_PARSING_EXTENSION:",
-	})
 	testCases = append(testCases, testCase{
 		name: "Ticket-Forbidden-TLS13",
 		config: Config{
@@ -5203,15 +5211,6 @@ func addExtensionTests() {
 	// offered in ClientHello. The runner's server will fail if this occurs,
 	// so we exercise the offering path. (EMS and Renegotiation Info are
 	// implicit in every test.)
-	testCases = append(testCases, testCase{
-		testType: serverTest,
-		name:     "ChannelID-Declined-TLS13",
-		config: Config{
-			MaxVersion: VersionTLS13,
-			ChannelID:  channelIDKey,
-		},
-		flags: []string{"-enable-channel-id"},
-	})
 	testCases = append(testCases, testCase{
 		testType: serverTest,
 		name:     "NPN-Declined-TLS13",
@@ -5222,22 +5221,6 @@ func addExtensionTests() {
 		flags: []string{"-advertise-npn", "\x03foo\x03bar\x03baz"},
 	})
 
-	testCases = append(testCases, testCase{
-		testType: serverTest,
-		name:     "InvalidChannelIDSignature",
-		config: Config{
-			MaxVersion: VersionTLS12,
-			ChannelID:  channelIDKey,
-			Bugs: ProtocolBugs{
-				InvalidChannelIDSignature: true,
-			},
-		},
-		flags:              []string{"-enable-channel-id"},
-		shouldFail:         true,
-		expectedError:      ":CHANNEL_ID_SIGNATURE_INVALID:",
-		expectedLocalError: "remote error: error decrypting message",
-	})
-
 	// OpenSSL sends the status_request extension on resumption in TLS 1.2. Test that this is
 	// tolerated.
 	testCases = append(testCases, testCase{

ssl/tls13_both.c

@@ -79,6 +79,11 @@ int tls13_handshake(SSL *ssl) {
         hs->wait = ssl_hs_ok;
         return -1;
 
+      case ssl_hs_channel_id_lookup:
+        ssl->rwstate = SSL_CHANNEL_ID_LOOKUP;
+        hs->wait = ssl_hs_ok;
+        return -1;
+
       case ssl_hs_private_key_operation:
         ssl->rwstate = SSL_PRIVATE_KEY_OPERATION;
         hs->wait = ssl_hs_ok;
@@ -105,8 +110,9 @@ int tls13_handshake(SSL *ssl) {
   }
 }
 
-static int tls13_get_cert_verify_signature_input(SSL *ssl, uint8_t **out,
-                                                 size_t *out_len, int server) {
+int tls13_get_cert_verify_signature_input(
+    SSL *ssl, uint8_t **out, size_t *out_len,
+    enum ssl_cert_verify_context_t cert_verify_context) {
   CBB cbb;
   if (!CBB_init(&cbb, 64 + 33 + 1 + 2 * EVP_MAX_MD_SIZE)) {
     goto err;
@@ -118,17 +124,27 @@ static int tls13_get_cert_verify_signature_input(SSL *ssl, uint8_t **out,
     }
   }
 
-  if (server) {
+  const uint8_t *context;
+  size_t context_len;
+  if (cert_verify_context == ssl_cert_verify_server) {
     /* Include the NUL byte. */
     static const char kContext[] = "TLS 1.3, server CertificateVerify";
-    if (!CBB_add_bytes(&cbb, (const uint8_t *)kContext, sizeof(kContext))) {
-      goto err;
-    }
-  } else {
+    context = (const uint8_t *)kContext;
+    context_len = sizeof(kContext);
+  } else if (cert_verify_context == ssl_cert_verify_client) {
     static const char kContext[] = "TLS 1.3, client CertificateVerify";
-    if (!CBB_add_bytes(&cbb, (const uint8_t *)kContext, sizeof(kContext))) {
-      goto err;
-    }
+    context = (const uint8_t *)kContext;
+    context_len = sizeof(kContext);
+  } else if (cert_verify_context == ssl_cert_verify_channel_id) {
+    static const char kContext[] = "TLS 1.3, Channel ID";
+    context = (const uint8_t *)kContext;
+    context_len = sizeof(kContext);
+  } else {
+    goto err;
+  }
+
+  if (!CBB_add_bytes(&cbb, context, context_len)) {
+    goto err;
   }
 
   uint8_t context_hashes[2 * EVP_MAX_MD_SIZE];
@@ -245,8 +261,9 @@ int tls13_process_certificate_verify(SSL *ssl) {
   }
   ssl->s3->tmp.peer_signature_algorithm = signature_algorithm;
 
-  if (!tls13_get_cert_verify_signature_input(ssl, &msg, &msg_len,
-                                             !ssl->server)) {
+  if (!tls13_get_cert_verify_signature_input(
+          ssl, &msg, &msg_len,
+          ssl->server ? ssl_cert_verify_client : ssl_cert_verify_server)) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
     goto err;
   }
@@ -352,8 +369,9 @@ enum ssl_private_key_result_t tls13_prepare_certificate_verify(
 
   enum ssl_private_key_result_t sign_result;
   if (is_first_run) {
-    if (!tls13_get_cert_verify_signature_input(ssl, &msg, &msg_len,
-                                               ssl->server)) {
+    if (!tls13_get_cert_verify_signature_input(
+            ssl, &msg, &msg_len,
+            ssl->server ? ssl_cert_verify_server : ssl_cert_verify_client)) {
       ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
       goto err;
     }

ssl/tls13_client.c

@@ -41,6 +41,7 @@ enum client_hs_state_t {
   state_send_client_certificate,
   state_send_client_certificate_verify,
   state_complete_client_certificate_verify,
+  state_send_channel_id,
   state_send_client_finished,
   state_flush,
   state_done,
@@ -519,7 +520,7 @@ static enum ssl_hs_wait_t do_process_server_finished(SSL *ssl,
 static enum ssl_hs_wait_t do_certificate_callback(SSL *ssl, SSL_HANDSHAKE *hs) {
   /* The peer didn't request a certificate. */
   if (!ssl->s3->hs->cert_request) {
-    hs->state = state_send_client_finished;
+    hs->state = state_send_channel_id;
     return ssl_hs_ok;
   }
 
@@ -566,13 +567,13 @@ static enum ssl_hs_wait_t do_send_client_certificate_verify(SSL *ssl,
                                                             int is_first_run) {
   /* Don't send CertificateVerify if there is no certificate. */
   if (!ssl_has_certificate(ssl)) {
-    hs->state = state_send_client_finished;
+    hs->state = state_send_channel_id;
     return ssl_hs_ok;
   }
 
   switch (tls13_prepare_certificate_verify(ssl, is_first_run)) {
     case ssl_private_key_success:
-      hs->state = state_send_client_finished;
+      hs->state = state_send_channel_id;
       return ssl_hs_write_message;
 
     case ssl_private_key_retry:
@@ -587,6 +588,32 @@ static enum ssl_hs_wait_t do_send_client_certificate_verify(SSL *ssl,
   return ssl_hs_error;
 }
 
+static enum ssl_hs_wait_t do_send_channel_id(SSL *ssl, SSL_HANDSHAKE *hs) {
+  if (!ssl->s3->tlsext_channel_id_valid) {
+    hs->state = state_send_client_finished;
+    return ssl_hs_ok;
+  }
+
+  if (!ssl_do_channel_id_callback(ssl)) {
+    return ssl_hs_error;
+  }
+
+  if (ssl->tlsext_channel_id_private == NULL) {
+    return ssl_hs_channel_id_lookup;
+  }
+
+  CBB cbb, body;
+  if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CHANNEL_ID) ||
+      !tls1_write_channel_id(ssl, &body) ||
+      !ssl->method->finish_message(ssl, &cbb)) {
+    CBB_cleanup(&cbb);
+    return ssl_hs_error;
+  }
+
+  hs->state = state_send_client_finished;
+  return ssl_hs_write_message;
+}
+
 static enum ssl_hs_wait_t do_send_client_finished(SSL *ssl, SSL_HANDSHAKE *hs) {
   if (!tls13_prepare_finished(ssl)) {
     return ssl_hs_error;
@@ -651,10 +678,13 @@ enum ssl_hs_wait_t tls13_client_handshake(SSL *ssl) {
         break;
       case state_send_client_certificate_verify:
         ret = do_send_client_certificate_verify(ssl, hs, 1 /* first run */);
-      break;
+        break;
       case state_complete_client_certificate_verify:
         ret = do_send_client_certificate_verify(ssl, hs, 0 /* complete */);
-      break;
+        break;
+      case state_send_channel_id:
+        ret = do_send_channel_id(ssl, hs);
+        break;
       case state_send_client_finished:
         ret = do_send_client_finished(ssl, hs);
         break;

ssl/tls13_server.c

@@ -43,6 +43,7 @@ enum server_hs_state_t {
   state_flush,
   state_process_client_certificate,
   state_process_client_certificate_verify,
+  state_process_channel_id,
   state_process_client_finished,
   state_send_new_session_ticket,
   state_flush_new_session_ticket,
@@ -476,7 +477,7 @@ static enum ssl_hs_wait_t do_process_client_certificate(SSL *ssl,
     ssl->s3->new_session->verify_result = X509_V_OK;
 
     /* Skip this state. */
-    hs->state = state_process_client_finished;
+    hs->state = state_process_channel_id;
     return ssl_hs_ok;
   }
 
@@ -503,7 +504,7 @@ static enum ssl_hs_wait_t do_process_client_certificate_verify(
     SSL *ssl, SSL_HANDSHAKE *hs) {
   if (ssl->s3->new_session->peer == NULL) {
     /* Skip this state. */
-    hs->state = state_process_client_finished;
+    hs->state = state_process_channel_id;
     return ssl_hs_ok;
   }
 
@@ -513,6 +514,22 @@ static enum ssl_hs_wait_t do_process_client_certificate_verify(
     return 0;
   }
 
+  hs->state = state_process_channel_id;
+  return ssl_hs_read_message;
+}
+
+static enum ssl_hs_wait_t do_process_channel_id(SSL *ssl, SSL_HANDSHAKE *hs) {
+  if (!ssl->s3->tlsext_channel_id_valid) {
+    hs->state = state_process_client_finished;
+    return ssl_hs_ok;
+  }
+
+  if (!tls13_check_message_type(ssl, SSL3_MT_CHANNEL_ID) ||
+      !tls1_verify_channel_id(ssl) ||
+      !ssl->method->hash_current_message(ssl)) {
+    return ssl_hs_error;
+  }
+
   hs->state = state_process_client_finished;
   return ssl_hs_read_message;
 }
@@ -645,6 +662,9 @@ enum ssl_hs_wait_t tls13_server_handshake(SSL *ssl) {
       case state_process_client_certificate_verify:
         ret = do_process_client_certificate_verify(ssl, hs);
         break;
+      case state_process_channel_id:
+        ret = do_process_channel_id(ssl, hs);
+        break;
       case state_process_client_finished:
         ret = do_process_client_finished(ssl, hs);
         break;