feat(aws): keyring storage for AWS access keys, carried by settings export
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e8bbe90327
commit
60dad7b6b6
3 files changed
+331
-1
No files matched your search
@@ -43,6 +43,7 @@ use zeroize::Zeroizing;
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use crate::models::host_connection::{AwsAuth, HostConnection, HostKind};
|
||||
use crate::models::marketplace::{AccountMethod, MarketplaceAccount};
|
||||
use crate::models::{
|
||||
AppSettings, ExportedSecrets, SettingsExportPayload, SettingsImportOutcome,
|
||||
@@ -138,6 +139,7 @@ fn split_settings_and_secrets(current: AppSettings) -> (AppSettings, ExportedSec
|
||||
&settings.marketplace_accounts,
|
||||
secure::get_marketplace_token,
|
||||
),
|
||||
aws_connection_keys: exported_aws_connection_keys(&settings.hosts, secure::get_aws_keys),
|
||||
};
|
||||
|
||||
(settings, secrets)
|
||||
@@ -184,6 +186,107 @@ fn restore_marketplace_tokens(
|
||||
warnings
|
||||
}
|
||||
|
||||
/// The stored keys of every `Keys` host connection, by connection id, as the
|
||||
/// `AwsKeys` JSON. A `Profile` connection holds none. A missing or unreadable
|
||||
/// entry is left out, like the other keychain secrets above.
|
||||
fn exported_aws_connection_keys(
|
||||
hosts: &[HostConnection],
|
||||
get: impl Fn(&str) -> Result<Option<secure::AwsKeys>, String>,
|
||||
) -> BTreeMap<String, String> {
|
||||
hosts
|
||||
.iter()
|
||||
.filter(|h| matches!(&h.kind, HostKind::Aws { auth: AwsAuth::Keys { .. }, .. }))
|
||||
.filter_map(|h| {
|
||||
let keys = get(&h.id).unwrap_or_default()?;
|
||||
Some((h.id.clone(), serde_json::to_string(&keys).ok()?))
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Write each imported set of AWS keys to the keychain, returning a warning
|
||||
/// (naming the connection id, never the value) for each that could not be.
|
||||
fn restore_aws_connection_keys(
|
||||
keys: &BTreeMap<String, String>,
|
||||
mut store: impl FnMut(&str, &secure::AwsKeys) -> Result<(), String>,
|
||||
) -> Vec<String> {
|
||||
let mut warnings = Vec::new();
|
||||
for (id, json) in keys {
|
||||
if json.trim().is_empty() {
|
||||
continue;
|
||||
}
|
||||
let Ok(parsed) = serde_json::from_str::<secure::AwsKeys>(json) else {
|
||||
log::warn!("Settings import: the AWS keys of connection {} are malformed", id);
|
||||
warnings.push(format!(
|
||||
"The AWS access keys of connection {} in the file are malformed; enter them again.",
|
||||
id
|
||||
));
|
||||
continue;
|
||||
};
|
||||
if let Err(e) = store(id, &parsed) {
|
||||
log::warn!(
|
||||
"Settings import: could not restore the AWS keys of connection {}: {}",
|
||||
id,
|
||||
e
|
||||
);
|
||||
warnings.push(format!(
|
||||
"Could not restore the AWS access keys of connection {} ({}); enter them again.",
|
||||
id, e
|
||||
));
|
||||
}
|
||||
}
|
||||
warnings
|
||||
}
|
||||
|
||||
/// Checks the imported host connections with the rules the connection
|
||||
/// commands use, before anything is written; refuses the import (like
|
||||
/// `validate_imported_marketplace_state`) rather than applying a bad entry.
|
||||
/// A `Keys` connection's `keyring_ref` must be its own (valid) id, and keys
|
||||
/// may only be carried for a `Keys` connection in the file.
|
||||
fn validate_imported_hosts(
|
||||
settings: &mut AppSettings,
|
||||
keys: &BTreeMap<String, String>,
|
||||
) -> Result<(), String> {
|
||||
use crate::models::host_connection::{
|
||||
validate_connection_name, validate_profile_name, validate_region,
|
||||
};
|
||||
let wrap = |e: String| format!("The file's host connections were refused: {e}");
|
||||
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
for h in &mut settings.hosts {
|
||||
if !secure::is_valid_connection_id(&h.id) {
|
||||
return Err(wrap("a connection has an invalid id.".into()));
|
||||
}
|
||||
if !seen.insert(h.id.clone()) {
|
||||
return Err(wrap("a connection appears twice.".into()));
|
||||
}
|
||||
h.name = validate_connection_name(&h.name).map_err(wrap)?;
|
||||
let HostKind::Aws { auth, default_region } = &h.kind;
|
||||
validate_region(default_region).map_err(wrap)?;
|
||||
match auth {
|
||||
AwsAuth::Profile { name } => {
|
||||
validate_profile_name(name).map_err(wrap)?;
|
||||
}
|
||||
AwsAuth::Keys { keyring_ref } => {
|
||||
if !secure::is_valid_connection_id(keyring_ref) || *keyring_ref != h.id {
|
||||
return Err(wrap(format!(
|
||||
"the connection \"{}\" has an invalid key reference.",
|
||||
h.name
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
for id in keys.keys() {
|
||||
let is_keys_connection = settings.hosts.iter().any(|h| {
|
||||
h.id == *id && matches!(&h.kind, HostKind::Aws { auth: AwsAuth::Keys { .. }, .. })
|
||||
});
|
||||
if !is_keys_connection {
|
||||
return Err(wrap("access keys are present for an unknown connection.".into()));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Export the current global settings and secrets to a password-encrypted
|
||||
/// file. `Ok(false)` means the save dialog was dismissed — not an error, and
|
||||
/// deliberately distinguishable from one so the frontend shows nothing
|
||||
@@ -376,6 +479,9 @@ pub async fn apply_settings_import(
|
||||
&marketplace_tokens,
|
||||
)?;
|
||||
|
||||
let aws_keys = payload.secrets.aws_connection_keys;
|
||||
validate_imported_hosts(&mut settings, &aws_keys)?;
|
||||
|
||||
let mut secret_restore_warnings = Vec::new();
|
||||
let mut gateway_secret_changed = false;
|
||||
|
||||
@@ -423,6 +529,11 @@ pub async fn apply_settings_import(
|
||||
secure::store_marketplace_token,
|
||||
));
|
||||
|
||||
secret_restore_warnings.extend(restore_aws_connection_keys(
|
||||
&aws_keys,
|
||||
secure::store_aws_keys,
|
||||
));
|
||||
|
||||
let imported_marketplace = (
|
||||
settings.marketplace_accounts.clone(),
|
||||
settings.marketplaces.clone(),
|
||||
@@ -827,4 +938,109 @@ mod tests {
|
||||
assert_eq!(warnings.len(), 1);
|
||||
assert!(!warnings[0].contains("test-token-not-real"));
|
||||
}
|
||||
|
||||
fn keys_host(id: &str) -> HostConnection {
|
||||
HostConnection {
|
||||
id: id.into(),
|
||||
name: "Prod".into(),
|
||||
kind: HostKind::Aws {
|
||||
auth: AwsAuth::Keys { keyring_ref: id.into() },
|
||||
default_region: "us-west-2".into(),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn profile_host(id: &str) -> HostConnection {
|
||||
HostConnection {
|
||||
id: id.into(),
|
||||
name: "Dev".into(),
|
||||
kind: HostKind::Aws {
|
||||
auth: AwsAuth::Profile { name: "default".into() },
|
||||
default_region: "us-west-2".into(),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn test_keys() -> secure::AwsKeys {
|
||||
secure::AwsKeys {
|
||||
access_key_id: "AKIATESTNOTREAL".into(),
|
||||
secret_access_key: "test-secret-not-real".into(),
|
||||
session_token: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn aws_connection_keys_round_trip_through_export() {
|
||||
let hosts = vec![keys_host("k1"), profile_host("p1")];
|
||||
let exported = exported_aws_connection_keys(&hosts, |id| match id {
|
||||
"k1" => Ok(Some(test_keys())),
|
||||
other => panic!("only a Keys connection is read, got {other}"),
|
||||
});
|
||||
assert_eq!(exported.keys().collect::<Vec<_>>(), vec!["k1"]);
|
||||
assert_eq!(serde_json::from_str::<secure::AwsKeys>(&exported["k1"]).unwrap(), test_keys());
|
||||
|
||||
let mut stored = Vec::new();
|
||||
let warnings = restore_aws_connection_keys(&exported, |id, keys| {
|
||||
stored.push((id.to_string(), keys.clone()));
|
||||
Ok(())
|
||||
});
|
||||
assert!(warnings.is_empty());
|
||||
assert_eq!(stored, vec![("k1".to_string(), test_keys())]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn aws_connection_keys_restore_failures_name_the_id_not_the_value() {
|
||||
let keys = BTreeMap::from([
|
||||
("bad".to_string(), "not json test-secret-not-real".to_string()),
|
||||
("locked".to_string(), serde_json::to_string(&test_keys()).unwrap()),
|
||||
]);
|
||||
let warnings = restore_aws_connection_keys(&keys, |_, _| Err("keychain locked".into()));
|
||||
assert_eq!(warnings.len(), 2);
|
||||
assert!(warnings[0].contains("bad"));
|
||||
assert!(warnings[1].contains("locked"));
|
||||
assert!(warnings.iter().all(|w| !w.contains("test-secret-not-real")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn imported_hosts_are_validated() {
|
||||
let check = |hosts: Vec<HostConnection>, keys: BTreeMap<String, String>| {
|
||||
let mut s = AppSettings::default();
|
||||
s.hosts = hosts;
|
||||
validate_imported_hosts(&mut s, &keys)
|
||||
};
|
||||
let none = BTreeMap::new();
|
||||
assert!(check(vec![keys_host("k1"), profile_host("p1")], none.clone()).is_ok());
|
||||
assert!(check(vec![keys_host("k1")], BTreeMap::from([("k1".into(), "{}".into())])).is_ok());
|
||||
|
||||
let mut bad_name = profile_host("p1");
|
||||
bad_name.name = " ".into();
|
||||
assert!(check(vec![bad_name], none.clone()).is_err());
|
||||
|
||||
let mut bad_region = profile_host("p1");
|
||||
bad_region.kind = HostKind::Aws {
|
||||
auth: AwsAuth::Profile { name: "default".into() },
|
||||
default_region: "nowhere".into(),
|
||||
};
|
||||
assert!(check(vec![bad_region], none.clone()).is_err());
|
||||
|
||||
let mut bad_profile = profile_host("p1");
|
||||
bad_profile.kind = HostKind::Aws {
|
||||
auth: AwsAuth::Profile { name: "a b".into() },
|
||||
default_region: "us-west-2".into(),
|
||||
};
|
||||
assert!(check(vec![bad_profile], none.clone()).is_err());
|
||||
|
||||
let mut other_ref = keys_host("k1");
|
||||
other_ref.kind = HostKind::Aws {
|
||||
auth: AwsAuth::Keys { keyring_ref: "someone-else".into() },
|
||||
default_region: "us-west-2".into(),
|
||||
};
|
||||
assert!(check(vec![other_ref], none.clone()).is_err());
|
||||
|
||||
assert!(check(vec![profile_host("../x")], none.clone()).is_err());
|
||||
assert!(check(vec![profile_host("p1"), profile_host("p1")], none.clone()).is_err());
|
||||
// Keys carried for a connection that is not a Keys connection.
|
||||
assert!(check(vec![profile_host("p1")], BTreeMap::from([("p1".into(), "{}".into())])).is_err());
|
||||
assert!(check(vec![], BTreeMap::from([("ghost".into(), "{}".into())])).is_err());
|
||||
}
|
||||
}
|
||||
@@ -64,6 +64,11 @@ pub struct ExportedSecrets {
|
||||
/// here or an imported account could never fetch.
|
||||
#[serde(default)]
|
||||
pub marketplace_account_tokens: BTreeMap<String, String>,
|
||||
/// Access keys of `AwsAuth::Keys` host connections, keyed by connection
|
||||
/// id; each value is the `AwsKeys` JSON. Like the tokens above they live
|
||||
/// in the keychain, not in `AppSettings::hosts`.
|
||||
#[serde(default)]
|
||||
pub aws_connection_keys: BTreeMap<String, String>,
|
||||
}
|
||||
|
||||
impl ExportedSecrets {
|
||||
@@ -74,6 +79,7 @@ impl ExportedSecrets {
|
||||
&& blank(&self.gateway_master_key)
|
||||
&& blank(&self.web_terminal_access_token)
|
||||
&& self.marketplace_account_tokens.values().all(|v| v.trim().is_empty())
|
||||
&& self.aws_connection_keys.values().all(|v| v.trim().is_empty())
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -167,7 +167,12 @@ fn read_entry(service: &str, label: &str) -> Result<Option<String>, String> {
|
||||
fn delete_entry(service: &str, label: &str) -> Result<(), String> {
|
||||
let entry = keyring::Entry::new(service, KEYCHAIN_ACCOUNT)
|
||||
.map_err(|e| format!("Keyring error: {}", e))?;
|
||||
match entry.delete_credential() {
|
||||
delete_outcome(entry.delete_credential(), label)
|
||||
}
|
||||
|
||||
/// "Wasn't there" is success; any other failure names the entry, not its value.
|
||||
fn delete_outcome(result: Result<(), keyring::Error>, label: &str) -> Result<(), String> {
|
||||
match result {
|
||||
Ok(()) | Err(keyring::Error::NoEntry) => Ok(()),
|
||||
Err(e) => Err(format!("Failed to delete {}: {}", label, e)),
|
||||
}
|
||||
@@ -408,6 +413,74 @@ pub fn delete_marketplace_token(account_id: &str) -> Result<(), String> {
|
||||
delete_entry(&marketplace_token_service(account_id)?, "the marketplace account token")
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// AWS connection access keys (global, one entry per connection)
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// Keychain service prefix; the connection id completes it.
|
||||
const AWS_CONNECTION_SERVICE_PREFIX: &str = "triple-c-aws-connection-";
|
||||
|
||||
/// Static AWS credentials for a `HostConnection` using `AwsAuth::Keys`. Held in
|
||||
/// the keychain as one JSON value. `Debug` redacts the secret and the token.
|
||||
#[derive(Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq)]
|
||||
pub struct AwsKeys {
|
||||
pub access_key_id: String,
|
||||
pub secret_access_key: String,
|
||||
pub session_token: Option<String>,
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for AwsKeys {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("AwsKeys")
|
||||
.field("access_key_id", &self.access_key_id)
|
||||
.field("secret_access_key", &"<redacted>")
|
||||
.field("session_token", &self.session_token.as_ref().map(|_| "<redacted>"))
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether `id` may be used as a connection id / keyring reference: 1..=64 of
|
||||
/// `[A-Za-z0-9-]`. The same rule as marketplace account ids.
|
||||
pub fn is_valid_connection_id(id: &str) -> bool {
|
||||
!id.is_empty()
|
||||
&& id.len() <= 64
|
||||
&& id.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-')
|
||||
}
|
||||
|
||||
fn aws_connection_service(connection_id: &str) -> Result<String, String> {
|
||||
if !is_valid_connection_id(connection_id) {
|
||||
return Err(format!("Invalid AWS connection id {:?}", connection_id));
|
||||
}
|
||||
Ok(format!("{}{}", AWS_CONNECTION_SERVICE_PREFIX, connection_id))
|
||||
}
|
||||
|
||||
pub fn store_aws_keys(connection_id: &str, keys: &AwsKeys) -> Result<(), String> {
|
||||
let service = aws_connection_service(connection_id)?;
|
||||
let json = serde_json::to_string(keys)
|
||||
.map_err(|_| "Failed to encode the AWS access keys".to_string())?;
|
||||
let entry = keyring::Entry::new(&service, KEYCHAIN_ACCOUNT)
|
||||
.map_err(|e| format!("Keyring error: {}", e))?;
|
||||
entry
|
||||
.set_password(&json)
|
||||
.map_err(|e| format!("Failed to store the AWS access keys: {}", e))
|
||||
}
|
||||
|
||||
pub fn get_aws_keys(connection_id: &str) -> Result<Option<AwsKeys>, String> {
|
||||
let Some(json) = read_entry(&aws_connection_service(connection_id)?, "the AWS access keys")?
|
||||
else {
|
||||
return Ok(None);
|
||||
};
|
||||
serde_json::from_str(&json)
|
||||
.map(Some)
|
||||
.map_err(|_| "The stored AWS access keys are unreadable".to_string())
|
||||
}
|
||||
|
||||
/// Missing entry is success. Unused until connection removal lands.
|
||||
#[allow(dead_code)]
|
||||
pub fn delete_aws_keys(connection_id: &str) -> Result<(), String> {
|
||||
delete_entry(&aws_connection_service(connection_id)?, "the AWS access keys")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -496,4 +569,39 @@ mod tests {
|
||||
// Pasted credentials routinely carry a trailing newline.
|
||||
assert_eq!(secret_to_store(Some(" ghp_abc123\n")), Some("ghp_abc123"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn aws_keys_debug_redacts_secrets() {
|
||||
let keys = AwsKeys {
|
||||
access_key_id: "AKIATESTNOTREAL".into(),
|
||||
secret_access_key: "test-secret-not-real".into(),
|
||||
session_token: Some("test-session-token-not-real".into()),
|
||||
};
|
||||
let shown = format!("{:?}", keys);
|
||||
assert!(!shown.contains("test-secret-not-real"), "{shown}");
|
||||
assert!(!shown.contains("test-session-token-not-real"), "{shown}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn aws_key_service_names_reject_bad_ids() {
|
||||
for bad in ["../x", "", &"a".repeat(65)] {
|
||||
assert!(aws_connection_service(bad).is_err(), "{bad:?}");
|
||||
assert!(get_aws_keys(bad).is_err());
|
||||
assert!(delete_aws_keys(bad).is_err());
|
||||
}
|
||||
assert_eq!(
|
||||
aws_connection_service("0b9e6a2c-1111").unwrap(),
|
||||
"triple-c-aws-connection-0b9e6a2c-1111"
|
||||
);
|
||||
}
|
||||
|
||||
/// There is no mock keyring here, so the "no entry is fine" rule is tested
|
||||
/// as the pure mapping `delete_entry` applies.
|
||||
#[test]
|
||||
fn removing_a_keys_connection_with_no_keyring_entry_succeeds() {
|
||||
assert_eq!(delete_outcome(Err(keyring::Error::NoEntry), "x"), Ok(()));
|
||||
assert_eq!(delete_outcome(Ok(()), "x"), Ok(()));
|
||||
let err = delete_outcome(Err(keyring::Error::NoStorageAccess("denied".into())), "x");
|
||||
assert!(err.is_err());
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user