Commit bf8b8aa7 authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

weblayer: adds support for encryption of navigation/tab state

User expectations on Android is that if the browser goes into the
background and is killed by the OS, and the user switches back to
the app state is restored (Chrome does this now). This is true even
of incognito data.

To limit the potential of leaking private information when incognito
this patch adds encryption of the tab/navigation stack. Chrome on
Android does this as well.

The key used for encryption is randomly generated and stored by the
system. This way, if the system doesn't provide the data (because
the system has determined the app shouldn't restore state), WebLayer
won't be able to decrypt the data.

This means restoring state of incognito data has a different lifetime
then non-incognito data, but that is by design.

Again, this logic matches that of Chrome on Android.

BUG=1033924
TEST=weblayer_support_instrumentation_test_apk
     BrowserFragmentLifecycleTest.restoresPreviousSession

Change-Id: Iaeb721ae942efa1db19ee8366d1cfbf30747cd43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2018482
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735995}
parent 39829d65
......@@ -145,6 +145,7 @@ jumbo_static_library("weblayer_lib") {
"common/features.h",
"common/weblayer_paths.cc",
"common/weblayer_paths.h",
"public/browser.cc",
"public/browser.h",
"public/browser_observer.h",
"public/common/switches.cc",
......
......@@ -74,10 +74,7 @@ public class BrowserFragmentLifecycleTest {
latch.await();
}
@Test
@SmallTest
public void restoresPreviousSession() throws InterruptedException {
Bundle extras = new Bundle();
private void restoresPreviousSession(Bundle extras) throws InterruptedException {
extras.putString(InstrumentationActivity.EXTRA_PERSISTENCE_ID, "x");
final String url = mActivityTestRule.getTestDataURL("simple_page.html");
InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(url, extras);
......@@ -108,4 +105,19 @@ public class BrowserFragmentLifecycleTest {
});
latch.await();
}
@Test
@SmallTest
public void restoresPreviousSession() throws InterruptedException {
restoresPreviousSession(new Bundle());
}
@Test
@SmallTest
public void restoresPreviousSessionIncognito() throws InterruptedException {
Bundle extras = new Bundle();
// This forces incognito.
extras.putString(InstrumentationActivity.EXTRA_PROFILE_NAME, null);
restoresPreviousSession(extras);
}
}
......@@ -17,6 +17,7 @@
#if defined(OS_ANDROID)
#include "base/android/callback_android.h"
#include "base/android/jni_array.h"
#include "base/android/jni_string.h"
#include "base/json/json_writer.h"
#include "weblayer/browser/java/jni/BrowserImpl_jni.h"
......@@ -24,26 +25,28 @@
namespace weblayer {
std::unique_ptr<Browser> Browser::Create(Profile* profile,
const std::string& persistence_id) {
std::unique_ptr<Browser> Browser::Create(
Profile* profile,
const PersistenceInfo* persistence_info) {
return std::make_unique<BrowserImpl>(static_cast<ProfileImpl*>(profile),
persistence_id);
persistence_info);
}
#if defined(OS_ANDROID)
BrowserImpl::BrowserImpl(ProfileImpl* profile,
const std::string& persistence_id,
const PersistenceInfo* persistence_info,
const base::android::JavaParamRef<jobject>& java_impl)
: BrowserImpl(profile, persistence_id) {
: BrowserImpl(profile, persistence_info) {
java_impl_ = java_impl;
}
#endif
BrowserImpl::BrowserImpl(ProfileImpl* profile,
const std::string& persistence_id)
: profile_(profile), persistence_id_(persistence_id) {
if (!persistence_id.empty())
CreateSessionServiceAndRestore();
const PersistenceInfo* persistence_info)
: profile_(profile),
persistence_id_(persistence_info ? persistence_info->id : std::string()) {
if (persistence_info)
CreateSessionServiceAndRestore(*persistence_info);
}
BrowserImpl::~BrowserImpl() {
......@@ -121,7 +124,23 @@ base::android::ScopedJavaLocalRef<jstring> BrowserImpl::GetPersistenceId(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller) {
return base::android::ScopedJavaLocalRef<jstring>(
base::android::ConvertUTF8ToJavaString(env, persistence_id_));
base::android::ConvertUTF8ToJavaString(env, GetPersistenceId()));
}
void BrowserImpl::SaveSessionServiceIfNecessary(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller) {
session_service_->SaveIfNecessary();
}
base::android::ScopedJavaLocalRef<jbyteArray>
BrowserImpl::GetSessionServiceCryptoKey(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller) {
std::vector<uint8_t> key;
if (session_service_)
key = session_service_->GetCryptoKey();
return base::android::ToJavaByteArray(env, key);
}
#endif
......@@ -197,7 +216,7 @@ void BrowserImpl::PrepareForShutdown() {
session_service_.reset();
}
const std::string& BrowserImpl::GetPersistenceId() {
std::string BrowserImpl::GetPersistenceId() {
return persistence_id_;
}
......@@ -217,27 +236,42 @@ base::FilePath BrowserImpl::GetSessionServiceDataPath() {
} else {
base_path = profile_->data_path().AppendASCII("Restore Data");
}
const std::string encoded_name = base32::Base32Encode(persistence_id_);
DCHECK(!GetPersistenceId().empty());
const std::string encoded_name = base32::Base32Encode(GetPersistenceId());
return base_path.AppendASCII("State" + encoded_name);
}
void BrowserImpl::CreateSessionServiceAndRestore() {
session_service_ =
std::make_unique<SessionService>(GetSessionServiceDataPath(), this);
void BrowserImpl::CreateSessionServiceAndRestore(
const PersistenceInfo& persistence_info) {
session_service_ = std::make_unique<SessionService>(
GetSessionServiceDataPath(), this, persistence_info.last_crypto_key);
}
#if defined(OS_ANDROID)
static jlong JNI_BrowserImpl_CreateBrowser(
JNIEnv* env,
jlong profile,
const base::android::JavaParamRef<jstring>& persistence_id,
const base::android::JavaParamRef<jstring>& j_persistence_id,
const base::android::JavaParamRef<jbyteArray>& j_persistence_crypto_key,
const base::android::JavaParamRef<jobject>& java_impl) {
return reinterpret_cast<intptr_t>(new BrowserImpl(
reinterpret_cast<ProfileImpl*>(profile),
persistence_id.obj()
? base::android::ConvertJavaStringToUTF8(persistence_id)
: std::string(),
java_impl));
Browser::PersistenceInfo persistence_info;
Browser::PersistenceInfo* persistence_info_ptr = nullptr;
if (j_persistence_id.obj()) {
const std::string persistence_id =
base::android::ConvertJavaStringToUTF8(j_persistence_id);
if (!persistence_id.empty()) {
persistence_info.id = persistence_id;
if (j_persistence_crypto_key.obj()) {
base::android::JavaByteArrayToByteVector(
env, j_persistence_crypto_key, &(persistence_info.last_crypto_key));
}
persistence_info_ptr = &persistence_info;
}
}
return reinterpret_cast<intptr_t>(
new BrowserImpl(reinterpret_cast<ProfileImpl*>(profile),
persistence_info_ptr, java_impl));
}
static void JNI_BrowserImpl_DeleteBrowser(JNIEnv* env, jlong browser) {
......
......@@ -34,10 +34,10 @@ class BrowserImpl : public Browser {
public:
#if defined(OS_ANDROID)
BrowserImpl(ProfileImpl* profile,
const std::string& persistence_id,
const PersistenceInfo* persistence_info,
const base::android::JavaParamRef<jobject>& java_impl);
#endif
BrowserImpl(ProfileImpl* profile, const std::string& persistence_id);
BrowserImpl(ProfileImpl* profile, const PersistenceInfo* persistence_info);
~BrowserImpl() override;
BrowserImpl(const BrowserImpl&) = delete;
BrowserImpl& operator=(const BrowserImpl&) = delete;
......@@ -70,6 +70,12 @@ class BrowserImpl : public Browser {
base::android::ScopedJavaLocalRef<jstring> GetPersistenceId(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller);
void SaveSessionServiceIfNecessary(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller);
base::android::ScopedJavaLocalRef<jbyteArray> GetSessionServiceCryptoKey(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller);
#endif
// Browser:
......@@ -79,12 +85,12 @@ class BrowserImpl : public Browser {
Tab* GetActiveTab() override;
const std::vector<Tab*>& GetTabs() override;
void PrepareForShutdown() override;
const std::string& GetPersistenceId() override;
std::string GetPersistenceId() override;
void AddObserver(BrowserObserver* observer) override;
void RemoveObserver(BrowserObserver* observer) override;
private:
void CreateSessionServiceAndRestore();
void CreateSessionServiceAndRestore(const PersistenceInfo& persistence_info);
// Returns the path used by |session_service_|.
base::FilePath GetSessionServiceDataPath();
......
......@@ -93,6 +93,13 @@ public class BrowserFragmentImpl extends RemoteFragmentImpl {
mContext = null;
}
@Override
public void onSaveInstanceState(Bundle outState) {
StrictModeWorkaround.apply();
mBrowser.onSaveInstanceState(outState);
super.onSaveInstanceState(outState);
}
public IBrowserFragment asIBrowserFragment() {
return new IBrowserFragment.Stub() {
@Override
......
......@@ -36,6 +36,9 @@ import java.util.List;
public class BrowserImpl extends IBrowser.Stub {
private static final ObserverList<Observer> sLifecycleObservers = new ObserverList<Observer>();
public static final String SAVED_STATE_SESSION_SERVICE_CRYPTO_KEY =
"SAVED_STATE_SESSION_SERVICE_CRYPTO_KEY";
private long mNativeBrowser;
private final ProfileImpl mProfile;
private BrowserViewController mViewController;
......@@ -64,8 +67,11 @@ public class BrowserImpl extends IBrowser.Stub {
public BrowserImpl(ProfileImpl profile, String persistenceId, Bundle savedInstanceState) {
mProfile = profile;
mNativeBrowser =
BrowserImplJni.get().createBrowser(profile.getNativeProfile(), persistenceId, this);
byte[] cryptoKey = savedInstanceState != null
? savedInstanceState.getByteArray(SAVED_STATE_SESSION_SERVICE_CRYPTO_KEY)
: null;
mNativeBrowser = BrowserImplJni.get().createBrowser(
profile.getNativeProfile(), persistenceId, cryptoKey, this);
for (Observer observer : sLifecycleObservers) {
observer.onBrowserCreated();
......@@ -104,6 +110,17 @@ public class BrowserImpl extends IBrowser.Stub {
updateAllTabs();
}
public void onSaveInstanceState(Bundle outState) {
if (mProfile.isIncognito()
&& !BrowserImplJni.get().getPersistenceId(mNativeBrowser, this).isEmpty()) {
// Trigger a save now as saving may generate a new crypto key. This doesn't actually
// save synchronously, rather triggers a save on a background task runner.
BrowserImplJni.get().saveSessionServiceIfNecessary(mNativeBrowser, this);
outState.putByteArray(SAVED_STATE_SESSION_SERVICE_CRYPTO_KEY,
BrowserImplJni.get().getSessionServiceCryptoKey(mNativeBrowser, this));
}
}
public void onActivityResult(int requestCode, int resultCode, Intent data) {
if (mWindowAndroid != null) {
mWindowAndroid.onActivityResult(requestCode, resultCode, data);
......@@ -286,7 +303,8 @@ public class BrowserImpl extends IBrowser.Stub {
@NativeMethods
interface Natives {
long createBrowser(long profile, String persistenceId, BrowserImpl caller);
long createBrowser(long profile, String persistenceId, byte[] persistenceCryptoKey,
BrowserImpl caller);
void deleteBrowser(long browser);
void addTab(long nativeBrowserImpl, BrowserImpl browser, long nativeTab);
void removeTab(long nativeBrowserImpl, BrowserImpl browser, long nativeTab);
......@@ -295,5 +313,7 @@ public class BrowserImpl extends IBrowser.Stub {
TabImpl getActiveTab(long nativeBrowserImpl, BrowserImpl browser);
void prepareForShutdown(long nativeBrowserImpl, BrowserImpl browser);
String getPersistenceId(long nativeBrowserImpl, BrowserImpl browser);
void saveSessionServiceIfNecessary(long nativeBrowserImpl, BrowserImpl browser);
byte[] getSessionServiceCryptoKey(long nativeBrowserImpl, BrowserImpl browser);
}
}
......@@ -51,6 +51,10 @@ public final class ProfileImpl extends IProfile.Stub {
return mName;
}
public boolean isIncognito() {
return mName.isEmpty();
}
@Override
public void clearBrowsingData(@NonNull @BrowsingDataType int[] dataTypes, long fromMillis,
long toMillis, @NonNull IObjectWrapper completionCallback) {
......
......@@ -59,23 +59,37 @@ constexpr int kWritesPerReset = 250;
// SessionService -------------------------------------------------------------
SessionService::SessionService(const base::FilePath& path, BrowserImpl* browser)
SessionService::SessionService(const base::FilePath& path,
BrowserImpl* browser,
const std::vector<uint8_t>& decryption_key)
: browser_(browser),
browser_session_id_(SessionID::NewUnique()),
command_storage_manager_(
std::make_unique<sessions::CommandStorageManager>(path, this)),
rebuild_on_next_save_(false) {
std::make_unique<sessions::CommandStorageManager>(
path,
this,
browser->profile()->GetBrowserContext()->IsOffTheRecord())),
rebuild_on_next_save_(false),
crypto_key_(decryption_key) {
browser_->AddObserver(this);
command_storage_manager_->ScheduleGetCurrentSessionCommands(
base::BindOnce(&SessionService::OnGotCurrentSessionCommands,
base::Unretained(this)),
std::vector<uint8_t>(), &cancelable_task_tracker_);
decryption_key, &cancelable_task_tracker_);
}
SessionService::~SessionService() {
SaveIfNecessary();
browser_->RemoveObserver(this);
}
void SessionService::SaveIfNecessary() {
if (command_storage_manager_->HasPendingSave())
command_storage_manager_->Save();
browser_->RemoveObserver(this);
}
const std::vector<uint8_t>& SessionService::GetCryptoKey() const {
return crypto_key_;
}
bool SessionService::ShouldUseDelayedSave() {
......@@ -93,6 +107,10 @@ void SessionService::OnWillSaveCommands() {
BuildCommandsForBrowser();
}
void SessionService::OnGeneratedNewCryptoKey(const std::vector<uint8_t>& key) {
crypto_key_ = key;
}
void SessionService::OnTabAdded(Tab* tab) {
content::WebContents* web_contents =
static_cast<TabImpl*>(tab)->web_contents();
......
......@@ -5,6 +5,8 @@
#ifndef WEBLAYER_BROWSER_SESSION_SERVICE_H_
#define WEBLAYER_BROWSER_SESSION_SERVICE_H_
#include <stddef.h>
#include <map>
#include <memory>
#include <string>
......@@ -39,13 +41,21 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
public sessions::SessionTabHelperDelegate,
public BrowserObserver {
public:
SessionService(const base::FilePath& path, BrowserImpl* browser);
SessionService(const base::FilePath& path,
BrowserImpl* browser,
const std::vector<uint8_t>& decryption_key);
SessionService(const SessionService&) = delete;
SessionService& operator=(const SessionService&) = delete;
~SessionService() override;
void SaveIfNecessary();
// Returns the key used to encrypt the file. Empty if not encrypted.
// Encryption is done when saving and the profile is off the record.
const std::vector<uint8_t>& GetCryptoKey() const;
private:
friend class SessionServiceTestHelper;
......@@ -54,6 +64,7 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
// CommandStorageManagerDelegate:
bool ShouldUseDelayedSave() override;
void OnWillSaveCommands() override;
void OnGeneratedNewCryptoKey(const std::vector<uint8_t>& key) override;
// BrowserObserver;
void OnTabAdded(Tab* tab) override;
......@@ -112,6 +123,8 @@ class SessionService : public sessions::CommandStorageManagerDelegate,
// Force session commands to be rebuild before next save event.
bool rebuild_on_next_save_;
std::vector<uint8_t> crypto_key_;
base::CancelableTaskTracker cancelable_task_tracker_;
};
......
......@@ -163,6 +163,13 @@ void ShutdownSessionServiceAndWait(BrowserImpl* browser) {
run_loop.Run();
}
std::unique_ptr<BrowserImpl> CreateBrowser(ProfileImpl* profile,
const std::string& persistence_id) {
Browser::PersistenceInfo info;
info.id = persistence_id;
return std::make_unique<BrowserImpl>(profile, &info);
}
} // namespace
using SessionServiceTest = WebLayerBrowserTest;
......@@ -170,8 +177,7 @@ using SessionServiceTest = WebLayerBrowserTest;
IN_PROC_BROWSER_TEST_F(SessionServiceTest, SingleTab) {
ASSERT_TRUE(embedded_test_server()->Start());
std::unique_ptr<BrowserImpl> browser =
std::make_unique<BrowserImpl>(GetProfile(), "x");
std::unique_ptr<BrowserImpl> browser = CreateBrowser(GetProfile(), "x");
std::unique_ptr<Tab> tab = Tab::Create(GetProfile());
browser->AddTab(tab.get());
const GURL url = embedded_test_server()->GetURL("/simple_page.html");
......@@ -180,7 +186,7 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, SingleTab) {
tab.reset();
browser.reset();
browser = std::make_unique<BrowserImpl>(GetProfile(), "x");
browser = CreateBrowser(GetProfile(), "x");
// Should be no tabs while waiting for restore.
EXPECT_TRUE(browser->GetTabs().empty());
// Wait for the restore and navigation to complete.
......@@ -197,8 +203,7 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, SingleTab) {
IN_PROC_BROWSER_TEST_F(SessionServiceTest, TwoTabs) {
ASSERT_TRUE(embedded_test_server()->Start());
std::unique_ptr<BrowserImpl> browser =
std::make_unique<BrowserImpl>(GetProfile(), "x");
std::unique_ptr<BrowserImpl> browser = CreateBrowser(GetProfile(), "x");
std::unique_ptr<Tab> tab1 = Tab::Create(GetProfile());
browser->AddTab(tab1.get());
const GURL url1 = embedded_test_server()->GetURL("/simple_page.html");
......@@ -218,7 +223,7 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, TwoTabs) {
tab2.reset();
browser.reset();
browser = std::make_unique<BrowserImpl>(GetProfile(), "x");
browser = CreateBrowser(GetProfile(), "x");
// Should be no tabs while waiting for restore.
EXPECT_TRUE(browser->GetTabs().empty()) << "iteration " << i;
// Wait for the restore and navigation to complete. This waits for the
......@@ -246,8 +251,7 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, MoveBetweenBrowsers) {
ASSERT_TRUE(embedded_test_server()->Start());
// Create a browser with two tabs.
std::unique_ptr<BrowserImpl> browser1 =
std::make_unique<BrowserImpl>(GetProfile(), "x");
std::unique_ptr<BrowserImpl> browser1 = CreateBrowser(GetProfile(), "x");
std::unique_ptr<Tab> tab1 = Tab::Create(GetProfile());
browser1->AddTab(tab1.get());
const GURL url1 = embedded_test_server()->GetURL("/simple_page.html");
......@@ -260,8 +264,7 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, MoveBetweenBrowsers) {
browser1->SetActiveTab(tab2.get());
// Create another browser with a single tab.
std::unique_ptr<BrowserImpl> browser2 =
std::make_unique<BrowserImpl>(GetProfile(), "y");
std::unique_ptr<BrowserImpl> browser2 = CreateBrowser(GetProfile(), "y");
std::unique_ptr<Tab> tab3 = Tab::Create(GetProfile());
browser2->AddTab(tab3.get());
const GURL url3 = embedded_test_server()->GetURL("/simple_page3.html");
......@@ -281,7 +284,7 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, MoveBetweenBrowsers) {
browser2.reset();
// Restore the browsers.
browser1 = std::make_unique<BrowserImpl>(GetProfile(), "x");
browser1 = CreateBrowser(GetProfile(), "x");
BrowserNavigationObserverImpl::WaitForNewTabToCompleteNavigation(
browser1.get(), url1, 1);
ASSERT_EQ(1u, browser1->GetTabs().size());
......@@ -289,7 +292,7 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, MoveBetweenBrowsers) {
->GetNavigationController()
->GetNavigationListSize());
browser2 = std::make_unique<BrowserImpl>(GetProfile(), "y");
browser2 = CreateBrowser(GetProfile(), "y");
BrowserNavigationObserverImpl::WaitForNewTabToCompleteNavigation(
browser2.get(), url2, 2);
ASSERT_EQ(2u, browser2->GetTabs().size());
......
// Copyright 2020 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.
#include "weblayer/public/browser.h"
namespace weblayer {
Browser::PersistenceInfo::PersistenceInfo() = default;
Browser::PersistenceInfo::PersistenceInfo(const PersistenceInfo& other) =
default;
Browser::PersistenceInfo::~PersistenceInfo() = default;
} // namespace weblayer
......@@ -5,6 +5,8 @@
#ifndef WEBLAYER_PUBLIC_BROWSER_H_
#define WEBLAYER_PUBLIC_BROWSER_H_
#include <stddef.h>
#include <memory>
#include <string>
#include <vector>
......@@ -19,10 +21,24 @@ class Tab;
// the set of Tabs.
class Browser {
public:
// Creates a new Browser. |persistence_id|, if non-empty, is used for saving
struct PersistenceInfo {
PersistenceInfo();
PersistenceInfo(const PersistenceInfo& other);
~PersistenceInfo();
// Uniquely identifies this browser for session restore, empty is a not a
// valid id.
std::string id;
// Last key used to encrypt incognito profile.
std::vector<uint8_t> last_crypto_key;
};
// Creates a new Browser. |persistence_info|, if non-null, is used for saving
// and restoring the state of the browser.
static std::unique_ptr<Browser> Create(Profile* profile,
const std::string& persistence_id);
static std::unique_ptr<Browser> Create(
Profile* profile,
const PersistenceInfo* persistence_info);
virtual ~Browser() {}
......@@ -36,7 +52,7 @@ class Browser {
virtual void PrepareForShutdown() = 0;
// Returns the id supplied to Create() that is used for persistence.
virtual const std::string& GetPersistenceId() = 0;
virtual std::string GetPersistenceId() = 0;
virtual void AddObserver(BrowserObserver* observer) = 0;
virtual void RemoveObserver(BrowserObserver* observer) = 0;
......
......@@ -396,8 +396,8 @@ public final class WebLayer {
* @param profileName Null to indicate in-memory profile. Otherwise, name cannot be empty
* and should contain only alphanumeric and underscore characters since it will be used as
* a directory name in the file system.
* @param persistenceId If non-null (which includes an empty string) uniquely identifies the
* Browser for saving state.
* @param persistenceId If non-null and not empty uniquely identifies the Browser for saving
* state.
*
* @since 81
*/
......
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