search

brave / sync

deprecated Brave sync server. (sync now uses a fork of the Chromium sync protocol.)
Mozilla Public License 2.0
203 stars 54 forks source link

Two different devices in same chain with same id #333

Closed AlexeyBarabash closed 4 years ago

AlexeyBarabash commented 5 years ago

It is possible situation when two different devices in the chain have the same id. Which is wrong.

Steps to reproduce with brave-core:

  1. Prepare 3 clear profiles: deviceA, deviceB, deviceC

  2. on deviceB and deviceC open brave://sync and click I have sync code , but don't enter any code for now

  3. on deviceA enable sync chain and copy sync code

  4. paste sync code on both deviceB and deviceC, but don't press Confirm sync code button

  5. arrange deviceB and deviceC windows to see both Confirm sync code buttons

  6. press Confirm sync code buttons as fast as possible on both deviceB and deviceC

  7. Actual result: deviceB and deviceC have two entries marked as This device , this means devices have the same deviceId equal to current device id. Screenshot from 2019-09-03 12-56-19

  8. Expected result: deviceB and deviceC are properly marked as This device just one in the list.

This is an issue for sync library, if two devices almost in the same time are doing connect to the sync chain, they can get response with the same id: https://github.com/brave/sync/blob/staging/client/sync.js#L71

Log from deviceC:

10:55:53.940 background.js:173 "sync-debug" message="set device ID: 1"

10:55:55.354 background.js:53 "send-sync-records" category_name="PREFERENCES" records=[{
"action":0,"device":{"name":"HAPPYUBU7"},"deviceId":{"0":1},"objectData":"device","objectId":{"0":9,"1":168,"2":24,"3":131,"4":6,"5":138,"6":248,"7":102,"8":3,"9":9,"10":66,"11":149,"12":58,"13":109,"14":96,"15":245},"syncTimestamp":0}]

10:55:55.603 background.js:193 "get-existing-objects" category_name="PREFERENCES" records=[{
"action":0,
"deviceId":{"0":0},"objectId":{"0":62,"1":163,"2":126,"3":198,"4":163,"5":95,"6":135,"7":192,"8":8,"9":146,"10":127,"11":154,"12":124,"13":208,"14":254,"15":240},
"device":{"name":"HAPPYUBU7"},"objectData":"device","syncTimestamp":1567497331561}] lastRecordTimeStamp=1567497331561 isTruncated=false

10:55:56.487 background.js:173 "sync-debug" message="got 3 decrypted records in PREFERENCES after 0"
10:55:56.487 background.js:193 "get-existing-objects" category_name="PREFERENCES" records=
[{
"action":0,
"deviceId":{"0":0},
"objectId":{"0":62,"1":163,"2":126,"3":198,"4":163,"5":95,"6":135,"7":192,"8":8,"9":146,"10":127,"11":154,"12":124,"13":208,"14":254,"15":240},"device":{"name":"HAPPYUBU7"},"objectData":"device","syncTimestamp":1567497331561},{
"action":0,
"deviceId":{"0":1},
"objectId":{"0":9,"1":168,"2":24,"3":131,"4":6,"5":138,"6":248,"7":102,"8":3,"9":9,"10":66,"11":149,"12":58,"13":109,"14":96,"15":245},"device":{"name":"HAPPYUBU7"},"objectData":"device","syncTimestamp":1567497355357},{
"action":0,
"deviceId":{"0":1},
"objectId":{"0":113,"1":69,"2":195,"3":57,"4":131,"5":245,"6":21,"7":70,"8":77,"9":54,"10":221,"11":227,"12":88,"13":227,"14":68,"15":255},"device":{"name":"HAPPYUBU7"},"objectData":"device","syncTimestamp":1567497355748
}] lastRecordTimeStamp=1567497355748 isTruncated=false

Log from deviceB:

10:55:54.568 background.js:178 "sync-debug" message="set device ID: 1"

10:55:55.746 background.js:58 "send-sync-records" category_name="PREFERENCES" records=[
{"action":0,
"device":{"name":"HAPPYUBU7"},
"deviceId":{"0":1},"objectData":"device",
"objectId":{"0":113,"1":69,"2":195,"3":57,"4":131,"5":245,"6":21,"7":70,"8":77,"9":54,"10":221,"11":227,"12":88,"13":227,"14":68,"15":255},"syncTimestamp":0}]

10:55:56.254 background.js:204 "resolved-sync-records" categoryName="PREFERENCES" records=[{
"action":0,
"deviceId":{"0":0},"objectId":{"0":62,"1":163,"2":126,"3":198,"4":163,"5":95,"6":135,"7":192,"8":8,"9":146,"10":127,"11":154,"12":124,"13":208,"14":254,"15":240},"device":{"name":"HAPPYUBU7"},"objectData":"device","syncTimestamp":1567497331561},{
"action":0,
"deviceId":{"0":1},"objectId":{"0":9,"1":168,"2":24,"3":131,"4":6,"5":138,"6":248,"7":102,"8":3,"9":9,"10":66,"11":149,"12":58,"13":109,"14":96,"15":245},"device":{"name":"HAPPYUBU7"},"objectData":"device","syncTimestamp":1567497355357},{
"action":0,
"deviceId":{"0":1},"objectId":{"0":113,"1":69,"2":195,"3":57,"4":131,"5":245,"6":21,"7":70,"8":77,"9":54,"10":221,"11":227,"12":88,"13":227,"14":68,"15":255},"device":{"name":"HAPPYUBU7"},"objectData":"device","syncTimestamp":1567497355748}]

So there are two devices with different object_id, but with the same deviceID.

darkdh commented 4 years ago

Here comes with the summary of discussion with @bridiver on slack,

  1. Each device will have uuid which is the real unique id and deviceId is just used for bookmark ordering backwards compatibility
  2. Add a uuid to the Device record https://github.com/brave/sync/blob/6f8795a56805fd229c75fd07af1e087d1737759e/lib/api.proto#L83
  3. Subscribe to SQS queues with uuid
  4. There will be a /useridprefix/devices/deviceid -> uuid mapping, so each device will try to write the map along with a long period recheck to achieve eventually consistent.
  5. If two devices have conflicts, device which has record with newest timestamp get to keep the id
  6. If we detect device id conflict we'll re-emit SAVE_INIT_DATA with an empty seed and new id, client will save new id and send new CREATE device record with new id.
  7. Client with device id conflict should also performs a resync(send all local records and fetch all remote records)