Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

quic: use OpenSSL built-in cert and hostname validation #34533

Closed
wants to merge 1 commit into from
Closed

quic: use OpenSSL built-in cert and hostname validation #34533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions node.gyp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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