diff --git a/README.md b/README.md index cca4eac..68e9f71 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ For a more detailed guide, please refer to our ./deploy.py --board=nrf52840_dongle --opensk ``` -1. Finally you need to inejct the cryptographic material if you enabled +1. Finally you need to inject the cryptographic material if you enabled batch attestation or CTAP1/U2F compatibility (which is the case by default): diff --git a/docs/install.md b/docs/install.md index 61866ff..d00b991 100644 --- a/docs/install.md +++ b/docs/install.md @@ -137,7 +137,7 @@ File | Purpose If you want to use your own attestation certificate and private key, simply replace `opensk_cert.pem` and `opensk.key` files. -Our build script `build.rs` is responsible for converting `aaguid.txt` file +Our build script `build.rs` is responsible for converting the `aaguid.txt` file into raw data that is then used by the Rust file `src/ctap/key_material.rs`. Our configuration script `tools/configure.py` is responsible for configuring diff --git a/src/ctap/command.rs b/src/ctap/command.rs index f2f2537..5dbd930 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -400,10 +400,10 @@ impl TryFrom for AuthenticatorAttestationMaterial { 2 => private_key, } = extract_map(cbor_value)?; } - let certificate = certificate.map(extract_byte_string).transpose()?.unwrap(); - let private_key = private_key.map(extract_byte_string).transpose()?.unwrap(); + let certificate = extract_byte_string(ok_or_missing(certificate)?)?; + let private_key = extract_byte_string(ok_or_missing(private_key)?)?; if private_key.len() != key_material::ATTESTATION_PRIVATE_KEY_LENGTH { - return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR); + return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } let private_key = array_ref!(private_key, 0, key_material::ATTESTATION_PRIVATE_KEY_LENGTH); Ok(AuthenticatorAttestationMaterial { @@ -638,4 +638,83 @@ mod test { let command = Command::deserialize(&cbor_bytes); assert_eq!(command, Ok(Command::AuthenticatorSelection)); } + + #[test] + fn test_vendor_configure() { + // Incomplete command + let mut cbor_bytes = vec![Command::AUTHENTICATOR_VENDOR_CONFIGURE]; + let command = Command::deserialize(&cbor_bytes); + assert_eq!(command, Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)); + + cbor_bytes.extend(&[0xA1, 0x01, 0xF5]); + let command = Command::deserialize(&cbor_bytes); + assert_eq!( + command, + Ok(Command::AuthenticatorVendorConfigure( + AuthenticatorVendorConfigureParameters { + lockdown: true, + attestation_material: None + } + )) + ); + + let dummy_cert = [0xddu8; 20]; + let dummy_pkey = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH]; + + // Attestation key is too short. + let cbor_value = cbor_map! { + 1 => false, + 2 => cbor_map! { + 1 => dummy_cert, + 2 => dummy_pkey[..key_material::ATTESTATION_PRIVATE_KEY_LENGTH - 1] + } + }; + assert_eq!( + AuthenticatorVendorConfigureParameters::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) + ); + + // Missing private key + let cbor_value = cbor_map! { + 1 => false, + 2 => cbor_map! { + 1 => dummy_cert + } + }; + assert_eq!( + AuthenticatorVendorConfigureParameters::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER) + ); + + // Missing certificate + let cbor_value = cbor_map! { + 1 => false, + 2 => cbor_map! { + 2 => dummy_pkey + } + }; + assert_eq!( + AuthenticatorVendorConfigureParameters::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER) + ); + + // Valid + let cbor_value = cbor_map! { + 1 => false, + 2 => cbor_map! { + 1 => dummy_cert, + 2 => dummy_pkey + } + }; + assert_eq!( + AuthenticatorVendorConfigureParameters::try_from(cbor_value), + Ok(AuthenticatorVendorConfigureParameters { + lockdown: false, + attestation_material: Some(AuthenticatorAttestationMaterial { + certificate: dummy_cert.to_vec(), + private_key: dummy_pkey + }) + }) + ); + } } diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 50e0bbf..b6cf5b4 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -931,22 +931,64 @@ where (self.check_user_presence)(cid)?; // Sanity checks - let has_priv_key = self.persistent_store.attestation_private_key()?.is_some(); - let has_cert = self.persistent_store.attestation_certificate()?.is_some(); + let current_priv_key = self.persistent_store.attestation_private_key()?; + let current_cert = self.persistent_store.attestation_certificate()?; - if params.attestation_material.is_some() { + let response = if params.attestation_material.is_some() { let data = params.attestation_material.unwrap(); - if !has_cert { - self.persistent_store - .set_attestation_certificate(&data.certificate)?; + + match (current_cert, current_priv_key) { + (Some(_), Some(_)) => { + // Device is fully programmed. + // We don't compare values to avoid giving an oracle + // about the private key. + AuthenticatorVendorResponse { + cert_programmed: true, + pkey_programmed: true, + } + } + // Device is not programmed. + (None, None) => { + self.persistent_store + .set_attestation_certificate(&data.certificate)?; + self.persistent_store + .set_attestation_private_key(&data.private_key)?; + AuthenticatorVendorResponse { + cert_programmed: true, + pkey_programmed: true, + } + } + // Device is partially programmed. Ensure the programmed value + // matched the given one before programming anything. + (Some(cert), None) => { + if cert != data.certificate { + return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); + } + self.persistent_store + .set_attestation_private_key(&data.private_key)?; + AuthenticatorVendorResponse { + cert_programmed: true, + pkey_programmed: true, + } + } + (None, Some(key)) => { + if key != data.private_key { + return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); + } + self.persistent_store + .set_attestation_certificate(&data.certificate)?; + AuthenticatorVendorResponse { + cert_programmed: true, + pkey_programmed: true, + } + } } - if !has_priv_key { - self.persistent_store - .set_attestation_private_key(&data.private_key)?; + } else { + AuthenticatorVendorResponse { + cert_programmed: current_cert.is_some(), + pkey_programmed: current_priv_key.is_some(), } }; - let has_priv_key = self.persistent_store.attestation_private_key()?.is_some(); - let has_cert = self.persistent_store.attestation_certificate()?.is_some(); if params.lockdown { // To avoid bricking the authenticator, we only allow lockdown // to happen if both values are programmed or if both U2F/CTAP1 and @@ -956,28 +998,13 @@ where #[cfg(not(feature = "with_ctap1"))] let need_certificate = USE_BATCH_ATTESTATION; - if (need_certificate && !(has_priv_key && has_cert)) + if (need_certificate && !(response.pkey_programmed && response.cert_programmed)) || libtock_drivers::crp::protect().is_err() { - Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR) - } else { - Ok(ResponseData::AuthenticatorVendor( - AuthenticatorVendorResponse { - cert_programmed: has_cert, - pkey_programmed: has_priv_key, - lockdown_enabled: true, - }, - )) + return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); } - } else { - Ok(ResponseData::AuthenticatorVendor( - AuthenticatorVendorResponse { - cert_programmed: has_cert, - pkey_programmed: has_priv_key, - lockdown_enabled: false, - }, - )) } + Ok(ResponseData::AuthenticatorVendor(response)) } pub fn generate_auth_data( @@ -2135,7 +2162,6 @@ mod test { AuthenticatorVendorResponse { cert_programmed: false, pkey_programmed: false, - lockdown_enabled: false } )) ); @@ -2159,7 +2185,6 @@ mod test { AuthenticatorVendorResponse { cert_programmed: true, pkey_programmed: true, - lockdown_enabled: false } )) ); @@ -2180,7 +2205,7 @@ mod test { dummy_key ); - // Try to inject other dummy values and check that intial values are retained. + // Try to inject other dummy values and check that initial values are retained. let other_dummy_key = [0x44u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH]; let response = ctap_state.process_vendor_configure( AuthenticatorVendorConfigureParameters { @@ -2198,7 +2223,6 @@ mod test { AuthenticatorVendorResponse { cert_programmed: true, pkey_programmed: true, - lockdown_enabled: false } )) ); @@ -2233,7 +2257,6 @@ mod test { AuthenticatorVendorResponse { cert_programmed: true, pkey_programmed: true, - lockdown_enabled: true } )) ); diff --git a/src/ctap/response.rs b/src/ctap/response.rs index 674a1f5..9b9efc1 100644 --- a/src/ctap/response.rs +++ b/src/ctap/response.rs @@ -238,7 +238,6 @@ impl From for cbor::Value { pub struct AuthenticatorVendorResponse { pub cert_programmed: bool, pub pkey_programmed: bool, - pub lockdown_enabled: bool, } impl From for cbor::Value { @@ -246,13 +245,11 @@ impl From for cbor::Value { let AuthenticatorVendorResponse { cert_programmed, pkey_programmed, - lockdown_enabled, } = vendor_response; cbor_map_options! { 1 => cert_programmed, 2 => pkey_programmed, - 3 => lockdown_enabled, } } } diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 110184f..a701325 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -964,8 +964,7 @@ mod test { .unwrap() .is_none()); - // Make sure the persistent keys are initialized. - // Put dummy values + // Make sure the persistent keys are initialized to dummy values. let dummy_key = [0x41u8; key_material::ATTESTATION_PRIVATE_KEY_LENGTH]; let dummy_cert = [0xddu8; 20]; persistent_store diff --git a/tools/configure.py b/tools/configure.py index eff1166..cc00a1a 100755 --- a/tools/configure.py +++ b/tools/configure.py @@ -75,11 +75,11 @@ def get_opensk_devices(batch_mode): def get_private_key(data, password=None): - # First we try without password + # First we try without password. try: return serialization.load_pem_private_key(data, password=None) except TypeError: - # Maybe we need a password then + # Maybe we need a password then. if sys.stdin.isatty(): password = getpass.getpass(prompt="Private key password: ") else: @@ -134,7 +134,7 @@ def main(args): for authenticator in tqdm(get_opensk_devices(args.batch)): # If the device supports it, wink to show which device - # we're going to program + # we're going to program. if authenticator.device.capabilities & hid.CAPABILITY.WINK: authenticator.device.wink() aaguid = uuid.UUID(bytes=authenticator.get_info().aaguid) @@ -149,11 +149,20 @@ def main(args): ) info("Certificate: {}".format("Present" if result[1] else "Missing")) info("Private Key: {}".format("Present" if result[2] else "Missing")) - if result[3]: - info("Device locked down!") + if args.lock: + info("Device is now locked down!") except ctap.CtapError as ex: if ex.code.value == ctap.CtapError.ERR.INVALID_COMMAND: error("Failed to configure OpenSK (unsupported command).") + elif ex.code.value == 0xF2: # VENDOR_INTERNAL_ERROR + error(("Failed to configure OpenSK (lockdown conditions not met " + "or hardware error).")) + elif ex.code.value == ctap.CtapError.ERR.INVALID_PARAMETER: + error( + ("Failed to configure OpenSK (device is partially programmed but " + "the given cert/key don't match the ones currently programmed).")) + else: + error("Failed to configure OpenSK (unknown error: {}".format(ex)) if __name__ == "__main__": @@ -174,7 +183,7 @@ if __name__ == "__main__": metavar="PEM_FILE", dest="certificate", help=("PEM file containing the certificate to inject into " - "OpenSK authenticator."), + "the OpenSK authenticator."), ) parser.add_argument( "--private-key",