Skip to content

Commit

Permalink
adds a feature flag for CTAP2.1, addresses comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kaczmarczyck committed Apr 28, 2020
1 parent 8f20a75 commit d9c4c72
Show file tree
Hide file tree
Showing 8 changed files with 274 additions and 60 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ std = ["cbor/std", "crypto/std", "crypto/derive_debug"]
ram_storage = []
verbose = ["debug_ctap"]
with_ctap1 = ["crypto/with_ctap1"]
with_ctap2_1 = []

[dev-dependencies]
elf2tab = "0.4.0"
Expand Down
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ few limitations:
Although we tested and implemented our firmware based on the published
[CTAP2.0 specifications](https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html),
our implementation was not reviewed nor officially tested and doesn't claim to
be FIDO Certified. With the upcoming next version of the
[CTAP2.1 specifications](https://fidoalliance.org/specs/fido2/fido-client-to-authenticator-protocol-v2.1-rd-20191217.html),
we started adding features, so master is currently between version 2.0 and 2.1.
be FIDO Certified.
We started adding features of the upcoming next version of the
[CTAP2.1 specifications](https://fidoalliance.org/specs/fido2/fido-client-to-authenticator-protocol-v2.1-rd-20191217.html).
The development is currently between 2.0 and 2.1, with updates hidden behind a feature flag.
Please add the flag `shell --ctap2-1` to the deploy command to include them.

### Cryptography

Expand Down
8 changes: 8 additions & 0 deletions deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,14 @@ def main(args):
help=("Compiles the OpenSK application without backward compatible "
"support for U2F/CTAP1 protocol."),
)
main_parser.add_argument(
"--ctap2-1",
action=RemoveConstAction,
const="with_ctap2_1",
dest="features",
help=("Compiles the OpenSK application with backward compatible "
"support for CTAP2.1 protocol."),
)
main_parser.add_argument(
"--regen-keys",
action="store_true",
Expand Down
7 changes: 7 additions & 0 deletions run_desktop_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ cargo build --manifest-path third_party/tock/tools/sha256sum/Cargo.toml
echo "Checking that CTAP2 builds properly..."
cargo check --release --target=thumbv7em-none-eabi
cargo check --release --target=thumbv7em-none-eabi --features with_ctap1
cargo check --release --target=thumbv7em-none-eabi --features with_ctap2_1
cargo check --release --target=thumbv7em-none-eabi --features debug_ctap
cargo check --release --target=thumbv7em-none-eabi --features panic_console
cargo check --release --target=thumbv7em-none-eabi --features debug_allocations
Expand Down Expand Up @@ -86,4 +87,10 @@ then

echo "Running unit tests on the desktop (debug mode + CTAP1)..."
cargo test --features std,with_ctap1

echo "Running unit tests on the desktop (release mode + CTAP2.1)..."
cargo test --release --features std,with_ctap2_1

echo "Running unit tests on the desktop (debug mode + CTAP2.1)..."
cargo test --features std,with_ctap2_1
fi
49 changes: 20 additions & 29 deletions src/ctap/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ use alloc::string::String;
use alloc::vec::Vec;
use core::convert::TryFrom;

// Depending on your memory, you can use Some(n) to limit request sizes.
// Depending on your memory, you can use Some(n) to limit request sizes in
// MakeCredential and GetAssertion. This affects allowList and excludeList.
// You might also want to set the max credential size in process_get_info then.
pub const MAX_CREDENTIAL_COUNT_IN_LIST: Option<u64> = None;
pub const MAX_CREDENTIAL_COUNT_IN_LIST: Option<usize> = None;

// CTAP specification (version 20190130) section 6.1
#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))]
Expand Down Expand Up @@ -136,25 +137,19 @@ impl TryFrom<cbor::Value> for AuthenticatorMakeCredentialParameters {
)?)?;

let cred_param_vec = read_array(ok_or_missing(param_map.get(&cbor_unsigned!(4)))?)?;
let mut pub_key_cred_params = vec![];
for cred_param_map_value in cred_param_vec {
if let Ok(cred_param) = PublicKeyCredentialParameter::try_from(cred_param_map_value) {
pub_key_cred_params.push(cred_param);
}
}
let pub_key_cred_params = cred_param_vec
.iter()
.map(PublicKeyCredentialParameter::try_from)
.collect::<Result<Vec<PublicKeyCredentialParameter>, Ctap2StatusCode>>()?;

let exclude_list = match param_map.get(&cbor_unsigned!(5)) {
Some(entry) => {
let exclude_list_vec = read_array(entry)?;
let mut exclude_list = vec![];
for exclude_list_value in exclude_list_vec {
if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST {
if exclude_list.len() as u64 >= count {
break;
}
}
exclude_list.push(PublicKeyCredentialDescriptor::try_from(exclude_list_value)?);
}
let exclude_list = exclude_list_vec
.iter()
.take(MAX_CREDENTIAL_COUNT_IN_LIST.unwrap_or(exclude_list_vec.len()))
.map(PublicKeyCredentialDescriptor::try_from)
.collect::<Result<Vec<PublicKeyCredentialDescriptor>, Ctap2StatusCode>>()?;
Some(exclude_list)
}
None => None,
Expand Down Expand Up @@ -222,15 +217,11 @@ impl TryFrom<cbor::Value> for AuthenticatorGetAssertionParameters {
let allow_list = match param_map.get(&cbor_unsigned!(3)) {
Some(entry) => {
let allow_list_vec = read_array(entry)?;
let mut allow_list = vec![];
for allow_list_value in allow_list_vec {
if let Some(count) = MAX_CREDENTIAL_COUNT_IN_LIST {
if allow_list.len() as u64 >= count {
break;
}
}
allow_list.push(PublicKeyCredentialDescriptor::try_from(allow_list_value)?);
}
let allow_list = allow_list_vec
.iter()
.take(MAX_CREDENTIAL_COUNT_IN_LIST.unwrap_or(allow_list_vec.len()))
.map(PublicKeyCredentialDescriptor::try_from)
.collect::<Result<Vec<PublicKeyCredentialDescriptor>, Ctap2StatusCode>>()?;
Some(allow_list)
}
None => None,
Expand Down Expand Up @@ -330,7 +321,7 @@ mod test {
AuthenticatorTransport, PublicKeyCredentialRpEntity, PublicKeyCredentialType,
PublicKeyCredentialUserEntity,
};
use super::super::CREDENTIAL_PARAMETER;
use super::super::ES256_CRED_PARAM;
use super::*;
use alloc::collections::BTreeMap;

Expand All @@ -349,7 +340,7 @@ mod test {
"displayName" => "bar",
"icon" => "example.com/foo/icon.png",
},
4 => cbor_array![CREDENTIAL_PARAMETER],
4 => cbor_array![ES256_CRED_PARAM],
5 => cbor_array![],
8 => vec![0x12, 0x34],
9 => 1,
Expand Down Expand Up @@ -380,7 +371,7 @@ mod test {
client_data_hash,
rp,
user,
pub_key_cred_params: vec![CREDENTIAL_PARAMETER],
pub_key_cred_params: vec![ES256_CRED_PARAM],
exclude_list: Some(vec![]),
extensions: None,
options,
Expand Down
Loading

0 comments on commit d9c4c72

Please sign in to comment.