Skip to content

Commit

Permalink
client: Return Secret instead of a blob
Browse files Browse the repository at this point in the history
Makes uses of the content-type from the DBus API and just hack
around for the portal backend until we figure out whether
it makes sense to store the content-type as an attribute
  • Loading branch information
bilelmoussaoui committed Dec 4, 2024
1 parent 3bc4434 commit 3fbbf46
Show file tree
Hide file tree
Showing 14 changed files with 83 additions and 68 deletions.
14 changes: 7 additions & 7 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,22 +245,22 @@ async fn print_item(
as_hex: bool,
) -> Result<(), Error> {
use std::fmt::Write;
let secret = item.secret().await?;
let bytes = secret.as_bytes();
if secret_only {
let bytes = item.secret().await?;
let mut stdout = std::io::stdout().lock();
if as_hex {
let hex = hex::encode(&bytes);
let hex = hex::encode(bytes);
stdout.write_all(hex.as_bytes())?;
} else {
stdout.write_all(&bytes)?;
stdout.write_all(bytes)?;
}
// Add a new line if we are writing to a tty
if stdout.is_terminal() {
stdout.write_all(b"\n")?;
}
} else {
let label = item.label().await?;
let bytes = item.secret().await?;
let mut attributes = item.attributes().await?;
let created = item.created().await?;
let modified = item.modified().await?;
Expand All @@ -277,15 +277,15 @@ async fn print_item(

// we still fallback to hex if it is not a string
if as_hex {
let hex = hex::encode(&bytes);
let hex = hex::encode(bytes);
writeln!(&mut result, "secret = {hex}").unwrap();
} else {
match std::str::from_utf8(&bytes) {
match std::str::from_utf8(bytes) {
Ok(secret) => {
writeln!(&mut result, "secret = {secret}").unwrap();
}
Err(_) => {
let hex = hex::encode(&bytes);
let hex = hex::encode(bytes);
writeln!(&mut result, "secret = {hex}").unwrap();
}
}
Expand Down
2 changes: 1 addition & 1 deletion client/examples/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ async fn main() -> oo7::Result<()> {
let keyring = Keyring::new().await?;
let attributes = HashMap::from([("attr", "value")]);
keyring
.create_item("Some Label", &attributes, b"secret", true)
.create_item("Some Label", &attributes, "secret", true)
.await?;

let items = keyring.search_items(&attributes).await?;
Expand Down
2 changes: 1 addition & 1 deletion client/examples/basic_2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ async fn main() -> oo7::Result<()> {
KEYRING
.get()
.unwrap()
.create_item("Some Label", &attributes, b"secret", true)
.create_item("Some Label", &attributes, "secret", true)
.await?;

let items = KEYRING.get().unwrap().search_items(&attributes).await?;
Expand Down
24 changes: 23 additions & 1 deletion client/src/dbus/api/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ use zbus::zvariant::{OwnedObjectPath, Type};
use zeroize::{Zeroize, ZeroizeOnDrop};

use super::Session;
use crate::{crypto, dbus::Error, Key, Secret};
use crate::{
crypto,
dbus::Error,
secret::{BLOB_CONTENT_TYPE, TEXT_CONTENT_TYPE},
Key, Secret,
};

#[derive(Debug, Serialize, Deserialize, Type)]
#[zvariant(signature = "(oayays)")]
Expand Down Expand Up @@ -61,6 +66,23 @@ impl<'a> DBusSecret<'a> {
})
}

pub(crate) fn decrypt(&self, key: Option<&Arc<Key>>) -> Result<Secret, Error> {
let value = match key {
Some(key) => &crypto::decrypt(&self.value, key, &self.parameters),
None => &self.value,
};

match self.content_type.as_str() {
TEXT_CONTENT_TYPE => Ok(Secret::Text(String::from_utf8(value.to_vec())?)),
BLOB_CONTENT_TYPE => Ok(Secret::blob(value)),
e => {
#[cfg(feature = "tracing")]
tracing::warn!("Unsupported content-type {e}, falling back to blob");
Ok(Secret::blob(value))
}
}
}

/// Session used to encode the secret
pub fn session(&self) -> &Session {
&self.session
Expand Down
6 changes: 3 additions & 3 deletions client/src/dbus/collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,18 +269,18 @@ mod tests {
"plain-type-test"
};
attributes.insert("type", value);
let secret = "a password";
let secret = crate::Secret::text("a password");

let collection = service.default_collection().await.unwrap();
let n_items = collection.items().await.unwrap().len();
let n_search_items = collection.search_items(&attributes).await.unwrap().len();

let item = collection
.create_item("A secret", &attributes, secret, true, None)
.create_item("A secret", &attributes, secret.clone(), true, None)
.await
.unwrap();

assert_eq!(*item.secret().await.unwrap(), secret.as_bytes());
assert_eq!(item.secret().await.unwrap(), secret);
assert_eq!(item.attributes().await.unwrap()["type"], value);

assert_eq!(collection.items().await.unwrap().len(), n_items + 1);
Expand Down
11 changes: 10 additions & 1 deletion client/src/dbus/error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::fmt;
use std::{fmt, string::FromUtf8Error};

/// DBus Secret Service specific errors.
/// <https://specifications.freedesktop.org/secret-service-spec/latest/errors.html>
Expand Down Expand Up @@ -31,6 +31,8 @@ pub enum Error {
NotFound(String),
/// Input/Output.
IO(std::io::Error),
/// Secret to string conversion failure.
Utf8(FromUtf8Error),
}

impl From<zbus::Error> for Error {
Expand Down Expand Up @@ -63,6 +65,12 @@ impl From<std::io::Error> for Error {
}
}

impl From<FromUtf8Error> for Error {
fn from(value: FromUtf8Error) -> Self {
Self::Utf8(value)
}
}

impl std::error::Error for Error {}

impl fmt::Display for Error {
Expand All @@ -74,6 +82,7 @@ impl fmt::Display for Error {
Self::Deleted => write!(f, "Item/Collection was deleted, can no longer be used"),
Self::NotFound(name) => write!(f, "The collection '{name}' doesn't exists"),
Self::Dismissed => write!(f, "Prompt was dismissed"),
Self::Utf8(e) => write!(f, "Failed to convert a text/plain secret to string, {e}"),
}
}
}
22 changes: 6 additions & 16 deletions client/src/dbus/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@ use async_lock::RwLock;
#[cfg(feature = "tokio")]
use tokio::sync::RwLock;
use zbus::zvariant::ObjectPath;
use zeroize::Zeroizing;

use super::{api, Algorithm, Error};
use crate::{crypto, AsAttributes, Key, Secret};
use crate::{AsAttributes, Key, Secret};

/// A secret with a label and attributes to identify it.
///
Expand Down Expand Up @@ -134,23 +133,14 @@ impl<'a> Item<'a> {
}

/// Retrieve the currently stored secret.
pub async fn secret(&self) -> Result<Zeroizing<Vec<u8>>, Error> {
pub async fn secret(&self) -> Result<Secret, Error> {
if !self.is_available().await {
Err(Error::Deleted)
} else {
let secret = self.inner.secret(&self.session).await?;

let value = match self.algorithm {
Algorithm::Plain => Zeroizing::new(secret.value.to_owned()),
Algorithm::Encrypted => {
let iv = &secret.parameters;
// Safe unwrap as it is encrypted
let aes_key = self.aes_key.as_ref().unwrap();

crypto::decrypt(&secret.value, aes_key, iv)
}
};
Ok(value)
self.inner
.secret(&self.session)
.await?
.decrypt(self.aes_key.as_ref())
}
}

Expand Down
4 changes: 2 additions & 2 deletions client/src/dbus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
//! let collection = service.default_collection().await?;
//! // Store a secret
//! collection
//! .create_item("My App's secret", &attributes, b"password", true, None)
//! .create_item("My App's secret", &attributes, "password", true, None)
//! .await?;
//!
//! // Retrieve it later thanks to it attributes
//! let items = collection.search_items(&attributes).await?;
//! let item = items.first().unwrap();
//! assert_eq!(*item.secret().await?, b"password");
//! assert_eq!(item.secret().await?, oo7::Secret::text("password"));
//!
//! # Ok(())
//! # }
Expand Down
5 changes: 2 additions & 3 deletions client/src/file/api/legacy_keyring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,12 @@ mod tests {
.join("legacy.keyring");
let blob = std::fs::read(path)?;
let keyring = Keyring::try_from(blob.as_slice())?;
let password = b"test";
let secret = Secret::from(password.to_vec());
let secret = Secret::blob("test");
let items = keyring.decrypt_items(&secret)?;

assert_eq!(items.len(), 1);
assert_eq!(items[0].label(), "foo");
assert_eq!(items[0].secret().as_ref(), b"foo".to_vec());
assert_eq!(items[0].secret(), Secret::blob("foo"));
let attributes = items[0].attributes();
assert_eq!(attributes.len(), 1);
assert_eq!(
Expand Down
6 changes: 3 additions & 3 deletions client/src/file/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ mod tests {

keyring
.items
.push(Item::new("Label", &needle, b"MyPassword").encrypt(&key)?);
.push(Item::new("Label", &needle, Secret::blob("MyPassword")).encrypt(&key)?);

assert_eq!(keyring.search_items(&needle, &key)?.len(), 1);

Expand All @@ -363,7 +363,7 @@ mod tests {
Item::new(
"My Label",
&HashMap::from([("my-tag", "my tag value")]),
"A Password".as_bytes(),
"A Password",
)
.encrypt(&key)?,
);
Expand All @@ -375,7 +375,7 @@ mod tests {
let loaded_items =
loaded_keyring.search_items(&HashMap::from([("my-tag", "my tag value")]), &key)?;

assert_eq!(*loaded_items[0].secret(), "A Password".as_bytes());
assert_eq!(loaded_items[0].secret(), Secret::blob("A Password"));

let _silent = std::fs::remove_file("/tmp/test.keyring");

Expand Down
4 changes: 2 additions & 2 deletions client/src/file/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ impl Item {
}

/// Retrieve the currently stored secret.
pub fn secret(&self) -> Zeroizing<Vec<u8>> {
Zeroizing::new(self.secret.clone())
pub fn secret(&self) -> Secret {
Secret::blob(&self.secret)
}

/// Store a new secret.
Expand Down
35 changes: 14 additions & 21 deletions client/src/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
//! .create_item(
//! "My Label",
//! &HashMap::from([("account", "alice")]),
//! b"My Password",
//! "My Password",
//! true,
//! )
//! .await?;
//!
//! let items = keyring
//! .search_items(&HashMap::from([("account", "alice")]))
//! .await?;
//! assert_eq!(*items[0].secret(), b"My Password");
//! assert_eq!(items[0].secret(), oo7::Secret::blob("My Password"));
//!
//! keyring
//! .delete(&HashMap::from([("account", "alice")]))
Expand Down Expand Up @@ -48,7 +48,6 @@ use tokio::{
io::AsyncReadExt,
sync::{Mutex, RwLock},
};
use zeroize::Zeroizing;

use crate::{AsAttributes, Key, Secret};

Expand All @@ -66,7 +65,7 @@ mod item;
pub use error::{Error, InvalidItemError, WeakKeyError};
pub use item::Item;

type ItemDefinition = (String, HashMap<String, String>, Zeroizing<Vec<u8>>, bool);
type ItemDefinition = (String, HashMap<String, String>, Secret, bool);

/// File backed keyring.
#[derive(Debug)]
Expand Down Expand Up @@ -575,7 +574,7 @@ mod tests {
let items = items.expect("unable to retrieve items");
assert_eq!(items.len(), 1);
assert_eq!(items[0].label(), "foo");
assert_eq!(items[0].secret().as_ref(), b"foo".to_vec());
assert_eq!(items[0].secret(), Secret::blob("foo"));
let attributes = items[0].attributes();
assert_eq!(attributes.len(), 1);
assert_eq!(
Expand Down Expand Up @@ -604,8 +603,7 @@ mod tests {

assert!(!v1_dir.join("default.keyring").exists());

let password = b"test";
let secret = Secret::from(password.to_vec());
let secret = Secret::blob("test");
let keyring = Keyring::open("default", secret).await?;

check_items(&keyring).await?;
Expand All @@ -630,8 +628,7 @@ mod tests {

std::env::set_var("XDG_DATA_HOME", data_dir.path());

let password = b"test";
let secret = Secret::from(password.to_vec());
let secret = Secret::blob("test");
let keyring = Keyring::open("default", secret).await?;

assert!(!v1_dir.join("default.keyring").exists());
Expand All @@ -658,15 +655,13 @@ mod tests {

std::env::set_var("XDG_DATA_HOME", data_dir.path());

let password = b"wrong";
let secret = Secret::from(password.to_vec());
let secret = Secret::blob("wrong");
let keyring = Keyring::open("default", secret).await;

assert!(keyring.is_err());
assert!(matches!(keyring.unwrap_err(), Error::IncorrectSecret));

let password = b"test";
let secret = Secret::from(password.to_vec());
let secret = Secret::blob("test");
let keyring = Keyring::open("default", secret).await;

assert!(keyring.is_ok());
Expand All @@ -688,8 +683,7 @@ mod tests {

std::env::set_var("XDG_DATA_HOME", data_dir.path());

let password = b"test";
let secret = Secret::from(password.to_vec());
let secret = Secret::blob("test");
let keyring = Keyring::open("default", secret).await?;

assert!(v1_dir.join("default.keyring").exists());
Expand All @@ -711,8 +705,7 @@ mod tests {

std::env::set_var("XDG_DATA_HOME", data_dir.path());

let password = b"test";
let secret = Secret::from(password.to_vec());
let secret = Secret::blob("test");
let keyring = Keyring::open("default", secret).await?;

assert!(!v1_dir.join("default.keyring").exists());
Expand Down Expand Up @@ -742,11 +735,11 @@ mod tests {
.create_item("test", &attributes, "password", false)
.await?;

let new_secret = Secret::from(b"password".to_vec());
keyring.change_secret(new_secret).await?;
let secret = Secret::blob("new_secret");
keyring.change_secret(secret).await?;

let new_secret = Secret::from(b"password".to_vec());
let keyring = Keyring::load(&path, new_secret).await?;
let secret = Secret::blob("new_secret");
let keyring = Keyring::load(&path, secret).await?;
let item_now = keyring.lookup_item(&attributes).await?.unwrap();

assert_eq!(item_before.label(), item_now.label());
Expand Down
Loading

0 comments on commit 3fbbf46

Please sign in to comment.