Skip to content

Commit

Permalink
quic: use OpenSSL built-in cert and hostname validation
Browse files Browse the repository at this point in the history 
PR-URL: #34533
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
@jasnell
jasnell committed Jul 31, 2020
1 parent 1e47051 commit a97b5f9
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 608 deletions.
3 changes: 1 addition & 2 deletions node.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1235,8 +1235,7 @@
],
'sources': [
'test/cctest/test_quic_buffer.cc',
'test/cctest/test_quic_cid.cc',
'test/cctest/test_quic_verifyhostnameidentity.cc'
'test/cctest/test_quic_cid.cc'
]
}],
['v8_enable_inspector==1', {
Expand Down
232 changes: 8 additions & 224 deletions src/quic/node_quic_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,229 +331,6 @@ bool InvalidRetryToken(
return false;
}

namespace {

bool SplitHostname(
const char* hostname,
std::vector<std::string>* parts,
const char delim = '.') {
static std::string check_str =
"\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2A\x2B\x2C\x2D\x2E\x2F\x30"
"\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3A\x3B\x3C\x3D\x3E\x3F\x40"
"\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4A\x4B\x4C\x4D\x4E\x4F\x50"
"\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5A\x5B\x5C\x5D\x5E\x5F\x60"
"\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6A\x6B\x6C\x6D\x6E\x6F\x70"
"\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7A\x7B\x7C\x7D\x7E\x7F";

std::stringstream str(hostname);
std::string part;
while (getline(str, part, delim)) {
// if (part.length() == 0 ||
// part.find_first_not_of(check_str) != std::string::npos) {
// return false;
// }
for (size_t n = 0; n < part.length(); n++) {
if (part[n] >= 'A' && part[n] <= 'Z')
part[n] = (part[n] | 0x20); // Lower case the letter
if (check_str.find(part[n]) == std::string::npos)
return false;
}
parts->push_back(part);
}
return true;
}

bool CheckCertNames(
const std::vector<std::string>& host_parts,
const std::string& name,
bool use_wildcard = true) {

if (name.length() == 0)
return false;

std::vector<std::string> name_parts;
if (!SplitHostname(name.c_str(), &name_parts))
return false;

if (name_parts.size() != host_parts.size())
return false;

for (size_t n = host_parts.size() - 1; n > 0; --n) {
if (host_parts[n] != name_parts[n])
return false;
}

if (name_parts[0].find('*') == std::string::npos ||
name_parts[0].find("xn--") != std::string::npos) {
return host_parts[0] == name_parts[0];
}

if (!use_wildcard)
return false;

std::vector<std::string> sub_parts;
SplitHostname(name_parts[0].c_str(), &sub_parts, '*');

if (sub_parts.size() > 2)
return false;

if (name_parts.size() <= 2)
return false;

std::string prefix;
std::string suffix;
if (sub_parts.size() == 2) {
prefix = sub_parts[0];
suffix = sub_parts[1];
} else {
prefix = "";
suffix = sub_parts[0];
}

if (prefix.length() + suffix.length() > host_parts[0].length())
return false;

if (host_parts[0].compare(0, prefix.length(), prefix))
return false;

if (host_parts[0].compare(
host_parts[0].length() - suffix.length(),
suffix.length(), suffix)) {
return false;
}

return true;
}

} // namespace

int VerifyHostnameIdentity(
const crypto::SSLPointer& ssl,
const char* hostname) {
int err = X509_V_ERR_HOSTNAME_MISMATCH;
crypto::X509Pointer cert(SSL_get_peer_certificate(ssl.get()));
if (!cert)
return err;

// There are several pieces of information we need from the cert at this point
// 1. The Subject (if it exists)
// 2. The collection of Alt Names (if it exists)
//
// The certificate may have many Alt Names. We only care about the ones that
// are prefixed with 'DNS:', 'URI:', or 'IP Address:'. We might check
// additional ones later but we'll start with these.
//
// Ideally, we'd be able to *just* use OpenSSL's built in name checking for
// this (SSL_set1_host and X509_check_host) but it does not appear to do
// checking on URI or IP Address Alt names, which is unfortunate. We need
// both of those to retain compatibility with the peer identity verification
// Node.js already does elsewhere. At the very least, we'll use
// X509_check_host here first as a first step. If it is successful, awesome,
// there's nothing else for us to do. Return and be happy!
if (X509_check_host(
cert.get(),
hostname,
strlen(hostname),
X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT |
X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS |
X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS,
nullptr) > 0) {
return 0;
}

if (X509_check_ip_asc(
cert.get(),
hostname,
X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT) > 0) {
return 0;
}

// If we've made it this far, then we have to perform a more check
return VerifyHostnameIdentity(
hostname,
crypto::GetCertificateCN(cert.get()),
crypto::GetCertificateAltNames(cert.get()));
}

int VerifyHostnameIdentity(
const char* hostname,
const std::string& cert_cn,
const std::unordered_multimap<std::string, std::string>& altnames) {

int err = X509_V_ERR_HOSTNAME_MISMATCH;

// 1. If the hostname is an IP address (v4 or v6), the certificate is valid
// if and only if there is an 'IP Address:' alt name specifying the same
// IP address. The IP address must be canonicalized to ensure a proper
// check. It's possible that the X509_check_ip_asc covers this. If so,
// we can remove this check.

if (SocketAddress::is_numeric_host(hostname)) {
auto ips = altnames.equal_range("ip");
for (auto ip = ips.first; ip != ips.second; ++ip) {
if (ip->second.compare(hostname) == 0) {
// Success!
return 0;
}
}
// No match, and since the hostname is an IP address, skip any
// further checks
return err;
}

auto dns_names = altnames.equal_range("dns");
auto uri_names = altnames.equal_range("uri");

size_t dns_count = std::distance(dns_names.first, dns_names.second);
size_t uri_count = std::distance(uri_names.first, uri_names.second);

std::vector<std::string> host_parts;
SplitHostname(hostname, &host_parts);

// 2. If there no 'DNS:' or 'URI:' Alt names, if the certificate has a
// Subject, then we need to extract the CN field from the Subject. and
// check that the hostname matches the CN, taking into consideration
// the possibility of a wildcard in the CN. If there is a match, congrats,
// we have a valid certificate. Return and be happy.

if (dns_count == 0 && uri_count == 0) {
if (cert_cn.length() > 0 && CheckCertNames(host_parts, cert_cn))
return 0;
// No match, and since there are no dns or uri entries, return
return err;
}

// 3. If, however, there are 'DNS:' and 'URI:' Alt names, things become more
// complicated. Essentially, we need to iterate through each 'DNS:' and
// 'URI:' Alt name to find one that matches. The 'DNS:' Alt names are
// relatively simple but may include wildcards. The 'URI:' Alt names
// require the name to be parsed as a URL, then extract the hostname from
// the URL, which is then checked against the hostname. If you find a
// match, yay! Return and be happy. (Note, it's possible that the 'DNS:'
// check in this step is redundant to the X509_check_host check. If so,
// we can simplify by removing those checks here.)

// First, let's check dns names
for (auto name = dns_names.first; name != dns_names.second; ++name) {
if (name->first.length() > 0 &&
CheckCertNames(host_parts, name->second)) {
return 0;
}
}

// Then, check uri names
for (auto name = uri_names.first; name != uri_names.second; ++name) {
if (name->first.length() > 0 &&
CheckCertNames(host_parts, name->second, false)) {
return 0;
}
}

// 4. Failing all of the previous checks, we assume the certificate is
// invalid for an unspecified reason.
return err;
}

// Get the ALPN protocol identifier that was negotiated for the session
Local<Value> GetALPNProtocol(const QuicSession& session) {
QuicCryptoContext* ctx = session.crypto_context();
Expand Down Expand Up @@ -747,10 +524,17 @@ SSL_QUIC_METHOD quic_method = SSL_QUIC_METHOD{
void SetHostname(const crypto::SSLPointer& ssl, const std::string& hostname) {
// If the hostname is an IP address, use an empty string
// as the hostname instead.
if (SocketAddress::is_numeric_host(hostname.c_str())) {
X509_VERIFY_PARAM* param = SSL_get0_param(ssl.get());
X509_VERIFY_PARAM_set_hostflags(param, 0);

if (UNLIKELY(SocketAddress::is_numeric_host(hostname.c_str()))) {
SSL_set_tlsext_host_name(ssl.get(), "");
CHECK_EQ(X509_VERIFY_PARAM_set1_host(param, "", 0), 1);
} else {
SSL_set_tlsext_host_name(ssl.get(), hostname.c_str());
CHECK_EQ(
X509_VERIFY_PARAM_set1_host(param, hostname.c_str(), hostname.length()),
1);
}
}

Expand Down
6 changes: 0 additions & 6 deletions src/quic/node_quic_crypto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,6 @@ bool InvalidRetryToken(
const uint8_t* token_secret,
uint64_t verification_expiration);

int VerifyHostnameIdentity(const crypto::SSLPointer& ssl, const char* hostname);
int VerifyHostnameIdentity(
const char* hostname,
const std::string& cert_cn,
const std::unordered_multimap<std::string, std::string>& altnames);

// Get the ALPN protocol identifier that was negotiated for the session
v8::Local<v8::Value> GetALPNProtocol(const QuicSession& session);

Expand Down
37 changes: 11 additions & 26 deletions src/quic/node_quic_session.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -570,19 +570,22 @@ void JSQuicSessionListener::OnHandshakeCompleted() {
String::NewFromUtf8(env->isolate(), hostname).ToLocalChecked();
}

int err = ctx->VerifyPeerIdentity(
hostname != nullptr ?
hostname :
session()->hostname().c_str());
Local<Value> validationErrorReason = v8::Null(env->isolate());
Local<Value> validationErrorCode = v8::Null(env->isolate());
int err = ctx->VerifyPeerIdentity();
if (err != X509_V_OK) {
crypto::GetValidationErrorReason(env, err).ToLocal(&validationErrorReason);
crypto::GetValidationErrorCode(env, err).ToLocal(&validationErrorCode);
}

Local<Value> argv[] = {
servername,
GetALPNProtocol(*session()),
ctx->cipher_name().ToLocalChecked(),
ctx->cipher_version().ToLocalChecked(),
Integer::New(env->isolate(), session()->max_pktlen_),
crypto::GetValidationErrorReason(env, err).ToLocalChecked(),
crypto::GetValidationErrorCode(env, err).ToLocalChecked(),
validationErrorReason,
validationErrorCode,
session()->crypto_context()->early_data() ?
v8::True(env->isolate()) :
v8::False(env->isolate())
Expand Down Expand Up @@ -1144,26 +1147,8 @@ bool QuicCryptoContext::InitiateKeyUpdate() {
uv_hrtime()) == 0;
}

int QuicCryptoContext::VerifyPeerIdentity(const char* hostname) {
int err = crypto::VerifyPeerCertificate(ssl_);
if (err)
return err;

// QUIC clients are required to verify the peer identity, servers are not.
switch (side_) {
case NGTCP2_CRYPTO_SIDE_CLIENT:
if (LIKELY(is_option_set(
QUICCLIENTSESSION_OPTION_VERIFY_HOSTNAME_IDENTITY))) {
return VerifyHostnameIdentity(ssl_, hostname);
}
break;
case NGTCP2_CRYPTO_SIDE_SERVER:
// TODO(@jasnell): In the future, we may want to implement this but
// for now we keep things simple and skip peer identity verification.
break;
}

return 0;
int QuicCryptoContext::VerifyPeerIdentity() {
return crypto::VerifyPeerCertificate(ssl_);
}

// Write outbound TLS handshake data into the ngtcp2 connection
Expand Down
2 changes: 1 addition & 1 deletion src/quic/node_quic_session.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ class QuicCryptoContext final : public MemoryRetainer {

bool InitiateKeyUpdate();

int VerifyPeerIdentity(const char* hostname);
int VerifyPeerIdentity();

QuicSession* session() const { return session_.get(); }

Expand Down
Loading

0 comments on commit a97b5f9

Please sign in to comment.