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.

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

fix(secrets): Reduce keychain unlock prompts on MacOS #2394

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/secrets/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

All notable changes to the Zowe Secrets SDK package will be documented in this file.

## Recent Changes

- BugFix: Reduced number of keychain unlock prompts on MacOS for simultaneous access to secrets by the same process.
t1m0thyj marked this conversation as resolved.
Show resolved Hide resolved

## `8.1.2`

- BugFix: Updated dependencies for technical currency. [#2289](https://github.com/zowe/zowe-cli/pull/2289)
Expand Down
12 changes: 11 additions & 1 deletion packages/secrets/core/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions packages/secrets/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ version = "0.48.0"
[target.'cfg(target_os = "macos")'.dependencies]
core-foundation = "0.9.3"
core-foundation-sys = "0.8.4"
fmutex = "0.1.0"

[target.'cfg(any(target_os = "freebsd", target_os = "linux"))'.dependencies]
gio = "0.18.2"
glib = "0.18.2"
glib-sys = "0.18.1"
gio = "0.18.2"
libsecret = "0.4.0"
libsecret-sys = "0.4.0"
libsecret-sys = "0.4.0"
1 change: 0 additions & 1 deletion packages/secrets/core/src/os/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ pub enum KeyringError {
#[error("[keyring] {name:?} library returned an error:\n\n{details:?}")]
Library { name: String, details: String },

#[cfg(not(target_os = "macos"))]
#[error("[keyring] An OS error has occurred:\n\n{0}")]
Os(String),

Expand Down
27 changes: 27 additions & 0 deletions packages/secrets/core/src/os/mac/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use error::Error;

use crate::os::mac::error::ERR_SEC_ITEM_NOT_FOUND;
use crate::os::mac::keychain_search::{KeychainSearch, SearchResult};
use fmutex::Guard;
use keychain::SecKeychain;

impl From<Error> for KeyringError {
Expand All @@ -22,6 +23,27 @@ impl From<Error> for KeyringError {
}
}

impl From<std::io::Error> for KeyringError {
fn from(error: std::io::Error) -> Self {
KeyringError::Os(error.to_string())
}
}

fn keyring_mutex() -> Result<Guard, KeyringError> {
// MacOS shows keychain prompt after secret has been modified by another process. We use cross-process mutex to
// block keychain access if there are multiple concurrent keychain operations invoked by the same process. This
// prevents multiple instances of the same app (e.g. VS Code) from triggering several keychain prompts at once.
let exe_path = std::env::current_exe()
.unwrap()
.to_string_lossy()
.replace(std::path::MAIN_SEPARATOR, "_");
let lock_path = std::env::temp_dir()
.join(format!("keyring_{}.lock", exe_path));
std::fs::OpenOptions::new().create(true).write(true).open(&lock_path)
.and_then(|_| fmutex::lock(lock_path))
.map_err(KeyringError::from)
}

///
/// Attempts to set a password for a given service and account.
///
Expand All @@ -38,6 +60,7 @@ pub fn set_password(
password: &String,
) -> Result<bool, KeyringError> {
let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex().unwrap();
match keychain.set_password(service.as_str(), account.as_str(), password.as_bytes()) {
Ok(()) => Ok(true),
Err(err) => Err(KeyringError::from(err)),
Expand All @@ -56,6 +79,7 @@ pub fn set_password(
///
pub fn get_password(service: &String, account: &String) -> Result<Option<String>, KeyringError> {
let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex().unwrap();
match keychain.find_password(service.as_str(), account.as_str()) {
Ok((pw, _)) => Ok(Some(String::from_utf8(pw.to_owned())?)),
Err(err) if err.code() == ERR_SEC_ITEM_NOT_FOUND => Ok(None),
Expand Down Expand Up @@ -83,6 +107,7 @@ pub fn find_password(service: &String) -> Result<Option<String>, KeyringError> {
}

let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex().unwrap();
match keychain.find_password(cred_attrs[0], cred_attrs[1]) {
Ok((pw, _)) => {
let pw_str = String::from_utf8(pw.to_owned())?;
Expand All @@ -104,6 +129,7 @@ pub fn find_password(service: &String) -> Result<Option<String>, KeyringError> {
///
pub fn delete_password(service: &String, account: &String) -> Result<bool, KeyringError> {
let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex().unwrap();
match keychain.find_password(service.as_str(), account.as_str()) {
Ok((_, item)) => {
item.delete()?;
Expand All @@ -128,6 +154,7 @@ pub fn find_credentials(
service: &String,
credentials: &mut Vec<(String, String)>,
) -> Result<bool, KeyringError> {
let _lock = keyring_mutex().unwrap();
match KeychainSearch::new()
.label(service.as_str())
.with_attrs()
Expand Down
12 changes: 11 additions & 1 deletion packages/secrets/src/keyring/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions packages/secrets/src/keyring/Cross.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ passthrough = [
image = "ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main"

[target.aarch64-unknown-linux-musl]
image = "rust:alpine"
image = "rust:alpine3.20"
pre-build = [
"wget -qO- https://musl.cc/aarch64-linux-musl-cross.tgz | tar -xzC / && export PATH=\"/aarch64-linux-musl-cross/bin:$PATH\"",
"apk add --no-cache musl-dev pkgconfig",
"apk add -p /aarch64-linux-musl-cross --initdb --arch aarch64 --allow-untrusted -X \"https://dl-cdn.alpinelinux.org/alpine/latest-stable/main/\" --no-cache --no-scripts libsecret-dev",
"apk add -p /aarch64-linux-musl-cross --initdb --arch aarch64 --allow-untrusted -X $(head -n 1 /etc/apk/repositories) --no-cache --no-scripts libsecret-dev",
"rustup target add aarch64-unknown-linux-musl"
]

Expand All @@ -33,5 +33,5 @@ image = "ghcr.io/cross-rs/i686-unknown-linux-gnu:main"
image = "ghcr.io/cross-rs/x86_64-unknown-linux-gnu:main"

[target.x86_64-unknown-linux-musl]
image = "rust:alpine"
image = "rust:alpine3.20"
pre-build = ["apk add libsecret-dev musl-dev"]
Loading