Skip to content

Commit

Permalink
Cleans up legacy credential parsing
Browse files Browse the repository at this point in the history 
This is a backwards incompatible change. This PR already introduces
backwards incompatible new credential parsing, and therefore we can also
remove all other legacy parsing.
  • Loading branch information
@kaczmarczyck
kaczmarczyck committed Aug 9, 2023
1 parent 468202b commit c4f62fa
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 187 deletions.
94 changes: 10 additions & 84 deletions libraries/opensk/src/api/key_store.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,22 +212,18 @@ impl<T: Helper> KeyStore for T {
return Ok(None);
}

let credential_source = if bytes.len() == LEGACY_CREDENTIAL_ID_SIZE {
decrypt_legacy_credential_id::<T>(&master_keys.encryption, &bytes[..hmac_message_size])?
} else {
match bytes[0] {
CBOR_CREDENTIAL_ID_VERSION => {
if bytes.len() != CBOR_CREDENTIAL_ID_SIZE {
return Ok(None);
}
decrypt_cbor_credential_id::<T>(
self,
&master_keys.encryption,
&bytes[1..hmac_message_size],
)?
let credential_source = match bytes[0] {
CBOR_CREDENTIAL_ID_VERSION => {
if bytes.len() != CBOR_CREDENTIAL_ID_SIZE {
return Ok(None);
}
_ => return Ok(None),
decrypt_cbor_credential_id::<T>(
self,
&master_keys.encryption,
&bytes[1..hmac_message_size],
)?
}
_ => return Ok(None),
};

if let Some(credential_source) = &credential_source {
Expand Down Expand Up @@ -326,30 +322,6 @@ fn remove_padding(data: &[u8]) -> Result<&[u8], Error> {
Ok(&data[..data.len() - pad_length as usize])
}

fn decrypt_legacy_credential_id<E: Env>(
encryption_key_bytes: &[u8; 32],
bytes: &[u8],
) -> Result<Option<CredentialSource>, Error> {
let aes_key = AesKey::<E>::new(encryption_key_bytes);
let plaintext = aes256_cbc_decrypt::<E>(&aes_key, bytes, true)
.map_err(|_| Error)?
.expose_secret_to_vec();
if plaintext.len() != 64 {
return Ok(None);
}
let private_key = if let Some(key) = PrivateKey::new_ecdsa_from_bytes(&plaintext[..32]) {
key
} else {
return Ok(None);
};
Ok(Some(CredentialSource {
private_key,
rp_id_hash: plaintext[32..64].try_into().unwrap(),
cred_protect_policy: None,
cred_blob: None,
}))
}

fn decrypt_cbor_credential_id<E: Env>(
env: &mut E,
encryption_key_bytes: &[u8; 32],
Expand Down Expand Up @@ -591,52 +563,6 @@ mod test {
test_wrap_unwrap_credential_missing_blocks(SignatureAlgorithm::Eddsa);
}

/// This is a copy of the function that genereated deprecated key handles.
fn legacy_encrypt_to_credential_id(
env: &mut TestEnv,
private_key: EcdsaSk<TestEnv>,
application: &[u8; 32],
) -> Result<Vec<u8>, Error> {
let master_keys = get_master_keys(env).unwrap();
let aes_key = AesKey::<TestEnv>::new(&master_keys.encryption);
let hmac_key = master_keys.authentication;
let mut plaintext = [0; 64];
private_key.to_slice(array_mut_ref!(plaintext, 0, 32));
plaintext[32..64].copy_from_slice(application);

let mut encrypted_id = aes256_cbc_encrypt::<TestEnv>(env.rng(), &aes_key, &plaintext, true)
.map_err(|_| Error)?;
let mut id_hmac = [0; HASH_SIZE];
Hmac::<TestEnv>::mac(&hmac_key, &encrypted_id[..], &mut id_hmac);
encrypted_id.extend(&id_hmac);
Ok(encrypted_id)
}

#[test]
fn test_encrypt_decrypt_credential_legacy() {
let mut env = TestEnv::default();
let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
let rp_id_hash = [0x55; 32];
let credential_source = CredentialSource {
private_key,
rp_id_hash,
cred_protect_policy: None,
cred_blob: None,
};
let ecdsa_key = credential_source
.private_key
.ecdsa_key::<TestEnv>()
.unwrap();
let credential_id =
legacy_encrypt_to_credential_id(&mut env, ecdsa_key, &rp_id_hash).unwrap();
let unwrapped = env
.key_store()
.unwrap_credential(&credential_id, &rp_id_hash)
.unwrap()
.unwrap();
assert_eq!(credential_source, unwrapped);
}

#[test]
fn test_wrap_credential_size() {
let mut env = TestEnv::default();
Expand Down
7 changes: 3 additions & 4 deletions libraries/opensk/src/api/private_key.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ impl PrivateKey {
}

/// Helper function that creates a private key of type ECDSA.
///
/// This function is public for legacy credential source parsing only.
pub fn new_ecdsa_from_bytes(bytes: &[u8]) -> Option<Self> {
fn new_ecdsa_from_bytes(bytes: &[u8]) -> Option<Self> {
if bytes.len() != 32 {
return None;
}
Expand All @@ -80,8 +78,9 @@ impl PrivateKey {
Some(PrivateKey::Ecdsa(seed))
}

/// Helper function that creates a private key of type Ed25519.
#[cfg(feature = "ed25519")]
pub fn new_ed25519_from_bytes(bytes: &[u8]) -> Option<Self> {
fn new_ed25519_from_bytes(bytes: &[u8]) -> Option<Self> {
if bytes.len() != 32 {
return None;
}
Expand Down
11 changes: 5 additions & 6 deletions libraries/opensk/src/ctap/ctap1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,6 @@ impl Ctap1Command {
.map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?;
let credential_source = filter_listed_credential(credential_source, false)
.ok_or(Ctap1StatusCode::SW_WRONG_DATA)?;
let ecdsa_key = credential_source
.private_key
.ecdsa_key::<E>()
.map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?;
if flags == Ctap1Flags::CheckOnly {
return Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED);
}
Expand All @@ -351,10 +347,13 @@ impl Ctap1Command {
)
.map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?;
signature_data.extend(&challenge);
let signature = ecdsa_key.sign(&signature_data);
let signature = credential_source
.private_key
.sign_and_encode::<E>(&signature_data)
.map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?;

let mut response = signature_data[application.len()..application.len() + 5].to_vec();
response.extend(signature.to_der());
response.extend(&signature);
Ok(response)
}
}
Expand Down
95 changes: 2 additions & 93 deletions libraries/opensk/src/ctap/data_formats.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,6 @@ pub struct PublicKeyCredentialSource {
// is associated with a unique tag, implemented with a CBOR unsigned key.
enum PublicKeyCredentialSourceField {
CredentialId = 0,
// Deprecated, we still read this field for backwards compatibility.
EcdsaPrivateKey = 1,
RpId = 2,
UserHandle = 3,
UserDisplayName = 4,
Expand All @@ -617,6 +615,7 @@ enum PublicKeyCredentialSourceField {
// When a field is removed, its tag should be reserved and not used for new fields. We document
// those reserved tags below.
// Reserved tags:
// - EcdsaPrivateKey = 1,
// - CredRandom = 5,
}

Expand Down Expand Up @@ -661,7 +660,6 @@ impl PublicKeyCredentialSource {
destructure_cbor_map! {
let {
PublicKeyCredentialSourceField::CredentialId => credential_id,
PublicKeyCredentialSourceField::EcdsaPrivateKey => ecdsa_private_key,
PublicKeyCredentialSourceField::RpId => rp_id,
PublicKeyCredentialSourceField::UserHandle => user_handle,
PublicKeyCredentialSourceField::UserDisplayName => user_display_name,
Expand All @@ -687,19 +685,7 @@ impl PublicKeyCredentialSource {
let user_icon = user_icon.map(extract_text_string).transpose()?;
let cred_blob = cred_blob.map(extract_byte_string).transpose()?;
let large_blob_key = large_blob_key.map(extract_byte_string).transpose()?;

// Parse the private key from the deprecated field if necessary.
let ecdsa_private_key = ecdsa_private_key.map(extract_byte_string).transpose()?;
let private_key = private_key
.map(|v| PrivateKey::from_cbor::<E>(wrap_key, v))
.transpose()?;
let private_key = match (ecdsa_private_key, private_key) {
(None, None) => return Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER),
(Some(_), Some(_)) => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR),
(Some(k), None) => PrivateKey::new_ecdsa_from_bytes(&k)
.ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?,
(None, Some(k)) => k,
};
let private_key = PrivateKey::from_cbor::<E>(wrap_key, ok_or_missing(private_key)?)?;

// We don't return whether there were unknown fields in the CBOR value. This means that
// deserialization is not injective. In particular deserialization is only an inverse of
Expand Down Expand Up @@ -2221,83 +2207,6 @@ mod test {
);
}

#[test]
fn test_credential_source_cbor_read_legacy() {
let mut env = TestEnv::default();
let wrap_key = env.key_store().wrap_key::<TestEnv>().unwrap();
let private_key = PrivateKey::new_ecdsa(&mut env);
let key_bytes = private_key.to_bytes();
let credential = PublicKeyCredentialSource {
key_type: PublicKeyCredentialType::PublicKey,
credential_id: env.rng().gen_uniform_u8x32().to_vec(),
private_key,
rp_id: "example.com".to_string(),
user_handle: b"foo".to_vec(),
user_display_name: None,
cred_protect_policy: None,
creation_order: 0,
user_name: None,
user_icon: None,
cred_blob: None,
large_blob_key: None,
};

let source_cbor = cbor_map! {
PublicKeyCredentialSourceField::CredentialId => credential.credential_id.clone(),
PublicKeyCredentialSourceField::EcdsaPrivateKey => key_bytes,
PublicKeyCredentialSourceField::RpId => credential.rp_id.clone(),
PublicKeyCredentialSourceField::UserHandle => credential.user_handle.clone(),
};
assert_eq!(
PublicKeyCredentialSource::from_cbor::<TestEnv>(&wrap_key, source_cbor),
Ok(credential)
);
}

#[test]
fn test_credential_source_cbor_legacy_error() {
let mut env = TestEnv::default();
let wrap_key = env.key_store().wrap_key::<TestEnv>().unwrap();
let private_key = PrivateKey::new_ecdsa(&mut env);
let key_bytes = private_key.to_bytes();
let credential = PublicKeyCredentialSource {
key_type: PublicKeyCredentialType::PublicKey,
credential_id: env.rng().gen_uniform_u8x32().to_vec(),
private_key: private_key.clone(),
rp_id: "example.com".to_string(),
user_handle: b"foo".to_vec(),
user_display_name: None,
cred_protect_policy: None,
creation_order: 0,
user_name: None,
user_icon: None,
cred_blob: None,
large_blob_key: None,
};

let source_cbor = cbor_map! {
PublicKeyCredentialSourceField::CredentialId => credential.credential_id.clone(),
PublicKeyCredentialSourceField::RpId => credential.rp_id.clone(),
PublicKeyCredentialSourceField::UserHandle => credential.user_handle.clone(),
};
assert_eq!(
PublicKeyCredentialSource::from_cbor::<TestEnv>(&wrap_key, source_cbor),
Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)
);

let source_cbor = cbor_map! {
PublicKeyCredentialSourceField::CredentialId => credential.credential_id,
PublicKeyCredentialSourceField::EcdsaPrivateKey => key_bytes,
PublicKeyCredentialSourceField::RpId => credential.rp_id,
PublicKeyCredentialSourceField::UserHandle => credential.user_handle,
PublicKeyCredentialSourceField::PrivateKey => private_key.to_cbor::<TestEnv>(env.rng(), &wrap_key).unwrap(),
};
assert_eq!(
PublicKeyCredentialSource::from_cbor::<TestEnv>(&wrap_key, source_cbor),
Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)
);
}

#[test]
fn test_credential_source_invalid_cbor() {
let mut env = TestEnv::default();
Expand Down

0 comments on commit c4f62fa

Please sign in to comment.