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 Nov 2, 2024
1 parent b600ead commit c31e34a
Show file tree
Hide file tree
Showing 14 changed files with 72 additions and 65 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<'a>(
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<'a>(

// 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
17 changes: 17 additions & 0 deletions client/src/dbus/api/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,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/plain" => Ok(Secret::Text(String::from_utf8(value.to_vec())?)),
"application/octet-stream" => 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
7 changes: 3 additions & 4 deletions client/src/keyring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::{collections::HashMap, sync::Arc, time::Duration};
use async_lock::RwLock;
#[cfg(feature = "tokio")]
use tokio::sync::RwLock;
use zeroize::Zeroizing;

use crate::{dbus, portal, AsAttributes, Result, Secret};

Expand Down Expand Up @@ -265,7 +264,7 @@ impl Item {
}

/// Retrieves the stored secret.
pub async fn secret(&self) -> Result<Zeroizing<Vec<u8>>> {
pub async fn secret(&self) -> Result<Secret> {
let secret = match self {
Self::File(item, _) => item.read().await.secret(),
Self::DBus(item) => item.secret().await?,
Expand Down Expand Up @@ -366,7 +365,7 @@ mod tests {
assert_eq!(items.len(), 1);
let item = items.remove(0);
assert_eq!(item.label().await?, "my item");
assert_eq!(*item.secret().await?, b"my_secret");
assert_eq!(item.secret().await?, Secret::blob("my_secret"));
let attrs = item.attributes().await?;
assert_eq!(attrs.len(), 1);
assert_eq!(attrs.get("key").unwrap(), "value");
Expand All @@ -378,7 +377,7 @@ mod tests {
assert_eq!(items.len(), 1);
let item = items.remove(0);
assert_eq!(item.label().await?, "my item");
assert_eq!(*item.secret().await?, b"my_secret");
assert_eq!(item.secret().await?, Secret::blob("my_secret"));
let attrs = item.attributes().await?;
assert_eq!(attrs.len(), 2);
assert_eq!(attrs.get("key").unwrap(), "changed_value");
Expand Down
5 changes: 2 additions & 3 deletions client/src/portal/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/portal/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/portal/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
Loading

0 comments on commit c31e34a

Please sign in to comment.