Sign a TLK share's extra record fields when verifying it - #4
Open
gregakespret wants to merge 1 commit into
Open
Conversation
-[CKKSTLKShare dataForSigning:] signs the seven positional fields and then every record field it doesn't know (bar server_*), sorted by key. We signed only the positional part, so a share carrying any extra field failed its check, and the ? in fetch_shares_for aborted the keychain join on the first one. A beta account fails this way on every attempt, in a different zone each time. Extras are serialized as Apple does: strings as UTF-8, bytes raw, dates as ISO 8601 whole seconds UTC counted from CloudKit's 2001 epoch, numbers as eight little-endian bytes of unsignedLongLongValue. References, lists, assets and locations are skipped. A failed check still aborts the join; it now first logs the record's field names (never values) so the next mismatch can be pinned from the logs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S8kaPMUXu4LSU1K91q6FDN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fetch_shares_fornow verifies a TLK share over the same bytes Apple signs.-[CKKSTLKShare dataForSigning:](apple-oss-distributions/Security,keychain/ckks/CKKSTLKShare.m) signs the seven positional fields, then every record field it doesn't know, exceptserver_*, sorted by key.data_for_signingtakes the share's record fields and appends those extras the way Apple serializes them:unsignedLongLongValueWithout the extras, a share carrying any extra field fails its check, and the
?aborts the keychain join on the first one. A beta account hits this on every attempt, in a different zone each time (Entering on key Health, thenBackstop), after its bottle has already decrypted.A failed check still aborts the join, but it first logs a
warn!listing the record's field names (never values), so the next mismatch shows which field caused it. That also confirms the diagnosis for this account: its field names haven't been seen yet.Tests: six unit tests in
keychain.rs, with expected payloads built by hand from Apple's source rather than from this code:server_*is left outEach test was checked against a targeted mutation of the code. Run them with
cargo test --release --features 'macos-validation-data,remote-anisette-v3' --package rustpush --lib(7 pass). CI runs tests only once #3 lands.Related: OpenBubbles#34 fixes the same payload but also skips shares that fail; this PR keeps the abort. Out of scope: the CKKSItem AAD code (~line 147) also reads a CloudKit date as Unix time.
🤖 Generated with Claude Code
https://claude.ai/code/session_01S8kaPMUXu4LSU1K91q6FDN