quic: support cert selection by SNI, non-PEM formats#32260
Merged
ggreenway merged 32 commits intoenvoyproxy:mainfrom Mar 6, 2024
Merged
quic: support cert selection by SNI, non-PEM formats#32260ggreenway merged 32 commits intoenvoyproxy:mainfrom
ggreenway merged 32 commits intoenvoyproxy:mainfrom
Conversation
This brings feature parity between quic and non-quic TLS use for certificate selection. Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Contributor
RyanTheOptimist
left a comment
There was a problem hiding this comment.
Looks awesome! One small nit.
| // we can reuse all the code that loads from different formats, allows using passwords on the key, | ||
| // etc. | ||
| std::string certs_str; | ||
| auto process_one_cert = [&](X509* cert) { |
Contributor
There was a problem hiding this comment.
nit: Would it make sense to just make this function in the anonymous namespace?
Member
Author
There was a problem hiding this comment.
I prefer having it in the function that uses it to make it clear the scope in which it's used, and it can capture a variable instead of needing a parameter. But it's just a small style preference. I'll change it if you insist.
Signed-off-by: Greg Greenway <ggreenway@apple.com>
danzh2010
reviewed
Feb 13, 2024
danzh2010
reviewed
Feb 13, 2024
danzh2010
reviewed
Feb 13, 2024
| ASSERT(result == 1); | ||
| BUF_MEM* buf_mem = nullptr; | ||
| result = BIO_get_mem_ptr(bio.get(), &buf_mem); | ||
| certs_str.append(buf_mem->data, buf_mem->length); |
Contributor
There was a problem hiding this comment.
Can you directly populate a std::vector<std::string> instead of concatenating the pem string here and then calling CertificateView::LoadPemFromStream() to split them apart?
Member
Author
There was a problem hiding this comment.
I took at stab at this in latest commit; PTAL and let me know if this is what you had in mind.
My understanding is that each string in this vector should be a pem-encoded (base64) string of a cert, without newlines or the begin/end block. Is there a boringssl API to get that directly? I didn't find a more direct way than what's in my latest commit.
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Some paths can't be hit anymore because they're caught earlier when constructing the ContextImpl, but they should still remain in the code for sanity Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Member
Author
|
/retest |
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Contributor
|
Needs main merge again? |
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Member
Author
|
/retest |
RyanTheOptimist
approved these changes
Mar 6, 2024
mattjo
added a commit
to mattjo/envoy
that referenced
this pull request
Mar 6, 2024
* origin: (34 commits) update CODEOWNER (envoyproxy#32457) Delete unused runtime flag. (envoyproxy#32739) mobile: Use direct ByteBuffer to pass data between C++ and Java (envoyproxy#32715) quic: support cert selection by SNI, non-PEM formats (envoyproxy#32260) mobile: Replace std::thread with Envoy::Thread::PosixThread (envoyproxy#32610) grpc: Add support for max frame length in gPRC frame decoding (envoyproxy#32511) build(deps): bump google.golang.org/protobuf from 1.32.0 to 1.33.0 (envoyproxy#32728) build(deps): bump the examples-golang-network group in /examples/golang-network/simple with 1 update (envoyproxy#32732) build(deps): bump google.golang.org/protobuf from 1.32.0 to 1.33.0 in /contrib/golang/filters/http/test/test_data/property (envoyproxy#32731) build(deps): bump otel/opentelemetry-collector from `246dfe9` to `71ac13c` in /examples/opentelemetry (envoyproxy#32730) build(deps): bump the examples-grpc-bridge group in /examples/grpc-bridge/server with 2 updates (envoyproxy#32720) build(deps): bump the contrib-golang group in /contrib/golang/router/cluster_specifier/test/test_data/simple with 1 update (envoyproxy#32721) build(deps): bump the contrib-golang group in /contrib/golang/filters/http/test/test_data/echo with 1 update (envoyproxy#32722) build(deps): bump the examples-ext-authz group in /examples/ext_authz/auth/grpc-service with 1 update (envoyproxy#32723) build(deps): bump the contrib-golang group in /contrib/golang/filters/http/test/test_data/routeconfig with 1 update (envoyproxy#32724) build(deps): bump the examples-load-reporting group in /examples/load-reporting-service with 1 update (envoyproxy#32726) build(deps): bump the contrib-golang group in /contrib/golang/filters/http/test/test_data/buffer with 1 update (envoyproxy#32727) build(deps): bump the examples-golang-http group in /examples/golang-http/simple with 1 update (envoyproxy#32729) opentelemetrytracer: Add User-Agent header to OTLP trace exporters (envoyproxy#32659) build: remove incorrect cc_library after tls code move (envoyproxy#32714) ...
RyanTheOptimist
pushed a commit
that referenced
this pull request
Mar 7, 2024
Commit Message: the test failed after #32260 because the test server uses a mocked MockTransportSocketFactoryContext which doesn't return a real ContextManager object in sslContextManager(). The mocked object returns nullptr in createSslServerContext(), thus fail to initialize the new member ssl_ctx_ in QuicServerTransportSocketFactory. Additional Description: bazel test //test/java/io/envoyproxy/envoymobile/engine/testing:quic_test_server_test passes in this PR Risk Level: low, test only Testing: existing tests pass Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A Signed-off-by: Dan Zhang <danzh@google.com>
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 3, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 3, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 3, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 4, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 4, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 17, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 17, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 17, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
sayboras
added a commit
to cilium/proxy
that referenced
this pull request
Jul 23, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
github-merge-queue bot
pushed a commit
to cilium/proxy
that referenced
this pull request
Jul 23, 2024
Relates: envoyproxy/envoy#31841 Relates: envoyproxy/envoy#32203 Relates: envoyproxy/envoy#32260 Relates: envoyproxy/envoy#33019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Message: Added support for QUIC listeners to choose certificates based on SNI and load certificates from formats other than PEM, such as pkcs12. This brings feature parity between quic and non-quic TLS use for certificate selection and loading formats.
Additional Description:
Risk Level: Medium
Testing: existing tests
Docs Changes: none
Release Notes: added
Platform Specific Features:
Runtime guard:
envoy.restart_features.quic_handle_certs_with_shared_tls_code[Optional Fixes #Issue]