Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
bruschetta: Use SimpleURLLoader to download installer files
DownloadService doesn't handle SSL-cert-secured connections properly, try using SimpleURLLoader which, after http://crrev/c/4511794 lands, will. Bug: b:270656010 Test: Unit, browser, run with injected policy (but not SSL URLs) Change-Id: I0680f01fac57f7ff764e7fbc215af9bc4693bc31 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4547079 Reviewed-by: Emil Mikulic <easy@google.com> Reviewed-by: Rainhard Findling <rainhard@chromium.org> Reviewed-by: Fergus Dall <sidereal@google.com> Commit-Queue: David Munro <davidmunro@google.com> Cr-Commit-Position: refs/heads/main@{#1147091}
- Loading branch information
David Munro
authored and
Chromium LUCI CQ
committed
May 22, 2023
1 parent
7cb4a5b
commit e960246
Showing
11 changed files
with
474 additions
and
50 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#include "chrome/browser/ash/bruschetta/bruschetta_download.h" | ||
|
||
#include "base/files/scoped_temp_dir.h" | ||
#include "base/memory/ptr_util.h" | ||
#include "base/task/task_traits.h" | ||
#include "base/task/thread_pool.h" | ||
#include "chrome/browser/extensions/cws_info_service.h" | ||
#include "chrome/browser/profiles/profile.h" | ||
#include "content/public/browser/storage_partition.h" | ||
#include "crypto/secure_hash.h" | ||
#include "crypto/sha2.h" | ||
#include "services/network/public/cpp/resource_request.h" | ||
#include "services/network/public/cpp/simple_url_loader.h" | ||
|
||
namespace bruschetta { | ||
|
||
const net::NetworkTrafficAnnotationTag kBruschettaTrafficAnnotation = | ||
net::DefineNetworkTrafficAnnotation("bruschetta_installer_download", | ||
R"( | ||
semantics { | ||
sender: "Bruschetta VM Installer", | ||
description: "Request sent to download firmware and VM image for " | ||
"a Bruschetta VM, which allows the user to run the VM." | ||
trigger: "User installing a Bruschetta VM" | ||
internal { | ||
contacts { | ||
email: "clumptini+oncall@google.com" | ||
} | ||
} | ||
user_data: { | ||
type: ACCESS_TOKEN | ||
} | ||
data: "Request to download Bruschetta firmware and VM image. " | ||
"Sends cookies associated with the source to authenticate the user." | ||
destination: WEBSITE | ||
last_reviewed: "2023-01-09" | ||
} | ||
policy { | ||
cookies_allowed: YES | ||
cookies_store: "user" | ||
chrome_policy { | ||
BruschettaVMConfiguration { | ||
BruschettaVMConfiguration: "{}" | ||
} | ||
} | ||
} | ||
)"); | ||
|
||
namespace { | ||
|
||
std::unique_ptr<base::ScopedTempDir> MakeTempDir() { | ||
auto dir = std::make_unique<base::ScopedTempDir>(); | ||
CHECK(dir->CreateUniqueTempDir()); | ||
return dir; | ||
} | ||
|
||
// Calculates the sha256 hash of the file at `path` incrementally i.e. without | ||
// loading the entire thing into memory at once. Blocking. | ||
std::string Sha256File(const base::FilePath& path) { | ||
base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ); | ||
if (!file.IsValid()) { | ||
return ""; | ||
} | ||
|
||
std::unique_ptr<crypto::SecureHash> ctx( | ||
crypto::SecureHash::Create(crypto::SecureHash::SHA256)); | ||
const size_t kReadBufferSize = 4096; | ||
char buffer[kReadBufferSize]; | ||
while (true) { | ||
int count = file.ReadAtCurrentPos(buffer, kReadBufferSize); | ||
|
||
// Treat EOF the same as any other error, stop reading and return the hash | ||
// of what we read. If there was a disk error or something we'll end up with | ||
// an invalid hash, same as if the file were truncated. | ||
if (count <= 0) { | ||
break; | ||
} | ||
ctx->Update(buffer, count); | ||
} | ||
|
||
char digest_bytes[crypto::kSHA256Length]; | ||
ctx->Finish(digest_bytes, crypto::kSHA256Length); | ||
|
||
return base::HexEncode(digest_bytes, crypto::kSHA256Length); | ||
} | ||
|
||
} // namespace | ||
|
||
std::string Sha256FileForTesting(const base::FilePath& path) { | ||
return Sha256File(path); | ||
} | ||
|
||
std::unique_ptr<SimpleURLLoaderDownload> SimpleURLLoaderDownload::StartDownload( | ||
Profile* profile, | ||
GURL url, | ||
base::OnceCallback<void(base::FilePath path, std::string sha256)> | ||
callback) { | ||
return base::WrapUnique(new SimpleURLLoaderDownload(profile, std::move(url), | ||
std::move(callback))); | ||
} | ||
|
||
SimpleURLLoaderDownload::SimpleURLLoaderDownload( | ||
Profile* profile, | ||
GURL url, | ||
base::OnceCallback<void(base::FilePath path, std::string sha256)> callback) | ||
: profile_(profile), url_(std::move(url)), callback_(std::move(callback)) { | ||
base::ThreadPool::PostTaskAndReplyWithResult( | ||
FROM_HERE, {base::MayBlock()}, base::BindOnce(&MakeTempDir), | ||
base::BindOnce(&SimpleURLLoaderDownload::Download, | ||
weak_ptr_factory_.GetWeakPtr())); | ||
} | ||
|
||
SimpleURLLoaderDownload::~SimpleURLLoaderDownload() { | ||
auto seq = base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()}); | ||
seq->DeleteSoon(FROM_HERE, std::move(scoped_temp_dir_)); | ||
if (post_deletion_closure_for_testing_) { | ||
seq->PostTask(FROM_HERE, std::move(post_deletion_closure_for_testing_)); | ||
} | ||
} | ||
|
||
void SimpleURLLoaderDownload::Download( | ||
std::unique_ptr<base::ScopedTempDir> dir) { | ||
scoped_temp_dir_ = std::move(dir); | ||
auto path = scoped_temp_dir_->GetPath().Append("download"); | ||
auto req = std::make_unique<network::ResourceRequest>(); | ||
req->url = url_; | ||
loader_ = network::SimpleURLLoader::Create(std::move(req), | ||
kBruschettaTrafficAnnotation); | ||
auto factory = profile_->GetDefaultStoragePartition() | ||
->GetURLLoaderFactoryForBrowserProcess(); | ||
loader_->DownloadToFile(factory.get(), | ||
base::BindOnce(&SimpleURLLoaderDownload::Finished, | ||
weak_ptr_factory_.GetWeakPtr()), | ||
std::move(path)); | ||
} | ||
|
||
void SimpleURLLoaderDownload::Finished(base::FilePath path) { | ||
if (path.empty()) { | ||
LOG(ERROR) << "Download failed"; | ||
std::move(callback_).Run(path, ""); | ||
return; | ||
} | ||
|
||
base::ThreadPool::PostTaskAndReplyWithResult( | ||
FROM_HERE, {base::MayBlock()}, base::BindOnce(&Sha256File, path), | ||
base::BindOnce(std::move(callback_), path)); | ||
} | ||
|
||
} // namespace bruschetta |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#ifndef CHROME_BROWSER_ASH_BRUSCHETTA_BRUSCHETTA_DOWNLOAD_H_ | ||
#define CHROME_BROWSER_ASH_BRUSCHETTA_BRUSCHETTA_DOWNLOAD_H_ | ||
|
||
#include "base/files/scoped_temp_dir.h" | ||
#include "base/functional/callback_forward.h" | ||
#include "base/functional/callback_helpers.h" | ||
#include "base/memory/raw_ptr.h" | ||
#include "services/network/public/cpp/simple_url_loader.h" | ||
#include "url/gurl.h" | ||
|
||
class Profile; | ||
namespace network { | ||
class SimpleURLLoader; | ||
} // namespace network | ||
|
||
namespace bruschetta { | ||
|
||
extern const net::NetworkTrafficAnnotationTag kBruschettaTrafficAnnotation; | ||
|
||
// Only exposed so unit tests can test it. | ||
std::string Sha256FileForTesting(const base::FilePath& path); | ||
|
||
// Wraps SimpleURLLoader to make it even simpler for Bruschetta to use it for | ||
// downloading files. | ||
class SimpleURLLoaderDownload { | ||
public: | ||
// Starts downloading the file at `url`, will invoke `callback` upon | ||
// completion. Either with the path to the downloaded file and a sha256 hash | ||
// of its contents, or in case of error will run `callback` with an empty | ||
// path. Destroying the returned download instance will cancel any active | ||
// downloads and delete any downloaded files. | ||
static std::unique_ptr<SimpleURLLoaderDownload> StartDownload( | ||
Profile* profile, | ||
GURL url, | ||
base::OnceCallback<void(base::FilePath path, std::string sha256)> | ||
callback); | ||
|
||
void SetPostDeletionCallbackForTesting(base::OnceClosure closure) { | ||
post_deletion_closure_for_testing_ = std::move(closure); | ||
} | ||
|
||
~SimpleURLLoaderDownload(); | ||
|
||
private: | ||
SimpleURLLoaderDownload( | ||
Profile* profile, | ||
GURL url, | ||
base::OnceCallback<void(base::FilePath path, std::string sha256)> | ||
callback); | ||
void Download(std::unique_ptr<base::ScopedTempDir> dir); | ||
void Finished(base::FilePath path); | ||
|
||
base::raw_ptr<Profile> profile_; | ||
GURL url_; | ||
std::unique_ptr<base::ScopedTempDir> scoped_temp_dir_; | ||
base::OnceCallback<void(base::FilePath path, std::string sha256)> callback_; | ||
std::unique_ptr<network::SimpleURLLoader> loader_; | ||
base::OnceClosure post_deletion_closure_for_testing_; | ||
|
||
base::WeakPtrFactory<SimpleURLLoaderDownload> weak_ptr_factory_{this}; | ||
}; | ||
|
||
} // namespace bruschetta | ||
|
||
#endif // CHROME_BROWSER_ASH_BRUSCHETTA_BRUSCHETTA_DOWNLOAD_H_ |
84 changes: 84 additions & 0 deletions
84
chrome/browser/ash/bruschetta/bruschetta_download_browsertest.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#include "chrome/browser/ash/bruschetta/bruschetta_download.h" | ||
|
||
#include "base/files/file_util.h" | ||
#include "base/run_loop.h" | ||
#include "base/strings/string_util.h" | ||
#include "base/test/test_future.h" | ||
#include "base/threading/thread_restrictions.h" | ||
#include "chrome/browser/ui/browser.h" | ||
#include "chrome/test/base/in_process_browser_test.h" | ||
#include "content/public/test/browser_test.h" | ||
#include "testing/gtest/include/gtest/gtest.h" | ||
|
||
namespace bruschetta { | ||
namespace { | ||
|
||
class BruschettaDownloadBrowserTest : public InProcessBrowserTest { | ||
public: | ||
BruschettaDownloadBrowserTest() = default; | ||
~BruschettaDownloadBrowserTest() override = default; | ||
|
||
BruschettaDownloadBrowserTest(const BruschettaDownloadBrowserTest&) = delete; | ||
BruschettaDownloadBrowserTest& operator=( | ||
const BruschettaDownloadBrowserTest&) = delete; | ||
|
||
protected: | ||
void SetUpInProcessBrowserTestFixture() override { | ||
InProcessBrowserTest::SetUpInProcessBrowserTestFixture(); | ||
} | ||
}; | ||
|
||
bool PathExistsBlockingAllowed(const base::FilePath& path) { | ||
base::ScopedAllowBlockingForTesting allow_blocking; | ||
return base::PathExists(path); | ||
} | ||
|
||
IN_PROC_BROWSER_TEST_F(BruschettaDownloadBrowserTest, TestHappyPath) { | ||
auto expected_hash = base::ToUpperASCII( | ||
"f54d00e6d24844ee3b1d0d8c2b9d2ed80b967e94eb1055bb1fd43eb9522908cc"); | ||
|
||
net::EmbeddedTestServer server(net::EmbeddedTestServer::TYPE_HTTPS); | ||
server.ServeFilesFromSourceDirectory("chrome/test/data/bruschetta"); | ||
auto server_handle = server.StartAndReturnHandle(); | ||
ASSERT_TRUE(server_handle); | ||
GURL url = server.GetURL("/download_file.img"); | ||
|
||
base::test::TestFuture<base::FilePath, std::string> future; | ||
base::FilePath path; | ||
auto download = SimpleURLLoaderDownload::StartDownload( | ||
browser()->profile(), url, future.GetCallback()); | ||
|
||
path = future.Get<base::FilePath>(); | ||
auto hash = future.Get<std::string>(); | ||
|
||
ASSERT_FALSE(path.empty()); | ||
EXPECT_EQ(hash, expected_hash); | ||
|
||
// Deleting the download should clean up downloaded files. I found | ||
// RunUntilIdle to be flaky hence we have an explicit callback for after | ||
// deletion completes. | ||
base::RunLoop run_loop; | ||
download->SetPostDeletionCallbackForTesting(run_loop.QuitClosure()); | ||
download.reset(); | ||
run_loop.Run(); | ||
EXPECT_FALSE(PathExistsBlockingAllowed(path)); | ||
} | ||
|
||
IN_PROC_BROWSER_TEST_F(BruschettaDownloadBrowserTest, TestDownloadFailed) { | ||
base::test::TestFuture<base::FilePath, std::string> future; | ||
auto download = SimpleURLLoaderDownload::StartDownload( | ||
browser()->profile(), GURL("bad url"), future.GetCallback()); | ||
|
||
auto path = future.Get<base::FilePath>(); | ||
auto hash = future.Get<std::string>(); | ||
|
||
ASSERT_TRUE(path.empty()); | ||
EXPECT_EQ(hash, ""); | ||
} | ||
|
||
} // namespace | ||
} // namespace bruschetta |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
chrome/browser/ash/bruschetta/bruschetta_download_unittest.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
// Copyright 2023 The Chromium Authors | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#include "chrome/browser/ash/bruschetta/bruschetta_download.h" | ||
|
||
#include <cstring> | ||
|
||
#include "base/files/file_util.h" | ||
#include "base/files/scoped_temp_dir.h" | ||
#include "testing/gtest/include/gtest/gtest.h" | ||
|
||
namespace bruschetta { | ||
namespace { | ||
|
||
class BruschettaDownloadTest : public testing::Test {}; | ||
|
||
TEST_F(BruschettaDownloadTest, TestSha256File) { | ||
base::ScopedTempDir dir; | ||
ASSERT_TRUE(dir.CreateUniqueTempDir()); | ||
const int kBufSize = | ||
4096 * 2 + 1; // Sha256File uses 4k block size, so make sure the file is | ||
// multiple blocks and not a complete block. | ||
char buff[kBufSize]; | ||
memset(buff, 'd', kBufSize); | ||
auto path = dir.GetPath().Append("file"); | ||
const char* expected = | ||
"B1AAAD3DBE85816D70C94C35B873D45F0C68F9D3B3DB6F6AB858A1560540E4DF"; | ||
ASSERT_TRUE(base::WriteFile(path, buff, kBufSize)); | ||
auto hash = Sha256FileForTesting(path); | ||
|
||
ASSERT_EQ(hash, expected); | ||
} | ||
|
||
} // namespace | ||
} // namespace bruschetta |
Oops, something went wrong.