Skip to content

Commit

Permalink
Bug 787133 - (hpkp) Part 1/2. Header Parsing and interface within PSM…
Browse files Browse the repository at this point in the history
…. r=keeler, r=mcmanus
  • Loading branch information
Camilo Viecco committed Sep 3, 2014
1 parent 959b31e commit a2ed8e3
Show file tree
Hide file tree
Showing 21 changed files with 561 additions and 131 deletions.
1 change: 1 addition & 0 deletions b2g/chrome/content/devtools/hud.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ let consoleWatcher = {
'Mixed Content Message',
'CSP',
'Invalid HSTS Headers',
'Invalid HPKP Headers',
'Insecure Password Field',
'SSL',
'CORS'
Expand Down
1 change: 1 addition & 0 deletions browser/devtools/webconsole/webconsole.js
Original file line number Diff line number Diff line change
Expand Up @@ -4674,6 +4674,7 @@ var Utils = {
case "Mixed Content Message":
case "CSP":
case "Invalid HSTS Headers":
case "Invalid HPKP Headers":
case "Insecure Password Field":
case "SSL":
case "CORS":
Expand Down
2 changes: 2 additions & 0 deletions dom/locales/en-US/chrome/security/security.properties
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ CrossSiteRequestBlocked=Cross-Origin Request Blocked: The Same Origin Policy dis

# LOCALIZATION NOTE: Do not translate "Strict-Transport-Security" or "HSTS"
InvalidSTSHeaders=The site specified an invalid Strict-Transport-Security header.
# LOCALIZATION NOTE: Do not translate "Public-Key-Pins or HPKP"
InvalidPKPHeaders=The site specified an invalid Public-Key-Pins header.
InsecurePasswordsPresentOnPage=Password fields present on an insecure (http://) page. This is a security risk that allows user login credentials to be stolen.
InsecureFormActionPasswordsPresent=Password fields present in a form with an insecure (http://) form action. This is a security risk that allows user login credentials to be stolen.
InsecurePasswordsPresentOnIframe=Password fields present on an insecure (http://) iframe. This is a security risk that allows user login credentials to be stolen.
Expand Down
4 changes: 4 additions & 0 deletions modules/libpref/init/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -1754,6 +1754,10 @@ pref("security.mixed_content.block_display_content", false);

// Disable pinning checks by default.
pref("security.cert_pinning.enforcement_level", 0);
// Do not process hpkp headers rooted by not built in roots by default.
// This is to prevent accidental pinning from MITM devices and is used
// for tests.
pref("security.cert_pinning.process_headers_from_non_builtin_roots", false);

// Modifier key prefs: default to Windows settings,
// menu access key = alt, accelerator key = control.
Expand Down
22 changes: 20 additions & 2 deletions netwerk/base/public/nsISiteSecurityService.idl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
interface nsIURI;
interface nsIObserver;
interface nsIHttpChannel;
interface nsISSLStatus;

%{C++
template<class T> class nsTArray;
Expand All @@ -22,7 +23,7 @@ namespace mozilla
[ref] native nsCStringTArrayRef(nsTArray<nsCString>);
[ref] native mozillaPkixTime(mozilla::pkix::Time);

[scriptable, uuid(35816ea0-3ab5-11e4-8613-180373d97f23)]
[scriptable, uuid(46555f70-3ab5-11e4-8613-180373d97f23)]
interface nsISiteSecurityService : nsISupports
{
const uint32_t HEADER_HSTS = 0;
Expand All @@ -31,14 +32,19 @@ interface nsISiteSecurityService : nsISupports

/**
* Parses a given HTTP header and records the results internally.
* Currently the only header type supported is HSTS (aka STS).
* Currently two header types are supported: HSTS (aka STS) and HPKP
* The format of the HSTS header is defined by the HSTS specification:
* https://tools.ietf.org/html/rfc6797
* and allows a host to specify that future HTTP requests should be
* upgraded to HTTPS.
* The Format of the HPKP header is currently defined by:
* https://tools.ietf.org/html/draft-ietf-websec-key-pinning-20
* and allows a host to speficy a subset of trusted anchors to be used
* in future HTTPS connections.
*
* @param aType the type of security header in question.
* @param aSourceURI the URI of the resource with the HTTP header.
* @param aSSLStatus the SSLStatus of the current channel
* @param aHeader the HTTP response header specifying security data.
* @param aFlags options for this request as defined in nsISocketProvider:
* NO_PERMANENT_STORAGE
Expand All @@ -52,10 +58,22 @@ interface nsISiteSecurityService : nsISupports
void processHeader(in uint32_t aType,
in nsIURI aSourceURI,
in string aHeader,
in nsISSLStatus aSSLStatus,
in uint32_t aFlags,
[optional] out unsigned long long aMaxAge,
[optional] out boolean aIncludeSubdomains);

/**
* Same as processHeader but without checking for the security properties
* of the connection. Use ONLY for testing.
*/
void unsafeProcessHeader(in uint32_t aType,
in nsIURI aSourceURI,
in string aHeader,
in uint32_t aFlags,
[optional] out unsigned long long aMaxAge,
[optional] out boolean aIncludeSubdomains);

/**
* Given a header type, removes state relating to that header of a host,
* including the includeSubdomains state that would affect subdomains.
Expand Down
140 changes: 83 additions & 57 deletions netwerk/protocol/http/nsHttpChannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,66 @@ nsHttpChannel::ProcessFailedProxyConnect(uint32_t httpStatus)
return rv;
}

/**
* Process a single security header. Only two types are suported HSTS and HPKP.
*/
nsresult
nsHttpChannel::ProcessSingleSecurityHeader(uint32_t aType,
nsISSLStatus *aSSLStatus,
uint32_t aFlags)
{
nsHttpAtom atom;
switch (aType) {
case nsISiteSecurityService::HEADER_HSTS:
atom = nsHttp::ResolveAtom("Strict-Transport-Security");
break;
case nsISiteSecurityService::HEADER_HPKP:
atom = nsHttp::ResolveAtom("Public-Key-Pins");
break;
default:
NS_NOTREACHED("Invalid security header type");
return NS_ERROR_FAILURE;
}

nsAutoCString securityHeader;
nsresult rv = mResponseHead->GetHeader(atom, securityHeader);
if (NS_SUCCEEDED(rv)) {
nsISiteSecurityService* sss = gHttpHandler->GetSSService();
NS_ENSURE_TRUE(sss, NS_ERROR_OUT_OF_MEMORY);
// Process header will now discard the headers itself if the channel
// wasn't secure (whereas before it had to be checked manually)
rv = sss->ProcessHeader(aType, mURI, securityHeader.get(), aSSLStatus,
aFlags, nullptr, nullptr);
if (NS_FAILED(rv)) {
nsAutoString consoleErrorCategory;
nsAutoString consoleErrorTag;
switch (aType) {
case nsISiteSecurityService::HEADER_HSTS:
consoleErrorTag = NS_LITERAL_STRING("InvalidSTSHeaders");
consoleErrorCategory = NS_LITERAL_STRING("Invalid HSTS Headers");
break;
case nsISiteSecurityService::HEADER_HPKP:
consoleErrorTag = NS_LITERAL_STRING("InvalidPKPHeaders");
consoleErrorCategory = NS_LITERAL_STRING("Invalid HPKP Headers");
break;
default:
return NS_ERROR_FAILURE;
}
AddSecurityMessage(consoleErrorTag, consoleErrorCategory);
LOG(("nsHttpChannel: Failed to parse %s header, continuing load.\n",
atom.get()));
}
} else {
if (rv != NS_ERROR_NOT_AVAILABLE) {
// All other errors are fatal
NS_ENSURE_SUCCESS(rv, rv);
}
LOG(("nsHttpChannel: No %s header, continuing load.\n",
atom.get()));
}
return NS_OK;
}

/**
* Decide whether or not to remember Strict-Transport-Security, and whether
* or not to enforce channel integrity.
Expand All @@ -1049,87 +1109,52 @@ nsHttpChannel::ProcessFailedProxyConnect(uint32_t httpStatus)
* it's an HTTPS connection.
*/
nsresult
nsHttpChannel::ProcessSTSHeader()
nsHttpChannel::ProcessSecurityHeaders()
{
nsresult rv;
bool isHttps = false;
rv = mURI->SchemeIs("https", &isHttps);
NS_ENSURE_SUCCESS(rv, rv);

// If this channel is not loading securely, STS doesn't do anything.
// The upgrade to HTTPS takes place earlier in the channel load process.
// If this channel is not loading securely, STS or PKP doesn't do anything.
// In the case of HSTS, the upgrade to HTTPS takes place earlier in the
// channel load process.
if (!isHttps)
return NS_OK;

nsAutoCString asciiHost;
rv = mURI->GetAsciiHost(asciiHost);
NS_ENSURE_SUCCESS(rv, NS_OK);

// If the channel is not a hostname, but rather an IP, STS doesn't do
// anything.
// If the channel is not a hostname, but rather an IP, do not process STS
// or PKP headers
PRNetAddr hostAddr;
if (PR_SUCCESS == PR_StringToNetAddr(asciiHost.get(), &hostAddr))
return NS_OK;

nsISiteSecurityService* sss = gHttpHandler->GetSSService();
NS_ENSURE_TRUE(sss, NS_ERROR_OUT_OF_MEMORY);

// mSecurityInfo may not always be present, and if it's not then it is okay
// to just disregard any STS headers since we know nothing about the
// to just disregard any security headers since we know nothing about the
// security of the connection.
NS_ENSURE_TRUE(mSecurityInfo, NS_OK);

// Check the trustworthiness of the channel (are there any cert errors?)
// If there are certificate errors, we still load the data, we just ignore
// any STS headers that are present.
bool tlsIsBroken = false;
rv = sss->ShouldIgnoreHeaders(mSecurityInfo, &tlsIsBroken);
NS_ENSURE_SUCCESS(rv, NS_OK);

// If this was already an STS host, the connection should have been aborted
// by the bad cert handler in the case of cert errors. If it didn't abort the connection,
// there's probably something funny going on.
// If this wasn't an STS host, errors are allowed, but no more STS processing
// will happen during the session.
bool wasAlreadySTSHost;
uint32_t flags =
NS_UsePrivateBrowsing(this) ? nsISocketProvider::NO_PERMANENT_STORAGE : 0;
rv = sss->IsSecureURI(nsISiteSecurityService::HEADER_HSTS, mURI, flags,
&wasAlreadySTSHost);
// Failure here means STS is broken. Don't prevent the load, but this
// shouldn't fail.
NS_ENSURE_SUCCESS(rv, NS_OK);
MOZ_ASSERT(!(wasAlreadySTSHost && tlsIsBroken),
"connection should have been aborted by nss-bad-cert-handler");

// Any STS header is ignored if the channel is not trusted due to
// certificate errors (STS Spec 7.1) -- there is nothing else to do, and
// the load may progress.
if (tlsIsBroken) {
LOG(("STS: Transport layer is not trustworthy, ignoring "
"STS headers and continuing load\n"));
return NS_OK;
}

// If there's a STS header, process it (STS Spec 7.1). At this point in
// processing, the channel is trusted, so the header should not be ignored.
const nsHttpAtom atom = nsHttp::ResolveAtom("Strict-Transport-Security");
nsAutoCString stsHeader;
rv = mResponseHead->GetHeader(atom, stsHeader);
if (rv == NS_ERROR_NOT_AVAILABLE) {
LOG(("STS: No STS header, continuing load.\n"));
return NS_OK;
}
// All other failures are fatal.
// Get the SSLStatus
nsCOMPtr<nsISSLStatusProvider> sslprov = do_QueryInterface(mSecurityInfo);
NS_ENSURE_TRUE(sslprov, NS_ERROR_FAILURE);
nsCOMPtr<nsISSLStatus> sslStatus;
rv = sslprov->GetSSLStatus(getter_AddRefs(sslStatus));
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_TRUE(sslStatus, NS_ERROR_FAILURE);

rv = sss->ProcessHeader(nsISiteSecurityService::HEADER_HSTS, mURI,
stsHeader.get(), flags, nullptr, nullptr);
if (NS_FAILED(rv)) {
AddSecurityMessage(NS_LITERAL_STRING("InvalidSTSHeaders"),
NS_LITERAL_STRING("Invalid HSTS Headers"));
LOG(("STS: Failed to parse STS header, continuing load.\n"));
}
rv = ProcessSingleSecurityHeader(nsISiteSecurityService::HEADER_HSTS,
sslStatus, flags);
NS_ENSURE_SUCCESS(rv, rv);

rv = ProcessSingleSecurityHeader(nsISiteSecurityService::HEADER_HPKP,
sslStatus, flags);
NS_ENSURE_SUCCESS(rv, rv);

return NS_OK;
}
Expand Down Expand Up @@ -1229,8 +1254,9 @@ nsHttpChannel::ProcessResponse()
// If proxy CONNECT response needs to complete, wait to process connection
// for Strict-Transport-Security.
} else {
// Given a successful connection, process any STS data that's relevant.
rv = ProcessSTSHeader();
// Given a successful connection, process any STS or PKP data that's
// relevant.
rv = ProcessSecurityHeaders();
MOZ_ASSERT(NS_SUCCEEDED(rv), "ProcessSTSHeader failed, continuing load.");
}

Expand Down
20 changes: 15 additions & 5 deletions netwerk/protocol/http/nsHttpChannel.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class nsDNSPrefetch;
class nsICancelable;
class nsIHttpChannelAuthProvider;
class nsInputStreamPump;
class nsISSLStatus;
class nsPerformance;

namespace mozilla { namespace net {
Expand Down Expand Up @@ -288,12 +289,21 @@ class nsHttpChannel MOZ_FINAL : public HttpBaseChannel
nsresult OpenRedirectChannel(nsresult rv);

/**
* A function that takes care of reading STS headers and enforcing STS
* load rules. After a secure channel is erected, STS requires the channel
* to be trusted or any STS header data on the channel is ignored.
* This is called from ProcessResponse.
* A function that takes care of reading STS and PKP headers and enforcing
* STS and PKP load rules. After a secure channel is erected, STS and PKP
* requires the channel to be trusted or any STS or PKP header data on
* the channel is ignored. This is called from ProcessResponse.
*/
nsresult ProcessSTSHeader();
nsresult ProcessSecurityHeaders();

/**
* A function to process a single security header (STS or PKP), assumes
* some basic sanity checks have been applied to the channel. Called
* from ProcessSecurityHeaders.
*/
nsresult ProcessSingleSecurityHeader(uint32_t aType,
nsISSLStatus *aSSLStatus,
uint32_t aFlags);

void InvalidateCacheEntryForLocation(const char *location);
void AssembleCacheKey(const char *spec, uint32_t postID, nsACString &key);
Expand Down
8 changes: 4 additions & 4 deletions netwerk/test/TestSTSParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ TestSuccess(const char* hdr, bool extraTokens,

uint64_t maxAge = 0;
bool includeSubdomains = false;
rv = sss->ProcessHeader(nsISiteSecurityService::HEADER_HSTS, dummyUri, hdr,
0, &maxAge, &includeSubdomains);
rv = sss->UnsafeProcessHeader(nsISiteSecurityService::HEADER_HSTS, dummyUri,
hdr, 0, &maxAge, &includeSubdomains);
EXPECT_SUCCESS(rv, "Failed to process valid header: %s", hdr);

REQUIRE_EQUAL(maxAge, expectedMaxAge, "Did not correctly parse maxAge");
Expand All @@ -68,8 +68,8 @@ bool TestFailure(const char* hdr,
nsresult rv = NS_NewURI(getter_AddRefs(dummyUri), "https://foo.com/bar.html");
EXPECT_SUCCESS(rv, "Failed to create URI");

rv = sss->ProcessHeader(nsISiteSecurityService::HEADER_HSTS, dummyUri, hdr,
0, nullptr, nullptr);
rv = sss->UnsafeProcessHeader(nsISiteSecurityService::HEADER_HSTS, dummyUri,
hdr, 0, nullptr, nullptr);
EXPECT_FAILURE(rv, "Parsed invalid header: %s", hdr);
passed(hdr);
return true;
Expand Down
5 changes: 5 additions & 0 deletions security/certverifier/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.

EXPORTS += [
'CertVerifier.h',
'OCSPCache.h',
]

UNIFIED_SOURCES += [
'CertVerifier.cpp',
'NSSCertDBTrustDomain.cpp',
Expand Down
7 changes: 7 additions & 0 deletions security/manager/boot/src/PublicKeyPinningService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,13 @@ TransportSecurityPreloadCompare(const void *key, const void *entry) {
return strcmp(keyStr, preloadEntry->mHost);
}

bool
PublicKeyPinningService::ChainMatchesPinset(const CERTCertList* certList,
const nsTArray<nsCString>& aSHA256keys)
{
return EvalChainWithHashType(certList, SEC_OID_SHA256, nullptr, &aSHA256keys);
}

/**
* Check PKPins on the given certlist against the specified hostname
*/
Expand Down
9 changes: 9 additions & 0 deletions security/manager/boot/src/PublicKeyPinningService.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#define PublicKeyPinningService_h

#include "cert.h"
#include "nsString.h"
#include "nsTArray.h"
#include "pkix/Time.h"

namespace mozilla {
Expand All @@ -29,6 +31,13 @@ class PublicKeyPinningService
const char* hostname,
mozilla::pkix::Time time,
bool enforceTestMode);
/**
* Returns true if there is any intersection between the certificate list
* and the pins specified in the aSHA256key array. Values passed in are
* assumed to be in base64 encoded form
*/
static bool ChainMatchesPinset(const CERTCertList* certList,
const nsTArray<nsCString>& aSHA256keys);
};

}} // namespace mozilla::psm
Expand Down
Loading

0 comments on commit a2ed8e3

Please sign in to comment.