You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ti...@apache.org on 2016/07/04 16:42:58 UTC
[2/5] mesos git commit: Updated certificate validation to check 'IP
Address' SAN.
Updated certificate validation to check 'IP Address' SAN.
Allows the verification of X509 certificates based on an IP address
instead of a hostname. Introduces a new environment variable;
\`SSL_VERIFY_IPADD\` which, when set to \`true\` will enable the
peer certificate verification to additionally rely on the IP
address of a connection.
Review: https://reviews.apache.org/r/49401/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/36a3c649
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/36a3c649
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/36a3c649
Branch: refs/heads/master
Commit: 36a3c6494be91e7a9b898c3c8eebe59883c050e4
Parents: 6133e72
Author: Till Toenshoff <to...@me.com>
Authored: Mon Jul 4 18:32:40 2016 +0200
Committer: Till Toenshoff <to...@me.com>
Committed: Mon Jul 4 18:32:40 2016 +0200
----------------------------------------------------------------------
3rdparty/libprocess/src/libevent_ssl_socket.cpp | 14 +-
3rdparty/libprocess/src/libevent_ssl_socket.hpp | 1 +
3rdparty/libprocess/src/openssl.cpp | 142 ++++++++++++++-----
3rdparty/libprocess/src/openssl.hpp | 8 +-
4 files changed, 125 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/36a3c649/3rdparty/libprocess/src/libevent_ssl_socket.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.cpp b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
index 19d9ae5..2e7f332 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -396,7 +396,7 @@ void LibeventSSLSocketImpl::event_callback(short events)
// Do post-validation of connection.
SSL* ssl = bufferevent_openssl_get_ssl(bev);
- Try<Nothing> verify = openssl::verify(ssl, peer_hostname);
+ Try<Nothing> verify = openssl::verify(ssl, peer_hostname, peer_ip);
if (verify.isError()) {
VLOG(1) << "Failed connect, verification error: " << verify.error();
SSL_free(ssl);
@@ -513,7 +513,7 @@ Future<Nothing> LibeventSSLSocketImpl::connect(const Address& address)
}
// Try and determine the 'peer_hostname' from the address we're
- // connecting to in order to properly verify the SSL connection later.
+ // connecting to in order to properly verify the certificate later.
const Try<string> hostname = address.hostname();
if (hostname.isError()) {
@@ -523,6 +523,10 @@ Future<Nothing> LibeventSSLSocketImpl::connect(const Address& address)
peer_hostname = hostname.get();
}
+ // Determine the 'peer_ip' from the address we're connecting to in
+ // order to properly verify the certificate later.
+ peer_ip = address.ip;
+
// Optimistically construct a 'ConnectRequest' and future.
Owned<ConnectRequest> request(new ConnectRequest());
Future<Nothing> future = request->promise.future();
@@ -1023,8 +1027,10 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request)
// post-verification. First, we need to determine the peer
// hostname.
Option<string> peer_hostname = None();
+
if (request->ip.isSome()) {
Try<string> hostname = net::getHostname(request->ip.get());
+
if (hostname.isError()) {
VLOG(2) << "Could not determine hostname of peer: "
<< hostname.error();
@@ -1037,7 +1043,9 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request)
SSL* ssl = bufferevent_openssl_get_ssl(bev);
CHECK_NOTNULL(ssl);
- Try<Nothing> verify = openssl::verify(ssl, peer_hostname);
+ Try<Nothing> verify =
+ openssl::verify(ssl, peer_hostname, request->ip);
+
if (verify.isError()) {
VLOG(1) << "Failed accept, verification error: " << verify.error();
request->promise.fail(verify.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/36a3c649/3rdparty/libprocess/src/libevent_ssl_socket.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.hpp b/3rdparty/libprocess/src/libevent_ssl_socket.hpp
index 1dbdaa8..4d376d8 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.hpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.hpp
@@ -178,6 +178,7 @@ private:
Queue<Future<Socket>> accept_queue;
Option<std::string> peer_hostname;
+ Option<net::IP> peer_ip;
// Socket descriptor/handle used by libevent_ssl.
// Ownership semantics:
http://git-wip-us.apache.org/repos/asf/mesos/blob/36a3c649/3rdparty/libprocess/src/openssl.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/openssl.cpp b/3rdparty/libprocess/src/openssl.cpp
index 0f62aa6..63916ff 100644
--- a/3rdparty/libprocess/src/openssl.cpp
+++ b/3rdparty/libprocess/src/openssl.cpp
@@ -80,6 +80,12 @@ Flags::Flags()
"certificate implies verifying it.",
false);
+ add(&Flags::verify_ipadd,
+ "verify_ipadd",
+ "Enable IP address verification in subject alternative name certificate "
+ "extension.",
+ false);
+
add(&Flags::verification_depth,
"verification_depth",
"Maximum depth for the certificate chain verification that shall be "
@@ -401,6 +407,11 @@ void reinitialize()
<< "verification";
}
+ if (ssl_flags->verify_ipadd) {
+ VLOG(2) << "Will use IP address verification in subject alternative name "
+ << "certificate extension.";
+ }
+
if (ssl_flags->require_cert && !ssl_flags->verify_cert) {
// Requiring a certificate implies that is should be verified.
ssl_flags->verify_cert = true;
@@ -543,7 +554,10 @@ SSL_CTX* context()
}
-Try<Nothing> verify(const SSL* const ssl, const Option<string>& hostname)
+Try<Nothing> verify(
+ const SSL* const ssl,
+ const Option<string>& hostname,
+ const Option<net::IP>& ip)
{
// Return early if we don't need to verify.
if (!ssl_flags->verify_cert) {
@@ -565,7 +579,7 @@ Try<Nothing> verify(const SSL* const ssl, const Option<string>& hostname)
return Error("Could not verify peer certificate");
}
- if (hostname.isNone()) {
+ if (!ssl_flags->verify_ipadd && hostname.isNone()) {
X509_free(cert);
return ssl_flags->require_cert
? Error("Cannot verify peer certificate: peer hostname unknown")
@@ -589,24 +603,62 @@ Try<Nothing> verify(const SSL* const ssl, const Option<string>& hostname)
for (int i = 0; i < san_names_num; i++) {
const GENERAL_NAME* current_name = sk_GENERAL_NAME_value(san_names, i);
- if (current_name->type == GEN_DNS) {
- // Current name is a DNS name, let's check it.
- const string dns_name =
- reinterpret_cast<char*>(ASN1_STRING_data(current_name->d.dNSName));
+ switch(current_name->type) {
+ case GEN_DNS: {
+ if (hostname.isSome()) {
+ // Current name is a DNS name, let's check it.
+ const string dns_name =
+ reinterpret_cast<char*>(ASN1_STRING_data(
+ current_name->d.dNSName));
+
+ // Make sure there isn't an embedded NUL character in the DNS name.
+ const size_t length = ASN1_STRING_length(current_name->d.dNSName);
+ if (length != dns_name.length()) {
+ sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free);
+ X509_free(cert);
+ return Error(
+ "X509 certificate malformed: "
+ "embedded NUL character in DNS name");
+ } else {
+ VLOG(2) << "Matching dNSName(" << i << "): " << dns_name;
+
+ // Compare expected hostname with the DNS name.
+ if (hostname.get() == dns_name) {
+ sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free);
+ X509_free(cert);
+
+ VLOG(2) << "dNSName match found for " << hostname.get();
+
+ return Nothing();
+ }
+ }
+ }
+ break;
+ }
+ case GEN_IPADD: {
+ if (ssl_flags->verify_ipadd && ip.isSome()) {
+ // Current name is an IPAdd, let's check it.
+ const ASN1_OCTET_STRING* current_ipadd = current_name->d.iPAddress;
- // Make sure there isn't an embedded NUL character in the DNS name.
- const size_t length = ASN1_STRING_length(current_name->d.dNSName);
- if (length != dns_name.length()) {
- sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free);
- X509_free(cert);
- return Error(
- "X509 certificate malformed: embedded NUL character in DNS name");
- } else { // Compare expected hostname with the DNS name.
- if (hostname.get() == dns_name) {
- sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free);
- X509_free(cert);
- return Nothing();
+ if (current_ipadd->type == V_ASN1_OCTET_STRING &&
+ current_ipadd->data != nullptr &&
+ current_ipadd->length == sizeof(uint32_t)) {
+ const net::IP ip_add(ntohl(
+ *reinterpret_cast<uint32_t*>(current_ipadd->data)));
+
+ VLOG(2) << "Matching iPAddress(" << i << "): " << ip_add;
+
+ if (ip.get() == ip_add) {
+ sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free);
+ X509_free(cert);
+
+ VLOG(2) << "iPAddress match found for " << ip.get();
+
+ return Nothing();
+ }
+ }
}
+ break;
}
}
}
@@ -614,34 +666,52 @@ Try<Nothing> verify(const SSL* const ssl, const Option<string>& hostname)
sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free);
}
- // If we still haven't verified the hostname, try doing it via
- // the certificate subject name.
- X509_NAME* name = X509_get_subject_name(cert);
+ if (hostname.isSome()) {
+ // If we still haven't verified the hostname, try doing it via
+ // the certificate subject name.
+ X509_NAME* name = X509_get_subject_name(cert);
+
+ if (name != nullptr) {
+ char text[_POSIX_HOST_NAME_MAX] {};
- if (name != nullptr) {
- char text[_POSIX_HOST_NAME_MAX] {};
+ if (X509_NAME_get_text_by_NID(
+ name,
+ NID_commonName,
+ text,
+ sizeof(text)) > 0) {
+ VLOG(2) << "Matching common name: " << text;
+
+ if (hostname.get() != text) {
+ X509_free(cert);
+ return Error(
+ "Presented Certificate Name: " + stringify(text) +
+ " does not match peer hostname name: " + hostname.get());
+ }
+
+ VLOG(2) << "Common name match found for " << hostname.get();
- if (X509_NAME_get_text_by_NID(
- name,
- NID_commonName,
- text,
- sizeof(text)) > 0) {
- if (hostname.get() != text) {
X509_free(cert);
- return Error(
- "Presented Certificate Name: " + stringify(text) +
- " does not match peer hostname name: " + hostname.get());
+ return Nothing();
}
-
- X509_free(cert);
- return Nothing();
}
}
// If we still haven't exited, we haven't verified it, and we give up.
X509_free(cert);
+
+ std::vector<string> details;
+
+ if (hostname.isSome()) {
+ details.push_back("hostname " + hostname.get());
+ }
+
+ if (ip.isSome()) {
+ details.push_back("IP " + stringify(ip.get()));
+ }
+
return Error(
- "Could not verify presented certificate with hostname " + hostname.get());
+ "Could not verify presented certificate with " +
+ strings::join(", ", details));
}
} // namespace openssl {
http://git-wip-us.apache.org/repos/asf/mesos/blob/36a3c649/3rdparty/libprocess/src/openssl.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/openssl.hpp b/3rdparty/libprocess/src/openssl.hpp
index 7d55025..68f8897 100644
--- a/3rdparty/libprocess/src/openssl.hpp
+++ b/3rdparty/libprocess/src/openssl.hpp
@@ -18,6 +18,7 @@
#include <string>
#include <stout/flags.hpp>
+#include <stout/ip.hpp>
#include <stout/nothing.hpp>
#include <stout/option.hpp>
#include <stout/try.hpp>
@@ -39,6 +40,7 @@ public:
Option<std::string> key_file;
bool verify_cert;
bool require_cert;
+ bool verify_ipadd;
unsigned int verification_depth;
Option<std::string> ca_dir;
Option<std::string> ca_file;
@@ -61,6 +63,7 @@ const Flags& flags();
// SSL_KEY_FILE=(path to key)
// SSL_VERIFY_CERT=(false|0,true|1)
// SSL_REQUIRE_CERT=(false|0,true|1)
+// SSL_VERIFY_IPADD=(false|0,true|1)
// SSL_VERIFY_DEPTH=(4)
// SSL_CA_DIR=(path to CA directory)
// SSL_CA_FILE=(path to CA file)
@@ -82,7 +85,10 @@ SSL_CTX* context();
// Verify that the hostname is properly associated with the peer
// certificate associated with the specified SSL connection.
-Try<Nothing> verify(const SSL* const ssl, const Option<std::string>& hostname);
+Try<Nothing> verify(
+ const SSL* const ssl,
+ const Option<std::string>& hostname = None(),
+ const Option<net::IP>& ip = None());
} // namespace openssl {
} // namespace network {