Optional tls_params in certificate to override TLS params in context by rudrakhp · Pull Request #45680 · envoyproxy/envoy

rudrakhp · 2026-06-17T14:21:48Z

Commit Message: optional tls params in certificate
Additional Description: Add optional tls_params in TLS certificate SDS that overrides context level TLS params for the certificate if provided.
Risk Level: Low (new API)
Testing: Unit tests
Docs Changes: N/A
Release Notes: Yes
Platform Specific Features: N/A
Related #44021

repokitteh-read-only · 2026-06-17T14:22:05Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #45680 was opened by rudrakhp.

see: more, trace.

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

wbpcode · 2026-06-24T12:09:20Z

I will leave this to the expert of TLS.

ggreenway

I don't think this works.

// SSL_set_SSL_CTX changes |ssl|'s |SSL_CTX|. |ssl| will use the
// certificate-related settings from |ctx|, and |SSL_get_SSL_CTX| will report
// |ctx|. This function may be used during the callbacks registered by
// |SSL_CTX_set_select_certificate_cb|,
// |SSL_CTX_set_tlsext_servername_callback|, and |SSL_CTX_set_cert_cb| or when
// the handshake is paused from them. It is typically used to switch
// certificates based on SNI.
//
// Note the session cache and related settings will continue to use the initial
// |SSL_CTX|. Callers should use |SSL_CTX_set_session_id_context| to partition
// the session cache between different domains.
//
// TODO(davidben): Should other settings change after this call?
OPENSSL_EXPORT SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx);

This only affects the certificate-related settings, and most of the newly-added settings wouldn't be applied.

/wait

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

rudrakhp · 2026-07-06T17:42:15Z

@ggreenway Thanks for the review! I updated the implementation to apply the selected certificate’s TLS params directly on SSL* after SSL_set_SSL_CTX() in onCertificateSelectionCompleted(), since SSL_set_SSL_CTX() only carries cert details. Please do give it a look, thanks again :)

ggreenway · 2026-07-06T23:42:14Z

Don't force-push to the PR branch; it makes it difficult to review.

ggreenway · 2026-07-06T23:46:43Z

+  // for that certificate during the TLS handshake; unset fields continue to use the context-level
+  // values. This allows different cipher suites, ECDH curves, protocol versions, signature
+  // algorithms, or compliance policies per certificate.
+  // Setting this on a client certificate has no effect.


Is there somewhere else this can be configured, so we don't have a setting that only works some of the time?

@wbpcode can you comment as the api-reviewer?

Some params like the ocsp_staple are configured under TlsCertificate but is supported in downstream context (server cert) only. Per cert TLS params made sense to exist under each corresponding cert.

We could rename it to downstream_tls_params or server_tls_params to be more explicit about its usage?

I don't know a better position for that. And if we can support this at upstream in theory, then it's fine to place it here and document it clear that this only make sense for now for downstream TLS.

And in the future, if we see similar requirement at upstream, we can implement it at upstream TLS also.

rudrakhp · 2026-07-07T06:17:04Z

Don't force-push to the PR branch; it makes it difficult to review.

Ack. I added a new commit after your last review to make it easier to see the changes since then. I only force pushed on the new commit while iterating on a few test failures. Will avoid doing that as well if you would prefer no force pushes at all :)

wbpcode

/lgtm api

My two cents to the API. :)

paul-r-gall · 2026-07-20T14:05:54Z

coverage finding looks relevant.

ggreenway

There's copy/pasted code all over this PR. Please refactor to avoid that.

The tests are currently not covering ecdh_curves or max_protocol_version, and they don't cover TLSv1.3 handshakes. Please add coverage for all the settings, using both TLSv1.2 and TLSv1.3.

/wait

ggreenway · 2026-07-21T00:03:55Z

+    if (p.compliance_policies_size() > 1) {
+      creation_status = absl::InvalidArgumentError(
+          "Only one compliance policy may be specified per certificate tls_params");
+      return;
+    }


Use compliancePolicyFromProto() here and line 149

Moving compliancePolicyFromProto() to utils to avoid cyclic dependencies

ggreenway · 2026-07-21T00:05:41Z

+      std::vector<absl::string_view> ciphers =
+          StringUtil::splitToken(tls_params_->cipher_suites, ":+![|]", false);


You've copied a lot of code from elsewhere. Refactor so there's not duplicated config handling code, so that it can't drift over time.

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

rudrakhp · 2026-07-25T19:51:36Z

+    // QUIC selects certs via ProofSource::GetProof(), not cert_cb, so tls_params_ never applies
+    // there.


I have not implemented this for QUIC/HTTP3 in this PR (also documented it in API docs and changelogs). We can implement this in a follow up if required?

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

repokitteh-read-only Bot added the api label Jun 17, 2026

repokitteh-read-only Bot assigned wbpcode Jun 17, 2026

rudrakhp force-pushed the tls_config_per_cert branch 2 times, most recently from 61f0b97 to 8d7cf14 Compare June 18, 2026 04:10

optional tls params in certificate

37ec457

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

rudrakhp force-pushed the tls_config_per_cert branch from 8d7cf14 to 37ec457 Compare June 18, 2026 07:19

wbpcode assigned ggreenway Jun 24, 2026

ggreenway requested changes Jun 24, 2026

View reviewed changes

repokitteh-read-only Bot added the waiting label Jun 24, 2026

repokitteh-read-only Bot removed the waiting label Jul 4, 2026

rudrakhp force-pushed the tls_config_per_cert branch 15 times, most recently from cad1603 to 5c44f82 Compare July 6, 2026 16:34

move setting cert tls params to onCertificateSelectionCompleted

0be0fc5

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

rudrakhp force-pushed the tls_config_per_cert branch from 5c44f82 to 0be0fc5 Compare July 6, 2026 16:39

ggreenway reviewed Jul 6, 2026

View reviewed changes

wbpcode reviewed Jul 12, 2026

View reviewed changes

repokitteh-read-only Bot removed the api label Jul 12, 2026

rudrakhp requested a review from ggreenway July 14, 2026 12:21

ggreenway requested changes Jul 21, 2026

View reviewed changes

repokitteh-read-only Bot added the waiting label Jul 21, 2026

move and reuse tls and compliance functions

25158f6

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

repokitteh-read-only Bot removed the waiting label Jul 24, 2026

Merge branch 'main' into tls_config_per_cert

f61b749

repokitteh-read-only Bot added the api label Jul 24, 2026

rudrakhp added 3 commits July 25, 2026 01:39

fixes

d09f6a3

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

refactor to dedup validations

3de488c

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

minor fixes

61f5303

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

rudrakhp commented Jul 25, 2026

View reviewed changes

rudrakhp added 3 commits July 26, 2026 01:28

move proto validation before compliance policy extraction

bb8b268

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

spell check

d3faf98

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

spell check

8cad5f2

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

		std::vector<absl::string_view> ciphers =
		StringUtil::splitToken(tls_params_->cipher_suites, ":+![\|]", false);

		// QUIC selects certs via ProofSource::GetProof(), not cert_cb, so tls_params_ never applies
		// there.

Uh oh!

Conversation

rudrakhp commented Jun 17, 2026

Uh oh!

repokitteh-read-only Bot commented Jun 17, 2026

Uh oh!

wbpcode commented Jun 24, 2026

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

rudrakhp commented Jul 6, 2026

Uh oh!

ggreenway commented Jul 6, 2026

Uh oh!

ggreenway Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

rudrakhp Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbpcode Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

rudrakhp commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbpcode left a comment

Choose a reason for hiding this comment

Uh oh!

paul-r-gall commented Jul 20, 2026

Uh oh!

ggreenway left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggreenway Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

rudrakhp Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggreenway Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

rudrakhp Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rudrakhp Jul 7, 2026 •

edited

Loading

rudrakhp commented Jul 7, 2026 •

edited

Loading

ggreenway left a comment •

edited

Loading

rudrakhp Jul 24, 2026 •

edited

Loading