From 5730ac7922cd9030d1c8efd4111cdb09db195a67 Mon Sep 17 00:00:00 2001
From: Bilal Elmoussaoui <belmouss@redhat.com>
Date: Sat, 2 Nov 2024 13:31:40 +0100
Subject: [PATCH 1/3] client: Add a generic Secret type

Avoids the duplicated types we have all over the place.

- We still don't make use of the content-type when creating or
retrieving portal secrets
- We still don't make use of content-type when retrieving dbus items

But it is an improvement over the current status.

Fixes #133
---
 cargo-credential/src/main.rs      |  3 +-
 cli/src/main.rs                   |  2 +-
 client/src/dbus/api/collection.rs |  4 +-
 client/src/dbus/api/item.rs       | 10 ++--
 client/src/dbus/api/mod.rs        |  4 +-
 client/src/dbus/api/secret.rs     | 42 ++++++-------
 client/src/dbus/api/service.rs    |  9 ++-
 client/src/dbus/collection.rs     | 20 +++----
 client/src/dbus/item.rs           | 14 ++---
 client/src/dbus/mod.rs            |  9 +--
 client/src/file/item.rs           | 10 ++--
 client/src/file/mod.rs            | 10 ++--
 client/src/file/secret.rs         | 26 --------
 client/src/keyring.rs             | 19 +++---
 client/src/lib.rs                 |  2 +
 client/src/secret.rs              | 99 +++++++++++++++++++++++++++++++
 portal/src/main.rs                |  4 +-
 server/src/collection.rs          |  6 +-
 server/src/item.rs                | 17 +++---
 server/src/main.rs                |  5 +-
 server/src/service.rs             |  8 +--
 21 files changed, 184 insertions(+), 139 deletions(-)
 delete mode 100644 client/src/file/secret.rs
 create mode 100644 client/src/secret.rs

diff --git a/cargo-credential/src/main.rs b/cargo-credential/src/main.rs
index bfc8eb899..c9e5491ab 100644
--- a/cargo-credential/src/main.rs
+++ b/cargo-credential/src/main.rs
@@ -50,7 +50,7 @@ impl SecretServiceCredential {
                 let token = cargo_credential::read_token(options, registry)?.expose();
 
                 if let Some(item) = items.first() {
-                    item.set_secret(token, "text/utf8")
+                    item.set_secret(token)
                         .await
                         .map_err(|err| Error::Other(Box::new(err)))?;
                 } else {
@@ -60,7 +60,6 @@ impl SecretServiceCredential {
                             &attributes,
                             token,
                             true,
-                            "text/utf8",
                             None,
                         )
                         .await
diff --git a/cli/src/main.rs b/cli/src/main.rs
index 479b50e7c..1458456c1 100644
--- a/cli/src/main.rs
+++ b/cli/src/main.rs
@@ -184,7 +184,7 @@ impl Commands {
                 };
 
                 collection
-                    .create_item(&label, &attributes, &secret, true, "text/plain", None)
+                    .create_item(&label, &attributes, secret, true, None)
                     .await?;
             }
             Commands::Lock => collection.lock(None).await?,
diff --git a/client/src/dbus/api/collection.rs b/client/src/dbus/api/collection.rs
index d07d0a70f..ab397a01a 100644
--- a/client/src/dbus/api/collection.rs
+++ b/client/src/dbus/api/collection.rs
@@ -5,7 +5,7 @@ use futures_util::{Stream, StreamExt};
 use serde::Serialize;
 use zbus::zvariant::{ObjectPath, OwnedObjectPath, Type};
 
-use super::{Item, Prompt, Properties, Secret, Unlockable, DESTINATION};
+use super::{DBusSecret, Item, Prompt, Properties, Unlockable, DESTINATION};
 use crate::{
     dbus::{Error, ServiceError},
     AsAttributes,
@@ -165,7 +165,7 @@ impl<'a> Collection<'a> {
         &self,
         label: &str,
         attributes: &impl AsAttributes,
-        secret: &Secret<'_>,
+        secret: &DBusSecret<'_>,
         replace: bool,
         window_id: Option<WindowIdentifier>,
     ) -> Result<Item<'a>, Error> {
diff --git a/client/src/dbus/api/item.rs b/client/src/dbus/api/item.rs
index cb313439c..24679405e 100644
--- a/client/src/dbus/api/item.rs
+++ b/client/src/dbus/api/item.rs
@@ -4,7 +4,7 @@ use ashpd::WindowIdentifier;
 use serde::Serialize;
 use zbus::zvariant::{ObjectPath, OwnedObjectPath, Type};
 
-use super::{secret::SecretInner, Prompt, Secret, Session, Unlockable, DESTINATION};
+use super::{DBusSecret, Prompt, Session, Unlockable, DESTINATION};
 use crate::{
     dbus::{Error, ServiceError},
     AsAttributes,
@@ -121,19 +121,19 @@ impl<'a> Item<'a> {
     }
 
     #[doc(alias = "GetSecret")]
-    pub async fn secret(&self, session: &Session<'_>) -> Result<Secret<'_>, Error> {
+    pub async fn secret(&self, session: &Session<'_>) -> Result<DBusSecret<'_>, Error> {
         let inner = self
             .inner()
             .call_method("GetSecret", &(session))
             .await
             .map_err::<ServiceError, _>(From::from)?
             .body()
-            .deserialize::<SecretInner>()?;
-        Secret::from_inner(self.inner().connection(), inner).await
+            .deserialize::<super::secret::DBusSecretInner>()?;
+        DBusSecret::from_inner(self.inner().connection(), inner).await
     }
 
     #[doc(alias = "SetSecret")]
-    pub async fn set_secret(&self, secret: &Secret<'_>) -> Result<(), Error> {
+    pub async fn set_secret(&self, secret: &DBusSecret<'_>) -> Result<(), Error> {
         self.inner()
             .call_method("SetSecret", &(secret,))
             .await
diff --git a/client/src/dbus/api/mod.rs b/client/src/dbus/api/mod.rs
index a546a430a..0236511cf 100644
--- a/client/src/dbus/api/mod.rs
+++ b/client/src/dbus/api/mod.rs
@@ -29,9 +29,9 @@ pub(crate) use properties::Properties;
 #[cfg(feature = "unstable")]
 #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
 pub use properties::Properties;
-pub use secret::Secret;
+pub use secret::DBusSecret;
 #[cfg(feature = "unstable")]
 #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
-pub use secret::SecretInner;
+pub use secret::DBusSecretInner;
 pub use service::Service;
 pub use session::Session;
diff --git a/client/src/dbus/api/secret.rs b/client/src/dbus/api/secret.rs
index 87371c500..9dd639a0f 100644
--- a/client/src/dbus/api/secret.rs
+++ b/client/src/dbus/api/secret.rs
@@ -5,15 +5,16 @@ use zbus::zvariant::{OwnedObjectPath, Type};
 use zeroize::{Zeroize, ZeroizeOnDrop};
 
 use super::Session;
-use crate::{crypto, dbus::Error, Key};
+use crate::{crypto, dbus::Error, Key, Secret};
 
 #[derive(Debug, Serialize, Deserialize, Type)]
 #[zvariant(signature = "(oayays)")]
-pub struct SecretInner(pub OwnedObjectPath, pub Vec<u8>, pub Vec<u8>, pub String);
+/// Same as [`DBusSecret`] without tying the session path to a [`Session`] type.
+pub struct DBusSecretInner(pub OwnedObjectPath, pub Vec<u8>, pub Vec<u8>, pub String);
 
 #[derive(Debug, Type, Zeroize, ZeroizeOnDrop)]
 #[zvariant(signature = "(oayays)")]
-pub struct Secret<'a> {
+pub struct DBusSecret<'a> {
     #[zeroize(skip)]
     pub(crate) session: Arc<Session<'a>>,
     pub(crate) parameters: Vec<u8>,
@@ -22,47 +23,42 @@ pub struct Secret<'a> {
     pub(crate) content_type: String,
 }
 
-impl<'a> Secret<'a> {
-    pub(crate) fn new(
-        session: Arc<Session<'a>>,
-        secret: impl AsRef<[u8]>,
-        content_type: &str,
-    ) -> Self {
+impl<'a> DBusSecret<'a> {
+    pub(crate) fn new(session: Arc<Session<'a>>, secret: impl Into<Secret>) -> Self {
+        let secret = secret.into();
         Self {
             session,
             parameters: vec![],
-            value: secret.as_ref().to_vec(),
-            content_type: content_type.to_owned(),
+            value: secret.as_bytes().to_vec(),
+            content_type: secret.content_type().to_owned(),
         }
     }
 
     pub(crate) fn new_encrypted(
         session: Arc<Session<'a>>,
-        secret: impl AsRef<[u8]>,
-        content_type: &str,
+        secret: impl Into<Secret>,
         aes_key: &Key,
     ) -> Self {
         let iv = crypto::generate_iv();
-        let secret = crypto::encrypt(secret.as_ref(), aes_key, &iv);
+        let secret = secret.into();
         Self {
             session,
+            value: crypto::encrypt(secret.as_bytes(), aes_key, &iv),
             parameters: iv,
-            value: secret,
-            content_type: content_type.to_owned(),
+            content_type: secret.content_type().to_owned(),
         }
     }
 
     pub(crate) async fn from_inner(
         cnx: &zbus::Connection,
-        inner: SecretInner,
-    ) -> Result<Secret<'_>, Error> {
-        let secret = Secret {
+        inner: DBusSecretInner,
+    ) -> Result<Self, Error> {
+        Ok(Self {
             session: Arc::new(Session::new(cnx, inner.0).await?),
             parameters: inner.1,
             value: inner.2,
             content_type: inner.3,
-        };
-        Ok(secret)
+        })
     }
 
     /// Session used to encode the secret
@@ -86,7 +82,7 @@ impl<'a> Secret<'a> {
     }
 }
 
-impl Serialize for Secret<'_> {
+impl Serialize for DBusSecret<'_> {
     fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
@@ -106,6 +102,6 @@ mod tests {
 
     #[test]
     fn signature() {
-        assert_eq!(Secret::SIGNATURE, "(oayays)");
+        assert_eq!(DBusSecret::SIGNATURE, "(oayays)");
     }
 }
diff --git a/client/src/dbus/api/service.rs b/client/src/dbus/api/service.rs
index d10815ed5..78aca075e 100644
--- a/client/src/dbus/api/service.rs
+++ b/client/src/dbus/api/service.rs
@@ -5,8 +5,7 @@ use futures_util::{Stream, StreamExt};
 use zbus::zvariant::{ObjectPath, OwnedObjectPath, OwnedValue, Type, Value};
 
 use super::{
-    secret::SecretInner, Collection, Item, Prompt, Properties, Secret, Session, Unlockable,
-    DESTINATION, PATH,
+    Collection, DBusSecret, Item, Prompt, Properties, Session, Unlockable, DESTINATION, PATH,
 };
 use crate::{
     dbus::{Algorithm, Error, ServiceError},
@@ -211,14 +210,14 @@ impl<'a> Service<'a> {
         &self,
         items: &[Item<'_>],
         session: &Session<'_>,
-    ) -> Result<HashMap<Item<'_>, Secret<'_>>, Error> {
+    ) -> Result<HashMap<Item<'_>, DBusSecret<'_>>, Error> {
         let secrets = self
             .inner()
             .call_method("GetSecrets", &(items, session))
             .await
             .map_err::<ServiceError, _>(From::from)?
             .body()
-            .deserialize::<HashMap<OwnedObjectPath, SecretInner>>()?;
+            .deserialize::<HashMap<OwnedObjectPath, super::secret::DBusSecretInner>>()?;
 
         let cnx = self.inner().connection();
         // Item's Hash implementation doesn't make use of any mutable internals
@@ -227,7 +226,7 @@ impl<'a> Service<'a> {
         for (path, secret_inner) in secrets {
             output.insert(
                 Item::new(cnx, path).await?,
-                Secret::from_inner(cnx, secret_inner).await?,
+                DBusSecret::from_inner(cnx, secret_inner).await?,
             );
         }
 
diff --git a/client/src/dbus/collection.rs b/client/src/dbus/collection.rs
index 64cfc1fd3..82b453c29 100644
--- a/client/src/dbus/collection.rs
+++ b/client/src/dbus/collection.rs
@@ -9,7 +9,7 @@ use tokio::sync::RwLock;
 use zbus::zvariant::{ObjectPath, OwnedObjectPath};
 
 use super::{api, Algorithm, Error, Item};
-use crate::{AsAttributes, Key};
+use crate::{AsAttributes, Key, Secret};
 
 /// A collection allows to store and retrieve items.
 ///
@@ -151,28 +151,22 @@ impl<'a> Collection<'a> {
     /// * `secret` - The secret to store.
     /// * `replace` - Whether to replace the value if the `attributes` matches
     ///   an existing `secret`.
-    /// * `content_type` - The content type of the secret, usually something
-    ///   like `text/plain`.
     pub async fn create_item(
         &self,
         label: &str,
         attributes: &impl AsAttributes,
-        secret: impl AsRef<[u8]>,
+        secret: impl Into<Secret>,
         replace: bool,
-        content_type: &str,
         window_id: Option<WindowIdentifier>,
     ) -> Result<Item<'a>, Error> {
         if !self.is_available().await {
             Err(Error::Deleted)
         } else {
             let secret = match self.algorithm {
-                Algorithm::Plain => {
-                    api::Secret::new(Arc::clone(&self.session), secret, content_type)
-                }
-                Algorithm::Encrypted => api::Secret::new_encrypted(
+                Algorithm::Plain => api::DBusSecret::new(Arc::clone(&self.session), secret),
+                Algorithm::Encrypted => api::DBusSecret::new_encrypted(
                     Arc::clone(&self.session),
                     secret,
-                    content_type,
                     self.aes_key.as_ref().unwrap(),
                 ),
             };
@@ -275,18 +269,18 @@ mod tests {
             "plain-type-test"
         };
         attributes.insert("type", value);
-        let secret = "a password".as_bytes();
+        let secret = "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, "text/plain", None)
+            .create_item("A secret", &attributes, secret, true, None)
             .await
             .unwrap();
 
-        assert_eq!(*item.secret().await.unwrap(), secret);
+        assert_eq!(*item.secret().await.unwrap(), secret.as_bytes());
         assert_eq!(item.attributes().await.unwrap()["type"], value);
 
         assert_eq!(collection.items().await.unwrap().len(), n_items + 1);
diff --git a/client/src/dbus/item.rs b/client/src/dbus/item.rs
index f19258a71..ea4d7e659 100644
--- a/client/src/dbus/item.rs
+++ b/client/src/dbus/item.rs
@@ -9,7 +9,7 @@ use zbus::zvariant::ObjectPath;
 use zeroize::Zeroizing;
 
 use super::{api, Algorithm, Error};
-use crate::{crypto, AsAttributes, Key};
+use crate::{crypto, AsAttributes, Key, Secret};
 
 /// A secret with a label and attributes to identify it.
 ///
@@ -159,19 +159,13 @@ impl<'a> Item<'a> {
     /// # Arguments
     ///
     /// * `secret` - The secret to store.
-    /// * `content_type` - The content type of the secret, usually something
-    ///   like `text/plain`.
     #[doc(alias = "SetSecret")]
-    pub async fn set_secret(
-        &self,
-        secret: impl AsRef<[u8]>,
-        content_type: &str,
-    ) -> Result<(), Error> {
+    pub async fn set_secret(&self, secret: impl Into<Secret>) -> Result<(), Error> {
         let secret = match self.algorithm {
-            Algorithm::Plain => api::Secret::new(Arc::clone(&self.session), secret, content_type),
+            Algorithm::Plain => api::DBusSecret::new(Arc::clone(&self.session), secret),
             Algorithm::Encrypted => {
                 let aes_key = self.aes_key.as_ref().unwrap();
-                api::Secret::new_encrypted(Arc::clone(&self.session), secret, content_type, aes_key)
+                api::DBusSecret::new_encrypted(Arc::clone(&self.session), secret, aes_key)
             }
         };
         self.inner.set_secret(&secret).await?;
diff --git a/client/src/dbus/mod.rs b/client/src/dbus/mod.rs
index 1e4291729..2a23afc10 100644
--- a/client/src/dbus/mod.rs
+++ b/client/src/dbus/mod.rs
@@ -14,14 +14,7 @@
 //! let collection = service.default_collection().await?;
 //! // Store a secret
 //! collection
-//!     .create_item(
-//!         "My App's secret",
-//!         &attributes,
-//!         b"password",
-//!         true,
-//!         "text/plain",
-//!         None,
-//!     )
+//!     .create_item("My App's secret", &attributes, b"password", true, None)
 //!     .await?;
 //!
 //! // Retrieve it later thanks to it attributes
diff --git a/client/src/file/item.rs b/client/src/file/item.rs
index 76306fe48..3021a707e 100644
--- a/client/src/file/item.rs
+++ b/client/src/file/item.rs
@@ -7,7 +7,7 @@ use super::{
     api::{AttributeValue, EncryptedItem, GVARIANT_ENCODING},
     Error,
 };
-use crate::{crypto, AsAttributes, Key};
+use crate::{crypto, AsAttributes, Key, Secret};
 
 /// An item stored in the file backend.
 #[derive(Deserialize, Serialize, zvariant::Type, Clone, Debug, Zeroize, ZeroizeOnDrop)]
@@ -27,7 +27,7 @@ impl Item {
     pub(crate) fn new(
         label: impl ToString,
         attributes: &impl AsAttributes,
-        secret: impl AsRef<[u8]>,
+        secret: impl Into<Secret>,
     ) -> Self {
         let now = std::time::SystemTime::UNIX_EPOCH
             .elapsed()
@@ -43,7 +43,7 @@ impl Item {
             label: label.to_string(),
             created: now,
             modified: now,
-            secret: secret.as_ref().to_vec(),
+            secret: secret.into().as_bytes().to_vec(),
         }
     }
 
@@ -81,12 +81,12 @@ impl Item {
     }
 
     /// Store a new secret.
-    pub fn set_secret(&mut self, secret: impl AsRef<[u8]>) {
+    pub fn set_secret(&mut self, secret: impl Into<Secret>) {
         self.modified = std::time::SystemTime::UNIX_EPOCH
             .elapsed()
             .unwrap()
             .as_secs();
-        self.secret = secret.as_ref().to_vec();
+        self.secret = secret.into().as_bytes().to_vec();
     }
 
     /// The UNIX time when the item was created.
diff --git a/client/src/file/mod.rs b/client/src/file/mod.rs
index c0363813c..6f15ade09 100644
--- a/client/src/file/mod.rs
+++ b/client/src/file/mod.rs
@@ -50,7 +50,7 @@ use tokio::{
 };
 use zeroize::Zeroizing;
 
-use crate::{AsAttributes, Key};
+use crate::{AsAttributes, Key, Secret};
 
 #[cfg(feature = "unstable")]
 #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
@@ -62,11 +62,9 @@ pub(crate) use api::AttributeValue;
 
 mod error;
 mod item;
-mod secret;
 
 pub use error::{Error, InvalidItemError, WeakKeyError};
 pub use item::Item;
-pub use secret::Secret;
 
 type ItemDefinition = (String, HashMap<String, String>, Zeroizing<Vec<u8>>, bool);
 
@@ -302,7 +300,7 @@ impl Keyring {
         &self,
         label: &str,
         attributes: &impl AsAttributes,
-        secret: impl AsRef<[u8]>,
+        secret: impl Into<Secret>,
         replace: bool,
     ) -> Result<Item, Error> {
         let item = {
@@ -367,7 +365,7 @@ impl Keyring {
             if replace {
                 keyring.remove_items(&attributes, &key)?;
             }
-            let item = Item::new(label, &attributes, &*secret);
+            let item = Item::new(label, &attributes, secret);
             let encrypted_item = item.encrypt(&key)?;
             keyring.items.push(encrypted_item);
         }
@@ -723,7 +721,7 @@ mod tests {
             .create_item(
                 "foo",
                 &HashMap::from([(crate::XDG_SCHEMA_ATTRIBUTE, "org.gnome.keyring.Note")]),
-                b"foo",
+                "foo",
                 false,
             )
             .await?;
diff --git a/client/src/file/secret.rs b/client/src/file/secret.rs
deleted file mode 100644
index 63c735c4f..000000000
--- a/client/src/file/secret.rs
+++ /dev/null
@@ -1,26 +0,0 @@
-use rand::Rng;
-use zeroize::{Zeroize, ZeroizeOnDrop};
-
-/// Secret used to unlock the keyring.
-#[derive(Debug, Zeroize, ZeroizeOnDrop)]
-pub struct Secret(Vec<u8>);
-
-impl From<Vec<u8>> for Secret {
-    fn from(secret: Vec<u8>) -> Self {
-        Self(secret)
-    }
-}
-
-impl std::ops::Deref for Secret {
-    type Target = [u8];
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
-impl Secret {
-    pub fn random() -> Self {
-        Self(rand::thread_rng().gen::<[u8; 8]>().to_vec())
-    }
-}
diff --git a/client/src/keyring.rs b/client/src/keyring.rs
index 78af0cd82..88479ec8e 100644
--- a/client/src/keyring.rs
+++ b/client/src/keyring.rs
@@ -6,7 +6,7 @@ use async_lock::RwLock;
 use tokio::sync::RwLock;
 use zeroize::Zeroizing;
 
-use crate::{dbus, file, AsAttributes, Result};
+use crate::{dbus, file, AsAttributes, Result, Secret};
 
 /// A [Secret Service](crate::dbus) or [file](crate::file) backed keyring
 /// implementation.
@@ -118,13 +118,13 @@ impl Keyring {
         &self,
         label: &str,
         attributes: &impl AsAttributes,
-        secret: impl AsRef<[u8]>,
+        secret: impl Into<Secret>,
         replace: bool,
     ) -> Result<()> {
         match self {
             Self::DBus(backend) => {
                 backend
-                    .create_item(label, attributes, secret, replace, "text/plain", None)
+                    .create_item(label, attributes, secret, replace, None)
                     .await?;
             }
             Self::File(backend) => {
@@ -194,7 +194,7 @@ impl Item {
                     .create_item(
                         item_guard.label(),
                         &item_guard.attributes(),
-                        &*item_guard.secret(),
+                        item_guard.secret(),
                         true,
                     )
                     .await?;
@@ -234,7 +234,7 @@ impl Item {
                     backend.replace_item_index(index, &item_guard).await?;
                 } else {
                     backend
-                        .create_item(item_guard.label(), attributes, &*item_guard.secret(), true)
+                        .create_item(item_guard.label(), attributes, item_guard.secret(), true)
                         .await?;
                 }
             }
@@ -244,7 +244,7 @@ impl Item {
     }
 
     /// Sets a new secret.
-    pub async fn set_secret(&self, secret: impl AsRef<[u8]>) -> Result<()> {
+    pub async fn set_secret(&self, secret: impl Into<Secret>) -> Result<()> {
         match self {
             Self::File(item, backend) => {
                 item.write().await.set_secret(secret);
@@ -254,12 +254,12 @@ impl Item {
                     .create_item(
                         item_guard.label(),
                         &item_guard.attributes(),
-                        &*item_guard.secret(),
+                        item_guard.secret(),
                         true,
                     )
                     .await?;
             }
-            Self::DBus(item) => item.set_secret(secret, "text/plain").await?,
+            Self::DBus(item) => item.set_secret(secret).await?,
         };
         Ok(())
     }
@@ -352,8 +352,7 @@ mod tests {
         fs::create_dir_all(&dir).await.unwrap();
         let path = dir.join("default.keyring");
 
-        let password = b"test";
-        let secret = file::Secret::from(password.to_vec());
+        let secret = crate::Secret::text("test");
         let keyring = Keyring::File(file::Keyring::load(&path, secret).await?.into());
 
         let items = keyring.items().await?;
diff --git a/client/src/lib.rs b/client/src/lib.rs
index a4159246e..a0e29e837 100644
--- a/client/src/lib.rs
+++ b/client/src/lib.rs
@@ -35,11 +35,13 @@ pub mod dbus;
 pub mod file;
 
 mod keyring;
+mod secret;
 
 pub use ashpd;
 pub use error::{Error, Result};
 pub use keyring::{Item, Keyring};
 pub use migration::migrate;
+pub use secret::Secret;
 pub use zbus;
 
 /// A schema attribute.
diff --git a/client/src/secret.rs b/client/src/secret.rs
new file mode 100644
index 000000000..c9747dc04
--- /dev/null
+++ b/client/src/secret.rs
@@ -0,0 +1,99 @@
+use rand::Rng;
+use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing};
+
+/// A safe wrapper around a combination of (secret, content-type).
+#[derive(Debug, Zeroize, ZeroizeOnDrop)]
+pub enum Secret {
+    /// Corresponds to `text/plain`
+    Text(String),
+    /// Corresponds to application/octet-stream
+    Blob(Vec<u8>),
+}
+
+impl Secret {
+    /// Generate a random secret, used when creating a session collection.
+    pub fn random() -> Self {
+        Self::Blob(rand::thread_rng().gen::<[u8; 8]>().to_vec())
+    }
+
+    /// Create a text secret, stored with `text/plain` content type.
+    pub fn text(value: impl AsRef<str>) -> Self {
+        Self::Text(value.as_ref().to_owned())
+    }
+
+    /// Create a blob secret, stored with `application/octet-stream` content
+    /// type.
+    pub fn blob(value: impl AsRef<[u8]>) -> Self {
+        Self::Blob(value.as_ref().to_owned())
+    }
+
+    pub fn content_type(&self) -> &'static str {
+        match self {
+            Self::Text(_) => "text/plain",
+            Self::Blob(_) => "application/octet-stream",
+        }
+    }
+
+    pub fn as_bytes(&self) -> &[u8] {
+        match self {
+            Self::Text(text) => text.as_bytes(),
+            Self::Blob(bytes) => bytes.as_ref(),
+        }
+    }
+}
+
+impl From<&[u8]> for Secret {
+    fn from(value: &[u8]) -> Self {
+        Self::blob(value)
+    }
+}
+
+impl From<Zeroizing<Vec<u8>>> for Secret {
+    fn from(value: Zeroizing<Vec<u8>>) -> Self {
+        Self::blob(value)
+    }
+}
+
+impl From<Vec<u8>> for Secret {
+    fn from(value: Vec<u8>) -> Self {
+        Self::blob(value)
+    }
+}
+
+impl From<&Vec<u8>> for Secret {
+    fn from(value: &Vec<u8>) -> Self {
+        Self::blob(value)
+    }
+}
+
+impl<const N: usize> From<&[u8; N]> for Secret {
+    fn from(value: &[u8; N]) -> Self {
+        Self::blob(value)
+    }
+}
+
+impl From<String> for Secret {
+    fn from(value: String) -> Self {
+        Self::text(value)
+    }
+}
+
+impl From<&str> for Secret {
+    fn from(value: &str) -> Self {
+        Self::text(value)
+    }
+}
+
+impl std::ops::Deref for Secret {
+    type Target = [u8];
+
+    fn deref(&self) -> &Self::Target {
+        self.as_bytes()
+    }
+}
+
+impl AsRef<[u8]> for Secret {
+    fn as_ref(&self) -> &[u8] {
+        self.as_bytes()
+    }
+}
diff --git a/portal/src/main.rs b/portal/src/main.rs
index dcb0097a7..93a86fce1 100644
--- a/portal/src/main.rs
+++ b/portal/src/main.rs
@@ -66,10 +66,8 @@ async fn send_secret_to_app(app_id: &AppID, fd: std::os::fd::OwnedFd) -> Result<
             .create_item(
                 &format!("Secret Portal token for {app_id}"),
                 &attributes,
-                &secret,
+                secret.clone(),
                 true,
-                // TODO Find a better one.
-                "text/plain",
                 None,
             )
             .await?;
diff --git a/server/src/collection.rs b/server/src/collection.rs
index 922bc597f..49e409522 100644
--- a/server/src/collection.rs
+++ b/server/src/collection.rs
@@ -8,7 +8,7 @@ use std::{
 
 use oo7::{
     dbus::{
-        api::{Properties, SecretInner},
+        api::{DBusSecretInner, Properties},
         ServiceError,
     },
     file::Keyring,
@@ -76,12 +76,12 @@ impl Collection {
     pub async fn create_item(
         &self,
         properties: Properties,
-        secret: SecretInner,
+        secret: DBusSecretInner,
         replace: bool,
         #[zbus(object_server)] object_server: &zbus::ObjectServer,
         #[zbus(signal_emitter)] signal_emitter: zbus::object_server::SignalEmitter<'_>,
     ) -> Result<(OwnedObjectPath, OwnedObjectPath), ServiceError> {
-        let SecretInner(session, iv, secret, _content_type) = secret;
+        let DBusSecretInner(session, iv, secret, _content_type) = secret;
         let label = properties.label();
         // Safe to unwrap as an item always has attributes
         let attributes = properties.attributes().unwrap();
diff --git a/server/src/item.rs b/server/src/item.rs
index e42cf1fa0..accadcd07 100644
--- a/server/src/item.rs
+++ b/server/src/item.rs
@@ -6,7 +6,7 @@ use std::{
 };
 
 use oo7::{
-    dbus::{api::SecretInner, ServiceError},
+    dbus::{api::DBusSecretInner, ServiceError},
     file,
 };
 use tokio::sync::Mutex;
@@ -58,7 +58,7 @@ impl Item {
     pub async fn get_secret(
         &self,
         session: OwnedObjectPath,
-    ) -> Result<(SecretInner,), ServiceError> {
+    ) -> Result<(DBusSecretInner,), ServiceError> {
         let Some(session) = self.service.session(&session).await else {
             tracing::error!("The session `{}` does not exist.", session);
             return Err(ServiceError::NoSession(format!(
@@ -77,6 +77,7 @@ impl Item {
 
         let inner = self.inner.lock().await;
         let secret = inner.secret();
+        let content_type = secret.content_type().to_owned();
 
         tracing::debug!("Secret retrieved from the item: {}.", self.path);
 
@@ -85,24 +86,24 @@ impl Item {
                 let iv = oo7::crypto::generate_iv();
                 let encrypted = oo7::crypto::encrypt(secret, &key, &iv);
 
-                Ok((SecretInner(
+                Ok((DBusSecretInner(
                     session.path().clone(),
                     iv,
                     encrypted,
-                    "text/plain".to_owned(),
+                    content_type,
                 ),))
             }
-            None => Ok((SecretInner(
+            None => Ok((DBusSecretInner(
                 session.path().clone(),
                 Vec::new(),
                 secret.to_vec(),
-                "text/plain".to_owned(),
+                content_type,
             ),)),
         }
     }
 
-    pub async fn set_secret(&self, secret: SecretInner) -> Result<(), ServiceError> {
-        let SecretInner(session, iv, secret, _content_type) = secret;
+    pub async fn set_secret(&self, secret: DBusSecretInner) -> Result<(), ServiceError> {
+        let DBusSecretInner(session, iv, secret, _content_type) = secret;
 
         let Some(session) = self.service.session(&session).await else {
             tracing::error!("The session `{}` does not exist.", session);
diff --git a/server/src/main.rs b/server/src/main.rs
index 76ce325d4..26219e9e5 100644
--- a/server/src/main.rs
+++ b/server/src/main.rs
@@ -6,7 +6,6 @@ mod service;
 mod session;
 
 use clap::Parser;
-use oo7::file::Secret;
 use service::Service;
 
 use crate::error::Error;
@@ -31,7 +30,7 @@ struct Args {
 async fn main() -> Result<(), Error> {
     tracing_subscriber::fmt::init();
     let args = Args::parse();
-    let mut secret: Option<Secret> = None;
+    let mut secret = None;
 
     if args.login {
         let password = rpassword::prompt_password("Enter the login password: ")?;
@@ -39,7 +38,7 @@ async fn main() -> Result<(), Error> {
             tracing::error!("Login password can't be empty.");
             return Err(Error::EmptyPassword);
         }
-        secret = Some(Secret::from(password.into_bytes()));
+        secret = Some(oo7::Secret::text(password));
     }
 
     let mut flags = zbus::fdo::RequestNameFlags::AllowReplacement.into();
diff --git a/server/src/service.rs b/server/src/service.rs
index 921c07e68..d012214bd 100644
--- a/server/src/service.rs
+++ b/server/src/service.rs
@@ -5,11 +5,11 @@ use std::{collections::HashMap, sync::Arc};
 use enumflags2::BitFlags;
 use oo7::{
     dbus::{
-        api::{Properties, SecretInner},
+        api::{DBusSecretInner, Properties},
         Algorithm, ServiceError,
     },
-    file::{Keyring, Secret},
-    Key,
+    file::Keyring,
+    Key, Secret,
 };
 use tokio::sync::{Mutex, RwLock};
 use zbus::{
@@ -135,7 +135,7 @@ impl Service {
         &self,
         items: Vec<OwnedObjectPath>,
         session: OwnedObjectPath,
-    ) -> Result<HashMap<OwnedObjectPath, SecretInner>, ServiceError> {
+    ) -> Result<HashMap<OwnedObjectPath, DBusSecretInner>, ServiceError> {
         let mut secrets = HashMap::new();
         let collections = self.collections.lock().await;
 

From 3bc4434e4c36ff6d219c480f10a7ef3aa27e2e50 Mon Sep 17 00:00:00 2001
From: Bilal Elmoussaoui <belmouss@redhat.com>
Date: Sat, 2 Nov 2024 13:40:26 +0100
Subject: [PATCH 2/3] client: Use getrandom for generating a session secret

Share the same code as the portal key
---
 Cargo.lock            |  2 +-
 client/Cargo.toml     |  1 +
 client/src/secret.rs  | 11 +++++++----
 portal/Cargo.toml     |  1 -
 portal/src/error.rs   |  9 ---------
 portal/src/main.rs    | 30 +++++++++++-------------------
 server/src/service.rs |  2 +-
 7 files changed, 21 insertions(+), 35 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 95ff10a05..d71293e67 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1175,6 +1175,7 @@ dependencies = [
  "endi",
  "futures-lite",
  "futures-util",
+ "getrandom",
  "hkdf",
  "hmac",
  "md-5",
@@ -1228,7 +1229,6 @@ version = "0.3.0"
 dependencies = [
  "ashpd",
  "clap",
- "getrandom",
  "oo7",
  "tokio",
  "tracing",
diff --git a/client/Cargo.toml b/client/Cargo.toml
index 81acaf989..5ab7ae4b8 100644
--- a/client/Cargo.toml
+++ b/client/Cargo.toml
@@ -28,6 +28,7 @@ digest = { version = "0.10", optional = true }
 endi.workspace = true
 futures-lite = { workspace = true, optional = true }
 futures-util.workspace = true
+getrandom = "0.2"
 hkdf = { version = "0.12", optional = true }
 hmac = { version = "0.12", optional = true }
 md-5 = { version = "0.10", optional = true }
diff --git a/client/src/secret.rs b/client/src/secret.rs
index c9747dc04..45726941c 100644
--- a/client/src/secret.rs
+++ b/client/src/secret.rs
@@ -1,8 +1,7 @@
-use rand::Rng;
 use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing};
 
 /// A safe wrapper around a combination of (secret, content-type).
-#[derive(Debug, Zeroize, ZeroizeOnDrop)]
+#[derive(Debug, Clone, Zeroize, ZeroizeOnDrop)]
 pub enum Secret {
     /// Corresponds to `text/plain`
     Text(String),
@@ -12,8 +11,12 @@ pub enum Secret {
 
 impl Secret {
     /// Generate a random secret, used when creating a session collection.
-    pub fn random() -> Self {
-        Self::Blob(rand::thread_rng().gen::<[u8; 8]>().to_vec())
+    pub fn random() -> Result<Self, getrandom::Error> {
+        let mut secret = [0; 64];
+        // Equivalent of `ring::rand::SecureRandom`
+        getrandom::getrandom(&mut secret)?;
+
+        Ok(Self::blob(secret))
     }
 
     /// Create a text secret, stored with `text/plain` content type.
diff --git a/portal/Cargo.toml b/portal/Cargo.toml
index cfea310a1..b7b450bee 100644
--- a/portal/Cargo.toml
+++ b/portal/Cargo.toml
@@ -15,7 +15,6 @@ version.workspace = true
 ashpd = {workspace = true, features = ["backend", "tracing"]}
 clap.workspace = true
 oo7.workspace = true
-getrandom = "0.2"
 tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
 tracing.workspace = true
 tracing-subscriber.workspace = true
diff --git a/portal/src/error.rs b/portal/src/error.rs
index c6a91ea0c..a8d9ed4d7 100644
--- a/portal/src/error.rs
+++ b/portal/src/error.rs
@@ -1,6 +1,5 @@
 #[derive(Debug)]
 pub enum Error {
-    Rand(getrandom::Error),
     Oo7(oo7::dbus::Error),
     Io(std::io::Error),
     Portal(ashpd::PortalError),
@@ -9,7 +8,6 @@ pub enum Error {
 impl std::fmt::Display for Error {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::Rand(e) => f.write_fmt(format_args!("Rand error {e}")),
             Self::Oo7(e) => f.write_fmt(format_args!("DBus error: {e}")),
             Self::Io(e) => f.write_fmt(format_args!("IO error: {e}")),
             Self::Portal(e) => f.write_fmt(format_args!("Portal error: {e}")),
@@ -20,7 +18,6 @@ impl std::fmt::Display for Error {
 impl std::error::Error for Error {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {
-            Self::Rand(_) => None,
             Self::Oo7(e) => Some(e),
             Self::Io(e) => Some(e),
             Self::Portal(e) => Some(e),
@@ -28,12 +25,6 @@ impl std::error::Error for Error {
     }
 }
 
-impl From<getrandom::Error> for Error {
-    fn from(err: getrandom::Error) -> Self {
-        Self::Rand(err)
-    }
-}
-
 impl From<oo7::dbus::Error> for Error {
     fn from(value: oo7::dbus::Error) -> Self {
         Self::Oo7(value)
diff --git a/portal/src/main.rs b/portal/src/main.rs
index 93a86fce1..eb4cbc1ac 100644
--- a/portal/src/main.rs
+++ b/portal/src/main.rs
@@ -11,7 +11,6 @@ pub use error::Result;
 use oo7::dbus::Service;
 use tokio::io::AsyncWriteExt;
 
-const PORTAL_SECRET_SIZE: usize = 64;
 const PORTAL_NAME: &str = "org.freedesktop.impl.portal.desktop.oo7";
 
 struct Secret;
@@ -36,13 +35,6 @@ impl ashpd::backend::secret::SecretImpl for Secret {
     }
 }
 
-fn generate_secret() -> Result<zeroize::Zeroizing<Vec<u8>>> {
-    let mut secret = [0; PORTAL_SECRET_SIZE];
-    // Equivalent of `ring::rand::SecureRandom`
-    getrandom::getrandom(&mut secret)?;
-    Ok(zeroize::Zeroizing::new(secret.to_vec()))
-}
-
 /// Generates, stores and send the secret back to the fd stream
 async fn send_secret_to_app(app_id: &AppID, fd: std::os::fd::OwnedFd) -> Result<()> {
     let service = Service::new().await?;
@@ -56,11 +48,17 @@ async fn send_secret_to_app(app_id: &AppID, fd: std::os::fd::OwnedFd) -> Result<
         (oo7::XDG_SCHEMA_ATTRIBUTE, GENERIC_SCHEMA_VALUE),
         ("app_id", app_id),
     ]);
-    let secret = if let Some(item) = collection.search_items(&attributes).await?.first() {
-        item.secret().await?
+
+    // Write the secret to the FD.
+    let std_stream = UnixStream::from(fd);
+    std_stream.set_nonblocking(true)?;
+    let mut stream = tokio::net::UnixStream::from_std(std_stream)?;
+
+    if let Some(item) = collection.search_items(&attributes).await?.first() {
+        stream.write_all(&item.secret().await?).await?;
     } else {
         tracing::debug!("Could not find secret for {app_id}, creating one");
-        let secret = generate_secret()?;
+        let secret = oo7::Secret::random().unwrap();
 
         collection
             .create_item(
@@ -72,14 +70,8 @@ async fn send_secret_to_app(app_id: &AppID, fd: std::os::fd::OwnedFd) -> Result<
             )
             .await?;
 
-        secret
-    };
-
-    // Write the secret to the FD.
-    let std_stream = UnixStream::from(fd);
-    std_stream.set_nonblocking(true)?;
-    let mut stream = tokio::net::UnixStream::from_std(std_stream)?;
-    stream.write_all(&secret).await?;
+        stream.write_all(&secret).await?;
+    }
 
     Ok(())
 }
diff --git a/server/src/service.rs b/server/src/service.rs
index d012214bd..f5c1bb9a7 100644
--- a/server/src/service.rs
+++ b/server/src/service.rs
@@ -288,7 +288,7 @@ impl Service {
             "session",
             false,
             service.clone(),
-            Arc::new(Keyring::temporary(Secret::random()).await?),
+            Arc::new(Keyring::temporary(Secret::random().unwrap()).await?),
         );
         collections.push(collection.clone());
         object_server

From 3fbbf466a3b0809da2618a6cb86b840254cf2c0b Mon Sep 17 00:00:00 2001
From: Bilal Elmoussaoui <belmouss@redhat.com>
Date: Sat, 2 Nov 2024 14:06:00 +0100
Subject: [PATCH 3/3] client: Return Secret instead of a blob

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
---
 cli/src/main.rs                       | 14 +++++------
 client/examples/basic.rs              |  2 +-
 client/examples/basic_2.rs            |  2 +-
 client/src/dbus/api/secret.rs         | 24 +++++++++++++++++-
 client/src/dbus/collection.rs         |  6 ++---
 client/src/dbus/error.rs              | 11 ++++++++-
 client/src/dbus/item.rs               | 22 +++++------------
 client/src/dbus/mod.rs                |  4 +--
 client/src/file/api/legacy_keyring.rs |  5 ++--
 client/src/file/api/mod.rs            |  6 ++---
 client/src/file/item.rs               |  4 +--
 client/src/file/mod.rs                | 35 +++++++++++----------------
 client/src/keyring.rs                 |  7 +++---
 client/src/secret.rs                  |  9 ++++---
 14 files changed, 83 insertions(+), 68 deletions(-)

diff --git a/cli/src/main.rs b/cli/src/main.rs
index 1458456c1..12e9601e6 100644
--- a/cli/src/main.rs
+++ b/cli/src/main.rs
@@ -245,14 +245,15 @@ 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() {
@@ -260,7 +261,6 @@ async fn print_item(
         }
     } 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?;
@@ -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();
                 }
             }
diff --git a/client/examples/basic.rs b/client/examples/basic.rs
index 654720001..5de5bfe8e 100644
--- a/client/examples/basic.rs
+++ b/client/examples/basic.rs
@@ -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?;
diff --git a/client/examples/basic_2.rs b/client/examples/basic_2.rs
index e7ead7b94..6e4ab53d8 100644
--- a/client/examples/basic_2.rs
+++ b/client/examples/basic_2.rs
@@ -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?;
diff --git a/client/src/dbus/api/secret.rs b/client/src/dbus/api/secret.rs
index 9dd639a0f..5df7e64ee 100644
--- a/client/src/dbus/api/secret.rs
+++ b/client/src/dbus/api/secret.rs
@@ -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)")]
@@ -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
diff --git a/client/src/dbus/collection.rs b/client/src/dbus/collection.rs
index 82b453c29..f1338a0ed 100644
--- a/client/src/dbus/collection.rs
+++ b/client/src/dbus/collection.rs
@@ -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);
diff --git a/client/src/dbus/error.rs b/client/src/dbus/error.rs
index bc112883e..23890de33 100644
--- a/client/src/dbus/error.rs
+++ b/client/src/dbus/error.rs
@@ -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>
@@ -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 {
@@ -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 {
@@ -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}"),
         }
     }
 }
diff --git a/client/src/dbus/item.rs b/client/src/dbus/item.rs
index ea4d7e659..8bf6a1b05 100644
--- a/client/src/dbus/item.rs
+++ b/client/src/dbus/item.rs
@@ -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.
 ///
@@ -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())
         }
     }
 
diff --git a/client/src/dbus/mod.rs b/client/src/dbus/mod.rs
index 2a23afc10..071915a2c 100644
--- a/client/src/dbus/mod.rs
+++ b/client/src/dbus/mod.rs
@@ -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(())
 //! # }
diff --git a/client/src/file/api/legacy_keyring.rs b/client/src/file/api/legacy_keyring.rs
index a8c973a88..54a1298fa 100644
--- a/client/src/file/api/legacy_keyring.rs
+++ b/client/src/file/api/legacy_keyring.rs
@@ -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!(
diff --git a/client/src/file/api/mod.rs b/client/src/file/api/mod.rs
index 01d549b5d..01f84f764 100644
--- a/client/src/file/api/mod.rs
+++ b/client/src/file/api/mod.rs
@@ -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);
 
@@ -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)?,
         );
@@ -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");
 
diff --git a/client/src/file/item.rs b/client/src/file/item.rs
index 3021a707e..5d2e8d6f8 100644
--- a/client/src/file/item.rs
+++ b/client/src/file/item.rs
@@ -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.
diff --git a/client/src/file/mod.rs b/client/src/file/mod.rs
index 6f15ade09..03b4a7cb6 100644
--- a/client/src/file/mod.rs
+++ b/client/src/file/mod.rs
@@ -11,7 +11,7 @@
 //!     .create_item(
 //!         "My Label",
 //!         &HashMap::from([("account", "alice")]),
-//!         b"My Password",
+//!         "My Password",
 //!         true,
 //!     )
 //!     .await?;
@@ -19,7 +19,7 @@
 //! 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")]))
@@ -48,7 +48,6 @@ use tokio::{
     io::AsyncReadExt,
     sync::{Mutex, RwLock},
 };
-use zeroize::Zeroizing;
 
 use crate::{AsAttributes, Key, Secret};
 
@@ -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)]
@@ -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!(
@@ -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?;
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
diff --git a/client/src/keyring.rs b/client/src/keyring.rs
index 88479ec8e..7c2827144 100644
--- a/client/src/keyring.rs
+++ b/client/src/keyring.rs
@@ -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, file, AsAttributes, Result, Secret};
 
@@ -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?,
@@ -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");
@@ -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");
diff --git a/client/src/secret.rs b/client/src/secret.rs
index 45726941c..e38dc490f 100644
--- a/client/src/secret.rs
+++ b/client/src/secret.rs
@@ -1,7 +1,10 @@
 use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing};
 
+pub(crate) const TEXT_CONTENT_TYPE: &str = "text/plain";
+pub(crate) const BLOB_CONTENT_TYPE: &str = "application/octet-stream";
+
 /// A safe wrapper around a combination of (secret, content-type).
-#[derive(Debug, Clone, Zeroize, ZeroizeOnDrop)]
+#[derive(Debug, Clone, PartialEq, Eq, Zeroize, ZeroizeOnDrop)]
 pub enum Secret {
     /// Corresponds to `text/plain`
     Text(String),
@@ -32,8 +35,8 @@ impl Secret {
 
     pub fn content_type(&self) -> &'static str {
         match self {
-            Self::Text(_) => "text/plain",
-            Self::Blob(_) => "application/octet-stream",
+            Self::Text(_) => TEXT_CONTENT_TYPE,
+            Self::Blob(_) => BLOB_CONTENT_TYPE,
         }
     }