Multipacket handling for credentials listing and other improvements

Cargo.toml

@@ -30,7 +30,13 @@ pretty_env_logger = "0.4.0"
 
 [features]
 default = ["apdu-dispatch"]
-devel = ["apdu-dispatch", "log-all", "delog/std-log"]
+devel = ["apdu-dispatch", "log-all", "delog/std-log", "devel-counters", "devel-ctaphid-bug"]
+
+# Count accesses to the read-only and read-write persistence storage
+devel-counters = []
+
+# Account ctaphid bug about 3072 buffer size. To be removed once fixed.
+devel-ctaphid-bug = []
 
 # Allow to use application over CTAPHID interface
 ctaphid = ["ctaphid-dispatch", "usbd-ctaphid"]

src/authenticator.rs

@@ -150,7 +150,7 @@ where
     ) -> Result {
         let no_authorization_needed = self
             .state
-            .persistent(&mut self.trussed, |_, state| !state.password_set());
+            .persistent_read_only(&mut self.trussed, |_, state| !state.password_set());
 
         // TODO: abstract out this idea to make it usable for all the PIV security indicators
 
@@ -219,6 +219,7 @@ where
             Command::SetPassword(set_password) => self.set_password(set_password),
             Command::ClearPassword => self.clear_password(),
             Command::VerifyCode(verify_code) => self.verify_code(verify_code, reply),
+            Command::SendRemaining => self.send_remaining(reply),
             _ => Err(Status::ConditionsOfUseNotSatisfied),
         }
     }
@@ -236,7 +237,7 @@ where
 
         let state = self
             .state
-            .persistent(&mut self.trussed, |_, state| state.clone());
+            .persistent_read_only(&mut self.trussed, |_, state| state.clone());
         let answer_to_select = AnswerToSelect::new(state.salt);
 
         let data: heapless::Vec<u8, 128> = if state.password_set() {
@@ -326,8 +327,26 @@ where
         Ok(Default::default())
     }
 
+    fn try_to_serialize_credential_for_list<const R: usize>(
+        credential: &Credential,
+        reply: &mut Data<R>,
+    ) -> core::result::Result<(), u8> {
+        reply.push(0x72)?;
+        reply.push((credential.label.len() + 1) as u8)?;
+        reply.push(oath::combine(credential.kind, credential.algorithm))?;
+        reply.extend_from_slice(&credential.label).map_err(|_| 0)?;
+        #[cfg(feature = "devel-ctaphid-bug")]
+        if reply.len() > 3072 {
+            // Finish early due to the usbd-ctaphid bug, which panics on bigger buffers than this
+            // FIXME Remove once fixed
+            return Err(1);
+        }
+        Ok(())
+    }
+
     /// The YK5 can store a Grande Totale of 32 OATH credentials.
     pub fn list_credentials<const R: usize>(&mut self, reply: &mut Data<R>) -> Result {
+        // TODO check if this one conflicts with send remaining
         if !self.state.runtime.client_authorized {
             return Err(Status::ConditionsOfUseNotSatisfied);
         }
@@ -339,40 +358,68 @@ where
         //          79 75 62 69  63 6F
         // 90 00
 
-        let maybe_credential_enc = syscall!(self.trussed.read_dir_files_first(
-            Location::Internal,
-            Self::credential_directory(),
-            None
-        ))
-        .data;
-        let mut maybe_credential: Option<Credential> = match maybe_credential_enc {
-            None => None,
-            Some(c) => self.state.decrypt_content(&mut self.trussed, c).ok(),
+        let mut file_index = if let Some(CommandState::ListCredentials(s_file_index)) =
+            self.state.runtime.previously
+        {
+            s_file_index
+        } else {
+            0
+        };
+
+        let mut maybe_credential = {
+            // To avoid creating additional buffer for the unfit data
+            // we will rewind the state and restart from there accordingly
+            let first_file = try_syscall!(self.trussed.read_dir_files_first(
+                Location::Internal,
+                Self::credential_directory(),
+                None
+            ))
+            .map_err(|_| iso7816::Status::UnspecifiedNonpersistentExecutionError)?
+            .data;
+
+            // Rewind if needed, otherwise return first file's content
+            let file = match &self.state.runtime.previously {
+                Some(CommandState::ListCredentials(s)) => {
+                    info_now!("found continuation state: {:?}", s);
+                    for _ in 0..s - 1 {
+                        try_syscall!(self.trussed.read_dir_files_next())
+                            .map_err(|_| iso7816::Status::UnspecifiedNonpersistentExecutionError)?;
+                    }
+                    try_syscall!(self.trussed.read_dir_files_next())
+                        .map_err(|_| iso7816::Status::UnspecifiedNonpersistentExecutionError)?
+                        .data
+                }
+                None => first_file,
+                _ => Err(iso7816::Status::FunctionNotSupported)?, // this one should never be reached
+            };
+
+            let maybe_credential: Option<Credential> = match file {
+                None => None,
+                Some(c) => self.state.decrypt_content(&mut self.trussed, c).ok(),
+            };
+            maybe_credential
         };
 
-        let mut file_index = 0;
         while let Some(credential) = maybe_credential {
+            // Try to serialize, abort if not succeeded
+            let current_reply_bytes_count = reply.len();
+            let res = Self::try_to_serialize_credential_for_list(&credential, reply);
+            if res.is_err() {
+                // Revert reply vector to the last good size, removing debris from the failed
+                // serialization
+                reply.truncate(current_reply_bytes_count);
+                return Err(Status::MoreAvailable(0xFF));
+            }
+
             // keep track, in case we need continuation
             file_index += 1;
             self.state.runtime.previously = Some(CommandState::ListCredentials(file_index));
 
-            // deserialize
-            reply.push(0x72).unwrap();
-            reply.push((credential.label.len() + 1) as u8).unwrap();
-            reply
-                .push(oath::combine(credential.kind, credential.algorithm))
-                .unwrap();
-            reply.extend_from_slice(&credential.label).unwrap();
-
             // check if there's more
             maybe_credential = match syscall!(self.trussed.read_dir_files_next()).data {
                 None => None,
                 Some(c) => self.state.decrypt_content(&mut self.trussed, c).ok(),
             };
-
-            if file_index % 8 == 0 {
-                // TODO: split response
-            }
         }
 
         // ran to completion
@@ -381,6 +428,13 @@ where
         Ok(())
     }
 
+    fn send_remaining<const R: usize>(&mut self, reply: &mut Data<{ R }>) -> Result {
+        match self.state.runtime.previously {
+            None => Err(Status::ConditionsOfUseNotSatisfied),
+            Some(CommandState::ListCredentials(_)) => self.list_credentials(reply),
+        }
+    }
+
     pub fn register(&mut self, register: command::Register<'_>) -> Result {
         self.user_present()?;
 
@@ -414,8 +468,21 @@ where
         let filename = self.filename_for_label(&credential.label);
 
         // 4. Serialize the credential (implicitly) and store it
-        self.state
-            .try_write_file(&mut self.trussed, filename, &credential)?;
+        let write_res = self
+            .state
+            .try_write_file(&mut self.trussed, filename, &credential);
+
+        if write_res.is_err() {
+            // TODO reuse delete() call
+            // 1. Try to delete the key from Trussed, ignore errors
+            try_syscall!(self.trussed.delete(credential.secret)).ok();
+            // 2. Try to delete the empty file, ignore errors
+            let filename = self.filename_for_label(&credential.label);
+            try_syscall!(self.trussed.remove_file(Location::Internal, filename)).ok();
+            // 3. Return the original error
+            write_res?
+        }
+
         Ok(())
     }
 
@@ -576,7 +643,7 @@ where
 
         if let Some(key) = self
             .state
-            .persistent(&mut self.trussed, |_, state| state.authorization_key)
+            .persistent_read_only(&mut self.trussed, |_, state| state.authorization_key)
         {
             debug_now!("key set: {:?}", key);
 
@@ -638,11 +705,15 @@ where
             return Err(Status::ConditionsOfUseNotSatisfied);
         }
         debug_now!("clearing password/key");
-        if let Some(key) = self.state.persistent(&mut self.trussed, |_, state| {
-            let existing_key = state.authorization_key;
-            state.authorization_key = None;
-            existing_key
-        }) {
+        if let Some(key) = self
+            .state
+            .try_persistent_read_write(&mut self.trussed, |_, state| {
+                let existing_key = state.authorization_key;
+                state.authorization_key = None;
+                Ok(existing_key)
+            })
+            .map_err(|_| Status::NotEnoughMemory)?
+        {
             syscall!(self.trussed.delete(key));
         }
         Ok(())
@@ -723,9 +794,10 @@ where
         }
 
         info_now!("injecting the key");
-        let tmp_key = syscall!(self
+        let tmp_key = try_syscall!(self
             .trussed
             .unsafe_inject_shared_key(key, Location::Volatile,))
+        .map_err(|_| Status::NotEnoughMemory)?
         .key;
 
         let verification = syscall!(self.trussed.sign_hmacsha1(tmp_key, challenge)).signature;
@@ -744,9 +816,12 @@ where
         .key;
 
         debug_now!("storing password/key");
-        self.state.persistent(&mut self.trussed, |_, state| {
-            state.authorization_key = Some(key)
-        });
+        self.state
+            .try_persistent_read_write(&mut self.trussed, |_, state| {
+                state.authorization_key = Some(key);
+                Ok(())
+            })
+            .map_err(|_| Status::NotEnoughMemory)?;
 
         // pub struct SetPassword<'l> {
         //     pub kind: oath::Kind,

src/command.rs

@@ -30,6 +30,8 @@ pub enum Command<'l> {
     Validate(Validate<'l>),
     /// Reverse HOTP validation
     VerifyCode(VerifyCode<'l>),
+    /// Send remaining data in the buffer
+    SendRemaining,
 }
 
 /// TODO: change into enum
@@ -491,6 +493,7 @@ impl<'l, const C: usize> TryFrom<&'l iso7816::Command<C>> for Command<'l> {
                 (0x00, oath::Instruction::VerifyCode, 0x00, 0x00) => {
                     Self::VerifyCode(VerifyCode::try_from(data)?)
                 }
+                (0x00, oath::Instruction::SendRemaining, 0x00, 0x00) => Self::SendRemaining,
                 _ => return Err(Status::InstructionNotSupportedOrInvalid),
             })
         }

src/ctaphid.rs

@@ -43,15 +43,22 @@ where
                         debug_now!("ISO conversion error: {:?}", _e);
                         app::Error::InvalidLength
                     })?;
-                self.respond(&ctap_to_iso7816_command, response)
-                    .map_err(|e| {
+                let res = self.respond(&ctap_to_iso7816_command, response);
+
+                match res {
+                    Ok(_) => return Ok(()),
+                    Err(Status::MoreAvailable(b)) => {
+                        response[0] = 0x61;
+                        response[1] = b;
+                        return Ok(());
+                    }
+                    Err(e) => {
                         debug_now!("OTP command execution error: {:?}", e);
                         let arr: [u8; 2] = e.into();
                         response.clear();
                         response.extend(arr);
-                        app::Error::InvalidCommand
-                    })
-                    .ok();
+                    }
+                }
             }
             _ => {
                 return Err(app::Error::InvalidCommand);

src/encrypted_container.rs

@@ -1,3 +1,4 @@
+use heapless_bytes::Bytes;
 use serde::{Deserialize, Serialize};
 use trussed::types::{KeyId, Message};
 use trussed::{cbor_deserialize, cbor_serialize, try_syscall};
@@ -88,14 +89,18 @@ impl From<Error> for trussed::error::Error {
 /// - Investigate postcard structure extensibility, as a means for smaller overhead for serialization
 #[derive(Debug, serde::Deserialize, serde::Serialize)]
 pub struct EncryptedDataContainer {
+    /// The ciphertext. 1024 bytes maximum. Reusing trussed::types::Message.
     #[serde(rename = "D")]
     data: trussed::types::Message,
     #[serde(rename = "T")]
-    tag: trussed::types::ShortData,
+    tag: ContainerTag,
     #[serde(rename = "N")]
-    nonce: trussed::types::ShortData,
+    nonce: ContainerNonce,
 }
 
+type ContainerTag = Bytes<16>;
+type ContainerNonce = Bytes<12>;
+
 impl TryFrom<&[u8]> for EncryptedDataContainer {
     type Error = Error;
 
@@ -189,8 +194,8 @@ impl EncryptedDataContainer {
 
         let encrypted_serialized_credential = EncryptedDataContainer {
             data: ciphertext,
-            nonce: encryption_results.nonce,
-            tag: encryption_results.tag,
+            nonce: encryption_results.nonce.try_convert_into().unwrap(), // should always be 12 bytes
+            tag: encryption_results.tag.try_convert_into().unwrap(), // should always be 16 bytes
         };
         Ok(encrypted_serialized_credential)
     }