1182001 - Proof of concept of Sync crypto library for Syncto client

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Sync, defect)

Description

[Fernando Jiménez Moreno [:ferjm]]

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

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] ferjm:bug1182001.weavecrypto > mozilla-b2g:master

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 3

[Richard Newman [:rnewman]]

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>

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

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.

Comment 5

[Michiel de Jong [:michielbdejong]]

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.

Comment 6

[Sean Lee [:seanlee][:weilonge]]

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.

Comment 7

[Michiel de Jong [:michielbdejong]]

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.

Comment 8

[[e-mail]]

Cool, 

If it will add a panel in settings or as a standalone app, could we get the UX spec?

Comment 9

[Michiel de Jong [:michielbdejong]]

@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.

Comment 10

[[e-mail]]

Yes, we can track UI spec & related UI status in bug 1168185, thanks!

Comment 11

[Michiel de Jong [:michielbdejong]]

@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?

Comment 12

[Fernando Jiménez Moreno [:ferjm]]

(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

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

(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!

Comment 14

[Michiel de Jong [:michielbdejong]]

Just saw that this bug was only for the PoC and was already closed, actual work is continued in bug 1191776.