Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: set macOS crypto keychain name earlier #35795

Merged
merged 3 commits into from Sep 28, 2022
Merged

fix: set macOS crypto keychain name earlier #35795

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8546902
fix: set macOS crypto keychain name earlier
MarshallOfSound Jun 21, 2022
d5e4e3e
spec: ensure arm64 mac tests are cleaned up
MarshallOfSound Sep 23, 2022
b16522d
fixup for password name difference in 19-x-y
jkleinsc Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .circleci/config/base.yml
View file
Open in desktop
Expand Up @@ -216,6 +216,7 @@ step-maybe-cleanup-arm64-mac: &step-maybe-cleanup-arm64-mac
rm -rf ~/Library/Application\ Support/electron*
security delete-generic-password -l "Chromium Safe Storage" || echo "✓ Keychain does not contain password from tests"
security delete-generic-password -l "Electron Test Main Safe Storage" || echo "✓ Keychain does not contain password from tests"
security delete-generic-password -a "electron-test-safe-storage" || echo "✓ Keychain does not contain password from tests"
elif [ "$TARGET_ARCH" == "arm" ] || [ "$TARGET_ARCH" == "arm64" ]; then
XVFB=/usr/bin/Xvfb
/sbin/start-stop-daemon --stop --exec $XVFB || echo "Xvfb not running"
Expand Down
9 changes: 8 additions & 1 deletion shell/browser/electron_browser_main_parts.cc
View file
Open in desktop
Expand Up @@ -90,6 +90,7 @@
#endif

#if BUILDFLAG(IS_MAC)
#include "components/os_crypt/keychain_password_mac.h"
#include "services/device/public/cpp/geolocation/geolocation_manager.h"
#include "shell/browser/ui/cocoa/views_delegate_mac.h"
#else
Expand Down Expand Up @@ -481,6 +482,9 @@ void ElectronBrowserMainParts::WillRunMainMessageLoop(
}

void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)
std::string app_name = electron::Browser::Get()->GetName();
#endif
#if BUILDFLAG(IS_LINUX)
auto shutdown_cb =
base::BindOnce(base::RunLoop::QuitCurrentWhenIdleClosureDeprecated());
Expand All @@ -491,7 +495,6 @@ void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
// Set up crypt config. This needs to be done before anything starts the
// network service, as the raw encryption key needs to be shared with the
// network service for encrypted cookie storage.
std::string app_name = electron::Browser::Get()->GetName();
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
std::unique_ptr<os_crypt::Config> config =
Expand All @@ -508,6 +511,10 @@ void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
base::PathService::Get(chrome::DIR_USER_DATA, &config->user_data_path);
OSCrypt::SetConfig(std::move(config));
#endif
#if BUILDFLAG(IS_MAC)
KeychainPassword::GetServiceName() = app_name + " Safe Storage";
KeychainPassword::GetAccountName() = app_name;
#endif
#if BUILDFLAG(IS_POSIX)
// Exit in response to SIGINT, SIGTERM, etc.
InstallShutdownSignalHandlers(
Expand Down
10 changes: 0 additions & 10 deletions shell/browser/net/system_network_context_manager.cc
View file
Open in desktop
Expand Up @@ -42,10 +42,6 @@
#include "shell/common/options_switches.h"
#include "url/gurl.h"

#if BUILDFLAG(IS_MAC)
#include "components/os_crypt/keychain_password_mac.h"
#endif

#if BUILDFLAG(IS_LINUX)
#include "components/os_crypt/key_storage_config_linux.h"
#endif
Expand Down Expand Up @@ -288,12 +284,6 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
base::FeatureList::IsEnabled(features::kAsyncDns),
default_secure_dns_mode, doh_config, additional_dns_query_types_enabled);

std::string app_name = electron::Browser::Get()->GetName();
#if BUILDFLAG(IS_MAC)
KeychainPassword::GetServiceName() = app_name + " Safe Storage";
KeychainPassword::GetAccountName() = app_name;
#endif

// The OSCrypt keys are process bound, so if network service is out of
// process, send it the required key.
if (content::IsOutOfProcessNetworkService() &&
Expand Down
6 changes: 3 additions & 3 deletions spec-main/api-safe-storage-spec.ts
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@ import { safeStorage } from 'electron/main';
import { expect } from 'chai';
import { emittedOnce } from './events-helpers';
import { ifdescribe } from './spec-helpers';
import * as fs from 'fs';
import * as fs from 'fs-extra';

/* isEncryptionAvailable returns false in Linux when running CI due to a mocked dbus. This stops
* Chrome from reaching the system's keyring or libsecret. When running the tests with config.store
Expand Down Expand Up @@ -36,8 +36,8 @@ describe('safeStorage module', () => {
ifdescribe(process.platform !== 'linux')('safeStorage module', () => {
after(async () => {
const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt');
if (fs.existsSync(pathToEncryptedString)) {
await fs.unlinkSync(pathToEncryptedString);
if (await fs.pathExists(pathToEncryptedString)) {
await fs.remove(pathToEncryptedString);
}
});

Expand Down