diff --git a/Cargo.toml b/Cargo.toml index f364d90d8..07a10da2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,3 +39,6 @@ log-debug = [] log-warn = [] log-error = [] + +[patch.crates-io] +flexiber = { git = "https://github.com/Nitrokey/flexiber", rev = "0.1.1.nitrokey" } \ No newline at end of file diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 000000000..66bd9ca0d --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,47 @@ +[package] +name = "oath-authenticator-fuzz" +version = "0.0.0" +publish = false +edition = "2021" + +[package.metadata] +cargo-fuzz = true + +[dependencies] +libfuzzer-sys = "0.4" +apdu-dispatch = { version = "0.1", optional = true } +flexiber = { version = "0.1.0", features = ["derive", "heapless"] } +heapless = "0.7" +heapless-bytes = "0.3" +hex-literal = "0.3" +interchange = "0.2" +iso7816 = "0.1" +serde = { version = "1", default-features = false } +trussed = { version = "0.1.0", features = ["virt", "verbose-tests"] } +ctaphid-dispatch = { version = "0.1", optional = true } +usbd-ctaphid = { git = "https://github.com/Nitrokey/nitrokey-3-firmware", optional = true } + +[dependencies.oath-authenticator] +path = ".." + +[features] +default = ["ctaphid-dispatch", "usbd-ctaphid", "apdu-dispatch"] + + +# Prevent this from interfering with workspaces +[workspace] +members = ["."] + +[profile.release] +debug = 1 + +[[bin]] +name = "fuzz_target_1" +path = "fuzz_targets/fuzz_target_1.rs" +test = false +doc = false + + +[patch.crates-io] +trussed = { git = "https://github.com/trussed-dev/trussed", branch = "main" } +flexiber = { git = "https://github.com/Nitrokey/flexiber", branch = "oath-authenticator" } \ No newline at end of file diff --git a/fuzz/Makefile b/fuzz/Makefile new file mode 100644 index 000000000..aca6cebc1 --- /dev/null +++ b/fuzz/Makefile @@ -0,0 +1,37 @@ +# Copyright (C) 2022 Nitrokey GmbH +# SPDX-License-Identifier: CC0-1.0 + +FUZZ_DURATION?="0" +FUZZ_JOBS?=$(shell nproc) +.NOTPARALLEL: + +.PHONY: check +check: + reuse lint + +.PHONY: fuzz +fuzz: + nice cargo +nightly fuzz run --jobs ${FUZZ_JOBS} fuzz_target_1 corpus -- -max_total_time=${FUZZ_DURATION} + +.PHONY: fuzz-cov +fuzz-cov: + cargo +nightly fuzz coverage fuzz_target_1 corpus + $(MAKE) fuzz-cov-show + +LLVMCOV=~/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-cov +.PHONY: fuzz-cov-show +fuzz-cov-show: + $(LLVMCOV) show --format=html \ + --instr-profile=coverage/fuzz_target_1/coverage.profdata \ + ${CARGO_TARGET_DIR}/x86_64-unknown-linux-gnu/release/fuzz_target_1 \ + > fuzz_coverage.html + +.PHONY: ci +ci: check + +.PHONY: setup +setup: + rustup component add clippy rustfmt && rustup toolchain install nightly + rustup component add llvm-tools-preview + cargo install cargo-tarpaulin cargo-fuzz --profile release + python3 -m pip install reuse diff --git a/fuzz/fuzz_targets/fuzz_target_1.rs b/fuzz/fuzz_targets/fuzz_target_1.rs new file mode 100644 index 000000000..82f10f61a --- /dev/null +++ b/fuzz/fuzz_targets/fuzz_target_1.rs @@ -0,0 +1,14 @@ +#![no_main] + +use libfuzzer_sys::fuzz_target; + +fuzz_target!(|data: &[u8]| { + trussed::virt::with_ram_client("oath", move |client| { + let mut oath = oath_authenticator::Authenticator::<_>::new(client); + let mut response = heapless::Vec::::new(); + + if let Ok(command) = iso7816::Command::<{ 10 * 255 }>::try_from(&data) { + oath.respond(&command, &mut response).ok(); + } + }) +}); diff --git a/src/authenticator.rs b/src/authenticator.rs index d36a0622c..f54c975d3 100644 --- a/src/authenticator.rs +++ b/src/authenticator.rs @@ -15,7 +15,7 @@ use trussed::{ types::{KeyId, Location, PathBuf}, }; -use crate::{command, Command, oath, state::{CommandState, State}}; +use crate::{ensure, command, Command, oath, state::{CommandState, State}}; use crate::command::VerifyCode; use crate::oath::Kind; @@ -93,6 +93,10 @@ pub struct ChallengingAnswerToSelect { // instead of '74 00', as with the tagged/Option derivation. #[tlv(simple = "0x74")] // Tag::Challenge challenge: [u8; 8], + + #[tlv(simple = "0x7b")] // Tag::Algorithm + // algorithm: oath::Algorithm, + algorithm: [u8; 1], } impl AnswerToSelect { @@ -114,6 +118,8 @@ impl AnswerToSelect { version: self.version, salt: self.salt, challenge: challenge, + // algorithm: oath::Algorithm::Sha1 // TODO set proper algo + algorithm: [0x01] // TODO set proper algo } } } @@ -174,9 +180,9 @@ where fn inner_respond(&mut self, command: &iso7816::Command, reply: &mut Data) -> Result { let class = command.class(); - assert!(class.chain().last_or_only()); - assert!(class.secure_messaging().none()); - assert!(class.channel() == Some(0)); + ensure(class.chain().last_or_only(), Status::CommandChainingNotSupported)?; + ensure(class.secure_messaging().none(), Status::SecureMessagingNotSupported)?; + ensure(class.channel() == Some(0), Status::ClassNotSupported)?; // parse Iso7816Command as PivCommand let command: Command = command.try_into()?; @@ -326,7 +332,8 @@ where self.state.runtime.previously = Some(CommandState::ListCredentials(file_index)); // deserialize - let credential: Credential = postcard_deserialize(&serialized_credential).unwrap(); + let credential: Credential = postcard_deserialize(&serialized_credential) + .map_err(|_| Status::IncorrectDataParameter)?; // append data in form: // 72 @@ -452,7 +459,8 @@ where // info_now!("serialized credential: {}", hex_str!(&serialized_credential)); // deserialize - let credential: Credential = postcard_deserialize(&serialized_credential).unwrap(); + let credential: Credential = postcard_deserialize(&serialized_credential) + .map_err(|_| Status::UnspecifiedPersistentExecutionError)?; // add to response reply.push(0x71).unwrap(); @@ -746,13 +754,17 @@ where } } - if found.is_none() { - // Failed verification - self.wink_bad(); - return Err(Status::VerificationFailed); - } + let found = match found { + None => { + // Failed verification + self.wink_bad(); + self.delay_on_failure(); + return Err(Status::VerificationFailed); + } + Some(val) => val + }; - self.bump_counter_for_cred(credential, found.unwrap())?; + self.bump_counter_for_cred(credential, found)?; self.wink_good(); // Verification passed @@ -787,13 +799,12 @@ where ); // load-bump counter let filename = self.filename_for_label(credential.label); - // TODO: use try_syscall - syscall!(self.trussed.write_file( + try_syscall!(self.trussed.write_file( Location::Internal, filename, postcard_serialize_bytes(&credential).unwrap(), None - )); + )).map_err(|_| Status::UnspecifiedPersistentExecutionError)?; Ok(()) } @@ -827,6 +838,12 @@ where // TODO blink green LED for 10 seconds, highest priority syscall!(self.trussed.wink(Duration::from_secs(10))); } + + fn delay_on_failure(&mut self){ + use crate::FAILURE_FORCED_DELAY_MILLISECONDS; + // TODO block for the time defined in the constant + // DESIGN allow only a couple of failures per power cycle? Similarly to the FIDO2 PIN + } } #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] diff --git a/src/command.rs b/src/command.rs index 80f826d9c..ba62aa18a 100644 --- a/src/command.rs +++ b/src/command.rs @@ -2,7 +2,7 @@ use core::convert::{TryFrom, TryInto}; use iso7816::{Data, Status}; -use crate::oath; +use crate::{ensure, oath}; const FAILED_PARSING_ERROR: Status = iso7816::Status::IncorrectDataParameter; @@ -72,7 +72,8 @@ impl<'l, const C: usize> TryFrom<&'l Data> for SetPassword<'l> { use flexiber::TaggedSlice; let mut decoder = flexiber::Decoder::new(data); let slice: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(slice.tag() == (oath::Tag::Key as u8).try_into().unwrap()); + ensure(slice.tag() == (oath::Tag::Key as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; + if slice.as_bytes().len() < 2 { return Err(FAILED_PARSING_ERROR) }; let (key_header, key) = slice.as_bytes().split_at(1); let kind: oath::Kind = key_header[0].try_into()?; @@ -81,12 +82,12 @@ impl<'l, const C: usize> TryFrom<&'l Data> for SetPassword<'l> { // assert!(algorithm == oath::Algorithm::Sha1); let slice: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(slice.tag() == (oath::Tag::Challenge as u8).try_into().unwrap()); + ensure(slice.tag() == (oath::Tag::Challenge as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let challenge = slice.as_bytes(); // assert_eq!(challenge.len(), 8); let slice: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(slice.tag() == (oath::Tag::Response as u8).try_into().unwrap()); + ensure(slice.tag() == (oath::Tag::Response as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let response = slice.as_bytes(); // assert_eq!(response.len(), 20); @@ -113,11 +114,11 @@ impl<'l, const C: usize> TryFrom<&'l Data> for Validate<'l> { let mut decoder = flexiber::Decoder::new(data); let slice: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(slice.tag() == (oath::Tag::Response as u8).try_into().unwrap()); + ensure(slice.tag() == (oath::Tag::Response as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let response = slice.as_bytes(); let slice: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(slice.tag() == (oath::Tag::Challenge as u8).try_into().unwrap()); + ensure(slice.tag() == (oath::Tag::Challenge as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let challenge = slice.as_bytes(); Ok(Validate { challenge, response }) @@ -137,11 +138,11 @@ impl<'l, const C: usize> TryFrom<&'l Data> for VerifyCode<'l> { let mut decoder = flexiber::Decoder::new(data); let first: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(first.tag() == (oath::Tag::Name as u8).try_into().unwrap()); + ensure(first.tag() == (oath::Tag::Name as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let label = first.as_bytes(); let slice: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(slice.tag() == (oath::Tag::Response as u8).try_into().unwrap()); + ensure(slice.tag() == (oath::Tag::Response as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let response = u32::from_be_bytes(slice.as_bytes().try_into().map_err(|_| FAILED_PARSING_ERROR )?); Ok(VerifyCode { label, response }) @@ -161,11 +162,11 @@ impl<'l, const C: usize> TryFrom<&'l Data> for Calculate<'l> { let mut decoder = flexiber::Decoder::new(data); let first: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(first.tag() == (oath::Tag::Name as u8).try_into().unwrap()); + ensure(first.tag() == (oath::Tag::Name as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let label = first.as_bytes(); let second: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(second.tag() == (oath::Tag::Challenge as u8).try_into().unwrap()); + ensure(second.tag() == (oath::Tag::Challenge as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let challenge = second.as_bytes(); Ok(Calculate { label, challenge }) @@ -184,7 +185,7 @@ impl<'l, const C: usize> TryFrom<&'l Data> for CalculateAll<'l> { let mut decoder = flexiber::Decoder::new(data); let first: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(first.tag() == (oath::Tag::Challenge as u8).try_into().unwrap()); + ensure(first.tag() == (oath::Tag::Challenge as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let challenge = first.as_bytes(); Ok(CalculateAll { challenge }) @@ -213,7 +214,7 @@ impl<'l, const C: usize> TryFrom<&'l Data> for Delete<'l> { let mut decoder = flexiber::Decoder::new(data); let first: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - assert!(first.tag() == (oath::Tag::Name as u8).try_into().unwrap()); + ensure(first.tag() == (oath::Tag::Name as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let label = first.as_bytes(); Ok(Delete { label }) @@ -284,7 +285,7 @@ impl<'a> flexiber::Decodable<'a> for Properties { let two_bytes: [u8; 2] = decoder.decode()?; let [tag, properties] = two_bytes; use flexiber::Tagged; - assert_eq!(flexiber::Tag::try_from(tag).unwrap(), Self::tag()); + ensure(flexiber::Tag::try_from(tag).unwrap() == Self::tag(), flexiber::ErrorKind::Failed)?; Ok(Properties(properties)) } } @@ -305,13 +306,14 @@ impl<'l, const C: usize> TryFrom<&'l Data> for Register<'l> { // first comes the label of the credential, with Tag::Name let first: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - // TODO make asserts recoverable errors - assert!(first.tag() == (oath::Tag::Name as u8).try_into().unwrap()); + ensure(first.tag() == (oath::Tag::Name as u8).try_into().unwrap(), FAILED_PARSING_ERROR)?; let label = first.as_bytes(); // then come (kind,algorithm,digits) and the actual secret (somewhat massaged) let second: TaggedSlice = decoder.decode().map_err(|_| FAILED_PARSING_ERROR)?; - second.tag().assert_eq((oath::Tag::Key as u8).try_into().unwrap()).unwrap(); + second.tag().assert_eq((oath::Tag::Key as u8).try_into().unwrap()).map_err(|_| FAILED_PARSING_ERROR)?; + + if second.as_bytes().len() < 3 { return Err(FAILED_PARSING_ERROR) }; let (secret_header, secret) = second.as_bytes().split_at(2); let kind: oath::Kind = secret_header[0].try_into()?; diff --git a/src/lib.rs b/src/lib.rs index 9547c65de..99ce938b7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,6 +23,7 @@ pub const YUBICO_OATH_AID: &[u8] = &hex!("A000000527 2101");// 01"); /// This constant defines timeout for the regular UP confirmation pub const UP_TIMEOUT_MILLISECONDS: u32 = 15 * 1000; +pub const FAILURE_FORCED_DELAY_MILLISECONDS: u32 = 1 * 1000; // class AID(bytes, Enum): // OTP = b'\xa0\x00\x00\x05\x27 \x20\x01' @@ -32,3 +33,11 @@ pub const UP_TIMEOUT_MILLISECONDS: u32 = 15 * 1000; // PIV = b'\xa0\x00\x00\x03\x08' // U2F = b'\xa0\x00\x00\x06\x47\x2f\x00\x01' # Official // U2F_YUBICO = b'\xa0\x00\x00\x05\x27\x10\x02' # Yubico - No longer used + + +fn ensure(cond: bool, err: T) -> core::result::Result<(), T> { + match cond { + true => Ok(()), + false => Err(err) + } +} \ No newline at end of file diff --git a/src/state.rs b/src/state.rs index 33cd08105..b9eddddd3 100644 --- a/src/state.rs +++ b/src/state.rs @@ -94,12 +94,12 @@ impl State { let result = f(trussed, &mut state); // 3. Always write it back - syscall!(trussed.write_file( + try_syscall!(trussed.write_file( Location::Internal, PathBuf::from(Self::FILENAME), postcard_serialize_bytes(&state).unwrap(), None, - )); + )).map_err(|_| Status::NotEnoughMemory)?; // 4. Return whatever result