Skip to content

Commit

Permalink
Expose direct socket APIs via [DirectSocketEnabled].
Browse files Browse the repository at this point in the history
This patch introduces a new placeholder IDL extended attribute for the
set of restrictions which might enable us to safely expose direct
sockets, as well as a set of other APIs that have similar
outside-the-web threat models. It then applies that IDL extended
attribute to the existing direct socket IDL files.

For the moment, this imposes two requirements on Direct Sockets:

1.  Opting-into COI via COOP/COEP.
2.  Toggling the kDirectSockets CLI flag.

I expect these requirements to shift in the future, but this is a
reasonable place to start.

Patch 5/5:
1.  https://chromium-review.googlesource.com/c/chromium/src/+/2871670
2.  https://chromium-review.googlesource.com/c/chromium/src/+/2874306
3.  https://chromium-review.googlesource.com/c/chromium/src/+/2874288
4.  This patch.
5.  https://chromium-review.googlesource.com/c/chromium/src/+/2875217

Note that this CL removes the Direct Socket interfaces from the
`webexposed` test suite. That's a little too complicated to fix in
this CL, but I aim to fix it in https://crbug.com/1206656.

Bug: 1206150
Change-Id: I519e1c5d164c43f5870c45823d08063f4a3af682
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2874890
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880498}
  • Loading branch information
mikewest authored and Chromium LUCI CQ committed May 7, 2021
1 parent 93da1b1 commit 8fbd3bb
Show file tree
Hide file tree
Showing 43 changed files with 1,549 additions and 423 deletions.
14 changes: 12 additions & 2 deletions content/browser/renderer_host/render_frame_host_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2836,8 +2836,18 @@ WebExposedIsolationInfo RenderFrameHostManager::GetWebExposedIsolationInfo(
navigation_request->coop_status().current_coop().value ==
network::mojom::CrossOriginOpenerPolicyValue::kSameOriginPlusCoep;
if (is_cross_origin_isolated) {
return WebExposedIsolationInfo::CreateIsolated(
url::Origin::Create(navigation_request->common_params().url));
url::Origin origin =
url::Origin::Create(navigation_request->common_params().url);

// For short-term testing, we'll treat COI as "good enough" to treat as
// an isolated application iff the kDirectSockets feature is also
// enabled.
//
// TODO(mkwst): Build a better distinction: https://crbug.com/1206150.
if (base::FeatureList::IsEnabled(features::kDirectSockets))
return WebExposedIsolationInfo::CreateIsolatedApplication(origin);

return WebExposedIsolationInfo::CreateIsolated(origin);
}
} else {
// If we are in an iframe, we inherit the isolation state of
Expand Down
4 changes: 4 additions & 0 deletions content/test/data/direct_sockets/open.html.mock-http-headers
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
HTTP/1.1 200 OK
Content-Type: text/html
Cross-Origin-Opener-Policy: same-origin
Cross-Origin-Embedder-Policy: require-corp
4 changes: 4 additions & 0 deletions content/test/data/direct_sockets/tcp.html.mock-http-headers
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
HTTP/1.1 200 OK
Content-Type: text/html
Cross-Origin-Opener-Policy: same-origin
Cross-Origin-Embedder-Policy: require-corp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Custom=|Getter|Setter|LegacyCallAsFunction|PropertyGetter|PropertyEnumerator|Pro
CustomConstructor
DefaultValue=Undefined
DeprecateAs=*
DirectSocketEnabled
DoNotCheckConstants
DoNotTestNewObject
EnforceRange
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ def expr_from_exposure(exposure,
# feature_selector-2nd-phase-term))
# which can be represented in more details as:
# (and cross_origin_isolated_term
# direct_socket_enabled_term
# secure_context_term
# uncond_exposed_term
# (or
Expand All @@ -192,6 +193,7 @@ def expr_from_exposure(exposure,
# feature_selector_term)))
# where
# cross_origin_isolated_term represents [CrossOriginIsolated]
# direct_socket_enabled_term represents [DirectSocketEnabled]
# secure_context_term represents [SecureContext=F1]
# uncond_exposed_term represents [Exposed=(G1, G2)]
# cond_exposed_term represents [Exposed(G1 F1, G2 F2)]
Expand Down Expand Up @@ -224,6 +226,12 @@ def ref_selected(features):
else:
cross_origin_isolated_term = _Expr(True)

# [DirectSocketEnabled]
if exposure.only_in_direct_socket_contexts:
direct_socket_enabled_term = _Expr("${is_direct_socket_enabled}")
else:
direct_socket_enabled_term = _Expr(True)

# [SecureContext]
if exposure.only_in_secure_contexts is True:
secure_context_term = _Expr("${is_in_secure_context}")
Expand Down Expand Up @@ -295,6 +303,7 @@ def ref_selected(features):
# Build an expression.
top_level_terms = []
top_level_terms.append(cross_origin_isolated_term)
top_level_terms.append(direct_socket_enabled_term)
top_level_terms.append(secure_context_term)
if uncond_exposed_terms:
top_level_terms.append(expr_or(uncond_exposed_terms))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ def bind_member_iteration_local_vars(code_node):
"is_cross_origin_isolated",
"const bool ${is_cross_origin_isolated} = "
"${execution_context}->CrossOriginIsolatedCapability();"),
SymbolNode(
"is_direct_socket_enabled",
"const bool ${is_direct_socket_enabled} = "
"${execution_context}->DirectSocketCapability();"),
SymbolNode(
"is_in_secure_context", "const bool ${is_in_secure_context} = "
"${execution_context}->IsSecureContext();"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1875,11 +1875,12 @@ def optimize_element_cereactions_reflect():
elif key == "Reflect":
has_reflect = True
elif key in ("Affects", "CrossOriginIsolated", "DeprecateAs",
"Exposed", "LogActivity", "LogAllWorlds", "Measure",
"MeasureAs", "ReflectEmpty", "ReflectInvalid",
"ReflectMissing", "ReflectOnly",
"RuntimeCallStatsCounter", "RuntimeEnabled",
"SecureContext", "URL", "Unscopable"):
"DirectSocketEnabled", "Exposed", "LogActivity",
"LogAllWorlds", "Measure", "MeasureAs",
"ReflectEmpty", "ReflectInvalid", "ReflectMissing",
"ReflectOnly", "RuntimeCallStatsCounter",
"RuntimeEnabled", "SecureContext", "URL",
"Unscopable"):
pass
else:
return None
Expand Down Expand Up @@ -4431,6 +4432,9 @@ def bind_installer_local_vars(code_node, cg_context):
S("is_cross_origin_isolated",
("const bool ${is_cross_origin_isolated} = "
"${execution_context}->CrossOriginIsolatedCapability();")),
S("is_direct_socket_enabled",
("const bool ${is_direct_socket_enabled} = "
"${execution_context}->DirectSocketCapability();")),
S("is_in_secure_context",
("const bool ${is_in_secure_context} = "
"${execution_context}->IsSecureContext();")),
Expand Down
9 changes: 9 additions & 0 deletions third_party/blink/renderer/bindings/scripts/code_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ def cross_origin_isolated_if(code, cross_origin_isolated_test):
code, 'execution_context && (%s)' % cross_origin_isolated_test)


# [DirectSocketEnabled]
def direct_socket_enabled_if(code, direct_socket_enabled_test):
if not direct_socket_enabled_test:
return code
return generate_indented_conditional(
code, 'execution_context && (%s)' % direct_socket_enabled_test)


# [SecureContext]
def secure_context_if(code, secure_context_test):
if secure_context_test is None:
Expand Down Expand Up @@ -123,6 +131,7 @@ def initialize_jinja_env(cache_dir):
'runtime_enabled': runtime_enabled_if,
'runtime_enabled_function': v8_utilities.runtime_enabled_function,
'cross_origin_isolated': cross_origin_isolated_if,
'direct_socket_enabled': direct_socket_enabled_if,
'secure_context': secure_context_if
})
jinja_env.filters.update(constant_filters())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def generate_global_constructors_list(interface_name, extended_attributes):
extended_attributes[name]) if extended_attributes[name] else '')
for name in [
'RuntimeEnabled', 'ContextEnabled', 'CrossOriginIsolated',
'SecureContext'
'DirectSocketEnabled', 'SecureContext'
] if name in extended_attributes
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
# attribute on the main interface.
DEPENDENCY_EXTENDED_ATTRIBUTES = frozenset([
'CrossOriginIsolated',
'DirectSocketEnabled',
'RuntimeEnabled',
'SecureContext',
])
Expand Down
33 changes: 22 additions & 11 deletions third_party/blink/renderer/bindings/scripts/v8_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,11 @@ def attribute_context(interface, attribute, interfaces, component_info):
'runtime_enabled_feature_name':
v8_utilities.runtime_enabled_feature_name(attribute, runtime_features),
# [CrossOriginIsolated]
'cross_origin_isolated_test': v8_utilities.cross_origin_isolated(attribute, interface),
'cross_origin_isolated_test':
v8_utilities.cross_origin_isolated(attribute, interface),
# [DirectSocketEnabled]
'direct_socket_enabled_test':
v8_utilities.direct_socket_enabled(attribute, interface),
# [SecureContext]
'secure_context_test': v8_utilities.secure_context(attribute, interface),
'use_output_parameter_for_result': idl_type.use_output_parameter_for_result,
Expand Down Expand Up @@ -344,25 +348,30 @@ def is_cross_origin_isolated(attribute):
return bool(attribute['cross_origin_isolated_test'])


def is_direct_socket_enabled(attribute):
return bool(attribute['direct_socket_enabled_test'])


def is_secure_context(attribute):
return bool(attribute['secure_context_test'])


def filter_accessors(attributes):
return [
attribute for attribute in attributes
if not (attribute['exposed_test'] or is_secure_context(attribute)
or is_cross_origin_isolated(attribute)
or attribute['context_enabled_feature_name']
or is_origin_trial_enabled(attribute)
or attribute['runtime_enabled_feature_name'])
attribute for attribute in attributes if not (
attribute['exposed_test'] or is_secure_context(attribute)
or is_cross_origin_isolated(attribute) or is_direct_socket_enabled(
attribute) or attribute['context_enabled_feature_name']
or is_origin_trial_enabled(attribute)
or attribute['runtime_enabled_feature_name'])
and not attribute['is_data_type_property']
]


def is_data_attribute(attribute):
return (not (attribute['exposed_test'] or is_secure_context(attribute)
or is_cross_origin_isolated(attribute)
or is_direct_socket_enabled(attribute)
or attribute['context_enabled_feature_name']
or is_origin_trial_enabled(attribute)
or attribute['runtime_enabled_feature_name'])
Expand All @@ -379,16 +388,18 @@ def filter_runtime_enabled(attributes):
return [
attribute for attribute in attributes
if not (attribute['exposed_test'] or is_secure_context(attribute)
or is_cross_origin_isolated(attribute))
or is_cross_origin_isolated(attribute)
or is_direct_socket_enabled(attribute))
and attribute['runtime_enabled_feature_name']
]


def filter_conditionally_enabled(attributes):
return [
attribute for attribute in attributes if attribute['exposed_test'] or
((is_secure_context(attribute) or is_cross_origin_isolated(attribute))
and not is_origin_trial_enabled(attribute))
attribute for attribute in attributes if attribute['exposed_test'] or (
(is_secure_context(attribute) or is_cross_origin_isolated(
attribute) or is_direct_socket_enabled(attribute))
and not is_origin_trial_enabled(attribute))
]


Expand Down
23 changes: 20 additions & 3 deletions third_party/blink/renderer/bindings/scripts/v8_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,13 @@ def member_filter_by_name(members, name):
feature['needs_cross_origin_isolated'] = any(
member.get('cross_origin_isolated_test', False)
for member in members)
feature['needs_direct_socket_enabled'] = any(
member.get('direct_socket_enabled_test', False)
for member in members)
feature['needs_context'] = feature['needs_secure_context'] or feature[
'needs_cross_origin_isolated'] or any(
member.get('exposed_test', False) for member in members)
'needs_cross_origin_isolated'] or feature[
'needs_direct_socket_enabled'] or any(
member.get('exposed_test', False) for member in members)

if features:
includes.add('platform/bindings/script_state.h')
Expand Down Expand Up @@ -538,9 +542,12 @@ def interface_context(interface, interfaces, component_info):
attr for attr in conditionally_enabled_attributes
if attr['constructor_type']
]
has_conditional_coi_attributes = any( #pylint: disable=invalid-name
has_conditional_coi_attributes = any( # pylint: disable=invalid-name
v8_attributes.is_cross_origin_isolated(attr)
for attr in conditionally_enabled_attributes)
has_conditional_direct_socket_attributes = any( # pylint: disable=invalid-name
v8_attributes.is_direct_socket_enabled(attr)
for attr in conditionally_enabled_attributes)
has_conditional_secure_attributes = any( # pylint: disable=invalid-name
v8_attributes.is_secure_context(attr)
for attr in conditionally_enabled_attributes)
Expand All @@ -551,6 +558,8 @@ def interface_context(interface, interfaces, component_info):
conditional_interface_objects,
'has_conditional_coi_attributes':
has_conditional_coi_attributes,
'has_conditional_direct_socket_attributes':
has_conditional_direct_socket_attributes,
'has_conditional_secure_attributes':
has_conditional_secure_attributes,
})
Expand All @@ -565,10 +574,15 @@ def interface_context(interface, interfaces, component_info):
has_conditional_coi_methods = any( # pylint: disable=invalid-name
v8_methods.is_cross_origin_isolated(method)
for method in conditional_methods)
has_conditional_direct_socket_methods = any( # pylint: disable=invalid-name
v8_methods.is_direct_socket_enabled(method)
for method in conditional_methods)
has_conditional_secure_methods = any( # pylint: disable=invalid-name
v8_methods.is_secure_context(method) for method in conditional_methods)
context.update({
'has_conditional_coi_methods': has_conditional_coi_methods,
'has_conditional_direct_socket_methods':
has_conditional_direct_socket_methods,
'has_conditional_secure_methods': has_conditional_secure_methods,
'conditional_methods': conditional_methods,
})
Expand Down Expand Up @@ -1224,6 +1238,9 @@ def overloads_context(interface, overloads):
# [CrossOriginIsolated]
'cross_origin_isolated_test_all':
common_value(overloads, 'cross_origin_isolated_test'),
# [DirectSocketEnabled]
'direct_socket_enabled_test_all':
common_value(overloads, 'direct_socket_enabled_test'),
# [SecureContext]
'secure_context_test_all':
common_value(overloads, 'secure_context_test'),
Expand Down
11 changes: 10 additions & 1 deletion third_party/blink/renderer/bindings/scripts/v8_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ def is_cross_origin_isolated(method):
method else method['cross_origin_isolated_test'])


def is_direct_socket_enabled(method):
return bool(
method['overloads']['direct_socket_enabled_test_all'] if 'overloads' in
method else method['direct_socket_enabled_test'])


def is_secure_context(method):
return bool(method['overloads']['secure_context_test_all'] if 'overloads'
in method else method['secure_context_test'])
Expand All @@ -78,7 +84,7 @@ def is_conditionally_enabled(method):
exposed = method['overloads']['exposed_test_all'] \
if 'overloads' in method else method['exposed_test']
return exposed or is_secure_context(method) or is_cross_origin_isolated(
method)
method) or is_direct_socket_enabled(method)


def filter_conditionally_enabled(methods, interface_is_partial):
Expand Down Expand Up @@ -315,6 +321,9 @@ def method_context(interface, method, component_info, is_visible=True):
# [CrossOriginIsolated]
'cross_origin_isolated_test':
v8_utilities.cross_origin_isolated(method, interface),
# [DirectSocketEnabled]
'direct_socket_enabled_test':
v8_utilities.direct_socket_enabled(method, interface),
# [SecureContext]
'secure_context_test':
v8_utilities.secure_context(method, interface),
Expand Down
18 changes: 18 additions & 0 deletions third_party/blink/renderer/bindings/scripts/v8_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,24 @@ def cross_origin_isolated(member, interface):
return 'is_cross_origin_isolated'


# [DirectSocketEnabled]
def direct_socket_enabled(member, interface):
"""Returns C++ code that checks whether an interface/method/attribute/etc.
is exposed to the current context. Requires that the surrounding code
defines an |is_direct_socket_enabled| variable prior to this check."""
member_is_direct_socket_enabled = (
'DirectSocketEnabled' in member.extended_attributes)
interface_is_direct_socket_enabled = (
(member.defined_in is None or member.defined_in == interface.name)
and 'DirectSocketEnabled' in interface.extended_attributes)

if not (member_is_direct_socket_enabled
or interface_is_direct_socket_enabled):
return None

return 'is_direct_socket_enabled'


# [SecureContext]
def secure_context(member, interface):
"""Returns C++ code that checks whether an interface/method/attribute/etc. is exposed
Expand Down
Loading

0 comments on commit 8fbd3bb

Please sign in to comment.