Commit ebe5bdf8 authored by Peter E Conn's avatar Peter E Conn Committed by Commit Bot

🤝 Use Origin class for canonically representing origins.

Constructing origins manually and storing them in Urls is error-prone
(eg the port could be missed out) and there are discrepancies in how
the origin could be formed (eg GURLUtils has a trailing '/' while
current manually formed origins don't). This is especially important as
origins are critical to security.

This CL creats an Origin class that offloads all the heavy lifting to
the well tested native origin class and allows the type system to
prevent the errors listed above.

Bug: 800422
Change-Id: I87759ea12a87f2b57bbdf40994d35ec468a43cff
Reviewed-on: https://chromium-review.googlesource.com/934289
Commit-Queue: Peter Conn <peconn@chromium.org>
Reviewed-by: default avatarBernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540825}
parent ad2e75c1
......@@ -93,7 +93,7 @@ public class BrowserSessionContentUtils {
CustomTabsConnection.getInstance().getClientPackageNameForSession(session);
if (TextUtils.isEmpty(packageName)) return false;
boolean valid = OriginVerifier.isValidOrigin(
packageName, referrer, CustomTabsService.RELATION_USE_AS_ORIGIN);
packageName, new Origin(referrer), CustomTabsService.RELATION_USE_AS_ORIGIN);
// OriginVerifier should only be allowing https schemes.
assert valid == UrlConstants.HTTPS_SCHEME.equals(referrer.getScheme());
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.browserservices;
import android.net.Uri;
import org.chromium.net.GURLUtils;
/**
* A class to canonically represent a web origin in Java. It requires the native library to be
* loaded as it uses {@link GURLUtils#getOrigin}.
*/
public class Origin {
private final String mOrigin;
/**
* Constructs a canonical Origin from a String.
*/
public Origin(String uri) {
mOrigin = GURLUtils.getOrigin(uri);
}
/**
* Constructs a canonical Origin from an Uri.
*/
public Origin(Uri uri) {
this(uri.toString());
}
/**
* Returns a Uri representing this Origin.
*/
public Uri uri() {
return Uri.parse(mOrigin);
}
@Override
public int hashCode() {
return mOrigin.hashCode();
}
@Override
public String toString() {
return mOrigin;
}
@Override
public boolean equals(Object other) {
if (this == other) return true;
if (other == null || getClass() != other.getClass()) return false;
return mOrigin.equals(((Origin) other).mOrigin);
}
}
......@@ -57,13 +57,13 @@ public class OriginVerifier {
private static final String USE_AS_ORIGIN = "delegate_permission/common.use_as_origin";
private static final String HANDLE_ALL_URLS = "delegate_permission/common.handle_all_urls";
private static Map<Pair<String, Integer>, Set<Uri>> sPackageToCachedOrigins;
private static Map<Pair<String, Integer>, Set<Origin>> sPackageToCachedOrigins;
private final OriginVerificationListener mListener;
private final String mPackageName;
private final String mSignatureFingerprint;
private final @Relation int mRelation;
private long mNativeOriginVerifier = 0;
private Uri mOrigin;
private Origin mOrigin;
/** Small helper class to post a result of origin verification. */
private class VerifiedCallback implements Runnable {
......@@ -79,9 +79,10 @@ public class OriginVerifier {
}
}
static Uri getPostMessageOriginFromVerifiedOrigin(String packageName, Uri verifiedOrigin) {
public static Uri getPostMessageUriFromVerifiedOrigin(String packageName,
Origin verifiedOrigin) {
return Uri.parse(IntentHandler.ANDROID_APP_REFERRER_SCHEME + "://"
+ verifiedOrigin.getHost() + "/" + packageName);
+ verifiedOrigin.uri().getHost() + "/" + packageName);
}
/** Clears all known relations. */
......@@ -98,15 +99,14 @@ public class OriginVerifier {
* @param relation The Digital Asset Links relation verified.
*/
public static void addVerifiedOriginForPackage(
String packageName, Uri origin, @Relation int relation) {
String packageName, Origin origin, @Relation int relation) {
ThreadUtils.assertOnUiThread();
if (sPackageToCachedOrigins == null) sPackageToCachedOrigins = new HashMap<>();
Set<Uri> cachedOrigins =
sPackageToCachedOrigins.get(new Pair<String, Integer>(packageName, relation));
Set<Origin> cachedOrigins =
sPackageToCachedOrigins.get(new Pair<>(packageName, relation));
if (cachedOrigins == null) {
cachedOrigins = new HashSet<Uri>();
sPackageToCachedOrigins.put(
new Pair<String, Integer>(packageName, relation), cachedOrigins);
cachedOrigins = new HashSet<>();
sPackageToCachedOrigins.put(new Pair<>(packageName, relation), cachedOrigins);
}
cachedOrigins.add(origin);
}
......@@ -121,11 +121,10 @@ public class OriginVerifier {
* @param origin The origin to verify
* @param relation The Digital Asset Links relation to verify for.
*/
public static boolean isValidOrigin(String packageName, Uri origin, @Relation int relation) {
public static boolean isValidOrigin(String packageName, Origin origin, @Relation int relation) {
ThreadUtils.assertOnUiThread();
if (sPackageToCachedOrigins == null) return false;
Set<Uri> cachedOrigins =
sPackageToCachedOrigins.get(new Pair<String, Integer>(packageName, relation));
Set<Origin> cachedOrigins = sPackageToCachedOrigins.get(new Pair<>(packageName, relation));
if (cachedOrigins == null) return false;
return cachedOrigins.contains(origin);
}
......@@ -140,12 +139,12 @@ public class OriginVerifier {
* @param origin The origin that was declared on the query for this result.
* @param verified Whether the given origin was verified to correspond to the given package.
*/
void onOriginVerified(String packageName, Uri origin, boolean verified);
void onOriginVerified(String packageName, Origin origin, boolean verified);
}
/**
* Main constructor.
* Use {@link OriginVerifier#start(Uri)}
* Use {@link OriginVerifier#start(Origin)}
* @param listener The listener who will get the verification result.
* @param packageName The package for the Android application for verification.
* @param relation Digital Asset Links {@link Relation} to use during verification.
......@@ -164,10 +163,10 @@ public class OriginVerifier {
* profile as context.
* @param origin The postMessage origin the application is claiming to have. Can't be null.
*/
public void start(@NonNull Uri origin) {
public void start(@NonNull Origin origin) {
ThreadUtils.assertOnUiThread();
mOrigin = origin;
String scheme = mOrigin.getScheme();
String scheme = mOrigin.uri().getScheme();
if (TextUtils.isEmpty(scheme)
|| !UrlConstants.HTTPS_SCHEME.equals(scheme.toLowerCase(Locale.US))) {
ThreadUtils.runOnUiThread(new VerifiedCallback(false));
......@@ -274,7 +273,6 @@ public class OriginVerifier {
private void originVerified(boolean originVerified) {
if (originVerified) {
addVerifiedOriginForPackage(mPackageName, mOrigin, mRelation);
mOrigin = getPostMessageOriginFromVerifiedOrigin(mPackageName, mOrigin);
}
if (mListener != null) mListener.onOriginVerified(mPackageName, mOrigin, originVerified);
cleanUp();
......
......@@ -32,7 +32,7 @@ public class PostMessageHandler
private boolean mMessageChannelCreated;
private boolean mBoundToService;
private AppWebMessagePort[] mChannel;
private Uri mOrigin;
private Uri mPostMessageUri;
private String mPackageName;
/**
......@@ -77,7 +77,7 @@ public class PostMessageHandler
// Can't reset with the same web contents twice.
if (webContents.equals(mWebContents)) return;
mWebContents = webContents;
if (mOrigin == null) return;
if (mPostMessageUri == null) return;
new WebContentsObserver(webContents) {
private boolean mNavigatedOnce;
......@@ -125,7 +125,7 @@ public class PostMessageHandler
mChannel[0].setMessageCallback(mMessageCallback, null);
webContents.postMessageToFrame(
null, "", mOrigin.toString(), "", new AppWebMessagePort[] {mChannel[1]});
null, "", mPostMessageUri.toString(), "", new AppWebMessagePort[] {mChannel[1]});
mMessageChannelCreated = true;
if (mBoundToService) notifyMessageChannelReady(null);
......@@ -139,11 +139,11 @@ public class PostMessageHandler
}
/**
* Sets the postMessage origin for this session to the given {@link Uri}.
* @param origin The origin value to be set.
* Sets the postMessage postMessageUri for this session to the given {@link Uri}.
* @param postMessageUri The postMessageUri value to be set.
*/
public void initializeWithOrigin(Uri origin) {
mOrigin = origin;
public void initializeWithPostMessageUri(Uri postMessageUri) {
mPostMessageUri = postMessageUri;
if (mWebContents != null && !mWebContents.isDestroyed()) {
initializeWithWebContents(mWebContents);
}
......@@ -191,17 +191,18 @@ public class PostMessageHandler
}
@Override
public void onOriginVerified(String packageName, Uri origin, boolean result) {
public void onOriginVerified(String packageName, Origin origin, boolean result) {
if (!result) return;
initializeWithOrigin(origin);
initializeWithPostMessageUri(
OriginVerifier.getPostMessageUriFromVerifiedOrigin(packageName, origin));
}
/**
* @return The origin that has been declared for this handler.
* @return The PostMessage Uri that has been declared for this handler.
*/
@VisibleForTesting
public Uri getOriginForTesting() {
return mOrigin;
public Uri getPostMessageUriForTesting() {
return mPostMessageUri;
}
/**
......
......@@ -27,6 +27,7 @@ import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeVersionInfo;
import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.browserservices.Origin;
import org.chromium.chrome.browser.browserservices.OriginVerifier;
import org.chromium.chrome.browser.browserservices.OriginVerifier.OriginVerificationListener;
import org.chromium.chrome.browser.browserservices.PostMessageHandler;
......@@ -359,13 +360,13 @@ class ClientManager {
}
/**
* See {@link PostMessageHandler#initializeWithOrigin(Uri)}.
* See {@link PostMessageHandler#initializeWithPostMessageUri(Uri)}.
*/
public synchronized void initializeWithPostMessageOriginForSession(
CustomTabsSessionToken session, Uri origin) {
SessionParams params = mSessionParams.get(session);
if (params == null) return;
params.postMessageHandler.initializeWithOrigin(origin);
params.postMessageHandler.initializeWithPostMessageUri(origin);
}
public synchronized boolean validateRelationship(
......@@ -374,7 +375,7 @@ class ClientManager {
}
/**
* See {@link PostMessageHandler#verifyAndInitializeWithOrigin(Uri, int)}.
* Validates the link between the client and the origin.
*/
public synchronized void verifyAndInitializeWithPostMessageOriginForSession(
CustomTabsSessionToken session, Uri origin, @Relation int relation) {
......@@ -391,7 +392,7 @@ class ClientManager {
OriginVerificationListener listener = null;
if (initializePostMessageChannel) listener = params.postMessageHandler;
params.originVerifier = new OriginVerifier(listener, params.getPackageName(), relation);
ThreadUtils.runOnUiThread(() -> { params.originVerifier.start(origin); });
ThreadUtils.runOnUiThread(() -> { params.originVerifier.start(new Origin(origin)); });
if (relation == CustomTabsService.RELATION_HANDLE_ALL_URLS
&& InstalledAppProviderImpl.isAppInstalledAndAssociatedWithOrigin(
params.getPackageName(), URI.create(origin.toString()),
......@@ -424,7 +425,7 @@ class ClientManager {
if (!isAppAssociatedWithOrigin) return false;
// Split path from the given Uri to get only the origin before web->native verification.
Uri origin = new Uri.Builder().scheme(url.getScheme()).authority(url.getHost()).build();
Origin origin = new Origin(url);
if (OriginVerifier.isValidOrigin(
packageName, origin, CustomTabsService.RELATION_HANDLE_ALL_URLS)) {
return true;
......@@ -444,7 +445,7 @@ class ClientManager {
synchronized Uri getPostMessageOriginForSessionForTesting(CustomTabsSessionToken session) {
SessionParams params = mSessionParams.get(session);
if (params == null) return null;
return params.postMessageHandler.getOriginForTesting();
return params.postMessageHandler.getPostMessageUriForTesting();
}
/**
......@@ -614,8 +615,8 @@ class ClientManager {
*/
public synchronized boolean isFirstPartyOriginForSession(
CustomTabsSessionToken session, Uri origin) {
return OriginVerifier.isValidOrigin(getClientPackageNameForSession(session), origin,
CustomTabsService.RELATION_USE_AS_ORIGIN);
return OriginVerifier.isValidOrigin(getClientPackageNameForSession(session),
new Origin(origin), CustomTabsService.RELATION_USE_AS_ORIGIN);
}
/** Tries to bind to a client to keep it alive, and returns true for success. */
......
......@@ -45,6 +45,7 @@ import org.chromium.chrome.browser.appmenu.AppMenuPropertiesDelegate;
import org.chromium.chrome.browser.browserservices.BrowserSessionContentHandler;
import org.chromium.chrome.browser.browserservices.BrowserSessionContentUtils;
import org.chromium.chrome.browser.browserservices.BrowserSessionDataProvider;
import org.chromium.chrome.browser.browserservices.Origin;
import org.chromium.chrome.browser.browserservices.OriginVerifier;
import org.chromium.chrome.browser.browserservices.OriginVerifier.OriginVerificationListener;
import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
......@@ -175,18 +176,12 @@ public class WebappActivity extends SingleTabActivity {
void verifyRelationship() {
mOriginVerifier = new OriginVerifier(mTrustedWebContentProvider,
getNativeClientPackageName(), CustomTabsService.RELATION_HANDLE_ALL_URLS);
// Split path from the url to get only the origin.
Uri origin = new Uri.Builder()
.scheme(mWebappInfo.uri().getScheme())
.authority(mWebappInfo.uri().getHost())
.build();
mOriginVerifier.start(origin);
mOriginVerifier.start(new Origin(mWebappInfo.uri()));
}
@Override
public void onOriginVerified(String packageName, Uri origin, boolean verified) {
public void onOriginVerified(String packageName, Origin origin, boolean verified) {
mVerificationFailed = !verified;
mOriginVerifier.cleanUp();
mOriginVerifier = null;
if (mVerificationFailed) getFullscreenManager().setPositionsForTabToNonFullscreen();
}
......
......@@ -140,6 +140,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentUtils.java",
"java/src/org/chromium/chrome/browser/browserservices/BrowserSessionDataProvider.java",
"java/src/org/chromium/chrome/browser/browserservices/OriginVerifier.java",
"java/src/org/chromium/chrome/browser/browserservices/Origin.java",
"java/src/org/chromium/chrome/browser/browserservices/PostMessageHandler.java",
"java/src/org/chromium/chrome/browser/browsing_data/UrlFilters.java",
"java/src/org/chromium/chrome/browser/childaccounts/ChildAccountFeedbackReporter.java",
......@@ -1519,6 +1520,7 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkTest.java",
"javatests/src/org/chromium/chrome/browser/browseractions/BrowserActionActivityTest.java",
"javatests/src/org/chromium/chrome/browser/browserservices/OriginVerifierTest.java",
"javatests/src/org/chromium/chrome/browser/browserservices/OriginTest.java",
"javatests/src/org/chromium/chrome/browser/browsing_data/BrowsingDataRemoverIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelBaseTest.java",
"javatests/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelEventFilterTest.java",
......
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.browserservices;
import android.net.Uri;
import android.support.test.filters.SmallTest;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.content.browser.test.NativeLibraryTestRule;
/**
* Tests for org.chromium.net.Origin.
*/
@RunWith(BaseJUnit4ClassRunner.class)
public class OriginTest {
@Rule
public NativeLibraryTestRule mNativeLibraryTestRule = new NativeLibraryTestRule();
@Before
public void setUp() {
mNativeLibraryTestRule.loadNativeLibraryNoBrowserProcess();
}
/**
* The actual conversion from a free form URL to an Origin is done in native and that is
* thoroughly tested there. Here we only check that the transformation is performed, not that
* it is complete and correct.
*/
@Test
@SmallTest
public void testConstruction() {
Origin origin = new Origin("http://www.example.com/path/to/page.html");
Assert.assertEquals("http://www.example.com/", origin.toString());
}
@Test
@SmallTest
public void testEquality() {
Origin origin1 = new Origin("http://www.example.com/page1.html");
Origin origin2 = new Origin("http://www.example.com/page2.html");
Assert.assertEquals(origin1, origin2);
Assert.assertEquals(origin1.hashCode(), origin2.hashCode());
// Note http*s*.
Origin origin3 = new Origin("https://www.example.com/page3.html");
Assert.assertNotEquals(origin1, origin3);
}
@Test
@SmallTest
public void testToUri() {
Origin origin = new Origin(Uri.parse("http://www.example.com/page.html"));
Uri uri = Uri.parse("http://www.example.com/");
Assert.assertEquals(uri, origin.uri());
}
@Test
@SmallTest
public void testToString() {
Origin origin = new Origin("http://www.example.com/page.html");
Assert.assertEquals("http://www.example.com/", origin.toString());
}
}
......@@ -40,13 +40,13 @@ public class OriginVerifierTest {
private static final String SHA_256_FINGERPRINT = ChromeVersionInfo.isOfficialBuild()
? SHA_256_FINGERPRINT_OFFICIAL
: SHA_256_FINGERPRINT_PUBLIC;
private static final Uri TEST_HTTPS_ORIGIN_1 = Uri.parse("https://www.example.com");
private static final Uri TEST_HTTPS_ORIGIN_2 = Uri.parse("https://www.android.com");
private static final Uri TEST_HTTP_ORIGIN = Uri.parse("http://www.android.com");
private static final Origin TEST_HTTPS_ORIGIN_1 = new Origin("https://www.example.com");
private static final Origin TEST_HTTPS_ORIGIN_2 = new Origin("https://www.android.com");
private static final Origin TEST_HTTP_ORIGIN = new Origin("http://www.android.com");
private class TestOriginVerificationListener implements OriginVerificationListener {
@Override
public void onOriginVerified(String packageName, Uri origin, boolean verified) {
public void onOriginVerified(String packageName, Origin origin, boolean verified) {
mLastPackageName = packageName;
mLastOrigin = origin;
mLastVerified = verified;
......@@ -58,7 +58,7 @@ public class OriginVerifierTest {
private OriginVerifier mUseAsOriginVerifier;
private OriginVerifier mHandleAllUrlsVerifier;
private volatile String mLastPackageName;
private volatile Uri mLastOrigin;
private volatile Origin mLastOrigin;
private volatile boolean mLastVerified;
@Before
......@@ -81,7 +81,8 @@ public class OriginVerifierTest {
@Test
@SmallTest
public void testOnlyHttpsAllowed() throws InterruptedException {
ThreadUtils.postOnUiThread(() -> mHandleAllUrlsVerifier.start(Uri.parse("LOL")));
ThreadUtils.postOnUiThread(()
-> mHandleAllUrlsVerifier.start(new Origin(Uri.parse("LOL"))));
Assert.assertTrue(
mVerificationResultSemaphore.tryAcquire(TIMEOUT_MS, TimeUnit.MILLISECONDS));
Assert.assertFalse(mLastVerified);
......@@ -117,8 +118,6 @@ public class OriginVerifierTest {
}
}));
Assert.assertEquals(mLastPackageName, PACKAGE_NAME);
Assert.assertEquals(mLastOrigin,
OriginVerifier.getPostMessageOriginFromVerifiedOrigin(
PACKAGE_NAME, TEST_HTTPS_ORIGIN_1));
Assert.assertEquals(mLastOrigin, TEST_HTTPS_ORIGIN_1);
}
}
......@@ -24,6 +24,7 @@ import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.MetricsUtils;
import org.chromium.base.test.util.RetryOnFailure;
import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.browserservices.Origin;
import org.chromium.chrome.browser.browserservices.OriginVerifier;
import org.chromium.chrome.browser.browserservices.PostMessageHandler;
import org.chromium.content.browser.test.NativeLibraryTestRule;
......@@ -185,7 +186,7 @@ public class ClientManagerTest {
// If there is a prepopulated origin, we should get a synchronous verification.
OriginVerifier.addVerifiedOriginForPackage(
ContextUtils.getApplicationContext().getPackageName(), Uri.parse(URL),
ContextUtils.getApplicationContext().getPackageName(), new Origin(URL),
CustomTabsService.RELATION_USE_AS_ORIGIN);
cm.verifyAndInitializeWithPostMessageOriginForSession(
mSession, Uri.parse(URL), CustomTabsService.RELATION_USE_AS_ORIGIN);
......@@ -229,7 +230,7 @@ public class ClientManagerTest {
ThreadUtils.runOnUiThreadBlocking(() -> {
// Prepopulated origins should depend on the relation used.
OriginVerifier.addVerifiedOriginForPackage(
ContextUtils.getApplicationContext().getPackageName(), Uri.parse(URL),
ContextUtils.getApplicationContext().getPackageName(), new Origin(URL),
CustomTabsService.RELATION_HANDLE_ALL_URLS);
// This uses CustomTabsService.RELATION_USE_AS_ORIGIN by default.
Assert.assertFalse(cm.isFirstPartyOriginForSession(mSession, Uri.parse(URL)));
......@@ -269,7 +270,7 @@ public class ClientManagerTest {
// Even if there is a prepopulated origin, non-https origins should get an early
// return with false.
OriginVerifier.addVerifiedOriginForPackage(
ContextUtils.getApplicationContext().getPackageName(), uri,
ContextUtils.getApplicationContext().getPackageName(), new Origin(uri),
CustomTabsService.RELATION_USE_AS_ORIGIN);
cm.verifyAndInitializeWithPostMessageOriginForSession(
mSession, uri, CustomTabsService.RELATION_USE_AS_ORIGIN);
......
......@@ -87,6 +87,7 @@ import org.chromium.chrome.browser.TabsOpenedFromExternalAppTest;
import org.chromium.chrome.browser.WarmupManager;
import org.chromium.chrome.browser.appmenu.AppMenuHandler;
import org.chromium.chrome.browser.browserservices.BrowserSessionContentUtils;
import org.chromium.chrome.browser.browserservices.Origin;
import org.chromium.chrome.browser.browserservices.OriginVerifier;
import org.chromium.chrome.browser.document.ChromeLauncherActivity;
import org.chromium.chrome.browser.firstrun.FirstRunStatus;
......@@ -1176,7 +1177,7 @@ public class CustomTabActivityTest {
connection.newSession(token);
connection.overridePackageNameForSessionForTesting(token, "app1");
ThreadUtils.runOnUiThreadBlocking(
() -> OriginVerifier.addVerifiedOriginForPackage("app1", Uri.parse(referrer),
() -> OriginVerifier.addVerifiedOriginForPackage("app1", new Origin(referrer),
CustomTabsService.RELATION_USE_AS_ORIGIN));
final CustomTabsSessionToken session = warmUpAndLaunchUrlWithSession(intent);
......
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