From 0e6dbe57f1c9a4e7aedcafedeb243c53526631c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 21 Nov 2022 16:02:32 +0100 Subject: [PATCH] Add support for AES keys in SET MANAGEMENT KEY and test AES keys for GENERAL AUTHENTICATE --- Cargo.toml | 2 +- src/lib.rs | 39 ++++++++++++++++++------- src/piv_types.rs | 6 ++++ src/state.rs | 18 +++++++++--- tests/command_response.ron | 50 ++++++++++++++++++++++++++++++-- tests/command_response.rs | 59 ++++++++++++++++++++++++++++++++------ 6 files changed, 148 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2711082..9d0eab3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,4 +58,4 @@ log-warn = [] log-error = [] [patch.crates-io] -trussed = { git = "https://github.com/trussed-dev/trussed", rev = "28478f8abed11d78c51e6a6a32326821ed61957a"} +trussed = { git = "https://github.com/sosthene-nitrokey/trussed", rev = "cbf8f3cc759fa79275fe06d3ce4661d6a4f306aa"} diff --git a/src/lib.rs b/src/lib.rs index 1e122ee..be0e796 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ extern crate log; delog::generate_macros!(); pub mod commands; +use commands::containers::KeyReference; use commands::GeneralAuthenticate; pub use commands::{Command, YubicoPivExtension}; pub mod constants; @@ -275,7 +276,7 @@ impl<'a, T: trussed::Client + trussed::client::Ed255> LoadedAuthenticator<'a, T> &mut self, data: &[u8], _touch_policy: TouchPolicy, - reply: &mut Data, + _reply: &mut Data, ) -> Result { // cmd := apdu{ // instruction: insSetMGMKey, @@ -286,25 +287,43 @@ impl<'a, T: trussed::Client + trussed::client::Ed255> LoadedAuthenticator<'a, T> // }, key[:]...), // } + // TODO _touch_policy + if !self.state.runtime.app_security_status.management_verified { return Err(Status::SecurityStatusNotSatisfied); } // example: 03 9B 18 // B0 20 7A 20 DC 39 0B 1B A5 56 CC EB 8D CE 7A 8A C8 23 E6 F5 0D 89 17 AA - if data.len() != 3 + 24 { + if data.len() < 4 { + warn!("Set management key with incorrect data"); return Err(Status::IncorrectDataParameter); } - let (prefix, new_management_key) = data.split_at(3); - if prefix != [0x03, 0x9b, 0x18] { + + let key_data = &data[3..]; + + let Ok(alg) = ManagementAlgorithm::try_from(data[0]) else { + warn!("Set management key with incorrect alg: {:x}", data[0]); + return Err(Status::IncorrectDataParameter); + }; + + if KeyReference::PivCardApplicationAdministration != data[1] { + warn!( + "Set management key with incorrect reference: {:x}, expected: {:x}", + data[1], + KeyReference::PivCardApplicationAdministration as u8 + ); return Err(Status::IncorrectDataParameter); } - let new_management_key: [u8; 24] = new_management_key.try_into().unwrap(); - self.state.persistent.set_management_key( - &new_management_key, - ManagementAlgorithm::Tdes, - self.trussed, - ); + + if data[2] as usize != key_data.len() || alg.key_len() != key_data.len() { + warn!("Set management key with incorrect data length: claimed: {}, required by algorithm: {}, real: {}", data[2], alg.key_len(), key_data.len()); + return Err(Status::IncorrectDataParameter); + } + + self.state + .persistent + .set_management_key(key_data, alg, self.trussed); Ok(()) } diff --git a/src/piv_types.rs b/src/piv_types.rs index be1975c..7f27da9 100644 --- a/src/piv_types.rs +++ b/src/piv_types.rs @@ -35,6 +35,12 @@ macro_rules! enum_u8 { } } } + + impl PartialEq for $name { + fn eq(&self, other: &u8) -> bool { + *self as u8 == *other + } + } } } diff --git a/src/state.rs b/src/state.rs index b7bfbd4..e6de4d7 100644 --- a/src/state.rs +++ b/src/state.rs @@ -156,6 +156,13 @@ impl ManagementAlgorithm { Self::Aes256 => Mechanism::Aes256Cbc, } } + + pub fn key_len(self) -> usize { + match self { + Self::Tdes => 24, + Self::Aes256 => 32, + } + } } #[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)] @@ -451,10 +458,13 @@ impl Persistent { client: &mut impl trussed::Client, ) { // let new_management_key = syscall!(self.trussed.unsafe_inject_tdes_key( - let id = - syscall!(client - .unsafe_inject_shared_key(management_key, trussed::types::Location::Internal,)) - .key; + let id = syscall!(client.unsafe_inject_key( + alg.mechanism(), + management_key, + trussed::types::Location::Internal, + KeySerialization::Raw + )) + .key; let old_management_key = self.keys.management_key.id; self.keys.management_key = ManagementKey { id, alg }; self.save(client); diff --git a/tests/command_response.ron b/tests/command_response.ron index 8cd1856..2b74bad 100644 --- a/tests/command_response.ron +++ b/tests/command_response.ron @@ -15,13 +15,57 @@ Select ] ), - // TODO: test with AES IoTest( name: "Default management key", cmd_resp: [ AuthenticateManagement( - algorithm: Tdes, - key: "0102030405060708 0102030405060708 0102030405060708" + key: ( + algorithm: Tdes, + key: "0102030405060708 0102030405060708 0102030405060708" + ) + ) + ] + ), + IoTest( + name: "Aes management key", + cmd_resp: [ + AuthenticateManagement( + key: ( + algorithm: Tdes, + key: "0102030405060708 0102030405060708 0102030405060708" + ) + ), + SetManagementKey( + key: ( + algorithm: Aes256, + key: "0102030405060708 0102030405060708 0102030405060708 0102030405060708" + ) + ), + AuthenticateManagement( + key: ( + algorithm: Aes256, + key: "0102030405060708 0102030405060708 0102030405060708 0102030405060708" + ) + ) + ] + ), + IoTest( + name: "unauthenticated set management key", + cmd_resp: [ + SetManagementKey( + key: ( + algorithm: Aes256, + key: "0102030405060708 0102030405060708 0102030405060708 0102030405060708" + ), + expected_status: SecurityStatusNotSatisfied, + ), + AuthenticateManagement( + key: ( + algorithm: Aes256, + key: "0102030405060708 0102030405060708 0102030405060708 0102030405060708" + ), + expected_status_challenge: IncorrectP1OrP2Parameter, + expected_status_response: IncorrectDataParameter, ) ] ), diff --git a/tests/command_response.rs b/tests/command_response.rs index 340ae1c..5cf95a8 100644 --- a/tests/command_response.rs +++ b/tests/command_response.rs @@ -73,6 +73,14 @@ impl Algorithm { _ => panic!(), } } + + pub fn key_len(self) -> usize { + match self { + Self::Tdes => 24, + Self::Aes256 => 32, + _ => panic!(), + } + } } fn serialize_len(len: usize) -> heapless::Vec { @@ -227,6 +235,13 @@ impl OutputMatcher { } } +#[derive(Deserialize, Debug)] +#[serde(deny_unknown_fields)] +struct ManagementKey { + algorithm: Algorithm, + key: String, +} + #[derive(Deserialize, Debug)] #[serde(deny_unknown_fields)] enum IoCmd { @@ -245,9 +260,13 @@ enum IoCmd { #[serde(default)] expected_status: Status, }, + SetManagementKey { + key: ManagementKey, + #[serde(default)] + expected_status: Status, + }, AuthenticateManagement { - algorithm: Algorithm, - key: String, + key: ManagementKey, #[serde(default)] expected_status_challenge: Status, #[serde(default)] @@ -274,21 +293,42 @@ impl IoCmd { Self::run_verify_default_global_pin(*expected_status, card) } Self::AuthenticateManagement { - algorithm, key, expected_status_challenge, expected_status_response, } => Self::run_authenticate_management( - algorithm, - key, + key.algorithm, + &key.key, *expected_status_challenge, *expected_status_response, card, ), + Self::SetManagementKey { + key, + expected_status, + } => Self::run_set_management_key(key.algorithm, &key.key, *expected_status, card), Self::Select => Self::run_select(card), } } + fn run_set_management_key( + alg: Algorithm, + key: &str, + expected_status: Status, + card: &mut setup::Piv, + ) { + let mut key_data = parse_hex(key); + let mut data = vec![alg as u8, 0x9b, key_data.len() as u8]; + data.append(&mut key_data); + + Self::run_bytes( + &build_command(0x00, 0xff, 0xff, 0xff, &data, 0), + &MATCH_ANY, + expected_status, + card, + ); + } + fn run_bytes( input: &[u8], output: &OutputMatcher, @@ -328,7 +368,7 @@ impl IoCmd { } fn run_authenticate_management( - alg: &Algorithm, + alg: Algorithm, key: &str, expected_status_challenge: Status, expected_status_response: Status, @@ -338,9 +378,12 @@ impl IoCmd { cipher::{BlockEncrypt, KeyInit}, TdesEde3, }; - let command = build_command(0x00, 0x87, *alg as u8, 0x9B, &hex!("7C 02 81 00"), 0); + let command = build_command(0x00, 0x87, alg as u8, 0x9B, &hex!("7C 02 81 00"), 0); let mut res = Self::run_bytes(&command, &MATCH_ANY, expected_status_challenge, card); let key = parse_hex(key); + if expected_status_challenge != Status::Success && res.is_empty() { + res = heapless::Vec::from_slice(&vec![0; alg.challenge_len() + 6]).unwrap(); + } // Remove header let challenge = &mut res[6..][..alg.challenge_len()]; @@ -356,7 +399,7 @@ impl IoCmd { _ => panic!(), } let second_data = tlv(&[0x7C], &tlv(&[0x82], challenge)); - let command = build_command(0x00, 0x87, *alg as u8, 0x9B, &second_data, 0); + let command = build_command(0x00, 0x87, alg as u8, 0x9B, &second_data, 0); Self::run_bytes(&command, &MATCH_ANY, expected_status_response, card); }