Skip to content

Skip a keychain item that cannot be read, instead of ending the export - #103

Merged
parawanderer merged 2 commits into
mainfrom
fix/servicekey-der-escapes
Aug 16, 2026
Merged

parawanderer merged 2 commits into
mainfrom
fix/servicekey-der-escapes

Conversation

@parawanderer

Copy link
Copy Markdown
Owner

Takes the FindMy.py fix for the crash reported in #89, and keeps the diagnosis that found it.

What was wrong

service_keys_from_der converted a DerError into ServiceKeyError for the outer parse_one
and not for the descent that follows. _from_v1 opens with element.children(), so a payload
whose first element parses but whose contents are not DER raised a raw DerError — and the caller
in session.py only caught (ItemError, ServiceKeyError).

So the skip-and-continue written for exactly this case was bypassed on which exception type came
out. The proof is three consecutive lines of the reporter's log:

DEBUG  Item for acct 504353426f756e64 holds no key: … Truncated DER: claims 42, 30 remain
DEBUG  Item for acct 64656661756c74   holds no key: … Truncated DER: claims 67, 19 remain

DerError: Truncated DER: element claims 109 bytes, 61 remain      <- fatal

Those hex accounts are PCSBound and default. Two items failed the same way and were skipped
correctly; the third ended the export. One item in sixty-one.

What it actually cost

More than one item. unlock reads Manatee and then ProtectedCloudStorage, accumulating keys
across both, and the reporter's log shows Manatee completing with 141 elliptic-curve keys
before the crash. The exception unwound the whole call, so those were found and then discarded.

The fix

Upstream, acd17c5 → 47c4f89: the whole body of service_keys_from_der inside the guard rather
than only parse_one — both arms descend, so both could raise it — plus der.DerError alongside
ServiceKeyError in the caller that skips unreadable items. That second half is a consistency fix
rather than defensiveness: pcs.py's key walk already caught both, and _pcs_keys_in had not
carried it across.

Only python/uv.lock changes here.

Verified against the new pin

The #89 payload shape DerError → ServiceKeyError, item skipped, run survives
A payload that already worked still ServiceKeyError, no regression
Caller catches DerError confirmed present in _pcs_keys_in
Exporter suite 441 pass, 2 skipped
flake8 clean

Reproduction, no account needed:

service_keys_from_der(bytes([0x30, 0x03, 0x02, 0x7F, 0x41]))
# before: DerError   after: ServiceKeyError

What is not claimed

This is not confirmed to fix the reported export, which is why it links #89 rather than closing
it. Whether the one fatal item was itself a key is not visible from the log — only the reporter's
next run answers that. What is certain is that the 61-item view now survives it, and the 141 keys
already recovered from Manatee are no longer thrown away.

If it still comes up short, the line to read is the holds no key DEBUG for the item that used to
be fatal.

Reported by @paula-coder-646 in #89, with a traceback that pinned the failure to the line.


PR description summarised by Claude Code.

Paula's traceback pinned it, which is what the stack trace was added for. The fault is one
inconsistent guard rather than anything about her account: she has five owned AirTags, all
visible under Items, which rules out both of the alternatives I had asked her to check.

`service_keys_from_der` converts a DerError to ServiceKeyError for the outer `parse_one` and
not for the descent that follows. `_from_v1` opens with `element.children()`, so a payload
whose first element parses but whose contents are not DER raises a raw DerError - and
`session.py` only catches `(ItemError, ServiceKeyError)`, so the skip-and-continue written for
exactly this case is bypassed on which exception type came out.

The proof is three consecutive lines of her log: two items failing this way were caught,
logged and skipped, and the third took the whole export down. One item in sixty-one.

Reproduced without an account in three lines, and the report carries it: an outer TLV with an
honest length and content that is not DER gives DerError where the docstring promises
ServiceKeyError.
acd17c5 -> 47c4f89, which makes service_keys_from_der answer in its own vocabulary for the
whole body rather than only the outer parse_one, and catches der.DerError alongside
ServiceKeyError in the caller that skips unreadable items.

Verified against the new pin rather than assumed. The payload from the report - an outer TLV
with an honest length wrapping content that is not DER - now raises ServiceKeyError where it
raised DerError before, so the item is skipped like the two beside it instead of ending the
run. 441 tests pass, flake8 clean.

Worth recording what the fix is expected to recover, because it is more than one item. `unlock`
reads Manatee and then ProtectedCloudStorage, accumulating across both, and the reporter's log
shows Manatee completing with 141 elliptic-curve keys before the crash. The exception unwound
the whole call, so those 141 were found and then thrown away. Surviving the item returns them.

Not yet confirmed to fix the reported export: whether the one fatal item was itself a key is
not visible from the log, and only their next run answers it.
@parawanderer
parawanderer merged commit bed7470 into main Aug 16, 2026
4 checks passed
@parawanderer
parawanderer deleted the fix/servicekey-der-escapes branch August 16, 2026 14:08
saamdotexe pushed a commit to saamdotexe/OpenTagViewer that referenced this pull request Oct 22, 2026
…der-escapes

Skip a keychain item that cannot be read, instead of ending the export
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant