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

Address PR comments

Browse Source
This commit is contained in:
Jean-Michel Picod
2020-11-26 16:29:14 +01:00
parent d491492554
commit 3d1d827984
2 changed files with 21 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
src/ctap/ctap1.rs
View File

@@ -35,6 +35,8 @@ pub enum Ctap1StatusCode {
SW_WRONG_LENGTH = 0x6700, SW_WRONG_LENGTH = 0x6700,
SW_CLA_NOT_SUPPORTED = 0x6E00, SW_CLA_NOT_SUPPORTED = 0x6E00,
SW_INS_NOT_SUPPORTED = 0x6D00, SW_INS_NOT_SUPPORTED = 0x6D00,
SW_MEMERR = 0x6501,
SW_COMMAND_ABORTED = 0x6F00,
SW_VENDOR_KEY_HANDLE_TOO_LONG = 0xF000, SW_VENDOR_KEY_HANDLE_TOO_LONG = 0xF000,
} }
@@ -49,6 +51,8 @@ impl TryFrom<u16> for Ctap1StatusCode {
0x6700 => Ok(Ctap1StatusCode::SW_WRONG_LENGTH), 0x6700 => Ok(Ctap1StatusCode::SW_WRONG_LENGTH),
0x6E00 => Ok(Ctap1StatusCode::SW_CLA_NOT_SUPPORTED), 0x6E00 => Ok(Ctap1StatusCode::SW_CLA_NOT_SUPPORTED),
0x6D00 => Ok(Ctap1StatusCode::SW_INS_NOT_SUPPORTED), 0x6D00 => Ok(Ctap1StatusCode::SW_INS_NOT_SUPPORTED),
0x6501 => Ok(Ctap1StatusCode::SW_MEMERR),
0x6F00 => Ok(Ctap1StatusCode::SW_COMMAND_ABORTED),
0xF000 => Ok(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG), 0xF000 => Ok(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG),
_ => Err(()), _ => Err(()),
} }
@@ -288,20 +292,22 @@ impl Ctap1Command {
let pk = sk.genpk(); let pk = sk.genpk();
let key_handle = ctap_state let key_handle = ctap_state
.encrypt_key_handle(sk, &application, None) .encrypt_key_handle(sk, &application, None)
.map_err(|_| Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG)?; .map_err(|_| Ctap1StatusCode::SW_COMMAND_ABORTED)?;
if key_handle.len() > 0xFF { if key_handle.len() > 0xFF {
// This is just being defensive with unreachable code. // This is just being defensive with unreachable code.
return Err(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG); return Err(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG);
} }
let certificate = match ctap_state.persistent_store.attestation_certificate() { let certificate = ctap_state
Ok(Some(value)) => value, .persistent_store
_ => return Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED), .attestation_certificate()
}; .map_err(|_| Ctap1StatusCode::SW_MEMERR)?
let private_key = match ctap_state.persistent_store.attestation_private_key() { .ok_or(Ctap1StatusCode::SW_COMMAND_ABORTED)?;
Ok(Some(value)) => value, let private_key = ctap_state
_ => return Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED), .persistent_store
}; .attestation_private_key()
.map_err(|_| Ctap1StatusCode::SW_MEMERR)?
.ok_or(Ctap1StatusCode::SW_COMMAND_ABORTED)?;
let mut response = Vec::with_capacity(105 + key_handle.len() + certificate.len()); let mut response = Vec::with_capacity(105 + key_handle.len() + certificate.len());
response.push(Ctap1Command::LEGACY_BYTE); response.push(Ctap1Command::LEGACY_BYTE);
@@ -442,7 +448,7 @@ mod test {
ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE);
let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE);
// Certificate and private key are missing // Certificate and private key are missing
assert_eq!(response, Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)); assert_eq!(response, Err(Ctap1StatusCode::SW_COMMAND_ABORTED));
let fake_key = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH]; let fake_key = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH];
assert!(ctap_state assert!(ctap_state
@@ -453,7 +459,7 @@ mod test {
ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE); ctap_state.u2f_up_state.grant_up(START_CLOCK_VALUE);
let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE); let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE);
// Certificate is still missing // Certificate is still missing
assert_eq!(response, Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)); assert_eq!(response, Err(Ctap1StatusCode::SW_COMMAND_ABORTED));
let fake_cert = [0x99u8; 100]; // Arbitrary length let fake_cert = [0x99u8; 100]; // Arbitrary length
assert!(ctap_state assert!(ctap_state

8
src/ctap/storage.rs
View File

@@ -230,8 +230,8 @@ impl PersistentStore {
.unwrap(); .unwrap();
} }
// TODO(jmichel): remove this when vendor command is in place // TODO(jmichel): remove this when vendor command is in place
#[cfg(not(any(test, feature = "ram_storage")))] #[cfg(not(test))]
self.load_attestation_from_firmware(); self.load_attestation_data_from_firmware();
if self.store.find_one(&Key::Aaguid).is_none() { if self.store.find_one(&Key::Aaguid).is_none() {
self.set_aaguid(key_material::AAGUID).unwrap(); self.set_aaguid(key_material::AAGUID).unwrap();
@@ -239,8 +239,8 @@ impl PersistentStore {
} }
// TODO(jmichel): remove this function when vendor command is in place. // TODO(jmichel): remove this function when vendor command is in place.
#[cfg(not(any(test, feature = "ram_storage")))] #[cfg(not(test))]
fn load_attestation_from_firmware(&mut self) { fn load_attestation_data_from_firmware(&mut self) {
// The following 2 entries are meant to be written by vendor-specific commands. // The following 2 entries are meant to be written by vendor-specific commands.
if self.store.find_one(&Key::AttestationPrivateKey).is_none() { if self.store.find_one(&Key::AttestationPrivateKey).is_none() {
self.set_attestation_private_key(key_material::ATTESTATION_PRIVATE_KEY) self.set_attestation_private_key(key_material::ATTESTATION_PRIVATE_KEY)