Skip to content

(ios) Sync encrypted contacts to iCloud as part of backup system#617

Merged
robbiehanson merged 5 commits intomasterfrom
ios-sync-contacts
Sep 9, 2024
Merged

(ios) Sync encrypted contacts to iCloud as part of backup system#617
robbiehanson merged 5 commits intomasterfrom
ios-sync-contacts

Conversation

@robbiehanson
Copy link
Copy Markdown
Contributor

@robbiehanson robbiehanson commented Aug 30, 2024

This PR updates the existing backup system (which was previously only used for payments) to also include contacts.

Very little is changed in terms of the UI. The section that was previously titled "Payments backup" is now called "Cloud backup". And the descriptions are changed to mention that both payments & contacts are backed up.

Just as we did when backing up payments, the contacts are also encrypted. That is, a "cloud key" is derived from the wallet's master key, and then the cloudKey is used with ChaCha20-Poly1305 to encrypt the serialized blob.

The contact photo is also backed up to the cloud. As mentioned in #600:

we resize [contact photos] down to 1 megapixel, which results in a file size of around 100K.

This is nice as we will probably do more with the contact photo in the future.

@robbiehanson robbiehanson requested a review from dpad85 August 30, 2024 19:48
Copy link
Copy Markdown
Member

@dpad85 dpad85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I was able to test a reset successfully. By the way the checkbox in the Reset wallet screen mentions that only the payments history will be removed, I think it's a mistake.

// - deterministic => by hashing the contactId
// - secure => by mixing in the secret cloudKey (derived from seed)

let prefix = SHA256.hash(data: cloudKey.rawRepresentation)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if it's hashed afterwards, it feels dangerous to use the cloud key data this way, especially as it's only to generate a record ID. Could we instead use a hash160 of the cloud key's public key for this prefix, like we do for the database name?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like for the database name we're doing this (in DatabaseManager.kt):

val nodeIdHash = nodeParams.nodeId.hash160().byteVector().toHex()
val channelsDb = SqliteChannelsDb(
  driver = createChannelsDbDriver(ctx, chain, nodeIdHash)
)

So I updated it to also use the nodeIdHash ba71501

@robbiehanson
Copy link
Copy Markdown
Contributor Author

By the way the checkbox in the Reset wallet screen mentions that only the payments history will be removed, I think it's a mistake.

Good catch. Fixed in 9411921

@robbiehanson
Copy link
Copy Markdown
Contributor Author

Note: I made some breaking changes in e6091d4. So if you have contacts currently stored in the cloud, then after pulling this change, you won't be able to pull them down on a restore.

What I did: I meant to store both payments & contacts in the same record zone (but different databases). But I was actually putting them in different record zones.

@robbiehanson robbiehanson requested a review from dpad85 September 2, 2024 20:15
@robbiehanson robbiehanson merged commit dc49aee into master Sep 9, 2024
@robbiehanson robbiehanson deleted the ios-sync-contacts branch September 9, 2024 14:16
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.

2 participants