Commit 09d03ac9 authored by mrefaat's avatar mrefaat Committed by Commit Bot

[IOS] Fix U2F scheme handling

After CRWWebDelegate removal & AppLauncher refactoring, U2F schemes are
not handeled by the app launcher correctly.
The reason is that the check for valid URL was happening after the x-callback
URL conversion by the U2F delegate. This CL moves that check to happen before
the conversion and add some simple tests on the applauncher tab helper to make
sure that errors in that part is caught.

Bug: 897329
Change-Id: I45445248a8d676528b362e0cae8f2ddec4ffbe3b
Reviewed-on: https://chromium-review.googlesource.com/c/1297293
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarPeter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602401}
parent 146ffa8e
......@@ -41,6 +41,7 @@ bool IsValidAppUrl(const GURL& app_url) {
// If the url is a direct FIDO U2F x-callback call, consider it as invalid, to
// prevent pages from spoofing requests with different origins.
// See https://crbug.com/897329#c2 for details on how U2F works.
if (app_url.SchemeIs("u2f-x-callback"))
return false;
......@@ -189,6 +190,9 @@ bool AppLauncherTabHelper::ShouldAllowRequest(
if (request_status == ExternalURLRequestStatus::kSubFrameRequestBlocked)
return false;
if (!IsValidAppUrl(request_url))
return false;
Tab* tab = LegacyTabHelper::GetTabForWebState(web_state_);
// If this is a Universal 2nd Factor (U2F) call, the origin needs to be
......@@ -200,6 +204,9 @@ bool AppLauncherTabHelper::ShouldAllowRequest(
->GetURL()
.GetOrigin();
request_url = [tab XCallbackFromRequestURL:request_url originURL:origin];
// If the URL was rejected by the U2F handler, |request_url| will be empty.
if (!request_url.is_valid())
return false;
}
GURL last_committed_url = web_state_->GetLastCommittedURL();
......@@ -220,14 +227,13 @@ bool AppLauncherTabHelper::ShouldAllowRequest(
if (model && model->loaded())
model->SetReadStatus(original_pending_url, true);
}
if (IsValidAppUrl(request_url) && last_committed_url.is_valid()) {
if (last_committed_url.is_valid()) {
RequestToLaunchApp(request_url, last_committed_url, is_link_transition);
}
return false;
}
if (IsValidAppUrl(request_url) &&
RequestToLaunchApp(request_url, last_committed_url, is_link_transition)) {
if (RequestToLaunchApp(request_url, last_committed_url, is_link_transition)) {
// Clears pending navigation history after successfully launching the
// external app.
web_state_->GetNavigationManager()->DiscardNonCommittedItems();
......
......@@ -299,10 +299,46 @@ TEST_F(AppLauncherTabHelperTest, InsecureUrls) {
EXPECT_FALSE(TestShouldAllowRequest(@"app-settings://",
/*target_frame_is_main=*/true,
/*has_user_gesture=*/false));
EXPECT_FALSE(TestShouldAllowRequest(@"u2f-x-callback://test",
EXPECT_EQ(0U, delegate_.countOfAppsLaunched);
}
// Tests that URLs with U2F schemes are handled correctly.
// This test is using https://chromeiostesting-dot-u2fdemo.appspot.com URL which
// is a whitelisted URL for the purpose of testing, but the test doesn't send
// any request to the server.
TEST_F(AppLauncherTabHelperTest, U2FUrls) {
// Add required tab helpers for the U2F check.
TabIdTabHelper::CreateForWebState(&web_state_);
LegacyTabHelper::CreateForWebState(&web_state_);
std::unique_ptr<web::NavigationItem> item = web::NavigationItem::Create();
// "u2f-x-callback" scheme should only be created by the browser. External
// URLs with that scheme should be blocked to prevent malicious sites from
// bypassing the browser origin/security check for u2f schemes.
item->SetURL(GURL("https://chromeiostesting-dot-u2fdemo.appspot.com"));
navigation_manager_->SetLastCommittedItem(item.get());
EXPECT_FALSE(TestShouldAllowRequest(@"u2f-x-callback://chromium.test",
/*target_frame_is_main=*/true,
/*has_user_gesture=*/false));
EXPECT_EQ(0U, delegate_.countOfAppsLaunched);
// Source URL is not trusted, so u2f scheme should not be allowed.
item->SetURL(GURL("https://chromium.test"));
navigation_manager_->SetLastCommittedItem(item.get());
EXPECT_FALSE(TestShouldAllowRequest(@"u2f://chromium.test",
/*target_frame_is_main=*/true,
/*has_user_gesture=*/false));
EXPECT_EQ(0U, delegate_.countOfAppsLaunched);
// Source URL is trusted, so u2f scheme should be allowed and an external app
// is launched via URL with u2f-x-callback scheme.
item->SetURL(GURL("https://chromeiostesting-dot-u2fdemo.appspot.com"));
navigation_manager_->SetLastCommittedItem(item.get());
EXPECT_FALSE(TestShouldAllowRequest(@"u2f://chromium.test",
/*target_frame_is_main=*/true,
/*has_user_gesture=*/false));
EXPECT_EQ(1U, delegate_.countOfAppsLaunched);
EXPECT_TRUE(delegate_.lastLaunchedAppURL.SchemeIs("u2f-x-callback"));
}
// Tests that URLs with Chrome Bundle schemes are blocked on iframes.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment