| Summary: | [WebAuthn] Allow same-site, cross-origin iframe get() | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | pascoe <pascoe> | ||||||
| Component: | WebKit Misc. | Assignee: | pascoe <pascoe> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, cdumez, changseok, commit-queue, esprehn+autocc, ews-watchlist, gyuyoung.kim, jiewen_tan, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 234283, 234309 | ||||||||
| Bug Blocks: | 222240 | ||||||||
| Attachments: |
|
||||||||
|
Description
pascoe@apple.com
2021-12-10 15:02:51 PST
Created attachment 446826 [details]
Patch
Comment on attachment 446826 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446826&action=review I think this patch needs a bit of work before it's ready to land. Please adjust the same-site test, and switch from a boolean to a two-state enum that documents its purpose. > Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:184 > + if (domain != RegistrableDomain(parentDocument->securityOrigin().data())) You should use "areRegistrableDomainsEqual" from the RegistrableDomains header. We identify same-site in Document like this: // Only prevent cross-site navigations. RefPtr targetDocument = targetFrame.document(); if (targetDocument && (targetDocument->securityOrigin().isSameOriginDomain(SecurityOrigin::create(destinationURL)) || areRegistrableDomainsEqual(targetDocument->url(), destinationURL))) return false; You should probably use something like that to be consistent. It should also handle all the weird cases of data/Blob URLs and so forth properly. > Source/WebCore/Modules/webauthn/WebAuthenticationUtils.h:55 > +WEBCORE_EXPORT Ref<ArrayBuffer> buildClientDataJson(ClientDataType /*type*/, const BufferSource& challenge, const SecurityOrigin& /*origin*/, const bool crossOrigin); Instead of this boolean, we should create a two-state enum to represent this state. Something like WebAuthn::CrossOrigin::Yes/No, or WebAuthn::Scope::CrossOrigin/SameOrigin. There are many examples in the sources of this pattern. Created attachment 447053 [details]
Patch
Comment on attachment 447053 [details]
Patch
Looks great -- thank you for making those changes! r=me
Committed r286993 (?): <https://commits.webkit.org/r286993> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447053 [details]. Re-opened since this is blocked by bug 234283 |