628338 - Ignore records that do not decrypt correctly

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Firefox Home, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

20:41 < mrz> my iphone always fails during history and tells me my key is wrong
20:41 <@philikon> mrz: interesting
20:41 <@philikon> st3fan: ^^^
20:41 <@philikon> st3fan: fx home should handle broken records gracefully
20:41 < st3fan> how? ignore them?
20:41 < atoll> mrz: that's a firefox home bug
20:42 < atoll> st3: better than logging the user out, yea
20:42 < st3fan> euh home expects proper data :)
20:42 <@mconnor> weak
20:42 <@philikon> st3fan: yes, ignore
20:42 -!- tchung [[e-mail]] has quit [Quit: tchung]
20:42 < st3fan> ok
20:42 <@philikon> st3fan: want me to file?
20:43 < st3fan> only if you have more info than 'ignore records for which decryption fails'
20:43 < atoll> can we get the bad record from mrz
20:44 < atoll> but.. that wouldn't help, n/m
20:44 <@philikon> st3fan: i don't
20:44 < st3fan> i'll file then
20:44 <@philikon>  ok :)
20:44 < st3fan> but do we know the actual cause of these invalid records?
20:45 < atoll> multiple client bugs
20:45 <@philikon> st3fan: yes
20:45 <@philikon> pretty much one client bug
20:45 < atoll> oh, better
20:45 < st3fan> so if you guys fix that, shoudl home still get a workaround?
20:45 <@philikon> gc bug in weavecrypto
20:45 <@philikon> st3fan: yes
20:45 <@philikon> st3fan: there are broken records on the server
20:46 < atoll> this will happen from time to time, no matter how awesome we are
20:46 <@mconnor> and no matter how awesome random-other-impls are
20:46 <@philikon> exactly
20:46 < atoll> yup

Comment 1

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Fixed in https://hg.mozilla.org/services/fx-home/rev/20e7e93fd80c

We now ignore bad records during sync.

To detect a real sync key change we check crypto/keys every time during sync. If we cannot decrypt crypto/keys with the secret that we have stored then we assume that the sync key has changed and we show the user the standard 'Cannot Sync, Incorrect Sync Key' dialog.

Comment 2

[Tony Chung [:tchung]]

how can i create a account with broken records to test this?  or better yet, does someone have one handy i can play with?

Comment 3

[Tony Chung [:tchung]]

Attachment: bad record screenshot

okay, i've retrieved a test account with bad records from rnewman, and was able to successfully connect to FF Home (despite no records shown, see screenshot)   Should the fix be that i cannot connect at all?   or is what we see here the right behavior?   

Fri Jan 28 10:21:24 unknown FirefoxHome[851] <Warning>: JPAKEClient#abort
Fri Jan 28 10:22:33 unknown FirefoxHome[851] <Warning>: unable to delete private key, already deleted
Fri Jan 28 10:22:37 unknown FirefoxHome[851] <Warning>: ### unexpected http error: 404
Fri Jan 28 10:22:37 unknown FirefoxHome[851] <Warning>: -JSONValue failed. Error trace is: (
    Error Domain=org.brautaset.JSON.ErrorDomain Code=11 UserInfo=0x61b8a70 "Unexpected end of string"
)
Fri Jan 28 10:22:37 unknown FirefoxHome[851] <Warning>: did not find matching mobile client record, adding.
Fri Jan 28 10:22:42 unknown FirefoxHome[851] <Warning>: getTimestamp (bookmarks) step: not an error
Fri Jan 28 10:22:44 unknown FirefoxHome[851] <Warning>: getTimestamp (history) step: not an error
Fri Jan 28 10:22:44 unknown FirefoxHome[851] <Warning>: getTimestamp (fullhistory) step: not an error

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Currently what happens in case of a bad record is that you see the 'Incorrect secret phrase' error.

The fix is that bad records are completely ignored. This means that no error is shown but also that some of your sync data will be skipped/ignored.

Sorry this is a difficult one to test. If you are syncing against an account with bad records then you should see one of these messages in the log:

Ignoring record: unable to decrypt
Ignoring record: unable to decrypt WBO JSON
Ignoring record: hmac does not verify encrypted object

Comment 5

[Tony Chung [:tchung]]

my test record from comment 3 isnt broken anymore.  need another broken record to resume the test pass