Replace iso7816::Command with iso7816::command::CommandView

This allows us to drop one generic parameter from the App trait and
potentially generate more efficient code.

Author: robin-nitrokey

Cargo.toml

@@ -9,7 +9,7 @@ license = "Apache-2.0 OR MIT"
 repository = "https://github.com/trussed-dev/apdu-dispatch"
 
 [workspace.dependencies]
-iso7816 = "0.1.1"
+iso7816 = "0.1.2"
 
 [patch.crates-io]
 apdu-app.path = "app"

app/CHANGELOG.md

@@ -7,3 +7,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 - Extract `app` module from `apdu-dispatch` 0.2.0 into a separate crate.
+- Replace `iso7816::Command` with `iso7816::command::CommandView` in the `App` trait and remove the `C` parameter.

app/src/lib.rs

@@ -1,21 +1,26 @@
 #![no_std]
 
-pub use iso7816::{Command, Data, Interface, Status};
+pub use iso7816::{command::CommandView, Data, Interface, Status};
 
 pub type Result = iso7816::Result<()>;
 
 /// An App can receive and respond APDUs at behest of the ApduDispatch.
-pub trait App<const C: usize, const R: usize>: iso7816::App {
+pub trait App<const R: usize>: iso7816::App {
     /// Given parsed APDU for select command.
     /// Write response data back to buf, and return length of payload.  Return APDU Error code on error.
     /// Alternatively, the app can defer the response until later by returning it in `poll()`.
-    fn select(&mut self, interface: Interface, apdu: &Command<C>, reply: &mut Data<R>) -> Result;
+    fn select(
+        &mut self,
+        interface: Interface,
+        apdu: CommandView<'_>,
+        reply: &mut Data<R>,
+    ) -> Result;
 
     /// Deselects the app. This is the result of another app getting selected.
     /// App should clear any sensitive state and reset security indicators.
     fn deselect(&mut self);
 
     /// Given parsed APDU for app when selected.
     /// Write response data back to buf, and return length of payload.  Return APDU Error code on error.
-    fn call(&mut self, interface: Interface, apdu: &Command<C>, reply: &mut Data<R>) -> Result;
+    fn call(&mut self, interface: Interface, apdu: CommandView<'_>, reply: &mut Data<R>) -> Result;
 }

dispatch/CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 - Move the `app` module into a separate crate, `apdu-app`, and re-export it.
+- Replace `iso7816::Command` with `iso7816::command::CommandView` in the `App` trait and remove the `C` parameter.
 
 ## [0.2.0]
 

dispatch/src/dispatch.rs

@@ -8,15 +8,17 @@
 //!
 //! Apps need to implement the App trait to be managed.
 //!
-use crate::command::SIZE as CommandSize;
 use crate::response::SIZE as ResponseSize;
 use crate::App;
 use crate::{
     interchanges::{self, Responder},
     response, Command,
 };
 
-use iso7816::{command::FromSliceError, Aid, Instruction, Result, Status};
+use iso7816::{
+    command::{CommandView, FromSliceError},
+    Aid, Instruction, Result, Status,
+};
 
 /// Maximum length of a data field of a response that can fit in an interchange message after
 /// concatenation of SW1SW2
@@ -50,17 +52,17 @@ struct ApduBuffer {
 }
 
 impl ApduBuffer {
-    fn request<const S: usize>(&mut self, command: &iso7816::Command<S>) {
+    fn request(&mut self, command: CommandView<'_>) {
         match &mut self.raw {
             RawApduBuffer::Request(buffered) => {
-                buffered.extend_from_command(command).ok();
+                buffered.extend_from_command_view(command).ok();
             }
             _ => {
                 if self.raw != RawApduBuffer::None {
                     info!("Was buffering the last response, but aborting that now for this new request.");
                 }
                 let mut new_cmd = iso7816::Command::try_from(&[0, 0, 0, 0]).unwrap();
-                new_cmd.extend_from_command(command).ok();
+                new_cmd.extend_from_command_view(command).ok();
                 self.raw = RawApduBuffer::Request(new_cmd);
             }
         }
@@ -84,7 +86,7 @@ pub struct ApduDispatch<'pipe> {
 }
 
 impl<'pipe> ApduDispatch<'pipe> {
-    fn apdu_type<const S: usize>(apdu: &iso7816::Command<S>, interface: Interface) -> RequestType {
+    fn apdu_type(apdu: CommandView<'_>, interface: Interface) -> RequestType {
         info!("instruction: {:?} {}", apdu.instruction(), apdu.p1);
         if apdu.instruction() == Instruction::Select && (apdu.p1 & 0x04) != 0 {
             Aid::try_new(apdu.data()).map_or_else(
@@ -119,8 +121,8 @@ impl<'pipe> ApduDispatch<'pipe> {
     // but that won't work due to ownership rules
     fn find_app<'a, 'b>(
         aid: Option<&Aid>,
-        apps: &'a mut [&'b mut dyn App<CommandSize, ResponseSize>],
-    ) -> Option<&'a mut &'b mut dyn App<CommandSize, ResponseSize>> {
+        apps: &'a mut [&'b mut dyn App<ResponseSize>],
+    ) -> Option<&'a mut &'b mut dyn App<ResponseSize>> {
         // match aid {
         //     Some(aid) => apps.iter_mut().find(|app| aid.starts_with(app.rid())),
         //     None => None,
@@ -145,9 +147,9 @@ impl<'pipe> ApduDispatch<'pipe> {
     }
 
     #[inline(never)]
-    fn buffer_chained_apdu_if_needed<const S: usize>(
+    fn buffer_chained_apdu_if_needed(
         &mut self,
-        command: iso7816::Command<S>,
+        command: CommandView<'_>,
         interface: Interface,
     ) -> RequestType {
         // iso 7816-4 5.1.1
@@ -156,7 +158,7 @@ impl<'pipe> ApduDispatch<'pipe> {
             let is_chaining = matches!(self.buffer.raw, RawApduBuffer::Request(_));
 
             if is_chaining {
-                self.buffer.request(&command);
+                self.buffer.request(command);
 
                 // Response now needs to be chained.
                 self.was_request_chained = true;
@@ -167,12 +169,12 @@ impl<'pipe> ApduDispatch<'pipe> {
                 if self.buffer.raw == RawApduBuffer::None {
                     self.was_request_chained = false;
                 }
-                let apdu_type = Self::apdu_type(&command, interface);
+                let apdu_type = Self::apdu_type(command, interface);
                 match apdu_type {
                     // Keep buffer the same in case of GetResponse
                     RequestType::GetResponse => (),
                     // Overwrite for everything else.
-                    _ => self.buffer.request(&command),
+                    _ => self.buffer.request(command),
                 }
                 apdu_type
             }
@@ -193,7 +195,7 @@ impl<'pipe> ApduDispatch<'pipe> {
 
             if !command.data().is_empty() {
                 info!("chaining {} bytes", command.data().len());
-                self.buffer.request(&command);
+                self.buffer.request(command);
             }
 
             // Nothing for the application to consume yet.
@@ -259,7 +261,7 @@ impl<'pipe> ApduDispatch<'pipe> {
                 Ok(command) => {
                     self.response_len_expected = command.expected();
                     // The Apdu may be standalone or part of a chain.
-                    self.buffer_chained_apdu_if_needed(command, interface)
+                    self.buffer_chained_apdu_if_needed(command.as_view(), interface)
                 }
                 Err(response) => {
                     // If not a valid APDU, return error and don't pass to app.
@@ -364,7 +366,7 @@ impl<'pipe> ApduDispatch<'pipe> {
     #[inline(never)]
     fn handle_app_select(
         &mut self,
-        apps: &mut [&mut dyn App<CommandSize, ResponseSize>],
+        apps: &mut [&mut dyn App<ResponseSize>],
         aid: Aid,
         interface: Interface,
     ) {
@@ -393,7 +395,9 @@ impl<'pipe> ApduDispatch<'pipe> {
             info!("Selected app");
             let mut response = response::Data::new();
             let result = match &self.buffer.raw {
-                RawApduBuffer::Request(apdu) => app.select(interface, apdu, &mut response),
+                RawApduBuffer::Request(apdu) => {
+                    app.select(interface, apdu.as_view(), &mut response)
+                }
                 _ => panic!("Unexpected buffer state."),
             };
 
@@ -409,14 +413,14 @@ impl<'pipe> ApduDispatch<'pipe> {
     #[inline(never)]
     fn handle_app_command(
         &mut self,
-        apps: &mut [&mut dyn App<CommandSize, ResponseSize>],
+        apps: &mut [&mut dyn App<ResponseSize>],
         interface: Interface,
     ) {
         // if there is a selected app, send it the command
         let mut response = response::Data::new();
         if let Some(app) = Self::find_app(self.current_aid.as_ref(), apps) {
             let result = match &self.buffer.raw {
-                RawApduBuffer::Request(apdu) => app.call(interface, apdu, &mut response),
+                RawApduBuffer::Request(apdu) => app.call(interface, apdu.as_view(), &mut response),
                 _ => panic!("Unexpected buffer state."),
             };
             self.handle_app_response(&result, &response);
@@ -426,10 +430,7 @@ impl<'pipe> ApduDispatch<'pipe> {
         };
     }
 
-    pub fn poll(
-        &mut self,
-        apps: &mut [&mut dyn App<CommandSize, ResponseSize>],
-    ) -> Option<Interface> {
+    pub fn poll(&mut self, apps: &mut [&mut dyn App<ResponseSize>]) -> Option<Interface> {
         // Only take on one transaction at a time.
         let request_type = self.check_for_request();
 

dispatch/tests/dispatch.rs

@@ -1,6 +1,5 @@
-use apdu_dispatch::app::{App, Result as AppResult};
+use apdu_dispatch::app::{App, CommandView, Result as AppResult};
 use apdu_dispatch::dispatch;
-use apdu_dispatch::Command;
 use apdu_dispatch::{interchanges, response};
 use hex_literal::hex;
 use interchange::Channel;
@@ -48,11 +47,11 @@ impl iso7816::App for TestApp1 {
 }
 
 // This app echos to Ins code 0x10
-impl App<{ apdu_dispatch::command::SIZE }, { apdu_dispatch::response::SIZE }> for TestApp1 {
+impl App<{ apdu_dispatch::response::SIZE }> for TestApp1 {
     fn select(
         &mut self,
         _interface: dispatch::Interface,
-        _apdu: &Command,
+        _apdu: CommandView<'_>,
         _reply: &mut response::Data,
     ) -> AppResult {
         Ok(())
@@ -63,7 +62,7 @@ impl App<{ apdu_dispatch::command::SIZE }, { apdu_dispatch::response::SIZE }> fo
     fn call(
         &mut self,
         _: dispatch::Interface,
-        apdu: &Command,
+        apdu: CommandView<'_>,
         reply: &mut response::Data,
     ) -> AppResult {
         println!("TestApp1::call");
@@ -123,11 +122,11 @@ impl iso7816::App for TestApp2 {
 }
 
 // This app echos to Ins code 0x20
-impl App<{ apdu_dispatch::command::SIZE }, { apdu_dispatch::response::SIZE }> for TestApp2 {
+impl App<{ apdu_dispatch::response::SIZE }> for TestApp2 {
     fn select(
         &mut self,
         _interface: dispatch::Interface,
-        _apdu: &Command,
+        _apdu: CommandView<'_>,
         _reply: &mut response::Data,
     ) -> AppResult {
         Ok(())
@@ -138,7 +137,7 @@ impl App<{ apdu_dispatch::command::SIZE }, { apdu_dispatch::response::SIZE }> fo
     fn call(
         &mut self,
         _: dispatch::Interface,
-        apdu: &Command,
+        apdu: CommandView<'_>,
         reply: &mut response::Data,
     ) -> AppResult {
         println!("TestApp2::call");
@@ -175,11 +174,11 @@ impl iso7816::App for PanicApp {
 }
 
 // This app echos to Ins code 0x20
-impl App<{ apdu_dispatch::command::SIZE }, { apdu_dispatch::response::SIZE }> for PanicApp {
+impl App<{ apdu_dispatch::response::SIZE }> for PanicApp {
     fn select(
         &mut self,
         _interface: dispatch::Interface,
-        _apdu: &Command,
+        _apdu: CommandView<'_>,
         _reply: &mut response::Data,
     ) -> AppResult {
         panic!("Dont call the panic app");
@@ -192,7 +191,7 @@ impl App<{ apdu_dispatch::command::SIZE }, { apdu_dispatch::response::SIZE }> fo
     fn call(
         &mut self,
         _: dispatch::Interface,
-        _apdu: &Command,
+        _apdu: CommandView<'_>,
         _reply: &mut response::Data,
     ) -> AppResult {
         panic!("Dont call the panic app");

fuzz/fuzz_targets/fuzz_target_1.rs

@@ -53,11 +53,11 @@ impl iso7816::App for FuzzAppImpl {
     }
 }
 
-impl App<{ apdu_dispatch::command::SIZE }, { apdu_dispatch::response::SIZE }> for FuzzAppImpl {
+impl App<{ apdu_dispatch::response::SIZE }> for FuzzAppImpl {
     fn select(
         &mut self,
         _interface: iso7816::Interface,
-        _apdu: &apdu_dispatch::Command,
+        _apdu: apdu_dispatch::app::CommandView<'_>,
         _reply: &mut apdu_dispatch::response::Data,
     ) -> AppResult {
         Ok(())
@@ -68,7 +68,7 @@ impl App<{ apdu_dispatch::command::SIZE }, { apdu_dispatch::response::SIZE }> fo
     fn call(
         &mut self,
         _: Interface,
-        _apdu: &apdu_dispatch::Command,
+        _apdu: apdu_dispatch::app::CommandView<'_>,
         reply: &mut apdu_dispatch::response::Data,
     ) -> AppResult {
         let (ref data, status) = &self.responses[self.count];
@@ -91,7 +91,7 @@ fuzz_target!(|input: Input| {
         .collect();
     let mut dyn_apps: Vec<_> = apps
         .iter_mut()
-        .map(|s| (s as &mut dyn apdu_dispatch::App<7609, 7609>))
+        .map(|s| (s as &mut dyn apdu_dispatch::App<7609>))
         .collect();
 
     let contact = Channel::new();