Implement cross-origin attributes using access check interceptors.

third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl

 {% filter format_blink_cpp_source_code %}
 
 {% include 'copyright_block.txt' %}
 #include "{{v8_class_or_partial}}.h"
 
 {% for filename in cpp_includes if filename != '%s.h' % cpp_class_or_partial %}
 #include "{{filename}}"
 {% endfor %}
 
 namespace blink {
@@ skipping 103 matching lines @@
 {% endif %}
 {% if attribute.has_setter %}
 {% if not attribute.has_custom_setter %}
 {{attribute_setter(attribute, world_suffix)}}
 {% endif %}
 {{attribute_setter_callback(attribute, world_suffix)}}
 {% endif %}
 {% endfor %}
 {% endfor %}
 {##############################################################################}
-{% block security_check_functions %}
-{% if has_access_check_callbacks and not is_partial %}
-bool securityCheck(v8::Local<v8::Context> accessingContext, v8::Local<v8::Object> accessedObject, v8::Local<v8::Value> data) {
-  {% if interface_name == 'Window' %}
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Local<v8::Object> window = V8Window::findInstanceInPrototypeChain(accessedObject, isolate);
-  if (window.IsEmpty())
-    return false; // the frame is gone.
-
-  const DOMWindow* targetWindow = V8Window::toImpl(window);
-  return BindingSecurity::shouldAllowAccessTo(toLocalDOMWindow(toDOMWindow(accessingContext)), targetWindow, BindingSecurity::ErrorReportOption::DoNotReport);
-  {% else %}{# if interface_name == 'Window' #}
-  {# Not 'Window' means it\'s Location. #}
-  {{cpp_class}}* impl = {{v8_class}}::toImpl(accessedObject);
-  return BindingSecurity::shouldAllowAccessTo(toLocalDOMWindow(toDOMWindow(accessingContext)), impl, BindingSecurity::ErrorReportOption::DoNotReport);
-  {% endif %}{# if interface_name == 'Window' #}
-}
-
-{% endif %}
-{% endblock %}
-{##############################################################################}
 {# Methods #}
 {% from 'methods.cpp.tmpl' import generate_method, overload_resolution_method,
        method_callback, origin_safe_method_getter, generate_constructor,
        method_implemented_in_private_script, generate_post_message_impl,
        runtime_determined_length_method, runtime_determined_maxarg_method
        with context %}
 {% for method in methods if method.should_be_exposed_to_script %}
 {% for world_suffix in method.world_suffixes %}
 {% if not method.is_custom and not method.is_post_message and method.visible %}
 {{generate_method(method, world_suffix)}}
@@ skipping 14 matching lines @@
 {# Document about the following condition: #}
 {# https://docs.google.com/document/d/1qBC7Therp437Jbt_QYAtNYMZs6zQ_7_tnMkNUG_ACqs/edit?usp=sharing #}
 {% if (method.overloads and method.overloads.visible and
        (not method.overloads.has_partial_overloads or not is_partial)) or
       (not method.overloads and method.visible) %}
 {# A single callback is generated for overloaded methods #}
 {# with considering partial overloads #}
 {{method_callback(method, world_suffix)}}
 {% endif %}
 {% endif %}
-{% if method.is_do_not_check_security and method.visible %}
+{% if method.is_cross_origin and method.visible %}
 {{origin_safe_method_getter(method, world_suffix)}}
 {% endif %}
 {% endfor %}
 {% endfor %}
 {% if iterator_method %}
 {{generate_method(iterator_method)}}
 {{method_callback(iterator_method)}}
 {% endif %}
 {% block origin_safe_method_setter %}{% endblock %}
 {# Constructors #}
@@ skipping 12 matching lines @@
 {% block named_property_query %}{% endblock %}
 {% block named_property_query_callback %}{% endblock %}
 {% block named_property_enumerator %}{% endblock %}
 {% block named_property_enumerator_callback %}{% endblock %}
 {% block indexed_property_getter %}{% endblock %}
 {% block indexed_property_getter_callback %}{% endblock %}
 {% block indexed_property_setter %}{% endblock %}
 {% block indexed_property_setter_callback %}{% endblock %}
 {% block indexed_property_deleter %}{% endblock %}
 {% block indexed_property_deleter_callback %}{% endblock %}
+{##############################################################################}
+{% block security_check_functions %}
+{% if has_access_check_callbacks and not is_partial %}
+bool securityCheck(v8::Local<v8::Context> accessingContext, v8::Local<v8::Object> accessedObject, v8::Local<v8::Value> data) {
+  {% if interface_name == 'Window' %}
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  v8::Local<v8::Object> window = V8Window::findInstanceInPrototypeChain(accessedObject, isolate);
+  if (window.IsEmpty())
+    return false; // the frame is gone.

[haraken, 2016/12/08 08:21:02]

Not really. window.IsEmpty happens when the prototype chain is tweaked. Whether
the frame is gone is checked by the following
BindingSecurity::shouldAllowAccessTo.

[dcheng, 2016/12/08 09:04:11]

I'm just copying and pasting code here =)

What would you suggest as a more accurate comment?

[haraken, 2016/12/09 02:26:55]

"It's not a window wrapper" :)

[dcheng, 2016/12/09 05:06:14]

Strictly speaking, is that true though? We only set access check callbacks for
Window and Location, and this code is specific for handling Window. So it must
be a window wrapper, but somehow got disassociated by changing the prototype
chain.

+
+  const DOMWindow* targetWindow = V8Window::toImpl(window);
+  return BindingSecurity::shouldAllowAccessTo(toLocalDOMWindow(toDOMWindow(accessingContext)), targetWindow, BindingSecurity::ErrorReportOption::DoNotReport);
+  {% else %}{# if interface_name == 'Window' #}
+  {# Not 'Window' means it\'s Location. #}
+  {{cpp_class}}* impl = {{v8_class}}::toImpl(accessedObject);
+  return BindingSecurity::shouldAllowAccessTo(toLocalDOMWindow(toDOMWindow(accessingContext)), impl, BindingSecurity::ErrorReportOption::DoNotReport);
+  {% endif %}{# if interface_name == 'Window' #}
+}
+
+{% if has_cross_origin_named_enumerator %}
+// TODO(dcheng): Can we / should we use AtomicString here? That means using DEFINE_STATIC_LOCAL here.

[haraken, 2016/12/08 08:21:02]

I'm fine with const char*.

[dcheng, 2016/12/08 09:04:11]

Done. I removed the TODO... I think it might still be an interesting
optimization, but maybe it's not a big deal.

+static const struct {
+  using GetterCallback = void(*)(const v8::PropertyCallbackInfo<v8::Value>&);
+  using SetterCallback = void(*)(v8::Local<v8::Value>, const V8CrossOriginSetterInfo&);
+
+  const char* const name;
+  const GetterCallback getter;
+  const SetterCallback setter;
+} kCrossOriginAttributeTable[] = {
+  {##### Cross-origin attributes #####}
+  {% for attribute in attributes if attribute.has_cross_origin_getter or attribute.has_cross_origin_setter %}
+  {
+    "{{attribute.name}}",
+    {%+ if attribute.has_cross_origin_getter %}&{{cpp_class}}V8Internal::{{attribute.name}}AttributeGetter{% else %}nullptr{% endif %},
+    {%+ if attribute.has_cross_origin_setter %}&{{cpp_class}}V8Internal::{{attribute.name}}AttributeSetter{% else %}nullptr{% endif %},
+  },
+  {% endfor %}
+  {##### Cross-origin methods #####}
+  {% for method in methods if method.is_cross_origin %}
+  {"{{method.name}}", &{{cpp_class}}V8Internal::{{method.name}}OriginSafeMethodGetter, nullptr},

[haraken, 2016/12/08 08:21:02]

After this CL, OriginSafeMethodSetter is no longer used, right? If so, can we
remove code for OriginSafeMethodSetter?

[dcheng, 2016/12/08 09:04:11]

Unfortunately, it is still needed. Originally, I tried removing this, and this
broke things like postMessage: event.source would be set incorrectly.

postMessage and the location setters need the calling context (incumbent realm
in spec terminology): they use this to determine if the calling context has
access to the target context. Since v8 is trying to get rid of
GetCallingContext(), we need to support this another way: instead, we create
function objects for each realm, so that when we call GetCurrentContext(), it
ends up pointing to the context which we created the function object for.

With this hack, if document A post messages document B, GetCurrentContext() ==
document A, and we enforce security checks / set event.source correctly, etc.

However, if we do not create a per-realm function object, then
GetCurrentContext() == the context we are calling *into*: that means that when
document A post messages document B, GetCurrentContext() == document B. This
breaks security checks for Location, as well as event.source (since event.source
doesn't point back at the caller).

[haraken, 2016/12/09 02:26:56]

Yes, I understand why we still need to keep OriginSafeMethodGetter. My question
is if we still need OriginSafeMethodSetter.

[dcheng, 2016/12/09 05:06:13]

I thought that this would be needed so that if JS overwrites the property, we
can return the user-supplied value rather than the function object. I might be
missing something, but I don't see how I'd be able to make this work without a
setter.

+  {% endfor %}
+};
+{% endif %}
+
+{% if has_cross_origin_named_getter %}
+void crossOriginNamedGetter(v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {

[haraken, 2016/12/08 08:21:02]

Just to confirm: V8 calls back crossOriginNamedGetter only when securityCheck
returns false, right? I just want to check if the overhead of
crossOriginNamedGetter doesn't affect normal window.foo accesses.

[dcheng, 2016/12/08 09:04:11]

That's correct: this interceptor is only invoked when the access check fails.

+  if (!name->IsString())
+    return;
+  const AtomicString& propertyName = toCoreAtomicString(name.As<v8::String>());
+
+  for (const auto& attribute: kCrossOriginAttributeTable) {

[Yuki, 2016/12/07 12:03:40]

nit: attribute : kCrossOrigin...
(one space before the colon)

[dcheng, 2016/12/08 09:04:11]

Done.

+    if (propertyName == attribute.name && attribute.getter) {
+      attribute.getter(info);
+      return;
+    }
+  }
+
+  {% if named_property_getter and named_property_getter.is_cross_origin %}
+  {% if named_property_getter.is_custom %}
+  {{v8_class}}::namedPropertyGetterCustom(propertyName, info);
+  {% else %}
+  {{cpp_class}}V8Internal::namedPropertyGetter(propertyName, info);
+  {% endif %}
+  {% else %}
+  BindingSecurity::failedAccessCheckFor(
+      info.GetIsolate(),
+      {{v8_class}}::toImpl(info.Holder())->frame());
+  {% endif %}
+}
+{% endif %}
+
+{% if has_cross_origin_named_setter %}
+void crossOriginNamedSetter(v8::Local<v8::Name> name, v8::Local<v8::Value> value, const v8::PropertyCallbackInfo<v8::Value>& info) {
+  if (!name->IsString())
+    return;
+  const AtomicString& propertyName = toCoreAtomicString(name.As<v8::String>());
+
+  for (const auto& attribute: kCrossOriginAttributeTable) {

[Yuki, 2016/12/07 12:03:40]

ditto

[dcheng, 2016/12/08 09:04:11]

Done.

+    if (propertyName == attribute.name && attribute.setter) {
+      attribute.setter(value, V8CrossOriginSetterInfo(info.GetIsolate(), info.Holder()));
+      return;
+    }
+  }
+

[haraken, 2016/12/08 08:21:02]

Why don't you need to call V8Internal::namedPropertySetter when
named_property_setter.is_cross_origin is set?

[dcheng, 2016/12/08 09:04:11]

I could add this for consistency... but there is no IDL that would use it, and
it would not be tested at all. So I just omitted it.

[haraken, 2016/12/09 02:26:56]

Okay, let's add a comment.

[dcheng, 2016/12/09 05:06:13]

Done.

+  BindingSecurity::failedAccessCheckFor(
+      info.GetIsolate(),
+      {{v8_class}}::toImpl(info.Holder())->frame());
+}
+{% endif %}
+
+{% if has_cross_origin_named_enumerator %}
+void crossOriginNamedEnumerator(const v8::PropertyCallbackInfo<v8::Array>& info) {
+  Vector<String> names;
+  for (const auto& attribute : kCrossOriginAttributeTable)
+    names.append(attribute.name);
+
+  v8SetReturnValue(
+      info,
+      toV8(names, info.Holder(), info.GetIsolate()).As<v8::Array>());
+}
+{% endif %}
+
+{% if has_cross_origin_indexed_getter %}
+void crossOriginIndexedGetter(uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& info) {
+  {% if indexed_property_getter.is_custom %}
+  {{v8_class}}::indexedPropertyGetterCustom(index, info);

[haraken, 2016/12/08 08:21:02]

Is there any real use case of indexedPropertyGetterCustom?

[dcheng, 2016/12/08 09:04:10]

See my previous comment about window[0], window[1], etc.

+  {% else %}
+  {{cpp_class}}V8Internal::indexedPropertyGetter(index, info);
+  {% endif %}
+}
+{% endif %}
+
+{% endif %}
+{% endblock %}
+{##############################################################################}
 } // namespace {{cpp_class_or_partial}}V8Internal
 
 {% block visit_dom_wrapper %}{% endblock %}
 {##############################################################################}
 {% block install_attributes %}
 {% from 'attributes.cpp.tmpl' import attribute_configuration with context %}
 {% if attributes | has_attribute_configuration %}
 // Suppress warning: global constructors, because AttributeConfiguration is trivial
 // and does not depend on another global objects.
 #if defined(COMPONENT_BUILD) && defined(WIN32) && COMPILER(CLANG)
@@ skipping 121 matching lines @@
   {% endif %}
   {% if attributes | has_accessor_configuration %}
   V8DOMConfiguration::installAccessors(isolate, world, instanceTemplate, prototypeTemplate, interfaceTemplate, signature, {{'%sAccessors' % v8_class}}, {{'WTF_ARRAY_LENGTH(%sAccessors)' % v8_class}});
   {% endif %}
   {% if methods | has_method_configuration(is_partial) %}
   V8DOMConfiguration::installMethods(isolate, world, instanceTemplate, prototypeTemplate, interfaceTemplate, signature, {{'%sMethods' % v8_class}}, {{'WTF_ARRAY_LENGTH(%sMethods)' % v8_class}});
   {% endif %}
   {% endfilter %}
   {%- if has_access_check_callbacks and not is_partial %}{{newline}}
   // Cross-origin access check
-  instanceTemplate->SetAccessCheckCallback({{cpp_class}}V8Internal::securityCheck, v8::External::New(isolate, const_cast<WrapperTypeInfo*>(&{{v8_class}}::wrapperTypeInfo)));
+  {% set cross_origin_named_getter = '%sV8Internal::crossOriginNamedGetter' % cpp_class if has_cross_origin_named_getter else 'nullptr' %}
+  {% set cross_origin_named_setter = '%sV8Internal::crossOriginNamedSetter' % cpp_class if has_cross_origin_named_setter else 'nullptr' %}
+  {% set cross_origin_named_enumerator = '%sV8Internal::crossOriginNamedEnumerator' % cpp_class if has_cross_origin_named_enumerator else 'nullptr' %}
+  {% set cross_origin_indexed_getter = '%sV8Internal::crossOriginIndexedGetter' % cpp_class if has_cross_origin_indexed_getter else 'nullptr' %}
+  instanceTemplate->SetAccessCheckCallbackAndHandler({{cpp_class}}V8Internal::securityCheck, v8::NamedPropertyHandlerConfiguration({{cross_origin_named_getter}}, {{cross_origin_named_setter}}, nullptr, nullptr, {{cross_origin_named_enumerator}}), v8::IndexedPropertyHandlerConfiguration({{cross_origin_indexed_getter}}), v8::External::New(isolate, const_cast<WrapperTypeInfo*>(&{{v8_class}}::wrapperTypeInfo)));
   {% endif %}
 
   {%- for group in attributes | purely_runtime_enabled_attributes | groupby('runtime_feature_name') %}{{newline}}
   if ({{group.list[0].runtime_enabled_function}}()) {
     {% for attribute in group.list | unique_by('name') | sort %}
     {% if attribute.is_data_type_property %}
     const V8DOMConfiguration::AttributeConfiguration attribute{{attribute.name}}Configuration = {{attribute_configuration(attribute)}};
     V8DOMConfiguration::installAttribute(isolate, world, instanceTemplate, prototypeTemplate, attribute{{attribute.name}}Configuration);
     {% else %}
     const V8DOMConfiguration::AccessorConfiguration accessor{{attribute.name}}Configuration = {{attribute_configuration(attribute)}};
@@ skipping 38 matching lines @@
   instanceTemplate->MarkAsUndetectable();
   {% endif %}
 
   {%- if methods | custom_registration(is_partial) %}{{newline}}
   {% for method in methods | custom_registration(is_partial) %}
   {# install_custom_signature #}
   {% filter exposed(method.overloads.exposed_test_all
                     if method.overloads else method.exposed_test) %}
   {% filter runtime_enabled(method.overloads.runtime_enabled_function_all
                             if method.overloads else method.runtime_enabled_function) %}
-  {% if method.is_do_not_check_security %}
-  {{install_do_not_check_security_method(method, '', 'instanceTemplate', 'prototypeTemplate') | indent(2)}}
+  {% if method.is_cross_origin %}
+  {# TODO(dcheng): This shouldn't be necessary with cross-origin interceptors,
+     but v8 doesn't support querying the incumbent context. For now, always
+     incorrectly create per-realm representations. #}
+  {{install_origin_safe_method(method, '', 'instanceTemplate', 'prototypeTemplate') | indent(2)}}
   {% else %}
   {{install_custom_signature(method, 'instanceTemplate', 'prototypeTemplate', 'interfaceTemplate', 'signature') | indent(2)}}
   {% endif %}
   {% endfilter %}
   {% endfilter %}
   {% endfor %}
   {% endif %}
 }
 
 {% endif %}{# not is_array_buffer_or_view #}
@@ skipping 65 matching lines @@
 {% for attribute in attributes if attribute.is_implemented_in_private_script %}
 {{attribute_getter_implemented_in_private_script(attribute)}}
 {% if attribute.has_setter %}
 {{attribute_setter_implemented_in_private_script(attribute)}}
 {% endif %}
 {% endfor %}
 {% block partial_interface %}{% endblock %}
 }  // namespace blink
 
 {% endfilter %}{# format_blink_cpp_source_code #}