Extending security interstitials to support subframe blocking

[Nohemi Fernandez]

Hi folks, 

Our team has been investigating extending the security interstitials framework to support displaying interstitials and handling mojom commands on subframes. 

I identified that there were two main blockers to this work, the first was supporting mojom commands for subframes in chrome/renderer for which I'd sent out crrev.com/c/5844948 and the second is to support multi-frame blocking in SecurityInterstitialTabHelper which currently operates under the assumption that there is a single interstitial per WebContents (I've attached a small diagram of the current to make the existing solution more visual). 

For this second task, I have successfully prototyped two options, which work at the surface level. The first based on the assumption that FrameTreeNodeIds are unique and tied to a single navigation ID (patchset#2) and the second that there is a single navigation ID associated with the WebContents and that changes to this invalidate the blocking functionality (patchset#4). However, it is less clear to me whether there are additional assumptions I need to keep in mind for edge case handling that may not be addressed by the two proposed solutions (e.g. handling redirects, navigations cancelled when interstitials are partially rendered, etc). 

I'm reaching out to navigation-dev@ to have a second opinion on my current approaches and to better understand the navigation and rendering test cases I should keep in mind when designing this solution. I really appreciate your time on this!

[Nate Fischer]

> the second is to support multi-frame blocking in SecurityInterstitialTabHelper which currently operates under the assumption that there is a single interstitial per WebContents

I tried this out today in WebView and it seems like multi-frame blocking is working correctly (as far as I can tell). I've attached a screenshot to show a single WebView which has two blocked iframes (based on this jsbin demo). This is running on an M130 official build, so I have not patched in any additional changes. I'm curious why this works in WebView but not in Chrome. Let me know if my test page is too simplistic and I'm overlooking an edge case though.

Nate Fischer | Software Engineer | ntf...@google.com

[Nohemi Fernandez]

For context on this thread, we are moving forward with a frame ID based solution that tracks Render Frames and their associated interstitial. Thanks to @Arthur Sonzogni for clarifying the expectations for navigation IDs. 

With respect to @Nate Fischer's testing on WebView, we've identified that the interstitial UI is already shown in both the WebView and Chrome browser cases, and this change will allow for commands to be matched to their interstitial origin (see conversation in crrev.com/c/5853475).   

[Charlie Reis]

Apologies for the delay!  We discussed this in the Chrome Security Architecture meeting yesterday (which happens each Tuesday) and we have some questions about your approach, to make sure there aren't other problems you'll run into.  At a high level, it seems great to add subframe support, but it's worth making sure the design will work.

1) Do you have a design doc for this work?  We found it tricky to piece together details of your plan from the CLs and bug, and a doc describing what support is needed and what changes are planned would help.

2) At a glance, it appears that SecurityInterstitialTabHelper is a WebContentsObserver and assumes there's only one interstitial per WebContents.  Are you planning to have one helper per RenderFrameHost, or re-architect the existing helper to manage multiple interstitials across RenderFrameHosts?

3) How does your use case compare with blocked ads (e.g., the heavy ad intervention), which show error pages in subframes?  We're guessing you need to be able to click a button to proceed with the navigation?  Presumably that's still a committed interstitial and the navigation has to be started again from scratch to proceed?

4) Regarding the assumptions you mentioned:

The first based on the assumption that FrameTreeNodeIds are unique and tied to a single navigation ID (patchset#2)

FrameTreedNodeIds are indeed unique, but they are not tied to a single navigation ID.  Multiple NavigationRequests can exist at the same time for a given FTN, such as with navigation queueing (when we're waiting between Commit and the DidCommit IPC from the renderer).

 

and the second that there is a single navigation ID associated with the WebContents and that changes to this invalidate the blocking functionality (patchset#4).

I might be misunderstanding what you mean, but there is not a single navigation ID or NavigationRequest per WebContents.  There can be multiple at once, both across FrameTreeNodes and within a single one.

Maybe it's possible to share more of what you discussed with Arthur, to help us understand where you're headed?

5) It sounds like this is mostly being tracked in Buganizer (i.e., not the Chromium issue tracker), which is fine for the parts relevant to your project.  Can you file a public Chromium tracker issue for adding subframe support to interstitials, though, to help Chrome folks follow along?

Thanks!

Charlie

--
You received this message because you are subscribed to the Google Groups "navigation-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to navigation-de...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/CANMaTomgO1MNauydpzcjyVPSjBNa8Q6d_ittdMT2N93t00Bt0A%40mail.gmail.com.

[Nohemi Fernandez]

Thanks Charlie for coming back to us on this thread. 

As discussed offline I would appreciate it if we could target M131 landing for this proposed change as this infrastructure restriction is blocking a larger set of changes both across WebView and Chrome. 

Please see CIL.

On Wed, Sep 18, 2024 at 5:32 PM Charlie Reis <cr...@chromium.org> wrote:

Apologies for the delay!  We discussed this in the Chrome Security Architecture meeting yesterday (which happens each Tuesday) and we have some questions about your approach, to make sure there aren't other problems you'll run into.  At a high level, it seems great to add subframe support, but it's worth making sure the design will work.

1) Do you have a design doc for this work?  We found it tricky to piece together details of your plan from the CLs and bug, and a doc describing what support is needed and what changes are planned would help.

I have shared a 1-pager for the current approach with the Chrome Security Architecture team. When I originally wrote the message I was not sure what support was needed since it was difficult to find the experts in this space. I think a review of the proposal and its associated change crrev.com/c/5853475 would be very useful for us. 

2) At a glance, it appears that SecurityInterstitialTabHelper is a WebContentsObserver and assumes there's only one interstitial per WebContents.  Are you planning to have one helper per RenderFrameHost, or re-architect the existing helper to manage multiple interstitials across RenderFrameHosts?

The plan is to re-architect the SecurityInterstitialTabHelper to manage multiple interstitials. 

3) How does your use case compare with blocked ads (e.g., the heavy ad intervention), which show error pages in subframes?  We're guessing you need to be able to click a button to proceed with the navigation?  Presumably that's still a committed interstitial and the navigation has to be started again from scratch to proceed?

The main goal of the change is to support a mapping between commands and error page UI. In the current infrastructure any page that has >1 interstitials will have their commands "disconnected" from the error page UI. 

I am not familiar with heavy ad intervention. I do need to click on a button, but this makes use of the open login command rather than altering the state of the navigation so it seems like our needs may be different (see internal slidedeck).

4) Regarding the assumptions you mentioned:

The first based on the assumption that FrameTreeNodeIds are unique and tied to a single navigation ID (patchset#2)

FrameTreedNodeIds are indeed unique, but they are not tied to a single navigation ID.  Multiple NavigationRequests can exist at the same time for a given FTN, such as with navigation queueing (when we're waiting between Commit and the DidCommit IPC from the renderer).

 

and the second that there is a single navigation ID associated with the WebContents and that changes to this invalidate the blocking functionality (patchset#4).

I might be misunderstanding what you mean, but there is not a single navigation ID or NavigationRequest per WebContents.  There can be multiple at once, both across FrameTreeNodes and within a single one.

Maybe it's possible to share more of what you discussed with Arthur, to help us understand where you're headed?

The helpful information I received from Arthur:

• FrameTreeNodeIds are unique 
• NavigationIds are unique and incrementing
• Redirects are treated as the same navigation ID
• Reloads are treated as different navigation IDs

5) It sounds like this is mostly being tracked in Buganizer (i.e., not the Chromium issue tracker), which is fine for the parts relevant to your project.  Can you file a public Chromium tracker issue for adding subframe support to interstitials, though, to help Chrome folks follow along?

The relevant Chromium bug is crbug/367439553. 

[Charlie Reis]

Thanks!  I'm looking through the doc and CL and will send any questions I have.

Charlie