Skip to content

Commit c2f39eb

Browse files
authored
Merge pull request #3408 from wwbmmm/verify-tls-peer-name
Add TLS peer name verification for clients
2 parents b52299c + ef7e5b9 commit c2f39eb

7 files changed

Lines changed: 176 additions & 7 deletions

File tree

src/brpc/channel.cpp

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include "brpc/policy/esp_authenticator.h"
4040
#include "brpc/transport_factory.h"
4141
#include "brpc/details/controller_private_accessor.h"
42+
#include "brpc/details/ssl_helper.h"
4243

4344
namespace brpc {
4445

@@ -128,7 +129,11 @@ static ChannelSignature ComputeChannelSignature(const ChannelOptions& opt) {
128129
buf.push_back('|');
129130
buf.append((char*)&verify.verify_depth, sizeof(verify.verify_depth));
130131
buf.push_back('|');
132+
buf.append((char*)&verify.verify_mode, sizeof(verify.verify_mode));
133+
buf.push_back('|');
131134
buf.append(verify.ca_file_path);
135+
buf.push_back('|');
136+
buf.append(verify.expected_peer_name);
132137
} else {
133138
// All disabled ChannelSSLOptions are the same
134139
}
@@ -324,6 +329,26 @@ static int CreateSocketSSLContext(const ChannelOptions& options,
324329
return 0;
325330
}
326331

332+
static void SetHttpsPeerName(const std::string& host,
333+
ChannelOptions* options) {
334+
ChannelSSLOptions* ssl = options->mutable_ssl_options();
335+
if (ssl->sni_name.empty()) {
336+
ssl->sni_name = host;
337+
}
338+
VerifyOptions& verify = ssl->verify;
339+
if (verify.verify_depth > 0 &&
340+
verify.verify_mode != VerifyMode::VERIFY_NONE &&
341+
verify.expected_peer_name.empty()) {
342+
if (SupportsPeerNameVerification()) {
343+
verify.expected_peer_name = host;
344+
} else {
345+
LOG_ONCE(WARNING)
346+
<< "The TLS backend does not support server identity "
347+
<< "verification; only the certificate chain will be verified";
348+
}
349+
}
350+
}
351+
327352
int Channel::Init(butil::EndPoint server_addr_and_port,
328353
const ChannelOptions* options) {
329354
return InitSingle(server_addr_and_port, "", options);
@@ -339,13 +364,12 @@ int Channel::InitSingle(const butil::EndPoint& server_addr_and_port,
339364
}
340365
int* port_out = raw_port == -1 ? &raw_port: NULL;
341366
ParseURL(raw_server_address, &_scheme, &_service_name, port_out);
367+
const std::string host = _service_name;
342368
if (raw_port != -1) {
343369
_service_name.append(":").append(std::to_string(raw_port));
344370
}
345371
if (_options.protocol == brpc::PROTOCOL_HTTP && _scheme == "https") {
346-
if (_options.mutable_ssl_options()->sni_name.empty()) {
347-
_options.mutable_ssl_options()->sni_name = _service_name;
348-
}
372+
SetHttpsPeerName(host, &_options);
349373
}
350374
const int port = server_addr_and_port.port;
351375
if (port < 0) {
@@ -392,13 +416,12 @@ int Channel::Init(const char* ns_url,
392416
}
393417
int raw_port = -1;
394418
ParseURL(ns_url, &_scheme, &_service_name, &raw_port);
419+
const std::string host = _service_name;
395420
if (raw_port != -1) {
396421
_service_name.append(":").append(std::to_string(raw_port));
397422
}
398423
if (_options.protocol == brpc::PROTOCOL_HTTP && _scheme == "https") {
399-
if (_options.mutable_ssl_options()->sni_name.empty()) {
400-
_options.mutable_ssl_options()->sni_name = _service_name;
401-
}
424+
SetHttpsPeerName(host, &_options);
402425
}
403426
butil::EndPoint client_endpoint;
404427
if (!_options.client_host.empty() &&

src/brpc/details/mesalink_ssl_helper.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@
3434

3535
namespace brpc {
3636

37+
bool SupportsPeerNameVerification() {
38+
return false;
39+
}
40+
3741
static const char* const PEM_START = "-----BEGIN";
3842

3943
static bool IsPemString(const std::string& input) {
@@ -240,6 +244,10 @@ static int LoadCertificate(SSL_CTX* ctx,
240244

241245
static int SetSSLOptions(SSL_CTX* ctx, const std::string& ciphers,
242246
int protocols, const VerifyOptions& verify) {
247+
if (!verify.expected_peer_name.empty()) {
248+
LOG(ERROR) << "Expected peer name verification is not supported by MesaLink";
249+
return -1;
250+
}
243251
if (verify.verify_depth > 0) {
244252
std::string cafile = verify.ca_file_path;
245253
if (!cafile.empty()) {

src/brpc/details/ssl_helper.cpp

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#ifndef USE_MESALINK
2424

2525
#include <sys/socket.h> // recv
26+
#include <arpa/inet.h> // inet_pton
2627
#include <pthread.h> // pthread_once
2728
#include <stdio.h> // fopen
2829
#include <stdlib.h> // getenv
@@ -39,6 +40,14 @@
3940

4041
namespace brpc {
4142

43+
bool SupportsPeerNameVerification() {
44+
#if defined(OPENSSL_IS_BORINGSSL) || OPENSSL_VERSION_NUMBER >= 0x10002000L
45+
return true;
46+
#else
47+
return false;
48+
#endif
49+
}
50+
4251
#ifndef OPENSSL_NO_DH
4352
static DH* g_dh_1024 = NULL;
4453
static DH* g_dh_2048 = NULL;
@@ -464,8 +473,12 @@ static int SetSSLOptions(SSL_CTX* ctx, const std::string& ciphers,
464473
return -1;
465474
}
466475

467-
// TODO: Verify the CNAME in certificate matches the requesting host
468476
if (verify.verify_depth > 0) {
477+
if (!verify.expected_peer_name.empty() &&
478+
verify.verify_mode == VerifyMode::VERIFY_NONE) {
479+
LOG(ERROR) << "Expected peer name requires peer verification";
480+
return -1;
481+
}
469482
if (verify.verify_mode == VerifyMode::VERIFY_FAIL_IF_NO_PEER_CERT) {
470483
SSL_CTX_set_verify(ctx, (SSL_VERIFY_PEER
471484
| SSL_VERIFY_FAIL_IF_NO_PEER_CERT), NULL);
@@ -493,7 +506,36 @@ static int SetSSLOptions(SSL_CTX* ctx, const std::string& ciphers,
493506
return -1;
494507
}
495508
}
509+
if (!verify.expected_peer_name.empty()) {
510+
#if defined(OPENSSL_IS_BORINGSSL) || OPENSSL_VERSION_NUMBER >= 0x10002000L
511+
X509_VERIFY_PARAM* param = SSL_CTX_get0_param(ctx);
512+
unsigned char address[sizeof(struct in6_addr)];
513+
int rc = 0;
514+
if (inet_pton(AF_INET, verify.expected_peer_name.c_str(), address) == 1 ||
515+
inet_pton(AF_INET6, verify.expected_peer_name.c_str(),
516+
address) == 1) {
517+
rc = X509_VERIFY_PARAM_set1_ip_asc(
518+
param, verify.expected_peer_name.c_str());
519+
} else {
520+
rc = X509_VERIFY_PARAM_set1_host(
521+
param, verify.expected_peer_name.c_str(), 0);
522+
}
523+
if (rc != 1) {
524+
LOG(ERROR) << "Fail to set expected peer name "
525+
<< verify.expected_peer_name << ": "
526+
<< SSLError(ERR_get_error());
527+
return -1;
528+
}
529+
#else
530+
LOG(ERROR) << "Expected peer name verification requires OpenSSL 1.0.2+";
531+
return -1;
532+
#endif
533+
}
496534
} else {
535+
if (!verify.expected_peer_name.empty()) {
536+
LOG(ERROR) << "Expected peer name requires peer verification";
537+
return -1;
538+
}
497539
SSL_CTX_set_verify(ctx, SSL_VERIFY_NONE, NULL);
498540
}
499541

src/brpc/details/ssl_helper.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ enum SSLProtocol {
5656
TLSv1_3 = 1 << 4,
5757
};
5858

59+
bool SupportsPeerNameVerification();
60+
5961
struct FreeSSLCTX {
6062
inline void operator()(SSL_CTX* ctx) const {
6163
if (ctx != NULL) {

src/brpc/ssl_options.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ struct VerifyOptions {
6666
// If empty, use the system default CA files
6767
// Default: ""
6868
std::string ca_file_path;
69+
70+
// Set the expected DNS name or IP address in the peer's certificate
71+
// Default: ""
72+
std::string expected_peer_name;
6973
};
7074

7175
// SSL options at client side

test/brpc_channel_unittest.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2425,10 +2425,27 @@ TEST_F(ChannelTest, parse_hostname) {
24252425
ASSERT_EQ(0, channel.Init("http://www.baidu.com:8888", "rr", &opt));
24262426
ASSERT_EQ("www.baidu.com:8888", channel._service_name);
24272427

2428+
opt.mutable_ssl_options()->verify.verify_mode = brpc::VerifyMode::VERIFY_PEER;
2429+
opt.mutable_ssl_options()->verify.verify_depth = 1;
2430+
opt.mutable_ssl_options()->verify.ca_file_path = "cert1.crt";
24282431
ASSERT_EQ(0, channel.Init("https://www.baidu.com", &opt));
24292432
ASSERT_EQ("www.baidu.com", channel._service_name);
2433+
#if defined(USE_MESALINK) || \
2434+
(!defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_NUMBER < 0x10002000L)
2435+
ASSERT_TRUE(channel._options.ssl_options().verify.expected_peer_name.empty());
2436+
#else
2437+
ASSERT_EQ("www.baidu.com",
2438+
channel._options.ssl_options().verify.expected_peer_name);
2439+
#endif
24302440
ASSERT_EQ(0, channel.Init("https://www.baidu.com:443", &opt));
24312441
ASSERT_EQ("www.baidu.com:443", channel._service_name);
2442+
#if defined(USE_MESALINK) || \
2443+
(!defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_NUMBER < 0x10002000L)
2444+
ASSERT_TRUE(channel._options.ssl_options().verify.expected_peer_name.empty());
2445+
#else
2446+
ASSERT_EQ("www.baidu.com",
2447+
channel._options.ssl_options().verify.expected_peer_name);
2448+
#endif
24322449
ASSERT_EQ(0, channel.Init("https://www.baidu.com", 443, &opt));
24332450
ASSERT_EQ("www.baidu.com:443", channel._service_name);
24342451
ASSERT_EQ(0, channel.Init("https://www.baidu.com:1443", &opt));

test/brpc_ssl_unittest.cpp

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,79 @@ TEST_F(SSLTest, force_ssl) {
229229
ASSERT_EQ(0, server.Join());
230230
}
231231

232+
void CallServerWithExpectedPeerName(int port, const char* server_address,
233+
const char* expected_peer_name,
234+
bool expect_success) {
235+
brpc::Channel channel;
236+
brpc::ChannelOptions options;
237+
options.protocol = brpc::PROTOCOL_HTTP;
238+
options.mutable_ssl_options()->verify.verify_mode =
239+
brpc::VerifyMode::VERIFY_PEER;
240+
options.mutable_ssl_options()->verify.verify_depth = 1;
241+
options.mutable_ssl_options()->verify.ca_file_path = "cert1.crt";
242+
if (expected_peer_name != NULL) {
243+
options.mutable_ssl_options()->verify.expected_peer_name = expected_peer_name;
244+
}
245+
std::string url = server_address;
246+
url.append(":").append(std::to_string(port));
247+
ASSERT_EQ(0, channel.Init(url.c_str(), &options));
248+
249+
test::EchoRequest req;
250+
test::EchoResponse res;
251+
req.set_message(EXP_REQUEST);
252+
brpc::Controller cntl;
253+
test::EchoService_Stub stub(&channel);
254+
stub.Echo(&cntl, &req, &res, NULL);
255+
if (expect_success) {
256+
EXPECT_FALSE(cntl.Failed()) << cntl.ErrorText();
257+
EXPECT_EQ(EXP_RESPONSE, res.message());
258+
} else {
259+
EXPECT_TRUE(cntl.Failed());
260+
}
261+
}
262+
263+
TEST_F(SSLTest, verify_peer_name) {
264+
const int port = 8613;
265+
brpc::Server server;
266+
brpc::ServerOptions server_options;
267+
brpc::CertInfo cert;
268+
cert.certificate = "cert1.crt";
269+
cert.private_key = "cert1.key";
270+
server_options.mutable_ssl_options()->default_cert = cert;
271+
272+
EchoServiceImpl echo_svc;
273+
ASSERT_EQ(0, server.AddService(
274+
&echo_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
275+
ASSERT_EQ(0, server.Start(port, &server_options));
276+
277+
CallServerWithExpectedPeerName(port, "https://localhost", NULL, true);
278+
CallServerWithExpectedPeerName(port, "https://127.0.0.1", NULL, false);
279+
CallServerWithExpectedPeerName(
280+
port, "https://localhost", "wrong.local", false);
281+
282+
ASSERT_EQ(0, server.Stop(0));
283+
ASSERT_EQ(0, server.Join());
284+
}
285+
286+
TEST_F(SSLTest, expected_peer_name_requires_peer_verification) {
287+
brpc::ChannelSSLOptions options;
288+
options.verify.expected_peer_name = "localhost";
289+
EXPECT_EQ(NULL, brpc::CreateClientSSLContext(options));
290+
291+
options.verify.verify_depth = 1;
292+
options.verify.verify_mode = brpc::VerifyMode::VERIFY_NONE;
293+
EXPECT_EQ(NULL, brpc::CreateClientSSLContext(options));
294+
}
295+
296+
TEST_F(SSLTest, peer_name_verification_capability) {
297+
#if defined(USE_MESALINK) || \
298+
(!defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_NUMBER < 0x10002000L)
299+
EXPECT_FALSE(brpc::SupportsPeerNameVerification());
300+
#else
301+
EXPECT_TRUE(brpc::SupportsPeerNameVerification());
302+
#endif
303+
}
304+
232305
void ProcessResponse(brpc::InputMessageBase* msg_base) {
233306
brpc::DestroyingPtr<brpc::policy::MostCommonMessage> msg(
234307
static_cast<brpc::policy::MostCommonMessage*>(msg_base));

0 commit comments

Comments
 (0)