Restrict origins for HLS keys [chromium/src : main]
Dale Curtis (Gerrit)
Dale Curtis added 1 comment![]( "Open in Gerrit")
} else if (declared_uri_value != resource_uri.spec()) {(1) I don't understand how this ensures the url is path-only; a comment would be helpful.
(2) I think this will be brittle, but we can't use SecurityOrigin since it's in blink. Is there someway this code could query up to the WebMediaPlayer instance?
Related details
- Ted (Chromium) Meyer
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Ted (Chromium) Meyer added 1 comment![]( "Open in Gerrit")
} else if (declared_uri_value != resource_uri.spec()) {(1) I don't understand how this ensures the url is path-only; a comment would be helpful.
(2) I think this will be brittle, but we can't use SecurityOrigin since it's in blink. Is there someway this code could query up to the WebMediaPlayer instance?
for (1), yeah I'll add a comment. for (2), we specifically do not want to compare to the security origin for this, but rather to the origin of the manifest. The manifest itself might be CORS for the page. there's more of a writeup for why in the linked bug.
Related details
- Dale Curtis
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dale Curtis
| Code-Review | +1 |
1 comment
Related details
- Ted (Chromium) Meyer
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Ted (Chromium) Meyer added 1 comment![]( "Open in Gerrit")
GURL uri,Can we rename this to playlist_uri?
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dale Curtis
| Code-Review | +1 |
1 comment
if (!resource_uri.is_valid()) {What is considered invalid? It seems like "https://example.com/manifest.m3u8".Resolve("https://anotherexample.com/file.key") would generate an invalid URI. Ditto with data://. Are the lines below even reachable post-Resolve? I assume so since you have a test which passes, but I don't really understand Resolve() then.
Related details
- Ted (Chromium) Meyer
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Ted (Chromium) Meyer voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ted (Chromium) Meyer
| Commit-Queue | +2 |
1 comment
if (!resource_uri.is_valid()) {What is considered invalid? It seems like "https://example.com/manifest.m3u8".Resolve("https://anotherexample.com/file.key") would generate an invalid URI. Ditto with data://. Are the lines below even reachable post-Resolve? I assume so since you have a test which passes, but I don't really understand Resolve() then.
`uri.Resolve(str path)` basically treats `path` as a GURL with potentially missing parts, and then fills those parts in from `uri`. In your example, the "str path" url isn't missing any parts, so it gets returned as a fully formed GURL. Some other examples:
- `"https://example.com/xyz/manifest.txt".resolve("secret.key") => "https://example.com/xyz/secret.key"` (added scheme, origin, and dirname(path))
- `"wss://example.com".resolve("//google.com/socket") => "wss://google.com/socket"` (added scheme)
- `"http://example.com/long/directory/path.txt".resolve("/secret.key") => "http://example.com/secret.key"` (added scheme and origin)
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Ted (Chromium) Meyer added 1 comment![]( "Open in Gerrit")
if (!resource_uri.is_valid()) {Ted (Chromium) MeyerWhat is considered invalid? It seems like "https://example.com/manifest.m3u8".Resolve("https://anotherexample.com/file.key") would generate an invalid URI. Ditto with data://. Are the lines below even reachable post-Resolve? I assume so since you have a test which passes, but I don't really understand Resolve() then.
`uri.Resolve(str path)` basically treats `path` as a GURL with potentially missing parts, and then fills those parts in from `uri`. In your example, the "str path" url isn't missing any parts, so it gets returned as a fully formed GURL. Some other examples:
- `"https://example.com/xyz/manifest.txt".resolve("secret.key") => "https://example.com/xyz/secret.key"` (added scheme, origin, and dirname(path))
- `"wss://example.com".resolve("//google.com/socket") => "wss://google.com/socket"` (added scheme)
- `"http://example.com/long/directory/path.txt".resolve("/secret.key") => "http://example.com/secret.key"` (added scheme and origin)
Oh, forgot the "invalid" part. Basically its for if either the GURL on which `::Resolve` is called is itself not valid, or if the first arg is empty, or some other non-URL friendly characters, that kind of thing. It's here so that we catch stuff like people setting the key url to `:\\://:\\://:\\://:` or something equally insane.
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Ted (Chromium) Meyer voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Restrict origins for HLS keys
HLS encrypted content should not be able to use a key which was loaded
from a source that is "insecure". A key-source is considered secure when
it meets one of these requirements:
- it is not a cross-origin key (hosted on the same domain as the frame
in which it is being accessed)
- it is a data:// url (fully embedded in the manifest)
- it is on the same domain as the manifest from which it is loaded.
- it has the proper Access-Control-Allow-Origin header for the top
frame in which the media is being played.
- M media/filters/hls_network_access_impl.cc
- M media/filters/hls_network_access_impl_unittest.cc
- M media/filters/hls_rendition_impl.cc
- M media/filters/hls_rendition_impl.h
- M media/formats/hls/media_playlist.cc
- M media/formats/hls/media_playlist.h
- M media/formats/hls/media_playlist_test_builder.h
- M media/formats/hls/media_playlist_unittest.cc
- M media/formats/hls/media_segment.cc
- M media/formats/hls/media_segment.h
- M media/formats/hls/media_segment_unittest.cc
Code-Review: +1 by Dale Curtis
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |