684074 - KeyExchange spec v3: allow sending party to connect to/create account

Status: RESOLVED FIXED

(Cloud Services :: General, defect)

Description

[Philipp von Weitershausen [:philikon]]

In bug 675823 we want to pair devices before we let users connect to or create a Sync account. This means that if "Desktop" doesn't have an account yet, we need to make "Mobile" hold after the J-PAKE pairing and before the credentials get sent over (because "Desktop" might not have credentials yet). Nothing changes in case "Desktop" is already connected to Sync, of course

This bug tracks the KeyExchange spec alterations necessary for this.

Comment 1

[Philipp von Weitershausen [:philikon]]

Attachment: Part 0 (v1): fix minor inaccuracies

Before addressing the actual changes to the spec, I went over it to fix any inaccuracies and document implicit assumptions that weren't actually written down yet:

* The channel ID is 4 characters, not 3.

* The If-None-Match / If-Match headers introduced in v2 are optional, so I clarified that by using "may be set" instead of "is set".

* Added explanatory text for all 412 Precondition Failed responses.

* Removed 412 Precondition Failed as a valid response to a GET request in steps 6 and 7. We return 304 Not Modified in those cases, 412 is only a valid response code for PUT requests.

* I added the polling timeouts for the initial message (300 seconds) and all subsequent messages (10 seconds) as well as the polling frequency.

Comment 2

[Philipp von Weitershausen [:philikon]]

Attachment: Part 1 (v1): v3 draft

Here's a draft for the v3 spec that would allow "Desktop" to connect to or create an account while "Mobile" is on hold.

My goal was to make this as little of a delta to v1/v2 as possible, mostly to ensure backwards compatibility with older clients (which is paramount). To summarize the changes:

* All transferred JSON blobs now contain a 'version' field in addition to the 'type' and 'payload' fields. For v3, this is set to be the number 3. The lack of this flag indicates to either party that the other party is a v1/v2 client. In that case, the v3 client must follow the v1/v2 flow.

* The timeout when "Mobile" polls for the credentials (step 9) has been increased from 10 to 300 seconds to allow for account setup. I believe expecting users to finish the vastly simplified account wizard in 5 minutes is very reasonable, but I'd be happy to increase that number further. However, I believe 300 seconds is the current maximum lifetime of a channel without activity on the server, so if we were to increase it further, we would have to tune server values accordingly.

So far so good. The devil is, as usual, in the details (such as dealing with a v1/v2 Mobile from a v3 Desktop that isn't connected to Sync yet), though I hope they are apparent from the document.

Comment 3

[Tarek Ziadé (:tarek)]

Comment on attachment 557681 [details] [diff] [review]
Part 0 (v1): fix minor inaccuracies

looks good

Comment 4

[Tarek Ziadé (:tarek)]

Comment on attachment 557683 [details] [diff] [review]
Part 1 (v1): v3 draft

It seems that 5. clearly indicates that it can be omitted if the user has an account,
so both scenarii seem to work. So I would simplify this paragraph by just 
explaining that they are two scenarii.

>+There are slightly different flows depending on the scenario, so this
>+document will only consider the most complicated scenario in which the
>+user does not have a Sync account yet and is trying to pair a device
>+with another one.
>+
>+For the sake of this document, "Mobile" refers to the a mobile device
>+or a desktop computer and "Desktop" refers to a desktop computer.
...


Why do we delete the session ?  If I understand correctly, v3 is doing the same number of operations on the
server -- just delaying the credentials transmission, and the channel is deleted after the last step by the server, 
so it seems unnecessary to force a session deletion at the end of a successful process


>+    delete session ----------------->|


Everything else seems fine.

Comment 5

[Philipp von Weitershausen [:philikon]]

(In reply to Tarek Ziadé (:tarek) from comment #4)
> It seems that 5. clearly indicates that it can be omitted if the user has an
> account, so both scenarii seem to work. So I would simplify this paragraph by
> just explaining that they are two scenarii.

Good point. I'll do that.

> >+There are slightly different flows depending on the scenario, so this
> >+document will only consider the most complicated scenario in which the
> >+user does not have a Sync account yet and is trying to pair a device
> >+with another one.
> >+
> >+For the sake of this document, "Mobile" refers to the a mobile device
> >+or a desktop computer and "Desktop" refers to a desktop computer.
> ...
> 
> 
> Why do we delete the session ?  If I understand correctly, v3 is doing the
> same number of operations on the server -- just delaying the credentials
> transmission, and the channel is deleted after the last step by the server, 
> so it seems unnecessary to force a session deletion at the end of a
> successful process

We've always done this, since v1. See the very bottom (step 10) of http://docs.services.mozilla.com/keyexchange/user-flow.html, so this is nothing new. It just wasn't in the flow overview before.

Comment 6

[Philipp von Weitershausen [:philikon]]

Attachment: Part 1 (v2): v3 draft

Addressed Tarek's comment regarding the first paragraph.

Pushed part 0: https://hg.mozilla.org/services/docs/rev/af051fe99a17

Comment 7

[Allison Naaktgeboren :ally]

Comment on attachment 558004 [details] [diff] [review]
Part 1 (v2): v3 draft

it seems reasonable to me. Feel free to commit to docs as a draft.

Comment 8

[Philipp von Weitershausen [:philikon]]

Landed Part 1: https://hg.mozilla.org/services/docs/rev/44ebf7ec9f09

Marked v3 as DRAFT in history.rst. Leaving this bug open to track any further changes to the v3 spec, should we find any during implementation.

Comment 9

[Allison Naaktgeboren :ally]

philikon, have there been changes made since it was moved to draft? The v3 spec is live in native fennec nightly and going live in firefox 10 & xul fennec 10. Can we consider the spec finished?

Comment 10

[Philipp von Weitershausen [:philikon]]

(In reply to :Ally Naaktgeboren from comment #9)
> Can we consider the spec finished?

I think so. https://hg.mozilla.org/services/docs/rev/8a3c90e8c96c