[mime-handler] Fix is_web_secure_context for MIME-handler OOPIF [chromium/src : main]
Maksim Sisov (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Alex Moshchuk
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov added 1 comment![]( "Open in Gerrit")
Hi Alex. This fixes service workers for mime handler's iframes and other features such as window.crypto.
Shortly speaking, the top level embedder mustn't result in the decision whether the mime handler's children are secure or not. At the moment, there seem to be no such a precedence. In case of normal extensions, they are toplevel and are always treated secure, which is no the case of the mime handler, which is typically a 1+x frame in the frame tree. And if anything above it is insecure, all the children are treated in secure, which results in failed SW registration or failing access to window.crypto.
Related details
- Alex Moshchuk
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov added 1 comment![]( "Open in Gerrit")
Hi Alex. This fixes service workers for mime handler's iframes and other features such as window.crypto.
Shortly speaking, the top level embedder mustn't result in the decision whether the mime handler's children are secure or not. At the moment, there seem to be no such a precedence. In case of normal extensions, they are toplevel and are always treated secure, which is no the case of the mime handler, which is typically a 1+x frame in the frame tree. And if anything above it is insecure, all the children are treated in secure, which results in failed SW registration or failing access to window.crypto.
s/this fixes/this is one of the patches that help to fix (another one is in the chain)
Related details
- Alex Moshchuk
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Maksim Sisov
| Commit-Queue | +1 |
1 comment
Nate, Alex, I decided to merge the 2 CLs above this is one into this CL as they become disconnected from each other and just add new stuff that is not really used. The production code is ~300 lines with the rest being tests. I hope it's fine
Related details
- Alex Moshchuk
- Chromium IPC Reviews
- Nate Chapin
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: d...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): d...@chromium.org
Reviewer source(s):
d...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Alex Moshchuk
- Dominic Farolino
- Nate Chapin
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nate Chapin (Gerrit)
Nate Chapin added 1 comment![]( "Open in Gerrit")
blink plumbing looks fine if other reviewers are ok with the overall design.
Related details
- Alex Moshchuk
- Dominic Farolino
- Maksim Sisov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Moshchuk (Gerrit)
Alex Moshchuk added 2 comments![]( "Open in Gerrit")
HTTPS frames inside a chrome-extension MIME-handler OOPIF incorrectly
report non-secure context when the embedder page is HTTP, blocking SWOn a high level the plumbing here is reasonable, but before doing a detailed review I wanted to double check that this is something that's worth supporting with the extra complexity. Are there compelling use cases here? I.e. is the SW support absolutely needed for Acrobat, etc? And they can't reasonably fix this on their side?
I do kind of see the point that we already say that all extensions are secure contexts regardless of where they're embedded (via secure_origin_allowlist.cc), and so their HTTPS subframes should be as well. But so far we seem to have gotten away with the current rules for normal extensions in these http(extension(https)) situations.
std::unique_ptr<PolicyContainerPolicies> parent_policies_override =This seems a bit suspicious to me, as it allows providing a full "fake" parent policy to just override one bit, which kind of violates least privilege. Could we explore alternatives to override just the bit that matters, either here or (probably better) by passing it to `ComputePolicies`?
Related details
- Dominic Farolino
- Maksim Sisov
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. |