Skip to content

Commit 5fbb972

Browse files
atergaCopilot
andauthored
feat: Add better observability features for the anchor migration (dfinity#3567)
# Motivation Observing exotic cases from the anchor migration. # Changes * Updates warning log messages to be more concise by removing "protected" qualifier * Adds filtering logic to only store the most exotic migration cases in memory * Adds pagination support to the special cases keys query function # Tests Not required. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent e1e81c2 commit 5fbb972

3 files changed

Lines changed: 74 additions & 12 deletions

File tree

src/internet_identity/src/main.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,18 @@ fn get_anchor_migration_special_cases(anchor_number: AnchorNumber) -> Vec<Specia
9090
}
9191

9292
#[query(hidden = true)]
93-
fn get_anchor_migration_special_cases_keys() -> Vec<AnchorNumber> {
94-
ANCHOR_MIGRATION_SPECIAL_CASES.with_borrow(|cases| cases.keys().cloned().collect())
93+
fn get_anchor_migration_special_cases_keys(lo: usize, hi: usize) -> Vec<AnchorNumber> {
94+
use std::cmp::{max, min};
95+
96+
let anchor_numbers: Vec<_> =
97+
ANCHOR_MIGRATION_SPECIAL_CASES.with_borrow(|cases| cases.keys().cloned().collect());
98+
99+
let lo = max(0, lo);
100+
let hi = min(anchor_numbers.len(), hi);
101+
102+
let (lo, hi) = (min(lo, hi), max(lo, hi));
103+
104+
anchor_numbers[lo..hi].to_vec()
95105
}
96106

97107
/// Temporary function to list migration errors.

src/internet_identity/src/storage.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,15 @@ impl<M: Memory + Clone> Storage<M> {
779779
// Read unbounded stable structures anchor
780780
let storable_fixed_anchor = StorableFixedAnchor::from_bytes(Cow::Owned(buf));
781781
let storable_anchor = self.stable_anchor_memory.get(&anchor_number);
782+
783+
if storable_anchor.is_none() {
784+
ic_cdk::println!(
785+
"Warning: Anchor number {} has no corresponding StorableAnchor in stable memory. \
786+
Falling back to reading from legacy anchor memory.",
787+
anchor_number,
788+
);
789+
}
790+
782791
Ok(Anchor::from((
783792
anchor_number,
784793
storable_fixed_anchor,

src/internet_identity/src/storage/anchor.rs

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ impl From<Anchor> for (StorableFixedAnchor, StorableAnchor) {
206206
) => {
207207
if protection == &DeviceProtection::Protected {
208208
ic_cdk::println!(
209-
"Warning: Passkey with protected device protection found. \
210-
PubKey: {:?}, Anchor: {}",
209+
"Warning: Passkey with device protection found. PubKey: {:?}, \
210+
Anchor: {}",
211211
hex::encode(&pubkey),
212212
anchor_number,
213213
);
@@ -230,8 +230,8 @@ impl From<Anchor> for (StorableFixedAnchor, StorableAnchor) {
230230
)));
231231
if protection == &DeviceProtection::Protected {
232232
ic_cdk::println!(
233-
"Warning: Passkey without origin with protected device protection \
234-
found. PubKey: {:?}, Anchor: {}",
233+
"Warning: Passkey without origin with device protection found. \
234+
PubKey: {:?}, Anchor: {}",
235235
hex::encode(&pubkey),
236236
anchor_number,
237237
);
@@ -254,6 +254,15 @@ impl From<Anchor> for (StorableFixedAnchor, StorableAnchor) {
254254
origin,
255255
)));
256256

257+
if protection == &DeviceProtection::Protected {
258+
ic_cdk::println!(
259+
"Warning: Recovery passkey with device protection found. PubKey: {:?}, \
260+
Anchor: {}",
261+
hex::encode(&pubkey),
262+
anchor_number,
263+
);
264+
}
265+
257266
// No logging for a legitimate legacy recovery device.
258267

259268
(credential_id, special_device_migration)
@@ -303,12 +312,46 @@ impl From<Anchor> for (StorableFixedAnchor, StorableAnchor) {
303312

304313
// Store special cases to aid observability (they will also be persisted in stable memory).
305314
if let Some(special_device_migration) = &special_device_migration {
306-
ANCHOR_MIGRATION_SPECIAL_CASES.with_borrow_mut(|cases| {
307-
cases
308-
.entry(anchor_number)
309-
.or_default()
310-
.push(special_device_migration.clone())
311-
})
315+
// To avoid storing too much data on the heap, we filter this down to only contain
316+
// the most exotic cases.
317+
318+
let is_valid_passkey_but_without_an_origin = matches!(
319+
(&credential_id, purpose, key_type, origin, protection),
320+
(
321+
Some(_),
322+
Purpose::Authentication,
323+
KeyType::Platform | KeyType::CrossPlatform | KeyType::Unknown,
324+
None,
325+
DeviceProtection::Unprotected,
326+
)
327+
);
328+
329+
let is_recovery_passkey = matches!(
330+
(&credential_id, purpose, key_type, protection),
331+
(
332+
Some(_),
333+
Purpose::Recovery,
334+
KeyType::Platform | KeyType::CrossPlatform | KeyType::Unknown,
335+
DeviceProtection::Unprotected,
336+
)
337+
);
338+
339+
let is_legacy_pin_flow = matches!(
340+
(&credential_id, purpose, key_type),
341+
(None, Purpose::Authentication, KeyType::BrowserStorageKey)
342+
);
343+
344+
if !is_valid_passkey_but_without_an_origin
345+
&& !is_recovery_passkey
346+
&& !is_legacy_pin_flow
347+
{
348+
ANCHOR_MIGRATION_SPECIAL_CASES.with_borrow_mut(|cases| {
349+
cases
350+
.entry(anchor_number)
351+
.or_default()
352+
.push(special_device_migration.clone())
353+
})
354+
}
312355
}
313356

314357
if let Some(credential_id) = credential_id {

0 commit comments

Comments
 (0)