[FeaturePolicy] Initial implementation of Feature Policy

third_party/WebKit/Source/platform/feature_policy/FeaturePolicy.cpp

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "platform/feature_policy/FeaturePolicy.h"
+
+#include "platform/json/JSONParser.h"
+#include "platform/json/JSONValues.h"
+#include "platform/weborigin/KURL.h"
+#include "platform/weborigin/SecurityOrigin.h"
+#include "wtf/text/StringBuilder.h"
+#include <algorithm>
+
+namespace blink {
+
+namespace {
+
+const FeaturePolicyFeature* featureForName(const String& featureName)
+{
+    // TODO: Use make_token_matcher.py to generate this code.
+    if (featureName == "cookie")
+        return &kDocumentCookie;
+    if (featureName == "domain")
+        return &kDocumentDomain;
+    if (featureName == "docwrite")
+        return &kDocumentWrite;
+    if (featureName == "vibrate")
+        return &kVibrate;
+    if (featureName == "webrtc")
+        return &kWebRTC;
+    return &kNoSuchFeature;
+}
+
+} // namespace
+
+const FeaturePolicyFeature kDocumentCookie { "cookie", true, true };
+const FeaturePolicyFeature kDocumentDomain { "domain", true, true };
+const FeaturePolicyFeature kDocumentWrite { "docwrite", true, true };
+const FeaturePolicyFeature kVibrate { "vibrate", true, false };
+const FeaturePolicyFeature kWebRTC { "webrtc", true, true };
+const FeaturePolicyFeature kNoSuchFeature { "", false, false };
+
+FeaturePolicy::Whitelist::Whitelist()
+    : m_matchesAllOrigins(false)
+{
+}
+
+void FeaturePolicy::Whitelist::addAll()
+{
+    m_matchesAllOrigins = true;
+}
+
+void FeaturePolicy::Whitelist::add(RefPtr<SecurityOrigin> origin)
+{
+    origins.append(origin);
+}
+
+bool FeaturePolicy::Whitelist::targets(const SecurityOrigin* origin) const

[raymes, 2016/09/19 09:19:05]

nit: would "contains" be slightly clearer?

[iclelland, 2016/09/27 15:06:27]

Probably; I was using the nomenclature from the original spec design. Changed
here because it makes sense.

[iclelland, 2016/10/14 19:37:49]

Done.

+{
+    if (m_matchesAllOrigins)
+        return true;
+    return (std::any_of(origins.begin(), origins.end(),
+        [origin](const RefPtr<SecurityOrigin>& target)
+        {
+            return target->isSameSchemeHostPortAndSuborigin(origin);
+        }));

[raymes, 2016/09/19 09:19:04]

(optionally) the long-hand way isn't much longer
for (const auto& target : origins) 
{
  if (target->isSameSchemeHostPortAndSuborigin(origin))
    return true
}
return false;

[iclelland, 2016/09/27 15:06:27]

Thanks, that's clearer. (I always have such high hopes for lambdas, but they
never work out :) )

+}
+
+// static
+FeaturePolicy* FeaturePolicy::createFromParentPolicy(const FeaturePolicy* parent, RefPtr<SecurityOrigin> currentOrigin)
+{
+    FeaturePolicy* newPolicy = new FeaturePolicy(currentOrigin, !parent);
+    if (parent) {
+        for (const auto& item : parent->m_whitelists) {
+            Whitelist newWhitelist;
+            if (item.value.targets(currentOrigin.get())) {
+                newWhitelist.add(currentOrigin);
+            }
+            newPolicy->m_whitelists.set(item.key, newWhitelist);
+        }
+    }
+    return newPolicy;
+}
+
+void FeaturePolicy::addPolicyFromString(const String& policy)

[raymes, 2016/09/19 09:19:04]

I think it's a little unclear what it means to "add" a policy. Will this ever
need to be called multiple times (it doesn't seem to in this CL)? Should we
instead just pass in the current frame's policy string on construction in the
function above? That would avoid this function being abused. But perhaps there
is a good reason for this :) If we do need it perhaps we can do something to
clarify it.

[iclelland, 2016/09/27 15:06:28]

I wouldn't want to pass in the current frame's string, since we'd have to
serialize it from the current policy (which might not be the same as the string
in the header or meta tag, depending on the parent frame policy)

The intention was to initialize a policy with the parent's policy, and then to
use this method to add the declarations from the HTTP header string, then use it
again with the <meta> tag content, if present. Each invocation would further
restrict the current policy.

The meta tag parsing isn't present yet, but I will add some unit tests to ensure
that this method works as intended.

+{
+    if (policy.isEmpty())
+        return;
+    for (const auto& whitelist : parse(policy)) {
+        if (isFeatureEnabled(whitelist.key)) {
+            m_whitelists.set(whitelist.key, whitelist.value);
+        }

[raymes, 2016/09/19 09:19:05]

Hmm, what about the case where the parent sets "docwrite: a.com" and then the
iframe a.com doesn't specify a policy. I think in that case we want the policy
for a.com to resolve to * (the default policy) but I think this code will cause
it to resolve to a.com for the subframe? (I might be wrong so please let me know
if I'm missing something :).

I actually think it might be useful to have a concept of a "resolved" policy for
a frame. It's similar to a feature policy but there is a well defined whitelist
for every feature (no features missing) which has taken into account header
policy, meta policy and all the parent policies for that frame. Given a resolved
policy of a parent frame and a {header policy, meta policy, embedding
information} of the current frame, we should be able to produce a new resolved
policy for the current frame. The distinction might help to avoid confusion
between the structure representing what a developer specified in a header
tag/meta tag and the internal structure representing what will actually be
enforced. This is a mouthful and might be too complicated but let me know your
thoughts :)

[iclelland, 2016/09/27 15:06:27]

No, I think you're right, and I didn't catch that case. I've added a step in
|createFromParentPolicy| that checks to see whether the feature should be
enabled by default in nested contexts. In that case, enabling it for a.com  in
the parent means that unless a.com says otherwise, it gets "*".

This isn't the case for policies like vibrate, which aren't enabled in nested
contexts, and so a.com needs to explicitly enable it's embeddees, and so on down
the chain.

I added two test cases for it as well.
That's an interesting idea; the current FeaturePolicy is sort-of that resolved
policy, or at least it's intended to be. The biggest difference, I think, is
that any features which haven't been mentioned by any policy aren't explicitly
represented; we use the defaults in that case. I'll give that some more thought,
though. It might help clarify some of the behaviour around inheritance and
policy layering.

[raymes, 2016/09/29 07:17:00]

Hmm, I'm still not sure I feel comfortable with this approach - it's hard to see
that it matches the algorithm in
https://docs.google.com/document/d/1saEBLE6d-qmMxao8Aw6A06oSP3yKdUG7lqMWBYlzE....
I realize that the algorithm assumes we have all the information at once, which
we might not in practice. Maybe we can set up a meeting to chat next week?

[iclelland, 2016/09/30 15:39:26]

Sounds like a good idea. I'll see if I can find some time on your calendar.

+    }
+}
+
+bool FeaturePolicy::isFeatureEnabledForOrigin(const FeaturePolicyFeature* feature, const SecurityOrigin* origin)
+{
+    if (m_whitelists.contains(feature)) {
+        const Whitelist& whitelist = m_whitelists.get(feature);
+        return whitelist.targets(origin);
+    }
+    return isFeatureEnabledByDefault(feature, m_isTopLevel);
+}
+
+bool FeaturePolicy::isFeatureEnabled(const FeaturePolicyFeature* feature)
+{
+    return isFeatureEnabledForOrigin(feature, m_origin.get());
+}
+
+// static
+bool FeaturePolicy::isFeatureEnabledByDefault(const FeaturePolicyFeature* feature, bool isTopLevel)
+{
+    if (isTopLevel)
+        return feature->enabledByDefault;
+    return feature->enabledInNestedContext;
+}
+
+void FeaturePolicy::setFeaturePolicyBindingsInstalled()
+{
+    m_bindingsInstalled = true;
+}
+
+bool FeaturePolicy::featurePolicyBindingsInstalled()
+{
+    return m_bindingsInstalled;
+}
+
+FeaturePolicy::FeaturePolicy(PassRefPtr<SecurityOrigin> currentOrigin, bool isTopLevel)
+    : m_origin(currentOrigin)
+    , m_isTopLevel(isTopLevel)
+{
+}
+
+HashMap<const FeaturePolicyFeature*, FeaturePolicy::Whitelist>
+FeaturePolicy::parse(const String& policy)
+{
+    HashMap<const FeaturePolicyFeature*, Whitelist> whitelists;
+    std::unique_ptr<JSONValue> policyJSON = parseJSON(policy);
+
+    if (!policyJSON)
+        return whitelists;
+
+    std::unique_ptr<JSONArray> items = JSONArray::cast(std::move(policyJSON));
+    if (!items)
+        return whitelists;
+
+    for (size_t i = 0; i < items->size(); ++i) {
+        JSONObject* item = JSONObject::cast(items->at(i));
+        if (!item)
+            continue;
+
+        for (size_t j = 0; j < item->size(); ++j) {
+            JSONObject::Entry entry = item->at(j);
+            String featureName = entry.first;
+            const FeaturePolicyFeature* feature = featureForName(featureName);
+            if (feature != &kNoSuchFeature) {
+                JSONArray* targets = JSONArray::cast(entry.second);
+                if (targets) {
+                    Whitelist whitelist;
+                    String targetString;
+                    for (size_t j = 0; j < targets->size(); ++j) {
+                        if (targets->at(j)->asString(&targetString)) {
+                            if (equalIgnoringCase(targetString, "self")) {
+                                whitelist.add(m_origin);
+                            } else if (targetString == "*") {
+                                whitelist.addAll();
+                            } else {
+                                KURL originUrl = KURL(KURL(), targetString);
+                                if (originUrl.isValid()) {
+                                    whitelist.add(SecurityOrigin::create(originUrl));
+                                }
+                            }
+                        }
+                    }
+                    whitelists.set(feature, whitelist);
+                }
+            } else {
+                continue; // Not a string or an array; invalid policy spec.
+            }
+        }
+    }
+    return whitelists;

[raymes, 2016/09/19 09:19:05]

I haven't reviewed the parsing yet :)

[iclelland, 2016/09/27 15:06:28]

NP; I haven't written a fuzzer for it yet either :)

[iclelland, 2016/10/14 19:37:49]

https://codereview.chromium.org/2420013004 FYI

+}
+
+DEFINE_TRACE(FeaturePolicy)
+{
+}
+
+} // namespace blink