kmwebnet/OpenSK
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Connect Vendor HID interface between USB driver and CTAP app (#490)

Browse Source 
* Connect Vendor HID endpoint to Ctap app

* tweaks from review

* formatting nit

* revert tock submodule revision

* fix formatting of deploy.py for yapf error

* Changes based on review

* Track state for each USB endpoint separately

* Rename patch file to ensure correct patching order

* Adjust patch from changes #494 and #500

* rustfmt

* rustfmt

* Deprecate patch 11 in favor of this full working code
This commit is contained in:
Liam Murphy
2022-06-20 15:31:31 +10:00
committed by GitHub
parent 1d53f3c921
commit 2544afbfee
4 changed files with 525 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
deploy.py
View File

@@ -473,8 +473,12 @@ class OpenSKInstaller:
def _check_invariants(self): def _check_invariants(self):
"""Runs selected unit tests to check preconditions in the code.""" """Runs selected unit tests to check preconditions in the code."""
print("Testing invariants in customization.rs...") print("Testing invariants in customization.rs...")
self.checked_command_output( features = ["std"]
["cargo", "test", "--features=std", "--lib", "customization"]) features.extend(self.args.features)
self.checked_command_output([
"cargo", "test", f"--features={','.join(features)}", "--lib",
"customization"
])
def generate_crypto_materials(self, force_regenerate: bool): def generate_crypto_materials(self, force_regenerate: bool):
"""Calls a shell script that generates cryptographic material.""" """Calls a shell script that generates cryptographic material."""

516
patches/tock/11-connect-vendor-hid-usb-interface.patch Normal file
View File

@@ -0,0 +1,516 @@
diff --git a/capsules/src/usb/usb_ctap.rs b/capsules/src/usb/usb_ctap.rs
index e8f1a87a4..2c91c0968 100644
--- a/capsules/src/usb/usb_ctap.rs
+++ b/capsules/src/usb/usb_ctap.rs
@@ -32,8 +32,7 @@ pub trait CtapUsbClient {
fn can_receive_packet(&self, app: &Option<&mut App>) -> bool;
// Signal to the client that a packet has been received.
- // If App is not supplied, it will be found from the implemntation's members.
- fn packet_received(&self, packet: &[u8; 64], app: Option<&mut App>);
+ fn packet_received(&self, packet: &[u8; 64], endpoint: usize, app: Option<&mut App>);
// Signal to the client that a packet has been transmitted.
fn packet_transmitted(&self);
@@ -49,7 +48,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> CtapUsbSyscallDriver<'a, 'b, C> {
CtapUsbSyscallDriver { usb_client, apps }
}
- fn app_packet_received(&self, packet: &[u8; 64], app: &mut App) {
+ fn app_packet_received(&self, packet: &[u8; 64], endpoint: usize, app: &mut App) {
if app.connected && app.waiting && app.side.map_or(false, |side| side.can_receive()) {
if let Some(buf) = &mut app.buffer {
// Copy the packet to the app's allowed buffer.
@@ -57,7 +56,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> CtapUsbSyscallDriver<'a, 'b, C> {
app.waiting = false;
// Signal to the app that a packet is ready.
app.callback
- .map(|mut cb| cb.schedule(CTAP_CALLBACK_RECEIVED, 0, 0));
+ .map(|mut cb| cb.schedule(CTAP_CALLBACK_RECEIVED, endpoint, 0));
}
}
}
@@ -81,16 +80,16 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> CtapUsbClient for CtapUsbSyscallDri
result
}
- fn packet_received(&self, packet: &[u8; 64], app: Option<&mut App>) {
+ fn packet_received(&self, packet: &[u8; 64], endpoint: usize, app: Option<&mut App>) {
match app {
None => {
for app in self.apps.iter() {
app.enter(|a, _| {
- self.app_packet_received(packet, a);
+ self.app_packet_received(packet, endpoint, a);
})
}
}
- Some(a) => self.app_packet_received(packet, a),
+ Some(a) => self.app_packet_received(packet, endpoint, a),
}
}
@@ -173,7 +172,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> Driver for CtapUsbSyscallDriver<'a,
.unwrap_or_else(|err| err.into())
}
- fn command(&self, cmd_num: usize, _arg1: usize, _arg2: usize, appid: AppId) -> ReturnCode {
+ fn command(&self, cmd_num: usize, endpoint: usize, _arg2: usize, appid: AppId) -> ReturnCode {
match cmd_num {
CTAP_CMD_CHECK => ReturnCode::SUCCESS,
CTAP_CMD_CONNECT => {
@@ -209,14 +208,14 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> Driver for CtapUsbSyscallDriver<'a,
if app.is_ready_for_command(Side::Transmit) {
if app.waiting {
ReturnCode::EALREADY
- } else if self
- .usb_client
- .transmit_packet(app.buffer.as_ref().unwrap().as_ref())
- {
- app.waiting = true;
- ReturnCode::SUCCESS
} else {
- ReturnCode::EBUSY
+ let r = self
+ .usb_client
+ .transmit_packet(app.buffer.as_ref().unwrap().as_ref(), endpoint);
+ if r == ReturnCode::SUCCESS {
+ app.waiting = true;
+ }
+ r
}
} else {
ReturnCode::EINVAL
@@ -263,14 +262,8 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> Driver for CtapUsbSyscallDriver<'a,
ReturnCode::SUCCESS
} else {
// Indicates to the driver that we have a packet to send.
- if self
- .usb_client
- .transmit_packet(app.buffer.as_ref().unwrap().as_ref())
- {
- ReturnCode::SUCCESS
- } else {
- ReturnCode::EBUSY
- }
+ self.usb_client
+ .transmit_packet(app.buffer.as_ref().unwrap().as_ref(), endpoint)
}
}
} else {
@@ -289,7 +282,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> Driver for CtapUsbSyscallDriver<'a,
// FIXME: if cancellation failed, the app should still wait. But that
// doesn't work yet.
app.waiting = false;
- if self.usb_client.cancel_transaction() {
+ if self.usb_client.cancel_transaction(endpoint) {
ReturnCode::SUCCESS
} else {
// Cannot cancel now because the transaction is already in process.
diff --git a/capsules/src/usb/usbc_ctap_hid.rs b/capsules/src/usb/usbc_ctap_hid.rs
index f6762b4b9..16b80cb10 100644
--- a/capsules/src/usb/usbc_ctap_hid.rs
+++ b/capsules/src/usb/usbc_ctap_hid.rs
@@ -18,13 +18,27 @@ use core::cell::Cell;
use kernel::common::cells::OptionalCell;
use kernel::debug;
use kernel::hil;
+use kernel::ReturnCode;
use kernel::hil::usb::TransferType;
static LANGUAGES: &'static [u16; 1] = &[
0x0409, // English (United States)
];
+#[cfg(not(feature = "vendor_hid"))]
+const NUM_ENDPOINTS: usize = 1;
+#[cfg(feature = "vendor_hid")]
+const NUM_ENDPOINTS: usize = 2;
+
const ENDPOINT_NUM: usize = 1;
+#[cfg(feature = "vendor_hid")]
+const VENDOR_ENDPOINT_NUM: usize = ENDPOINT_NUM + 1;
+
+static ENDPOINTS: &'static [usize] = &[
+ ENDPOINT_NUM,
+ #[cfg(feature = "vendor_hid")]
+ VENDOR_ENDPOINT_NUM
+];
static CTAP_REPORT_DESCRIPTOR: &'static [u8] = &[
0x06, 0xD0, 0xF1, // HID_UsagePage ( FIDO_USAGE_PAGE ),
@@ -98,21 +112,42 @@ static VENDOR_HID: HIDDescriptor<'static> = HIDDescriptor {
sub_descriptors: VENDOR_HID_SUB_DESCRIPTORS,
};
-pub struct ClientCtapHID<'a, 'b, C: 'a> {
- client_ctrl: ClientCtrl<'a, 'static, C>,
-
- // 64-byte buffers for the endpoint
+// The state of each endpoint.
+struct EndpointState {
+ endpoint: usize,
in_buffer: Buffer64,
out_buffer: Buffer64,
- // Interaction with the client
- client: OptionalCell<&'b dyn CtapUsbClient>,
tx_packet: OptionalCell<[u8; 64]>,
pending_in: Cell<bool>,
pending_out: Cell<bool>,
+ // Is there a delayed packet?
delayed_out: Cell<bool>,
}
+impl EndpointState {
+ pub fn new(endpoint:usize) -> Self {
+ EndpointState{
+ endpoint: endpoint,
+ in_buffer: Buffer64::default(),
+ out_buffer: Buffer64::default(),
+ tx_packet: OptionalCell::empty(),
+ pending_in: Cell::new(false),
+ pending_out: Cell::new(false),
+ delayed_out: Cell::new(false),
+ }
+ }
+}
+
+pub struct ClientCtapHID<'a, 'b, C: 'a> {
+ client_ctrl: ClientCtrl<'a, 'static, C>,
+
+ endpoints: [EndpointState; NUM_ENDPOINTS],
+
+ // Interaction with the client
+ client: OptionalCell<&'b dyn CtapUsbClient>,
+}
+
impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> {
pub fn new(
controller: &'a C,
@@ -168,7 +203,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> {
&[
EndpointDescriptor {
endpoint_address: EndpointAddress::new_const(
- ENDPOINT_NUM + 1,
+ VENDOR_ENDPOINT_NUM,
TransferDirection::HostToDevice,
),
transfer_type: TransferType::Interrupt,
@@ -177,7 +212,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> {
},
EndpointDescriptor {
endpoint_address: EndpointAddress::new_const(
- ENDPOINT_NUM + 1,
+ VENDOR_ENDPOINT_NUM,
TransferDirection::DeviceToHost,
),
transfer_type: TransferType::Interrupt,
@@ -229,99 +264,135 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> {
LANGUAGES,
strings,
),
- in_buffer: Buffer64::default(),
- out_buffer: Buffer64::default(),
+ endpoints: [
+ EndpointState::new(ENDPOINT_NUM),
+ #[cfg(feature = "vendor_hid")]
+ EndpointState::new(VENDOR_ENDPOINT_NUM),
+ ],
client: OptionalCell::empty(),
- tx_packet: OptionalCell::empty(),
- pending_in: Cell::new(false),
- pending_out: Cell::new(false),
- delayed_out: Cell::new(false),
}
}
+ fn get_endpoint(&'a self, endpoint: usize) -> Option<&'a EndpointState> {
+ for (i, ep) in ENDPOINTS.iter().enumerate() {
+ if endpoint == *ep {
+ return Some(&self.endpoints[i]);
+ }
+ }
+ None
+ }
+
pub fn set_client(&'a self, client: &'b dyn CtapUsbClient) {
self.client.set(client);
}
- pub fn transmit_packet(&'a self, packet: &[u8]) -> bool {
- if self.pending_in.get() {
- // The previous packet has not yet been transmitted, reject the new one.
- false
- } else {
- self.pending_in.set(true);
+ pub fn transmit_packet(&'a self, packet: &[u8], endpoint: usize) -> ReturnCode {
+ if let Some(s) = self.get_endpoint(endpoint) {
+ if s.pending_in.get() {
+ // The previous packet has not yet been transmitted, reject the new one.
+ return ReturnCode::EBUSY;
+ }
+ s.pending_in.set(true);
let mut buf: [u8; 64] = [0; 64];
buf.copy_from_slice(packet);
- self.tx_packet.set(buf);
+ s.tx_packet.set(buf);
// Alert the controller that we now have data to send on the Interrupt IN endpoint.
- self.controller().endpoint_resume_in(1);
- true
+ self.controller().endpoint_resume_in(endpoint);
+ ReturnCode::SUCCESS
+ } else {
+ // Unsupported endpoint
+ ReturnCode::EINVAL
}
}
- pub fn receive_packet(&'a self, app: &mut App) -> bool {
- if self.pending_out.get() {
- // The previous packet has not yet been received, reject the new one.
- false
- } else {
- self.pending_out.set(true);
- // In case we reported Delay before, send the pending packet back to the client.
- // Otherwise, there's nothing to do, the controller will send us a packet_out when a
- // packet arrives.
- if self.delayed_out.take() {
- if self.send_packet_to_client(Some(app)) {
- // If that succeeds, alert the controller that we can now
- // receive data on the Interrupt OUT endpoint.
- self.controller().endpoint_resume_out(1);
+ pub fn receive_packet(&'a self, app: &mut App) {
+ for (_, s) in self.endpoints.iter().enumerate() {
+ if s.pending_out.get() {
+ // The previous packet has not yet been received, reject the new one.
+ continue;
+ } else {
+ s.pending_out.set(true);
+ // In case we reported Delay before, send the pending packet back to the client.
+ // Otherwise, there's nothing to do, the controller will send us a packet_out when a
+ // packet arrives.
+ if s.delayed_out.take() {
+ if self.send_packet_to_client(s.endpoint, Some(app)) {
+ // If that succeeds, alert the controller that we can now
+ // receive data on the Interrupt OUT endpoint.
+ self.controller().endpoint_resume_out(s.endpoint);
+ }
}
}
- true
}
}
// Send an OUT packet available in the controller back to the client.
// This returns false if the client is not ready to receive a packet, and true if the client
// successfully accepted the packet.
- fn send_packet_to_client(&'a self, app: Option<&mut App>) -> bool {
- // Copy the packet into a buffer to send to the client.
- let mut buf: [u8; 64] = [0; 64];
- for (i, x) in self.out_buffer.buf.iter().enumerate() {
- buf[i] = x.get();
- }
-
- assert!(!self.delayed_out.get());
-
- // Notify the client
- if self
- .client
- .map_or(false, |client| client.can_receive_packet(&app))
- {
- assert!(self.pending_out.take());
-
- // Clear any pending packet on the transmitting side.
- // It's up to the client to handle the received packet and decide if this packet
- // should be re-transmitted or not.
- self.cancel_in_transaction();
+ fn send_packet_to_client(&'a self, endpoint: usize, app: Option<&mut App>) -> bool {
+ if let Some(s) = self.get_endpoint(endpoint) {
+ // Copy the packet into a buffer to send to the client.
+ let mut buf: [u8; 64] = [0; 64];
+ for (i, x) in s.out_buffer.buf.iter().enumerate() {
+ buf[i] = x.get();
+ }
- self.client.map(|client| client.packet_received(&buf, app));
- true
+ assert!(!s.delayed_out.get());
+
+ // Notify the client
+ if self
+ .client
+ .map_or(false, |client| client.can_receive_packet(&app))
+ {
+ assert!(s.pending_out.take());
+
+ // Clear any pending packet on the transmitting side.
+ // It's up to the client to handle the received packet and decide if this packet
+ // should be re-transmitted or not.
+ self.cancel_in_transaction(endpoint);
+
+ self.client.map(|client| client.packet_received(&buf, endpoint, app));
+ true
+ } else {
+ // Cannot receive now, indicate a delay to the controller.
+ s.delayed_out.set(true);
+ false
+ }
} else {
- // Cannot receive now, indicate a delay to the controller.
- self.delayed_out.set(true);
+ // Unsupported endpoint
false
}
}
- pub fn cancel_transaction(&'a self) -> bool {
- self.cancel_in_transaction() | self.cancel_out_transaction()
+ // Cancel transaction(s) in process. |endpoint| of 0 indicates all endpoints.
+ pub fn cancel_transaction(&'a self, endpoint: usize) -> bool {
+ if endpoint > 0 {
+ return self.cancel_in_transaction(endpoint) | self.cancel_out_transaction(endpoint);
+ }
+ let mut r = false;
+ for (_, s) in self.endpoints.iter().enumerate() {
+ r |= self.cancel_in_transaction(s.endpoint) | self.cancel_out_transaction(s.endpoint);
+ }
+ r
}
- fn cancel_in_transaction(&'a self) -> bool {
- self.tx_packet.take();
- self.pending_in.take()
+ fn cancel_in_transaction(&'a self, endpoint: usize) -> bool {
+ if let Some(s) = self.get_endpoint(endpoint) {
+ s.tx_packet.take();
+ s.pending_in.take()
+ } else {
+ // Unsupported endpoint
+ false
+ }
}
- fn cancel_out_transaction(&'a self) -> bool {
- self.pending_out.take()
+ fn cancel_out_transaction(&'a self, endpoint: usize) -> bool {
+ if let Some(s) = self.get_endpoint(endpoint) {
+ s.pending_out.take()
+ } else {
+ // Unsupported endpoint
+ false
+ }
}
#[inline]
@@ -335,13 +406,15 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> hil::usb::Client<'a> for ClientCtap
// Set up the default control endpoint
self.client_ctrl.enable();
- // Set up the interrupt in-out endpoint
- self.controller()
- .endpoint_set_in_buffer(1, &self.in_buffer.buf);
- self.controller()
- .endpoint_set_out_buffer(1, &self.out_buffer.buf);
- self.controller()
- .endpoint_in_out_enable(TransferType::Interrupt, 1);
+ // Set up the interrupt in-out endpoint(s).
+ for (i, endpoint) in ENDPOINTS.iter().enumerate() {
+ self.controller()
+ .endpoint_set_in_buffer(*endpoint, &self.endpoints[i].in_buffer.buf);
+ self.controller()
+ .endpoint_set_out_buffer(*endpoint, &self.endpoints[i].out_buffer.buf);
+ self.controller()
+ .endpoint_in_out_enable(TransferType::Interrupt, *endpoint);
+ }
}
fn attach(&'a self) {
@@ -384,20 +457,20 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> hil::usb::Client<'a> for ClientCtap
match transfer_type {
TransferType::Bulk => hil::usb::InResult::Error,
TransferType::Interrupt => {
- if endpoint != 1 {
- return hil::usb::InResult::Error;
- }
-
- if let Some(packet) = self.tx_packet.take() {
- let buf = &self.in_buffer.buf;
- for i in 0..64 {
- buf[i].set(packet[i]);
+ if let Some(s) = self.get_endpoint(endpoint) {
+ if let Some(packet) = s.tx_packet.take() {
+ let buf = &s.in_buffer.buf;
+ for i in 0..64 {
+ buf[i].set(packet[i]);
+ }
+ hil::usb::InResult::Packet(64)
+ } else {
+ // Nothing to send
+ hil::usb::InResult::Delay
}
-
- hil::usb::InResult::Packet(64)
} else {
- // Nothing to send
- hil::usb::InResult::Delay
+ // Unsupported endpoint
+ return hil::usb::InResult::Error
}
}
TransferType::Control | TransferType::Isochronous => unreachable!(),
@@ -414,7 +487,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> hil::usb::Client<'a> for ClientCtap
match transfer_type {
TransferType::Bulk => hil::usb::OutResult::Error,
TransferType::Interrupt => {
- if endpoint != 1 {
+ if endpoint == 0 || endpoint > NUM_ENDPOINTS {
return hil::usb::OutResult::Error;
}
@@ -422,7 +495,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> hil::usb::Client<'a> for ClientCtap
// Cannot process this packet
hil::usb::OutResult::Error
} else {
- if self.send_packet_to_client(None) {
+ if self.send_packet_to_client(endpoint, None) {
hil::usb::OutResult::Ok
} else {
hil::usb::OutResult::Delay
@@ -434,21 +507,21 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> hil::usb::Client<'a> for ClientCtap
}
fn packet_transmitted(&'a self, endpoint: usize) {
- if endpoint != 1 {
- panic!("Unexpected transmission on ep {}", endpoint);
- }
-
- if self.tx_packet.is_some() {
- panic!("Unexpected tx_packet while a packet was being transmitted.");
- }
- self.pending_in.set(false);
+ if let Some(s) = self.get_endpoint(endpoint) {
+ if s.tx_packet.is_some() {
+ panic!("Unexpected tx_packet while a packet was being transmitted.");
+ }
+ s.pending_in.set(false);
- // Clear any pending packet on the receiving side.
- // It's up to the client to handle the transmitted packet and decide if they want to
- // receive another packet.
- self.cancel_out_transaction();
+ // Clear any pending packet on the receiving side.
+ // It's up to the client to handle the transmitted packet and decide if they want to
+ // receive another packet.
+ self.cancel_out_transaction(endpoint);
- // Notify the client
- self.client.map(|client| client.packet_transmitted());
+ // Notify the client
+ self.client.map(|client| client.packet_transmitted());
+ } else {
+ panic!("Unexpected transmission on ep {}", endpoint);
+ }
}
-}
+ }

13
patches/tock/11-usb-endpoint-receive.patch
View File

@@ -1,13 +0,0 @@
diff --git a/capsules/src/usb/usb_ctap.rs b/capsules/src/usb/usb_ctap.rs
index e8f1a87a4..2c1ecf934 100644
--- a/capsules/src/usb/usb_ctap.rs
+++ b/capsules/src/usb/usb_ctap.rs
@@ -57,7 +57,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> CtapUsbSyscallDriver<'a, 'b, C> {
app.waiting = false;
// Signal to the app that a packet is ready.
app.callback
- .map(|mut cb| cb.schedule(CTAP_CALLBACK_RECEIVED, 0, 0));
+ .map(|mut cb| cb.schedule(CTAP_CALLBACK_RECEIVED, 1, 0));
}
}
}

5
third_party/libtock-drivers/src/usb_ctap_hid.rs vendored
View File

@@ -328,8 +328,9 @@ fn send_or_recv_with_timeout_detail(
if matches!(status, Ok(SendOrRecvStatus::Timeout)) { if matches!(status, Ok(SendOrRecvStatus::Timeout)) {
#[cfg(feature = "verbose_usb")] #[cfg(feature = "verbose_usb")]
writeln!(Console::new(), "Cancelling USB transaction due to timeout").unwrap(); writeln!(Console::new(), "Cancelling USB transaction due to timeout").unwrap();
let result_code = let result_code = unsafe {
unsafe { syscalls::raw::command(DRIVER_NUMBER, command_nr::CANCEL, 0, 0) }; syscalls::raw::command(DRIVER_NUMBER, command_nr::CANCEL, endpoint as usize, 0)
};
match result_code { match result_code {
// - SUCCESS means that we successfully cancelled the transaction. // - SUCCESS means that we successfully cancelled the transaction.
// - EALREADY means that the transaction was already completed. // - EALREADY means that the transaction was already completed.