Merge branch 'release-039' into fix-admin-auth

Author: sosthene-nitrokey

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## [v0.3.9][] (2025-02-11)
+
+[v0.3.9]: https://github.com/Nitrokey/piv-authenticator/releases/tag/v0.3.9
+
+- Fix admin authentication
+
+### Security
+
+- Fix [CVE-2025-25201](https://github.com/Nitrokey/piv-authenticator/security/advisories/GHSA-28p6-c99x-fg8j)
+
 ## [v0.3.8][] (2024-10-17)
 
 [v0.3.8]: https://github.com/Nitrokey/piv-authenticator/releases/tag/v0.3.8

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "piv-authenticator"
-version = "0.3.8"
+version = "0.3.9"
 authors = ["Nicolas Stalder <[email protected]>", "Nitrokey GmbH"]
 edition = "2021"
 license = "Apache-2.0 OR MIT"
@@ -72,8 +72,9 @@ rsa = ["trussed-rsa-alloc", "alloc"]
 
 log-all = []
 log-none = []
-log-info = []
+log-trace = []
 log-debug = []
+log-info = []
 log-warn = []
 log-error = []
 

src/lib.rs

@@ -255,7 +255,7 @@ where
     }
 }
 
-impl<'a, T: Client> LoadedAuthenticator<'a, T> {
+impl<T: Client> LoadedAuthenticator<'_, T> {
     pub fn yubico_set_administration_key<const R: usize>(
         &mut self,
         data: &[u8],
@@ -482,7 +482,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> {
             exponentiation: tlv::get_do(&[0x85], input),
         };
 
-        error!(
+        debug!(
             "witness: {}, challenge: {}, response: {}, exponentiation: {}",
             &parsed.witness.is_some(),
             &parsed.challenge.is_some(),
@@ -569,21 +569,22 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> {
     ) -> Result {
         info!("Single auth 1");
         let key = self.validate_auth_management(auth)?;
-        let pl = syscall!(self.trussed.random_bytes(key.alg.challenge_length())).bytes;
-        self.state.volatile.command_cache = Some(CommandCache::SingleAuthChallenge(
-            Bytes::from_slice(&pl).unwrap(),
+        let plaintext = syscall!(self.trussed.random_bytes(key.alg.challenge_length())).bytes;
+        let ciphertext =
+            syscall!(self
+                .trussed
+                .encrypt(key.alg.mechanism(), key.id, &plaintext, &[], None))
+            .ciphertext;
+        self.state.volatile.command_cache = Some(CommandCache::SingleAuthChallengeReference(
+            Bytes::from_slice(&ciphertext).unwrap(),
         ));
-        let data = syscall!(self
-            .trussed
-            .encrypt(key.alg.mechanism(), key.id, &pl, &[], None))
-        .ciphertext;
 
         reply.expand(&[0x7C])?;
         let offset = reply.len();
         {
             reply.expand(&[0x81])?;
-            reply.append_len(data.len())?;
-            reply.expand(&data)?;
+            reply.append_len(plaintext.len())?;
+            reply.expand(&plaintext)?;
         }
         reply.prepend_len(offset)?;
         Ok(())
@@ -599,14 +600,15 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> {
             return Err(Status::IncorrectDataParameter);
         }
 
-        let Some(plaintext_challenge) = self.state.volatile.take_single_challenge() else {
+        let Some(challenge_reference) = self.state.volatile.take_single_challenge_reference()
+        else {
             warn!("Missing cached challenge for auth");
             return Err(Status::ConditionsOfUseNotSatisfied);
         };
 
-        let is_eq: bool = response.ct_eq(&plaintext_challenge).into();
-        if is_eq {
-            warn!("Bad auth challenge");
+        let is_eq: bool = response.ct_eq(&challenge_reference).into();
+        if !is_eq {
+            warn!("Failed admin authentication. Challenge did not match");
             return Err(Status::IncorrectDataParameter);
         }
 
@@ -616,31 +618,37 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> {
             .administrator_verified = true;
         Ok(())
     }
+
     fn mutual_auth_1<const R: usize>(
         &mut self,
         auth: GeneralAuthenticate,
         mut reply: Reply<'_, R>,
     ) -> Result {
         info!("Mutual auth 1");
         let key = self.validate_auth_management(auth)?;
-        let pl = syscall!(self.trussed.random_bytes(key.alg.challenge_length())).bytes;
-        self.state.volatile.command_cache = Some(CommandCache::MutualAuthChallenge(
-            Bytes::from_slice(&pl).unwrap(),
+        let plaintext = syscall!(self.trussed.random_bytes(key.alg.challenge_length())).bytes;
+
+        let ciphertext =
+            syscall!(self
+                .trussed
+                .encrypt(key.alg.mechanism(), key.id, &plaintext, &[], None))
+            .ciphertext;
+
+        self.state.volatile.command_cache = Some(CommandCache::MutualAuthWitnessReference(
+            Bytes::from_slice(&plaintext).unwrap(),
         ));
-        let data = syscall!(self
-            .trussed
-            .encrypt(key.alg.mechanism(), key.id, &pl, &[], None))
-        .ciphertext;
+
         reply.expand(&[0x7C])?;
         let offset = reply.len();
         {
             reply.expand(&[0x80])?;
-            reply.append_len(data.len())?;
-            reply.expand(&data)?;
+            reply.append_len(ciphertext.len())?;
+            reply.expand(&ciphertext)?;
         }
         reply.prepend_len(offset)?;
         Ok(())
     }
+
     fn mutual_auth_2<const R: usize>(
         &mut self,
         auth: GeneralAuthenticate,
@@ -661,13 +669,14 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> {
             return Err(Status::IncorrectDataParameter);
         }
 
-        let Some(plaintext_challenge) = self.state.volatile.take_mutual_challenge() else {
+        let Some(witness_reference) = self.state.volatile.take_mutual_witness_reference() else {
             warn!("Missing cached challenge for auth");
             return Err(Status::ConditionsOfUseNotSatisfied);
         };
 
-        if challenge.ct_eq(&plaintext_challenge).into() {
-            warn!("Bad auth challenge");
+        let is_eq: bool = response.ct_eq(&witness_reference).into();
+        if !is_eq {
+            warn!("Failed admin authentication. Challenge did not match");
             return Err(Status::IncorrectDataParameter);
         }
 
@@ -701,7 +710,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> {
         just_verified: bool,
         mut reply: Reply<'_, R>,
     ) -> Result {
-        error!("Request for sign, data length: {}, data:", message.len());
+        debug!("Request for sign, data length: {}, data:", message.len());
         // error!("{}", delog::hexstr!(message));
 
         let Ok(key_ref) = auth.key_reference.try_into() else {
@@ -737,7 +746,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> {
             reply.append_len(response.len())?;
             reply.expand(&response)?;
         }
-        error!("Signed data len: {}, Data:", response.len());
+        debug!("Signed data len: {}, Data:", response.len());
         // error!("{}", delog::hexstr!(&response));
 
         reply.prepend_len(offset)?;

src/reply.rs

@@ -12,13 +12,13 @@ impl<'v, const R: usize> Deref for Reply<'v, R> {
     }
 }
 
-impl<'v, const R: usize> DerefMut for Reply<'v, R> {
+impl<const R: usize> DerefMut for Reply<'_, R> {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.0
     }
 }
 
-impl<'v, const R: usize> Reply<'v, R> {
+impl<const R: usize> Reply<'_, R> {
     /// Extend the reply and return an error otherwise
     /// The MoreAvailable and GET RESPONSE mechanisms are handled by adpu_dispatch
     ///

src/state.rs

@@ -305,7 +305,7 @@ impl Drop for UseValidKey {
     }
 }
 
-impl<'t> LoadedState<'t> {
+impl LoadedState<'_> {
     pub fn key_exists(
         &self,
         client: &mut impl crate::Client,
@@ -508,17 +508,17 @@ impl Volatile {
 
         Ok(ReadValid::Encrypted(unsealed_key.key))
     }
-    pub fn take_single_challenge(&mut self) -> Option<Bytes<16>> {
+    pub fn take_single_challenge_reference(&mut self) -> Option<Bytes<16>> {
         match self.command_cache.take() {
-            Some(CommandCache::SingleAuthChallenge(b)) => return Some(b),
+            Some(CommandCache::SingleAuthChallengeReference(b)) => return Some(b),
             old => self.command_cache = old,
         };
         None
     }
 
-    pub fn take_mutual_challenge(&mut self) -> Option<Bytes<16>> {
+    pub fn take_mutual_witness_reference(&mut self) -> Option<Bytes<16>> {
         match self.command_cache.take() {
-            Some(CommandCache::MutualAuthChallenge(b)) => return Some(b),
+            Some(CommandCache::MutualAuthWitnessReference(b)) => return Some(b),
             old => self.command_cache = old,
         };
         None
@@ -535,8 +535,8 @@ pub struct AppSecurityStatus {
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum CommandCache {
-    SingleAuthChallenge(Bytes<16>),
-    MutualAuthChallenge(Bytes<16>),
+    SingleAuthChallengeReference(Bytes<16>),
+    MutualAuthWitnessReference(Bytes<16>),
 }
 
 impl Persistent {

tests/bad_admin_key

@@ -0,0 +1 @@
+

��������