Closed Bug 1182001 Opened 7 years ago Closed 7 years ago

Proof of concept of Sync crypto library for Syncto client

Categories

(Firefox OS Graveyard :: Sync, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
FxOS-S4 (07Aug)

People

(Reporter: ferjm, Assigned: mbdejong)

References

Details

Attachments

(2 files)

In order to be able to synchronize data with other Sync clients (desktop, Android, iOS) the Syncto [1] client will need to encrypt and decrypt the data that it sends and receives from the Syncto server following the same crypto algorithms that other Sync clients use.

[1] https://wiki.mozilla.org/Firefox_OS/Syncto
Assignee: nobody → ferjmoreno
Blocks: fxos-sync
Depends on: 1182071
Attached patch v1Splinter Review
Be wary of using WeaveCrypto here. It's inefficient (Bug 756549), broken (Bug 702519), not a good design (Bug 608156), and easy to get wrong (Bug 627097).

Sync has a fairly self-contained set of crypto work; pretty much everything fits in one file[1].

If you have existing crypto primitives on the system, I suggest doing it right.

[1] <https://github.com/mozilla/firefox-ios/blob/master/Sync/KeyBundle.swift>
Ugh... I wasn't aware of the problems around WeaveCrypto :( Thanks for pointing them out here, Richard.

It seems that reimplementing the crypto algorithms on the System app is the best way to go. Unfortunately I am pretty bad with crypto stuff in general, so it'll take me a while to figure it out. In the mean time these patches will at least be useful to proof the Syncto architecture.
Target Milestone: --- → FxOS-S4 (07Aug)
Blocks: 1187387
Assignee: ferjmoreno → mbdejong
Summary: Add a way to encrypt/decrypt Sync records on Gaia → Proof of concept of Sync encryption library for Syncto client
Summary: Proof of concept of Sync encryption library for Syncto client → Proof of concept of Sync crypto library for Syncto client
Using the hkdf routine from https://github.com/mozilla-b2g/firefoxos-loop-client/blob/master/app/js/helpers/hawk_creds.js on top of WebCrypto I was able to decrypt the data that comes in from syncto.

There are still several problems:
* the crypto/keys entry does not always exist on the server (confirmed with the python sync client https://github.com/mozilla-services/syncclient). Not sure what to do in this case, needs more research.
* kinto.js does not always make the network request to retrieve the crypto collection. Needs more research.
* the syncclient.js code (https://github.com/michielbdejong/gaia/blob/354ed1984d1649b7e47e04a4330ae219c189b9ae/apps/kintodemo/synccrypto.js) which we will use today in the demo is too ugly even to be published as a poc.
* the poc can read-and-decrypt (syncing desktop->fxos), but not yet encrypt-and-write (syncing fxos->desktop).

I will clean up syncclient.js and add some error reporting, and then create a pull request which references this bug.
Here is the demo code for Paris WW:
https://github.com/weilonge/gaia/tree/demo_settings

In this demo, we provide the features as following:
* Implement Firefox Sync entry in Settings app for user to login/synchronize data.
* Decrypt the data fetching from Firefox Sync server.
* Import History data to Browser app (Places DataStore) by places.js service.

Improvement:
* (potential issue) The data for one collection is in one XHR. Is there any way to retrieve the partial data of a collection by some filter?
* When the data is retrieved at second time for the new entries, the new entries won't be existed. This is probably the issue of Kinto.js or synccrypto.js. We have to find the root cause.
Yes, in the end we used the Settings app as the proof-of-concept for the demo, so this bug can be closed. I'll use the crypto code as a basis for bug 1191776.

Demo was successful! :)

Francisco said he will share the video of all the demos of the Paris WW on one of the mailing lists once it's processed and uploaded.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Cool, 

If it will add a panel in settings or as a standalone app, could we get the UX spec?
Flags: needinfo?(mbdejong)
@gasolin: Does https://bugzilla.mozilla.org/show_bug.cgi?id=1168185#c8 answer that question sufficiently for the settings app? For the background app (the 'synchronizer app' from https://wiki.mozilla.org/Firefox_OS/Syncto#Implementation), there would I think just be a basic "Nothing to see here" index.html, probably with a link to the Sync tab in setting. This would normally not show up, but iiuc, if a user would list 'currently running apps' while the Synchronizer app happens to be running, they would have a way to get to its UI, so that's why this screen would still be necessary.
Flags: needinfo?(mbdejong)
Yes, we can track UI spec & related UI status in bug 1168185, thanks!
@ferjm What would be the review process for this? The code is in https://github.com/michielbdejong/fxsync-webcrypto/pull/17. Since this code deals with cryptography, I thought it would make sense to also ask for feedback from a security expert?
(In reply to Michiel de Jong [:michielbdejong] from comment #9)
> @gasolin: Does https://bugzilla.mozilla.org/show_bug.cgi?id=1168185#c8
> answer that question sufficiently for the settings app? For the background
> app (the 'synchronizer app' from
> https://wiki.mozilla.org/Firefox_OS/Syncto#Implementation), there would I
> think just be a basic "Nothing to see here" index.html, probably with a link
> to the Sync tab in setting. This would normally not show up, but iiuc, if a
> user would list 'currently running apps' while the Synchronizer app happens
> to be running, they would have a way to get to its UI, so that's why this
> screen would still be necessary.

We can hide apps by adding the "system" role to its manifest. Like in https://mxr.mozilla.org/gaia/source/apps/bookmark/manifest.webapp#5
(In reply to Michiel de Jong [:michielbdejong] from comment #11)
> @ferjm What would be the review process for this? The code is in
> https://github.com/michielbdejong/fxsync-webcrypto/pull/17.

If I am not wrong this should be part of the Synchronizer app, given that this seems to be the only client for this library for now. I would start by landing bug 1196232 and then we can start adding the code from https://github.com/michielbdejong/fxsync-webcrypto/pull/17 there.

> Since this code
> deals with cryptography, I thought it would make sense to also ask for
> feedback from a security expert?

Richard, do you think that you could do the reviews once Michiel adds the PR to Gaia with this code? If not, is anyone else familiar with the Firefox Sync crypto algorithm that we should be asking for review? Thanks!
Flags: needinfo?(rnewman)
Just saw that this bug was only for the PoC and was already closed, actual work is continued in bug 1191776.
Flags: needinfo?(rnewman)
You need to log in before you can comment on or make changes to this bug.