You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "wwbmmm (via GitHub)" <gi...@apache.org> on 2026/07/26 03:44:48 UTC
[PR] Add TLS peer name verification for clients (brpc)
wwbmmm opened a new pull request, #3408:
URL: https://github.com/apache/brpc/pull/3408
### What problem does this PR solve?
Issue Number: null
Problem Summary:
Client TLS verification validates the certificate chain, but callers cannot
configure the DNS name or IP address expected in the peer certificate. This
makes it possible for a trusted certificate issued for another endpoint to be
accepted when only chain verification is configured.
### What is changed and the side effects?
Changed:
- Add `VerifyOptions::expected_peer_name` for DNS name and IP SAN validation.
- Derive the expected peer name from the host in an HTTPS Channel URL when
certificate verification is enabled and no explicit peer name is configured.
- Configure OpenSSL peer name verification as part of certificate verification.
- Reject peer name verification when certificate verification is disabled or
the TLS backend cannot support it.
- Include the verification mode and expected peer name in the channel signature
to prevent connections with different verification settings from being
shared.
- Add tests for matching DNS names, mismatched DNS names, IP SAN mismatches, and
invalid verification configurations.
Side effects:
- Performance effects: one DNS name or IP SAN comparison is added during a TLS
handshake when `expected_peer_name` is configured.
- Breaking backward compatibility: HTTPS Channels with certificate verification
enabled now reject certificates that do not match the URL host. Callers that
intentionally connect through an alias can set `expected_peer_name` explicitly.
OpenSSL versions older than 1.0.2 and MesaLink reject configurations that
require peer name verification because they do not provide the required API.
---
### Check List:
- [x] The project builds with `make -j6` in `build`.
- [x] `build/test/brpc_ssl_unittest` passes.
- [x] Related tests cover matching and mismatched peer identities.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add TLS peer name verification for clients (brpc)
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3408:
URL: https://github.com/apache/brpc/pull/3408#discussion_r3651836107
##########
src/brpc/channel.cpp:
##########
@@ -324,6 +328,20 @@ static int CreateSocketSSLContext(const ChannelOptions& options,
return 0;
}
+static void SetHttpsPeerName(const std::string& host,
+ ChannelOptions* options) {
+ ChannelSSLOptions* ssl = options->mutable_ssl_options();
+ if (ssl->sni_name.empty()) {
+ ssl->sni_name = host;
+ }
+ VerifyOptions& verify = ssl->verify;
+ if (verify.verify_depth > 0 &&
+ verify.verify_mode != VerifyMode::VERIFY_NONE &&
+ verify.expected_peer_name.empty()) {
+ verify.expected_peer_name = host;
+ }
+}
Review Comment:
SetHttpsPeerName() uses the host string returned by ParseURL() verbatim. For IPv6 literal URLs, ParseURL() keeps the square brackets (e.g. "[::1]") and SplitHostAndPort() does not strip them, so the derived `sni_name` / `verify.expected_peer_name` will include brackets and fail IP parsing + certificate verification (and may send an invalid SNI). Consider normalizing bracketed IPv6 hosts before assigning these fields.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add TLS peer name verification for clients (brpc)
Posted by "zyearn (via GitHub)" <gi...@apache.org>.
zyearn commented on PR #3408:
URL: https://github.com/apache/brpc/pull/3408#issuecomment-5088490804
My main concern is the older MesaLink/old-OpenSSL backward compatibility break. Otherwise LGTM
Is there anything we can do to avoid it? Maybe a flag so existing users won't be silently broken?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add TLS peer name verification for clients (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #3408:
URL: https://github.com/apache/brpc/pull/3408#issuecomment-5126973010
> My main concern is the older MesaLink/old-OpenSSL backward compatibility break. Otherwise LGTM
>
> Is there anything we can do to avoid it? Maybe a flag so existing users won't be silently broken?
This will only affect users who set verify.verify_depth > 0 && verify.verify_mode != VerifyMode::VERIFY_NONE . I think they need to verify the peer host.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add TLS peer name verification for clients (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #3408:
URL: https://github.com/apache/brpc/pull/3408#issuecomment-5127978191
> My main concern is the older MesaLink/old-OpenSSL backward compatibility break. Otherwise LGTM
>
> Is there anything we can do to avoid it? Maybe a flag so existing users won't be silently broken?
You are right. I’ve updated the PR to preserve backward compatibility:
* OpenSSL 1.0.2+ and BoringSSL automatically verify the HTTPS URL host.
* MesaLink and older OpenSSL retain chain-only verification and emit a one-time warning.
* Explicitly setting expected_peer_name still fails on unsupported backends instead of being silently ignored.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add TLS peer name verification for clients (brpc)
Posted by "lorinlee (via GitHub)" <gi...@apache.org>.
lorinlee merged PR #3408:
URL: https://github.com/apache/brpc/pull/3408
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org