Update initiator state token on policy update [chromium/src : main]
Camille Lamy (Gerrit)
Camille Lamy added 3 comments![]( "Open in Gerrit")
// such changes and update the `initiator_state_token_to_commit`.It seems the DocumentLoader in Blink might attempt to modify the CSP in the PolicyContainerHost in DocumentLoader::CreateCSP called from DocumentLoader::InitializeWindow. I am not sure we have actually committed the navigation at that point, which is the time where the PolicyContainerHost gets moved into the RenderFrameHostImpl and the RFH sets itself as the PolicyContainerHost::Client. To avoid an update of the initiator_state_token getting missed during this timeframe, I am having the NavigationRequest be the client until we pass the PolicyContainerHost to the RFH. The NavigationRequest also stores the initiator_state_token_to_commit_, so it can just update it if needed, passing the PCH with the right policies and matching token to RFH at commit time.
CHECK(!client_);I had to remove those checks because the NavigationRequest now sets itself as the client of the PolicyContainerHost prior to those being called. See comment in NavigationRequest for why. Since we're still requesting the NavigationPolicyContainerBuilder as a PassKey this is fine I think, as that means the PCH is not assigned to an RFH yet, so modifying browser process-only policies should be fine.
Related details
- Alex Moshchuk
- Mike West
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. |
Alex Moshchuk (Gerrit)
Alex Moshchuk voted and added 4 comments![]( "Open in Gerrit")
Votes added by Alex Moshchuk
| Code-Review | +1 |
4 comments
Thanks! This CL LGTM for content/.
At some point, I do want to come back to the question at https://chromium-review.git.corp.google.com/c/chromium/src/+/8024863/comment/2c8923c1_34286659/ and specifically how to avoid trusting the renderer-provided initiator state tokens. Maybe we can scope them to a process somehow, or combine with the RFH's document token, so that if an attacker process A somehow learns about a token belonging to process B (via a side channel, etc), it can't start navigations with a token that maps to initiator state belonging to some random cross-site frame?
// such changes and update the `initiator_state_token_to_commit`.It seems the DocumentLoader in Blink might attempt to modify the CSP in the PolicyContainerHost in DocumentLoader::CreateCSP called from DocumentLoader::InitializeWindow. I am not sure we have actually committed the navigation at that point, which is the time where the PolicyContainerHost gets moved into the RenderFrameHostImpl and the RFH sets itself as the PolicyContainerHost::Client. To avoid an update of the initiator_state_token getting missed during this timeframe, I am having the NavigationRequest be the client until we pass the PolicyContainerHost to the RFH. The NavigationRequest also stores the initiator_state_token_to_commit_, so it can just update it if needed, passing the PCH with the right policies and matching token to RFH at commit time.
Good catch, I think capturing updates in NavigationRequest makes sense.
CHECK(!client_);I had to remove those checks because the NavigationRequest now sets itself as the client of the PolicyContainerHost prior to those being called. See comment in NavigationRequest for why. Since we're still requesting the NavigationPolicyContainerBuilder as a PassKey this is fine I think, as that means the PCH is not assigned to an RFH yet, so modifying browser process-only policies should be fine.
Acknowledged
Related details
- Camille Lamy
- Mike West
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. |