Skip to content

Commit

Permalink
[WebOTP] Prototyping WebOTP support in cross-origin iframes
Browse files Browse the repository at this point in the history
Major changes:
1. Adds a new policy-controlled feature "otp-credentials" to allow the
  iframe to get credentials from its parent frame.
2. Supports parsing SMSes with format "@top.com #code @iframe.com"
3. Passes along a list of origins in the system instead of a single
  origin previously.

More details on the context: WICG/web-otp#50

Bug: 1136506
Change-Id: Ic458e51c33b721a80204abb490776b436086bff4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2497992
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831865}
GitOrigin-RevId: 5874e3cfc841949d2a808d50be2374028d2306e3
  • Loading branch information
yi-gu authored and Copybara-Service committed Nov 30, 2020
1 parent c0ae97a commit 0397918
Show file tree
Hide file tree
Showing 10 changed files with 127 additions and 17 deletions.
1 change: 1 addition & 0 deletions blink/public/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ source_set("headers") {
"service_worker/service_worker_status_code.h",
"service_worker/service_worker_type_converters.h",
"service_worker/service_worker_types.h",
"sms/webotp_constants.h",
"sms/webotp_service_destroyed_reason.h",
"sms/webotp_service_outcome.h",
"switches.h",
Expand Down
14 changes: 14 additions & 0 deletions blink/public/common/sms/webotp_constants.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2020 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.

#ifndef THIRD_PARTY_BLINK_PUBLIC_COMMON_SMS_WEBOTP_CONSTANTS_H_
#define THIRD_PARTY_BLINK_PUBLIC_COMMON_SMS_WEBOTP_CONSTANTS_H_

namespace blink {

static constexpr int kMaxUniqueOriginInAncestorChainForWebOTP = 2;

} // namespace blink

#endif // THIRD_PARTY_BLINK_PUBLIC_COMMON_SMS_WEBOTP_CONSTANTS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ enum FeaturePolicyFeature {
// Controls access to gamepads interface
kGamepad = 79,

// Controls use of WebOTP API.
kOTPCredentials = 80,

// Don't change assigned numbers of any item, and don't reuse removed slots.
// Add new features at the end of the enum.
// Also, run update_feature_policy_enum.py in
Expand Down
1 change: 1 addition & 0 deletions blink/public/platform/web_runtime_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ class WebRuntimeFeatures {
BLINK_PLATFORM_EXPORT static void EnableWebGPU(bool);
BLINK_PLATFORM_EXPORT static void EnableWebID(bool);
BLINK_PLATFORM_EXPORT static void EnableWebNfc(bool);
BLINK_PLATFORM_EXPORT static void EnableWebOTPAssertionFeaturePolicy(bool);
BLINK_PLATFORM_EXPORT static void EnableWebShare(bool);
BLINK_PLATFORM_EXPORT static void EnableWebUsb(bool);
BLINK_PLATFORM_EXPORT static void EnableWebXR(bool);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@
feature_default: "EnableForAll",
depends_on: ["FeaturePolicyForSandbox"],
},
{
name: "OTPCredentials",
feature_policy_name: "otp-credentials",
depends_on: ["WebOTPAssertionFeaturePolicy"],
},
{
name: "OrientationLock",
feature_policy_name: "orientation-lock",
Expand Down
95 changes: 79 additions & 16 deletions blink/renderer/modules/credentialmanager/credentials_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/numerics/safe_conversions.h"
#include "base/rand_util.h"
#include "build/build_config.h"
#include "third_party/blink/public/common/sms/webotp_constants.h"
#include "third_party/blink/public/common/sms/webotp_service_outcome.h"
#include "third_party/blink/public/mojom/credentialmanager/credential_manager.mojom-blink.h"
#include "third_party/blink/public/mojom/feature_policy/feature_policy.mojom-blink.h"
Expand Down Expand Up @@ -111,7 +112,9 @@ enum class RequiredOriginType {
// expressed in various ways, e.g.: |allow| iframe attribute and/or
// feature-policy header, and may be inherited from parent browsing
// contexts. See Feature Policy spec.
kSecureAndPermittedByFeaturePolicy,
kSecureAndPermittedByWebAuthGetAssertionFeaturePolicy,
// Similar to the enum above, checks the "otp-credentials" feature policy.
kSecureAndPermittedByWebOTPAssertionFeaturePolicy,
};

bool IsSameOriginWithAncestors(const Frame* frame) {
Expand All @@ -128,6 +131,37 @@ bool IsSameOriginWithAncestors(const Frame* frame) {
return true;
}

// An ancestor chain is valid iff there are at most 2 unique origins on the
// chain (current origin included), the unique origins must be consecutive.
// e.g. the following are valid:
// A.com (calls WebOTP API)
// A.com -> A.com (calls WebOTP API)
// A.com -> A.com -> B.com (calls WebOTP API)
// A.com -> B.com -> B.com (calls WebOTP API)
// while the following are invalid:
// A.com -> B.com -> A.com (calls WebOTP API)
// A.com -> B.com -> C.com (calls WebOTP API)
// Note that there is additional requirement on feature permission being granted
// upon crossing origins but that is not verified by this function.
bool IsAncestorChainValidForWebOTP(const Frame* frame) {
const SecurityOrigin* current_origin =
frame->GetSecurityContext()->GetSecurityOrigin();
int number_of_unique_origin = 1;

const Frame* parent = frame->Tree().Parent();
while (parent) {
auto* parent_origin = parent->GetSecurityContext()->GetSecurityOrigin();
if (!parent_origin->IsSameOriginWith(current_origin)) {
++number_of_unique_origin;
current_origin = parent_origin;
}
if (number_of_unique_origin > kMaxUniqueOriginInAncestorChainForWebOTP)
return false;
parent = parent->Tree().Parent();
}
return true;
}

bool CheckSecurityRequirementsBeforeRequest(
ScriptPromiseResolver* resolver,
RequiredOriginType required_origin_type) {
Expand Down Expand Up @@ -163,7 +197,8 @@ bool CheckSecurityRequirementsBeforeRequest(
}
break;

case RequiredOriginType::kSecureAndPermittedByFeaturePolicy:
case RequiredOriginType::
kSecureAndPermittedByWebAuthGetAssertionFeaturePolicy:
// The 'publickey-credentials-get' feature's "default allowlist" is
// "self", which means the webauthn feature is allowed by default in
// same-origin child browsing contexts.
Expand All @@ -172,7 +207,7 @@ bool CheckSecurityRequirementsBeforeRequest(
resolver->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kNotAllowedError,
"The 'publickey-credentials-get' feature is not enabled in this "
"document. Feature Policy may be used to delegate Web "
"document. Permissions Policy may be used to delegate Web "
"Authentication capabilities to cross-origin child frames."));
return false;
} else {
Expand All @@ -181,6 +216,22 @@ bool CheckSecurityRequirementsBeforeRequest(
WebFeature::kCredentialManagerCrossOriginPublicKeyGetRequest);
}
break;

case RequiredOriginType::kSecureAndPermittedByWebOTPAssertionFeaturePolicy:
if (!resolver->GetExecutionContext()->IsFeatureEnabled(
mojom::blink::FeaturePolicyFeature::kOTPCredentials)) {
resolver->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kNotAllowedError,
"The 'otp-credentials` feature is not enabled in this document."));
return false;
}
if (!IsAncestorChainValidForWebOTP(resolver->DomWindow()->GetFrame())) {
resolver->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kNotAllowedError,
"More than two unique origins are detected in the origin chain."));
return false;
}
break;
}

return true;
Expand Down Expand Up @@ -208,10 +259,18 @@ void AssertSecurityRequirementsBeforeResponse(
IsSameOriginWithAncestors(resolver->DomWindow()->GetFrame()));
break;

case RequiredOriginType::kSecureAndPermittedByFeaturePolicy:
case RequiredOriginType::
kSecureAndPermittedByWebAuthGetAssertionFeaturePolicy:
SECURITY_CHECK(resolver->GetExecutionContext()->IsFeatureEnabled(
mojom::blink::FeaturePolicyFeature::kPublicKeyCredentialsGet));
break;

case RequiredOriginType::kSecureAndPermittedByWebOTPAssertionFeaturePolicy:
SECURITY_CHECK(
resolver->GetExecutionContext()->IsFeatureEnabled(
mojom::blink::FeaturePolicyFeature::kOTPCredentials) &&
IsAncestorChainValidForWebOTP(resolver->DomWindow()->GetFrame()));
break;
}
}

Expand Down Expand Up @@ -579,7 +638,11 @@ void OnSmsReceive(ScriptPromiseResolver* resolver,
mojom::blink::SmsStatus status,
const WTF::String& otp) {
AssertSecurityRequirementsBeforeResponse(
resolver, RequiredOriginType::kSecureAndSameWithAncestors);
resolver, resolver->GetExecutionContext()->IsFeatureEnabled(
mojom::blink::FeaturePolicyFeature::kOTPCredentials)
? RequiredOriginType::
kSecureAndPermittedByWebOTPAssertionFeaturePolicy
: RequiredOriginType::kSecureAndSameWithAncestors);
auto& window = *LocalDOMWindow::From(resolver->GetScriptState());
ukm::SourceId source_id = window.UkmSourceID();
ukm::UkmRecorder* recorder = window.UkmRecorder();
Expand Down Expand Up @@ -857,13 +920,18 @@ ScriptPromise CredentialsContainer::get(
auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
ScriptPromise promise = resolver->Promise();

auto required_origin_type = RequiredOriginType::kSecureAndSameWithAncestors;
// hasPublicKey() implies that this is a WebAuthn request.
auto required_origin_type =
options->hasPublicKey() &&
RuntimeEnabledFeatures::
WebAuthenticationGetAssertionFeaturePolicyEnabled()
? RequiredOriginType::kSecureAndPermittedByFeaturePolicy
: RequiredOriginType::kSecureAndSameWithAncestors;
if (options->hasPublicKey() &&
RuntimeEnabledFeatures::
WebAuthenticationGetAssertionFeaturePolicyEnabled()) {
required_origin_type = RequiredOriginType::
kSecureAndPermittedByWebAuthGetAssertionFeaturePolicy;
} else if (options->hasOtp() &&
RuntimeEnabledFeatures::WebOTPAssertionFeaturePolicyEnabled()) {
required_origin_type =
RequiredOriginType::kSecureAndPermittedByWebOTPAssertionFeaturePolicy;
}
if (!CheckSecurityRequirementsBeforeRequest(resolver, required_origin_type)) {
return promise;
}
Expand Down Expand Up @@ -996,11 +1064,6 @@ ScriptPromise CredentialsContainer::get(
WTF::Bind(&AbortOtpRequest, WrapPersistent(script_state)));
}

if (!CheckSecurityRequirementsBeforeRequest(
resolver, RequiredOriginType::kSecureAndSameWithAncestors)) {
return promise;
}

auto* webotp_service =
CredentialManagerProxy::From(script_state)->WebOTPService();
webotp_service->Receive(WTF::Bind(&OnSmsReceive, WrapPersistent(resolver),
Expand Down
4 changes: 4 additions & 0 deletions blink/renderer/platform/exported/web_runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,10 @@ void WebRuntimeFeatures::EnableWebAuthenticationGetAssertionFeaturePolicy(
enable);
}

void WebRuntimeFeatures::EnableWebOTPAssertionFeaturePolicy(bool enable) {
RuntimeEnabledFeatures::SetWebOTPAssertionFeaturePolicyEnabled(enable);
}

void WebRuntimeFeatures::EnableLazyInitializeMediaControls(bool enable) {
RuntimeEnabledFeatures::SetLazyInitializeMediaControlsEnabled(enable);
}
Expand Down
5 changes: 5 additions & 0 deletions blink/renderer/platform/runtime_enabled_features.json5
Original file line number Diff line number Diff line change
Expand Up @@ -2184,6 +2184,11 @@
name: "WebOTP",
status: {"default": "experimental", "Android": "stable"},
},
{
name: "WebOTPAssertionFeaturePolicy",
status: "experimental",
depends_on: ["WebOTP"],
},
{
name: "WebScheduler",
origin_trial_feature_name: "WebScheduler",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,19 @@

}, "Test OTPCredential enabled in same origin iframes");

promise_test(async t => {
const messageWatcher = new EventWatcher(t, window, "message");
var iframe = document.createElement("iframe");
iframe.src = remoteBaseURL + "support/otpcredential-iframe.html"
iframe.allow = "otp-credentials";
document.body.appendChild(iframe);

const message = await messageWatcher.wait_for("message");
assert_equals(message.data.result, "Pass");
assert_equals(message.data.code, "ABC123");

}, "OTPCredential enabled in cross origin iframes with permissions policy");

promise_test(async t => {
const messageWatcher = new EventWatcher(t, window, "message");
var iframe = document.createElement("iframe");
Expand All @@ -37,5 +50,5 @@
assert_equals(message.data.result, "Fail");
assert_equals(message.data.errorType, "NotAllowedError");

}, "Test OTPCredential disabled in cross origin iframes");
}, "OTPCredential disabled in cross origin iframes without permissions policy");
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ microphone
midi
modals
orientation-lock
otp-credentials
payment
picture-in-picture
pointer-lock
Expand Down

0 comments on commit 0397918

Please sign in to comment.