Partition Blob Registry by the top-level main document origin #7549
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
EWS run on previous version of this PR (hash e13f604)
|
webkit-ews-buildbot
added
the
merging-blocked
|
EWS run on previous version of this PR (hash e361b65)
|
|
EWS run on previous version of this PR (hash bb8585b)
|
|
EWS run on previous version of this PR (hash a5fe313)
|
sysrqb
requested review from
a team,
philn,
cdumez,
rniwa and
marcoscaceres
as code owners
|
EWS run on previous version of this PR (hash 51274c8)
|
|
EWS run on previous version of this PR (hash c43ee8c) |
|
EWS run on previous version of this PR (hash 42011b7) |
|
EWS run on previous version of this PR (hash bd9cd15) |
|
EWS run on previous version of this PR (hash 2241057) |
|
EWS run on previous version of this PR (hash 10e85ff) |
webkit-ews-buildbot
added
the
merging-blocked
sysrqb
added
the
DOM
|
EWS run on previous version of this PR (hash af5e232) |
webkit-ews-buildbot
added
the
merging-blocked
|
EWS run on previous version of this PR (hash 163d40e) |
szewai
reviewed
Aug 18, 2023
There was a problem hiding this comment.
Partitioning looks good to me; need @cdumez or @achristensen07 to review the changes for navigation.
|
EWS run on previous version of this PR (hash 051d764) |
🛠 jsc-mips
achristensen07
approved these changes
Aug 21, 2023
webkit-ews-buildbot
added
the
merging-blocked
|
EWS run on previous version of this PR (hash 692f8c5) |
|
EWS run on current version of this PR (hash 4806ee0) |
https://bugs.webkit.org/show_bug.cgi?id=260035 rdar://problem/113705298 Reviewed by Alex Christensen and Sihui Liu. Public blob URLs are only accessible from same-origin dcuments, but access is not restricted by the top-level origin. This means that Blob URLs can be used as a cross-origin tracking mechanism within iframes. In this patch we partition public blob URLs within the Blob Registry by top-level origin. This partitioning is controlled by a feature flag that is disabled by default. I took a few approaches at solving this. The most difficult challenge was finding a solution that allowed retrieving BlobData using a public blob URL from WKWebView APIs. In that case, the relevant top document may not be obvious, or may not exist. As a result, the design of this partitioning is more like access control rather than adding another key into the hashmap. Two alternative designs I considered include creating a second hashmap that is keyed by <URL, SecurityOriginData> and we lookup the BlobData in that map if we have a SecurityOriginData, otherwise we use the unpartitioned map. Or, we create a new map from URL -> SecurityOriginData where we can lookup the associated top origin SecurityOriginData if we don't already know it. However, both of these options are more complex than the chosen implementation, and neither of them seemed safer. This change also enforces a noopener policy on new windows when the top origin of the opener is cross-origin with the blob's security origin. This is a mitigation that was discussed in the blob URL storage partitioning issue [0] with cross-engine support, and that seemed reasonable to me. [0] w3c/FileAPI#153 * LayoutTests/TestExpectations: * LayoutTests/http/tests/local/blob/download-blob-from-iframe-expected.txt: Added. * LayoutTests/http/tests/local/blob/download-blob-from-iframe.html: Added. * LayoutTests/http/tests/local/blob/navigate-blob-expected.txt: Added. * LayoutTests/http/tests/local/blob/navigate-blob.html: Added. * LayoutTests/http/tests/local/blob/resources/broadcast-channel-proxy.html: Added. * LayoutTests/http/tests/local/blob/resources/iframe-creating-or-downloading-blob.html: Added. * LayoutTests/http/tests/local/blob/resources/iframe-for-creating-and-navigating-to-blob.html: Added. * LayoutTests/http/tests/local/blob/resources/main-frame-with-iframe-creating-or-navigating-to-blob.html: Added. * LayoutTests/http/tests/local/blob/resources/main-frame-with-iframe-downloading-blob.html: Added. * LayoutTests/http/tests/security/blob-null-url-location-origin-expected.txt: * LayoutTests/http/tests/security/blob-null-url-location-origin.html: * LayoutTests/http/tests/security/cross-origin-blob-transfer-expected.txt: Added. * LayoutTests/http/tests/security/cross-origin-blob-transfer.html: Added. * LayoutTests/http/tests/security/resources/iframe-cross-origin-blob-transfer.html: Added. * LayoutTests/http/tests/security/top-level-unique-origin2.https.html: * LayoutTests/platform/gtk-wk2/http/tests/local/blob/download-blob-from-iframe-expected.txt: Added. * LayoutTests/platform/mac-wk1/TestExpectations: * Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml: * Source/WebCore/fileapi/BlobURL.cpp: (WebCore::BlobURL::isInternalURL): * Source/WebCore/fileapi/BlobURL.h: * Source/WebCore/fileapi/ThreadableBlobRegistry.cpp: (WebCore::ThreadableBlobRegistry::registerInternalFileBlobURL): (WebCore::ThreadableBlobRegistry::registerInternalBlobURL): (WebCore::ThreadableBlobRegistry::registerInternalBlobURLOptionallyFileBacked): (WebCore::ThreadableBlobRegistry::registerInternalBlobURLForSlice): (WebCore::isInternalBlobURL): Deleted. * Source/WebCore/loader/FrameLoader.cpp: (WebCore::FrameLoader::loadURL): (WebCore::FrameLoader::loadPostRequest): (WebCore::createWindow): * Source/WebCore/platform/network/BlobRegistryImpl.cpp: (WebCore::BlobRegistryImpl::registerBlobURLOptionallyFileBacked): (WebCore::BlobRegistryImpl::unregisterBlobURL): (WebCore::BlobRegistryImpl::getBlobDataFromURL const): (WebCore::BlobRegistryImpl::addBlobData): (WebCore::BlobRegistryImpl::registerBlobURLHandle): (WebCore::BlobRegistryImpl::unregisterBlobURLHandle): * Source/WebCore/platform/network/BlobRegistryImpl.h: Canonical link: https://commits.webkit.org/267172@main
webkit-commit-queue
force-pushed
the
eng/241318
branch
from
4806ee0
to
7f2ea8f
Compare
|
Committed 267172@main (7f2ea8f): https://commits.webkit.org/267172@main Reviewed commits have been landed. Closing PR #7549 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
7f2ea8f
4806ee0