Bug 222240 – [WebAuthn] Using WebAuthn within cross-origin iframe elements

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Mathieu Perreault]

As mentioned in the  WebAuthn Level 2 editor's draft at https://w3c.github.io/webauthn/#sctn-iframe-guidance, functionality to get credentials from a cross-origin iframe should be enabled if the iframe has the allow="publickey-credentials-get" attribute/value pair. Other browsers implementers have implemented this capability and my group sees a relevant use case for this in our product.

[Radar WebKit Bug Importer]

<rdar://problem/74830748>

[login Llama]

Agreed This is important for Paments and a number of other use-cases.  It should be part of WebAuthn level 2 support.

[John Wilander]

WebKit on Cocoa platforms (and possibly other platforms too) block cookies for cross-site resources by default. All other state and storage is partitioned per first-party site. Whatever this support entails must not regress the privacy protections we already have in place. One way of resolving this is to tie it to the Storage Access API.

[John Wilander]

It would also be good to know if cross-origin but same-site would be sufficient since that is currently not a privacy boundary.

[[e-mail]]

I'm curious about what allowing WebAuthn within a cross-origin i-frame might reveal. The dialog messages show what site is requesting the WebAuthn panel. 

For example, if we implement this and site1.com embeds a google.com webauthn login page, the dialog would show "google.com" is asking to login.

I have seen some discussion of the embedded document obtaining information about the state of the embedding page via if certain calls pass/fail (therefore obtaining information about the i-frame's allow property) here: https://www.w3.org/TR/permissions-policy-1/#privacy-expose-policy . But we already implement several feature policies for camera, fullscreen, microphone, etc in https://github.com/WebKit/WebKit/blob/main/Source/WebCore/html/FeaturePolicy.cpp . I don't see anything anything special about WebAuthn where we might want to restrict the use further.

[John Wilander]

The Storage Access API is the only shipping way to request a cross-site identity and the prompt tells the user that their activity may be tracked by the requesting party if they grant access.

Cross-site WebAuthn would have to convey the same information since as soon as the user is identified, that identity can be shared with the first party website or stored by the third-party in first-party storage.

This is not the same as asking for permission to use device features such as camera. This is about user identity and linking of user identity across websites.

See WebKit's tracking prevention policy: https://webkit.org/tracking-prevention-policy/

[[e-mail]]

I don't see the change being asked for here as providing any special way of of transmitting information between the embedded document and the embedder, even if it's cross-origin. All the current restrictions on communication between the embedding document and the embedded document would still apply.

The webauthn spec specifies that Web Authentication should be disabled in i-frames with a feature policy to allow it here: https://www.w3.org/TR/webauthn-2/#sctn-iframe-guidance 

If a cross-origin embedded site needs to convey information after authentication, they would still need to use existing mechanisms to do it.

[John Wilander]

(In reply to [e-mail] from comment #7)
> I don't see the change being asked for here as providing any special way of
> of transmitting information between the embedded document and the embedder,
> even if it's cross-origin. All the current restrictions on communication
> between the embedding document and the embedded document would still apply.
> 
> The webauthn spec specifies that Web Authentication should be disabled in
> i-frames with a feature policy to allow it here:
> https://www.w3.org/TR/webauthn-2/#sctn-iframe-guidance 
> 
> If a cross-origin embedded site needs to convey information after
> authentication, they would still need to use existing mechanisms to do it.

It's not about transmitting. It's about cross-site access to user IDs.

Today, in shipping WebKit, there is no automatic or invisible way to know a user identity in a cross-site iframe unless it has already been established. Such iframes have to call the Storage Access API on a user gesture in their iframe to request access to cookies. The Storage Access API prompt will inform the user that granting access means their activity may be tracked by the requesting party on this first-party website.

Once a cross-site iframe has access to a user identity it can use regular means to share that identity with the first-party, other third-parties, or with itself in the form of a third-party script running in the first-party context. The most common method is postMessage.

The privacy boundary is to make sure cross-site resources cannot identify the user without the user understanding the potential tracking risk involved. Allowing cross-site WebAuthn without informing the user of that risk would create a loophole in how we try to make sure users understand the privacy implications of their choices.

[[e-mail]]

Created attachment 453678 [details]
Patch

[[e-mail]]

Created attachment 453752 [details]
Patch

[[e-mail]]

Created attachment 453754 [details]
Patch

[[e-mail]]

Created attachment 453772 [details]
Patch

[Brent Fulgham]

Comment on attachment 453772 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=453772&action=review

I think this is close, but needs a little work before it's ready to land. r- to adjust the naming and clean up some CredentialContainer:scope implementation.

> Source/WebCore/ChangeLog:11
> +        same-site to cross-origin with consent.

I had to read this sentence a few times. I propose:

"""
This patch relaxes the requirement to perform a Web Authentication assertion
inside an i-frame with the "publickey-credentials-get" feature policy from
'same-site' to 'cross-origin with consent'.
"""

> Source/WebCore/Modules/credentialmanagement/CredentialsContainer.cpp:49
> +std::pair<WebAuthn::Scope, std::optional<SecurityOriginData>> CredentialsContainer::scopeAndSingleParent()

Maybe "singleParent" should be called something like "AuthenticationOrigin", or "RelyingParty"? I think we are authenticating in a frame (which is the relying party) on behalf of the main frame (which is the site seeking to do something based on the authentication succeeding in the frame).

Maybe we should call it  "scopeAndCrossOriginParent"?

> Source/WebCore/Modules/credentialmanagement/CredentialsContainer.cpp:64
> +            parents.add(origin.data());

Collecting the set of parents really only makes sense if we have a plan to select one over the others due to some criteria. Since we seem only care about the first one we encounter, we should just store one:

if (!origin.isSameOriginAs(document->securityOrigin())) {
    isSameOrigin = false;
    if (!singleParent)
        singleParent = origin.data();
}

Or maybe:

if (isSameOrigin && !origin.isSameOriginAs(document->securityOrigin())) {
    isSameOrigin = false;
    singleParent = origin.data();
}

Or maybe:

if (!singleParent && !origin.isSameOriginAs(document->securityOrigin()))
    singleParent = origin.data()

... and get rid of the 'isSameOrigin' flag entirely.

And maybe "singleParent" should be called "crossOriginParent"

> Source/WebCore/Modules/credentialmanagement/CredentialsContainer.h:62
> +    std::pair<WebAuthn::Scope, std::optional<SecurityOriginData>> scopeAndSingleParent();

It might be worth declaring a typedef for this std::pair-based type, since you use it in a number of places.

> Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:205
> +        promise.reject(Exception { NotAllowedError, "The origin of the document is not the same as multiple of its ancestors."_s });

What does "... not the same as multiple of its ancestors" mean here? I think this would be clearer to say:

> Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp:87
>                  }

I looks like the indentation of everything inside HAVE(UNIFIED_ASC_AUTH_UI) needs to be fixed (I realize this predates your change, but we should fix it).

[[e-mail]]

Created attachment 453986 [details]
Patch

[Brent Fulgham]

Comment on attachment 453986 [details]
Patch

r=me

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 453986 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[[e-mail]]

Created attachment 454129 [details]
Patch for landing

[Chris Dumez]

Comment on attachment 454129 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=454129&action=review

> Source/WebCore/Modules/credentialmanagement/CredentialsContainer.cpp:49
> +ScopeAndCrossOriginParent CredentialsContainer::scopeAndCrossOriginParent()

Can this function be marked const? Doesn't seem like it should be causing mutations.

> Source/WebCore/Modules/credentialmanagement/CredentialsContainer.cpp:52
> +        return std::make_pair(WebAuthn::Scope::CrossOrigin, std::nullopt);

With C++17, you should be able to use `std::pair { WebAuthn::Scope::CrossOrigin, std::nullopt }`, instead of std::make_pair().

Same comment applies below.

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:185
> +static RetainPtr<ASCCredentialRequestContext> configureRegistrationRequestContext(const PublicKeyCredentialCreationOptions& options, Vector<uint8_t> hash, std::optional<WebCore::GlobalFrameIdentifier> globalFrameID)

We really shouldn't be passing vectors around by value.

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:250
> +static inline RetainPtr<ASCPublicKeyCredentialAssertionOptions> configureAssertionOptions(const PublicKeyCredentialRequestOptions& options, Vector<uint8_t> hash, ASCPublicKeyCredentialKind kind, const std::optional<SecurityOriginData>& parentOrigin, RetainPtr<NSMutableArray<ASCPublicKeyCredentialDescriptor *>> allowedCredentials, RetainPtr<NSString> userVerification)

We really shouldn't be passing vectors around by value.

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:269
> +static RetainPtr<ASCCredentialRequestContext> configurationAssertionRequestContext(const PublicKeyCredentialRequestOptions& options, Vector<uint8_t> hash, std::optional<WebCore::MediationRequirement> mediation, std::optional<WebCore::GlobalFrameIdentifier> globalFrameID, std::optional<WebCore::SecurityOriginData>& parentOrigin)

ditto.

[[e-mail]]

Created attachment 454132 [details]
Patch for landing

[EWS]

Committed r291018 (248192@main): <https://commits.webkit.org/248192@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454132 [details].

[Brent Fulgham]

This fix shipped with Safari 15.5 (all platforms).