Skip to content

Commit 5e446ff

Browse files
authored
fix: Adjust anchor migration to correctly handle key type unknown (#3547)
# Motivation We discovered that passkeys with key type unknown are in fact a legitimate case (not inconsistent data), originating from devices that for some reason do not follow the recommendation to specify either the platform or cross-platform key type. This PR thus adjusts the device migration logic to reflect this. # Changes * Make `KeyType::Unknown` indicate legitimate passkeys. # Tests * Added `should_handle_unknown_key_type_with_credential_id` to cover migrating normal and recovery passkeys with unknown key type.
1 parent edcb183 commit 5e446ff

File tree

2 files changed

+65
-2
lines changed

2 files changed

+65
-2
lines changed

src/internet_identity/src/storage/anchor.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ impl From<Anchor> for (StorableFixedAnchor, StorableAnchor) {
197197
(
198198
Some(credential_id),
199199
Purpose::Authentication,
200-
KeyType::Platform | KeyType::CrossPlatform,
200+
KeyType::Platform | KeyType::CrossPlatform | KeyType::Unknown,
201201
) => {
202202
if protection == &DeviceProtection::Protected {
203203
ic_cdk::println!(
@@ -214,7 +214,7 @@ impl From<Anchor> for (StorableFixedAnchor, StorableAnchor) {
214214
(
215215
Some(credential_id),
216216
Purpose::Recovery,
217-
KeyType::Platform | KeyType::CrossPlatform,
217+
KeyType::Platform | KeyType::CrossPlatform | KeyType::Unknown,
218218
) => {
219219
let credential_id = Some(credential_id);
220220
let special_device_migration = Some(SpecialDeviceMigration::from((

src/internet_identity/src/storage/anchor/tests.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,69 @@ mod from_conversion_tests {
10561056
assert_eq!(passkeys[0].credential_id, vec![0xde, 0xad, 0xbe, 0xef]);
10571057
}
10581058

1059+
/// Tests that passkey devices with unknown key type but having a credential_id are
1060+
/// treated as normal passkeys during migration (regardless of the purpose).
1061+
#[test]
1062+
fn should_handle_unknown_key_type_with_credential_id() {
1063+
let mut anchor = Anchor::new(ANCHOR_NUMBER, 123456789);
1064+
1065+
for purpose in [Purpose::Authentication, Purpose::Recovery] {
1066+
// Device with unknown key type but has credential_id
1067+
let device = Device {
1068+
pubkey: ByteBuf::from(vec![80, 81, 82, purpose.clone() as u8]),
1069+
alias: "Unknown Key Type Device".to_string(),
1070+
credential_id: Some(ByteBuf::from(vec![83, 84, 85])),
1071+
aaguid: None,
1072+
purpose,
1073+
key_type: KeyType::Unknown,
1074+
protection: DeviceProtection::Unprotected,
1075+
origin: Some("https://id.ai".to_string()),
1076+
last_usage_timestamp: Some(123456789),
1077+
metadata: None,
1078+
};
1079+
anchor.add_device(device.clone()).unwrap();
1080+
}
1081+
1082+
let (_fixed, storable): (StorableFixedAnchor, StorableAnchor) = anchor.into();
1083+
1084+
// Device with unknown key type but has credential_id is migrated as passkey.
1085+
let passkeys = storable.passkey_credentials.unwrap();
1086+
assert_eq!(
1087+
passkeys,
1088+
vec![
1089+
StorablePasskeyCredential {
1090+
pubkey: vec![80, 81, 82, Purpose::Authentication as u8],
1091+
credential_id: vec![83, 84, 85],
1092+
origin: "https://id.ai".to_string(),
1093+
created_at_ns: None,
1094+
last_usage_timestamp_ns: Some(123456789),
1095+
alias: Some("Unknown Key Type Device".to_string()),
1096+
aaguid: None,
1097+
// Happy case, therefore no special migration data.
1098+
special_device_migration: None,
1099+
},
1100+
StorablePasskeyCredential {
1101+
pubkey: vec![80, 81, 82, Purpose::Recovery as u8],
1102+
credential_id: vec![83, 84, 85],
1103+
origin: "https://id.ai".to_string(),
1104+
created_at_ns: None,
1105+
last_usage_timestamp_ns: Some(123456789),
1106+
alias: Some("Unknown Key Type Device".to_string()),
1107+
aaguid: None,
1108+
// Special case (recovery passkey); device migration data should be persisted.
1109+
special_device_migration: Some(SpecialDeviceMigration {
1110+
credential_id: Some(vec![83, 84, 85]),
1111+
purpose: Purpose::Recovery.into(),
1112+
key_type: KeyType::Unknown.into(),
1113+
}),
1114+
}
1115+
]
1116+
);
1117+
1118+
let recovery_keys = storable.recovery_keys.unwrap();
1119+
assert_eq!(recovery_keys, vec![]);
1120+
}
1121+
10591122
/// Verifies that devices without a credential_id and not marked as seed phrase are included
10601123
/// in the recovery_keys list.
10611124
#[test]

0 commit comments

Comments
 (0)