Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1712)

Unified Diff: Source/platform/weborigin/SecurityOrigin.cpp

Issue 27073003: CSP Suborigins Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Address abarth's comments Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/platform/weborigin/SecurityOrigin.cpp
diff --git a/Source/platform/weborigin/SecurityOrigin.cpp b/Source/platform/weborigin/SecurityOrigin.cpp
index 4c11b4110748af6511855f573c47bc9fa9d4bce2..250ac2a39ae480d05b919f040717789de7422973 100644
--- a/Source/platform/weborigin/SecurityOrigin.cpp
+++ b/Source/platform/weborigin/SecurityOrigin.cpp
@@ -62,6 +62,12 @@ static SecurityOrigin* cachedOrigin(const KURL& url)
return 0;
}
+static bool sameSuborigin(const SecurityOrigin* origin1, const SecurityOrigin* origin2)
+{
+ // If either origin has a suborigin set, both must have the same suborigin.
+ return (!origin1->hasSuborigin() && !origin2->hasSuborigin()) || (origin1->suboriginName() == origin2->suboriginName());
Mike West 2014/10/23 12:59:20 Is this any different from `origin1->suboriginName
jww 2015/03/20 22:50:03 Yup. I think I was considering something tricky wi
+}
+
bool SecurityOrigin::shouldUseInnerURL(const KURL& url)
{
// FIXME: Blob URLs don't have inner URLs. Their form is "blob:<inner-origin>/<UUID>", so treating the part after "blob:" as a URL is incorrect.
@@ -158,6 +164,7 @@ SecurityOrigin::SecurityOrigin(const SecurityOrigin* other)
, m_host(other->m_host.isolatedCopy())
, m_domain(other->m_domain.isolatedCopy())
, m_filePath(other->m_filePath.isolatedCopy())
+ , m_suboriginName(other->m_suboriginName)
, m_port(other->m_port)
, m_isUnique(other->m_isUnique)
, m_universalAccess(other->m_universalAccess)
@@ -200,6 +207,15 @@ PassRefPtr<SecurityOrigin> SecurityOrigin::createUnique()
return origin.release();
}
+void SecurityOrigin::addSuborigin(const String& suborigin)
+{
+ // Changing suborigins midstream is bad. Very bad. It should not happen.
+ // This is, in fact, one of the very basic invariants that makes suborigins
+ // an effective security tool.
+ RELEASE_ASSERT(m_suboriginName.isNull());
+ m_suboriginName = suborigin;
+}
+
PassRefPtr<SecurityOrigin> SecurityOrigin::isolatedCopy() const
{
return adoptRef(new SecurityOrigin(this));
@@ -235,6 +251,9 @@ bool SecurityOrigin::canAccess(const SecurityOrigin* other) const
if (isUnique() || other->isUnique())
return false;
+ if (!sameSuborigin(this, other))
+ return false;
Mike West 2014/10/23 12:59:20 I assume that bypassing the `document.domain` chec
jww 2015/03/20 22:50:03 Definitely.
+
// Here are two cases where we should permit access:
//
// 1) Neither document has set document.domain. In this case, we insist
@@ -298,6 +317,9 @@ bool SecurityOrigin::canRequest(const KURL& url) const
if (targetOrigin->isUnique())
return false;
+ if (!sameSuborigin(this, targetOrigin.get()))
+ return false;
+
// We call isSameSchemeHostPort here instead of canAccess because we want
// to ignore document.domain effects.
if (isSameSchemeHostPort(targetOrigin.get()))
@@ -392,6 +414,8 @@ SecurityOrigin::Policy SecurityOrigin::canShowNotifications() const
return AlwaysAllow;
if (isUnique())
return AlwaysDeny;
+ if (hasSuborigin())
+ return AlwaysDeny;
Mike West 2014/10/23 12:59:20 Why?
jww 2015/03/20 22:50:03 Same rationale as above. Start with the limitation
return Ask;
}
@@ -483,7 +507,14 @@ AtomicString SecurityOrigin::toRawAtomicString() const
inline void SecurityOrigin::buildRawString(StringBuilder& builder) const
{
- builder.reserveCapacity(m_protocol.length() + m_host.length() + 10);
+ if (hasSuborigin()) {
+ builder.reserveCapacity(11 + m_suboriginName.length() + m_protocol.length() + m_host.length() + 10);
+ builder.appendLiteral("suborigin:");
+ builder.append(m_suboriginName);
+ builder.append('+');
Mike West 2014/10/23 12:59:20 Is this the scheme we ended up with on the list?
jww 2015/03/20 22:50:03 There was no consensus, but it was the compromise
+ } else {
+ builder.reserveCapacity(m_protocol.length() + m_host.length() + 10);
+ }
builder.append(m_protocol);
builder.appendLiteral("://");
builder.append(m_host);
@@ -509,6 +540,9 @@ PassRefPtr<SecurityOrigin> SecurityOrigin::create(const String& protocol, const
bool SecurityOrigin::isSameSchemeHostPort(const SecurityOrigin* other) const
{
+ if (!sameSuborigin(this, other))
+ return false;
+
if (m_host != other->m_host)
return false;
« Source/platform/weborigin/SecurityOrigin.h ('K') | « Source/platform/weborigin/SecurityOrigin.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698