Permalink
Browse files

CVE-2015-4663: Support verifying certificates on ssl/tls streams, ena…

…ble by default

Summary: - thread through the StreamContext so it's actually possible to change
   SSLSocket settings - code to enable it was unreachable
 - affects both raw sockets and file_get_contents etc
 - CuRL was unaffected
 - enable verify_peer by default (behavior change in PHP 5.6.9)
 - use the system certificate store if none is specified

With thanks to @

Reviewed By: @siyengar

Differential Revision: D2171039
  • Loading branch information...
1 parent 8a57588 commit e282a459188a472e177b45ad2d2989289294df74 @fredemmott fredemmott committed with hhvm-bot Jun 18, 2015
@@ -136,7 +136,7 @@ int HttpClient::request(const char* verb,
curl_easy_setopt(cp, CURLOPT_DNS_USE_GLOBAL_CACHE, 0); // for thread-safe
curl_easy_setopt(cp, CURLOPT_DNS_CACHE_TIMEOUT, 120);
curl_easy_setopt(cp, CURLOPT_NOSIGNAL, 1); // for multithreading mode
- curl_easy_setopt(cp, CURLOPT_SSL_VERIFYPEER, 0);
+ curl_easy_setopt(cp, CURLOPT_SSL_VERIFYPEER, 1);
curl_easy_setopt(cp, CURLOPT_SSL_CTX_FUNCTION, curl_tls_workarounds_cb);
/*
@@ -214,8 +214,9 @@ int HttpClient::request(const char* verb,
if (m_stream_context_options[s_ssl].isArray()) {
const Array ssl = m_stream_context_options[s_ssl].toArray();
- if (ssl[s_verify_peer].toBoolean()) {
- curl_easy_setopt(cp, CURLOPT_SSL_VERIFYPEER, 1);
+ if (ssl.exists(s_verify_peer)) {
+ curl_easy_setopt(cp, CURLOPT_SSL_VERIFYPEER,
+ ssl[s_verify_peer].toBoolean());
}
if (ssl.exists(s_capath)) {
curl_easy_setopt(cp, CURLOPT_CAPATH,
@@ -126,6 +126,7 @@ HttpStreamWrapper::open(const String& filename,
auto file = makeSmartPtr<UrlFile>(method.data(), headers,
post_data, max_redirs,
timeout, ignore_errors);
+ file->setStreamContext(context);
file->setProxy(proxy_host, proxy_port, proxy_user, proxy_pass);
bool ret = file->open(filename, mode);
if (!ret) {
@@ -128,6 +128,10 @@ SSL *SSLSocket::createSSL(SSL_CTX *ctx) {
cafile.data(), capath.data());
return nullptr;
}
+ } else if (m_data->m_client && !SSL_CTX_set_default_verify_paths(ctx)) {
+ raise_warning(
+ "Unable to set default verify locations and no CA settings specified");
+ return nullptr;
}
int64_t depth = m_context[s_verify_depth].toInt64();
@@ -200,11 +204,19 @@ SSLSocket::SSLSocket()
m_data(static_cast<SSLSocketData*>(getSocketData()))
{}
-SSLSocket::SSLSocket(int sockfd, int type, const char *address /* = NULL */,
- int port /* = 0 */)
+StaticString s_ssl("ssl");
+
+SSLSocket::SSLSocket(int sockfd, int type, const SmartPtr<StreamContext>& ctx,
+ const char *address /* = NULL */, int port /* = 0 */)
: Socket(std::make_shared<SSLSocketData>(), sockfd, type, address, port),
m_data(static_cast<SSLSocketData*>(getSocketData()))
-{}
+{
+ if (!ctx) {
+ return;
+ }
+ this->setStreamContext(ctx);
+ this->m_context = ctx->getOptions()[s_ssl].toArray();
+}
SSLSocket::~SSLSocket() { }
@@ -322,7 +334,8 @@ bool SSLSocket::handleError(int64_t nr_bytes, bool is_init) {
///////////////////////////////////////////////////////////////////////////////
SmartPtr<SSLSocket> SSLSocket::Create(
- int fd, int domain, const HostURL &hosturl, double timeout) {
+ int fd, int domain, const HostURL &hosturl, double timeout,
+ const SmartPtr<StreamContext>& ctx) {
CryptoMethod method;
const std::string scheme = hosturl.getScheme();
@@ -339,7 +352,7 @@ SmartPtr<SSLSocket> SSLSocket::Create(
}
auto sock = makeSmartPtr<SSLSocket>(
- fd, domain, hosturl.getHost().c_str(), hosturl.getPort());
+ fd, domain, ctx, hosturl.getHost().c_str(), hosturl.getPort());
sock->m_data->m_method = method;
sock->m_data->m_connect_timeout = timeout;
@@ -400,6 +413,10 @@ bool SSLSocket::setupCrypto(SSLSocket *session /* = NULL */) {
return false;
}
+ if (!m_context.exists(s_verify_peer)) {
+ m_context.add(s_verify_peer, true);
+ }
+
/* need to do slightly different things, based on client/server method,
* so lets remember which method was selected */
#if OPENSSL_VERSION_NUMBER < 0x00909000L
@@ -524,6 +541,10 @@ bool SSLSocket::applyVerificationPolicy(X509 *peer) {
} else if (name_len != (int)strlen(buf)) {
raise_warning("Peer certificate CN=`%.*s' is malformed", name_len, buf);
return false;
+ } else if (name_len == sizeof(buf)) {
+ raise_warning("Peer certificate CN=`%.*s' is too long to verify",
+ name_len, buf);
+ return false;
}
bool match = (strcmp(cnmatch.c_str(), buf) == 0);
@@ -48,10 +48,12 @@ struct SSLSocket : Socket {
static int GetSSLExDataIndex();
static SmartPtr<SSLSocket> Create(int fd, int domain, const HostURL &hosturl,
- double timeout);
+ double timeout,
+ const SmartPtr<StreamContext>& ctx);
SSLSocket();
- SSLSocket(int sockfd, int type, const char *address = nullptr, int port = 0);
+ SSLSocket(int sockfd, int type, const SmartPtr<StreamContext>& ctx,
+ const char *address = nullptr, int port = 0);
virtual ~SSLSocket();
DECLARE_RESOURCE_ALLOCATION(SSLSocket);
@@ -79,6 +79,10 @@ bool UrlFile::open(const String& input_url, const String& mode) {
return false;
}
HttpClient http(m_timeout, m_maxRedirect);
+ auto ctx = this->getStreamContext();
+ if (ctx) {
+ http.setStreamContextOptions(ctx->getOptions());
+ }
m_response.clear();
if (!m_proxyHost.empty()) {
@@ -405,6 +405,7 @@ static int connect_with_timeout(int fd, struct sockaddr *sa_ptr,
}
static Variant new_socket_connect(const HostURL &hosturl, double timeout,
+ const SmartPtr<StreamContext>& streamctx,
Variant &errnum, Variant &errstr) {
int domain = AF_UNSPEC;
int type = SOCK_STREAM;
@@ -474,7 +475,7 @@ static Variant new_socket_connect(const HostURL &hosturl, double timeout,
fd = -1;
}
- sslsock = SSLSocket::Create(fd, domain, hosturl, timeout);
+ sslsock = SSLSocket::Create(fd, domain, hosturl, timeout, streamctx);
if (sslsock) {
sock = sslsock;
} else {
@@ -1341,7 +1342,8 @@ thread_local std::unordered_map<
}
Variant sockopen_impl(const HostURL &hosturl, VRefParam errnum,
- VRefParam errstr, double timeout, bool persistent) {
+ VRefParam errstr, double timeout, bool persistent,
+ const Variant& context) {
errnum = 0;
errstr = empty_string();
std::string key;
@@ -1368,7 +1370,12 @@ Variant sockopen_impl(const HostURL &hosturl, VRefParam errnum,
m_reqInjectionData.getSocketDefaultTimeout();
}
- auto socket = new_socket_connect(hosturl, timeout, errnum, errstr);
+
+ SmartPtr<StreamContext> streamctx;
+ if (context.isResource()) {
+ streamctx = cast<StreamContext>(context.toResource());
+ }
+ auto socket = new_socket_connect(hosturl, timeout, streamctx, errnum, errstr);
if (!socket.isResource()) {
return false;
}
@@ -1389,7 +1396,7 @@ Variant HHVM_FUNCTION(fsockopen,
VRefParam errstr /* = null */,
double timeout /* = -1.0 */) {
HostURL hosturl(static_cast<const std::string>(hostname), port);
- return sockopen_impl(hosturl, errnum, errstr, timeout, false);
+ return sockopen_impl(hosturl, errnum, errstr, timeout, false, null_variant);
}
Variant HHVM_FUNCTION(pfsockopen,
@@ -1399,7 +1406,7 @@ Variant HHVM_FUNCTION(pfsockopen,
VRefParam errstr /* = null */,
double timeout /* = -1.0 */) {
HostURL hosturl(static_cast<const std::string>(hostname), port);
- return sockopen_impl(hosturl, errnum, errstr, timeout, true);
+ return sockopen_impl(hosturl, errnum, errstr, timeout, true, null_variant);
}
String ipaddr_convert(struct sockaddr *addr, int addrlen) {
@@ -158,7 +158,8 @@ Variant socket_server_impl(const HostURL &hosturl,
VRefParam errnum = uninit_null(),
VRefParam errstr = uninit_null());
Variant sockopen_impl(const HostURL &hosturl, VRefParam errnum,
- VRefParam errstr, double timeout, bool persistent);
+ VRefParam errstr, double timeout, bool persistent,
+ const Variant& context);
///////////////////////////////////////////////////////////////////////////////
}
@@ -682,7 +682,7 @@ Variant HHVM_FUNCTION(stream_socket_client,
int flags /* = 0 */,
const Variant& context /* = null_variant */) {
HostURL hosturl(static_cast<const std::string>(remote_socket));
- return sockopen_impl(hosturl, errnum, errstr, timeout, false);
+ return sockopen_impl(hosturl, errnum, errstr, timeout, false, context);
}
Variant HHVM_FUNCTION(stream_socket_get_name,

0 comments on commit e282a45

Please sign in to comment.