1103120 - Authentication for WiFi RDP sockets

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Bug 1059001 adds TLS sockets w/o auth, so here we'll add the auth bits.

Design: https://wiki.mozilla.org/DevTools/WiFi_Debugging/Design

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: bz://1103120/jryans

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

/r/2393 - Bug 1103120 - Part 1: Add WiFi auth design doc. r=past
/r/2395 - Bug 1103120 - Part 2: Add authenticators for each auth mode. r=past
/r/2397 - Bug 1103120 - Part 3: Server: Advertise cert for authentication. r=past
/r/2399 - Bug 1103120 - Part 4: Server: Move default prompt to new file. r=past
/r/2401 - Bug 1103120 - Part 5: Server: Create connection object on accept. r=past
/r/2403 - Bug 1103120 - Part 6: Server: Move allowConnection to authenticator. r=past
/r/2405 - Bug 1103120 - Part 7: Server: Provide session to default prompt. r=past
/r/2407 - Bug 1103120 - Part 8: Server: Use promises and results in allowConnection. r=past
/r/2409 - Bug 1103120 - Part 9: Server: Require client cert, add cert to session. r=past
/r/2411 - Bug 1103120 - Part 10: Server: Send pending auth message to client. r=past
/r/2413 - Bug 1103120 - Part 11: Client: Pass auth settings from advertisement. r=past
/r/2415 - Bug 1103120 - Part 12: Client: Receive pending auth message from server. r=past
/r/2417 - Bug 1103120 - Part 13: Client: Set cert when connecting. r=past
/r/2419 - Bug 1103120 - Part 14: Client: Send OOB data to server. r=past
/r/2421 - Bug 1103120 - Part 15: Client: Compare server cert to advertisement. r=past
/r/2423 - Bug 1103120 - Part 16: Server: Receive OOB data from client. r=past
/r/2425 - Bug 1103120 - Part 17: Server: Default receive OOB dialog. r=past
/r/2427 - Bug 1103120 - Part 18: Client: Validate random value, debugging begins. r=past
/r/2429 - Bug 1103120 - Part 19: OOB cert auth tests. r=past
/r/2431 - Bug 1103120 - Part 20: B2G: Use OOB auth via WiFi. r=past

Pull down these commits:

hg pull review -r b94d33e828b749c8d00e8ad07a5aa5e474020e04

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=30885ffa9b32

(I am expecting some debug failures, will fix soon.)

If you'd like to try this locally, here's how I might do it:

1. Grab a desktop build from the above try run
2. Grab a simulator build from the above try run (in the b2g desktop build dir)
3. Install the simulator build, but don't run it from WebIDE
4. In the terminal, start the simulator's b2g manually with something like:

b2g/B2G.app/Contents/MacOS/b2g-bin -profile "$(pwd)/profile"

Argument to "profile" needs to be an absolute path for some reason.

5. In the desktop build, go to about:config.
6. Set devtools.remote.wifi.scan to true.
7. Start WebIDE.
8. You should see a WiFi runtime entry (confusingly...) with the same name as your machine (this is the simulator in a special testing mode).
9. Choose the WiFi runtime entry.
10. Approve the connection in the simulator window.
11. Copy the token from WebIDE to simulator.

It should be quite clear that the UX is terrible... :) To be fixed in later bugs, for sure!

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2391/#review1567

I added a note about how to test locally in the bug, check there if you want to attempt this.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

/r/2393 - Bug 1103120 - Part 1: Add WiFi auth design doc. r=past
/r/2395 - Bug 1103120 - Part 2: Add authenticators for each auth mode. r=past
/r/2397 - Bug 1103120 - Part 3: Server: Advertise cert for authentication. r=past
/r/2399 - Bug 1103120 - Part 4: Server: Move default prompt to new file. r=past
/r/2401 - Bug 1103120 - Part 5: Server: Create connection object on accept. r=past
/r/2403 - Bug 1103120 - Part 6: Server: Move allowConnection to authenticator. r=past
/r/2405 - Bug 1103120 - Part 7: Server: Provide session to default prompt. r=past
/r/2407 - Bug 1103120 - Part 8: Server: Use promises and results in allowConnection. r=past
/r/2409 - Bug 1103120 - Part 9: Server: Require client cert, add cert to session. r=past
/r/2411 - Bug 1103120 - Part 10: Server: Send pending auth message to client. r=past
/r/2413 - Bug 1103120 - Part 11: Client: Pass auth settings from advertisement. r=past
/r/2415 - Bug 1103120 - Part 12: Client: Receive pending auth message from server. r=past
/r/2417 - Bug 1103120 - Part 13: Client: Set cert when connecting. r=past
/r/2419 - Bug 1103120 - Part 14: Client: Send OOB data to server. r=past
/r/2421 - Bug 1103120 - Part 15: Client: Compare server cert to advertisement. r=past
/r/2423 - Bug 1103120 - Part 16: Server: Receive OOB data from client. r=past
/r/2425 - Bug 1103120 - Part 17: Server: Default receive OOB dialog. r=past
/r/2427 - Bug 1103120 - Part 18: Client: Validate random value, debugging begins. r=past
/r/2429 - Bug 1103120 - Part 19: OOB cert auth tests. r=past
/r/2431 - Bug 1103120 - Part 20: B2G: Use OOB auth via WiFi. r=past
/r/2487 - Bug 1103120 - Part 21: Client: Close streams when connection refused. r=past

Pull down these commits:

hg pull review -r 28e5f59ff73dc6650cb76085258c8d39c573115a

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Updated to resolve debug test failures.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ed591856010

Comment 7

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/2391/#review1659

MozReview handles commit ordering implicitly since it is an extension of version control. So, part numbers in commit messages add little value. They just create more work for you should you want to perform history reordering later. I recommend leaving them out next time :)

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2393/#review1793

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +security challenges to address, but we must also keep in **mind that if

This seems like a weird place to start the emphasis. Perhaps it should start at "if"?

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +connect to Servers through an encrypted, authenticate channel. After

Typo: authenticated

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +first connection from a new Client, the Client is saved on the Server

Typo: after *the* first

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +2.  User choose device to start connection on Computer

Typo: chooses

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +4.  Device sees that ComputerCert is from a unknown client (since it is

From *an* unknown client

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +5.  User is shown a Allow / Deny / Always Allow prompt on the Device

s/a/an/

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +9.  Out-of-band channel is used to move result of step 7 from Computer

This must be step 8, right?

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +    accepted via Wi-Fi

Do you mean that the socket will not be bound to the localhost interface, but only to the external IP of the wireless adapter? If so, perhaps it would be best to spell it out to avoid ambiguity.

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +    are exposing here

Nit: missing period.

::: toolkit/devtools/security/docs/wifi.md
(Diff revision 2)
> +    out-of-band channel

Maybe I'm misunderstanding things, but isn't the Device calculating hash(ComputerCert) and extracting it from what it received out-of-band in this step, in order to find K?

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2395/#review1795

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> + * connection, server listener) in case some methods are customized by the
> + * for a given use case.

Typo: by the for a...

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> + * client displays which is then scanned by camera on the server.

Typo: by *a* camera

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +exports.Authenticators = {
> +  get(mode) {

What's the point of having both Authenticators.Prompt and Authenticators.get("PROMPT")? The difference in spelling can be a cause of confusion, for one thing. We generally lean towards the former in existing code, so I'm curious as to what the get() method buys us.

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +// Expose the authenticator modes
> +for (let key in Authenticators) {
> +  let auth = Authenticators[key];
> +  exports.Authenticators[auth.mode] = auth.mode;

Same comment here. Without exposing a get() method, authenticator modes can already be retrieved by iterating over the Authenticators keys.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2397/#review1797

::: toolkit/devtools/security/socket.js
(Diff revision 2)
> +   * implements an authentication policy.  It is expected that different uses
> +   * cases may override pieces of the |Authenticator|.  See auth.js.

Typo: different use cases

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +   * Verify that listener settings are appropriate for this authentication mode.
> +   *

Since this method uses the unusual method of throwing on failed validation instead of returning an appropriate value, perhaps it would be a good idea to spell this out in the comments?

::: toolkit/devtools/security/socket.js
(Diff revision 2)
> +  authenticator: new (Authenticators.get().Server)(),

Droppping Authenticators.get() would make this use the more common idiom:
new Authenticators.Prompt.Server()

I also haven't seen any need for a constructor function in the authenticator so far, but maybe it comes up in one of the next patches.

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2399/#review1799

::: toolkit/devtools/security/prompt.js
(Diff revision 2)
> +let Server = exports.Server = {};

The Server namespace here seems unexpected, but perhaps its need is evident further in the patch series.

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2395/#review1801

> What's the point of having both Authenticators.Prompt and Authenticators.get("PROMPT")? The difference in spelling can be a cause of confusion, for one thing. We generally lean towards the former in existing code, so I'm curious as to what the get() method buys us.

Authenticators.Prompt is not currently exposed outside this module.  External callers should all use |get()|.

The only (meaningful) use of |get()| is on the client side (see part 11) because the server's authentication mode is serialized into the advertisement.  So, "PROMPT" is serialized in the advertisement, and then used to look up the right authenticator for the client.

I am happy to go a different route, I was not really that happy with it as-is.

Comment 13

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2397/#review1805

> Droppping Authenticators.get() would make this use the more common idiom:
> new Authenticators.Prompt.Server()
> 
> I also haven't seen any need for a constructor function in the authenticator so far, but maybe it comes up in one of the next patches.

The reasoning for construction is that some methods for notifications and other UX concerns are overridden per use case.  This starts to happen in part 6, when |allowConnection| is moved on the authenticator, but continues as even more such overriddable things are added.

Comment 14

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2401/#review1807

::: toolkit/devtools/security/socket.js
(Diff revision 2)
> +    }).then(() => this.allow(), e => this.deny(e));

Better add a final catch() or then(null, ...) to make sure a broken allow() doesn't go undetected.

Comment 15

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2393/#review1813

> Maybe I'm misunderstanding things, but isn't the Device calculating hash(ComputerCert) and extracting it from what it received out-of-band in this step, in order to find K?

The device receives info about the computer's cert in two channels:

* Via the WiFi connection as part of the TLS handshake
* Via the OOB channel

In this step, the device compares the OOB cert hash against the one from the TLS handshake.  This provides assurance that the TLS client connected to the device is the one the user expects, and not a MITM attacker, since an attacker can't easily control the OOB channel.  This allows the device to know it should trust the computer.

K is separate from the cert hash, and mainly allows the device to demonstrate back to the computer that the OOB channel was secure, so the computer can trust the device.

Of course, for DevTools, the critical security decision is on the server, so the checking of certs I would say is more critical than K, since the cert hashes are what determine the server's decision.

Comment 16

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2403/#review1949

Comment 17

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2405/#review1951

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +   * In PROMPT mode, the |authenticate| method is provided:
> +   * {
> +   *   client: {
> +   *     host,
> +   *     port
> +   *   },
> +   *   server: {
> +   *     host,
> +   *     port
> +   *   }
> +   * }
> +   *
>     * @return true if the connection should be permitted, false otherwise

Is this supposed to be the "session" parameter? If not, can we add a comment about it?

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +   * In OOB_CERT mode, the |authenticate| method is provided:
> +   * {
> +   *   client: {
> +   *     host,
> +   *     port,
> +   *     cert: {
> +   *       sha256
> +   *     },
> +   *   },
> +   *   server: {
> +   *     host,
> +   *     port,
> +   *     cert: {
> +   *       sha256
> +   *     }
> +   *   }
> +   * }
> +   *

Same here.

::: toolkit/devtools/security/prompt.js
(Diff revision 2)
> -  let msg = bundle.GetStringFromName("remoteIncomingPromptMessage");
> +  let title = bundle.GetStringFromName(key + "Title");

I don't think this is a net gain. I know I often look for the code that displays a string by first grepping for the displayed string and then grepping with the string's key. Finding where remoteIncomingPromptTitle is used now will not be that easy.

::: toolkit/devtools/security/socket.js
(Diff revision 2)
> +    } else {
>        return promise.reject(Cr.NS_ERROR_CONNECTION_REFUSED);

Nit: no need for "else" here.

::: toolkit/locales/en-US/chrome/global/devtools/debugger.properties
(Diff revision 2)
> +# LOCALIZATION NOTE (remoteIncomingPromptServerAddress): The message displayed
> +# dialog for the user to choose whether an incoming connection should be

Typo. (The message displayed dialog)

::: toolkit/locales/en-US/chrome/global/devtools/debugger.properties
(Diff revision 2)
> +remoteIncomingPromptClientAddress=Client Address: %1$S

Would it be clearer if we called it Endpoint instead of Address? I'm concerned that localhost:6000 is not an actual address.

Comment 18

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2407/#review1953

Comment 19

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2409/#review1955

Ship It!

Comment 20

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2411/#review1957

Ship It!

Comment 21

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2413/#review1961

Ship It!

Comment 22

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2415/#review1963

::: toolkit/devtools/security/socket.js
(Diff revision 2)
> + * error due to the use of unsigned certs, you should make another attempt after

You probably mean "unverified certs" here?

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +    let deferred = promise.defer();
> +    transport.hooks = {
> +      onPacket(packet) {
> +        let { authResult } = packet;
> +        // TODO: Examine the result
> +      }
> +    };
> +    transport.ready();
> +    return deferred.promise;

Don't you need to resolve the deferred in onPacket?

Comment 23

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2417/#review1965

Ship It!

Comment 24

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2419/#review1969

::: toolkit/locales/en-US/chrome/global/devtools/debugger.properties
(Diff revision 2)
> +clientSendOOBHeader=The server you are connecting needs more information to authenticate this connection.  Provide the token below in prompt that appears on the server.

A couple of typos and suggestions for clarification (I don't think the user will likely know about the client-server design we are using):
- "The *endpoint* you are connecting *to*..."
- "*Please* provide the token below in *the* prompt that appers on the *other end*"

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +    return {
> +      sha256: clientCert.sha256Fingerprint,
> +      k: this._createRandom()
> +    };

Ah, this is why I was confused earlier: I assumed a single value would be transferred and it would be the product of a transformation of hash and k.

::: toolkit/devtools/security/prompt.js
(Diff revision 2)
> +    throw new Error("Invalid auth mode / result");

Bonus points if you add the actual result to the message, for better debugging.

::: toolkit/devtools/security/prompt.js
(Diff revision 2)
> +  const key = "clientSendOOB";
> +  let title = bundle.GetStringFromName(key + "Title");

Same comment here as before, about not having the entire string key.

Comment 25

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2421/#review2083

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +    let advertisedCert = cert;

What's the point of this local variable? It's only used once.

Comment 26

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2423/#review2087

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +      case AuthenticationResult.ALLOW:
> +      case AuthenticationResult.ALLOW_PERSIST:

I think you'll have to swap the order of these two as ALLOW is a subset of ALLOW_PERSIST.

::: toolkit/devtools/security/auth.js
(Diff revision 2)
> +    // Client may decide to abort if K does not match
> +    // Server's portion of authentication is now complete

Nit: you aren't consistent about ending comments with a period in this file, but in this case I was actually confused as I assumed it was a single sentence :)

Comment 27

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2425/#review2089

::: toolkit/devtools/security/prompt.js
(Diff revision 2)
> +  const key = "serverReceiveOOB";
> +  let title = bundle.GetStringFromName(key + "Title");

<Insert my traditional whining about split string keys here>

::: toolkit/devtools/security/prompt.js
(Diff revision 2)
> +  let sha256 = input.value.substring(0, 64);

Shouldn't we tim() input.value here before taking the substring?

Comment 28

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2427/#review2091

Ship It!

Comment 29

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2429/#review2097

Ship It!

Comment 30

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2431/#review2099

Ship It!

Comment 31

[Panos Astithas (he/him) [:past] (please ni?)]

https://reviewboard.mozilla.org/r/2487/#review2101

Ship It!

Comment 32

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8547983 [details]
MozReview Request: bz://1103120/jryans

https://reviewboard.mozilla.org/r/2391/#review2103

Ship It!

Comment 33

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2393/#review2171

> Do you mean that the socket will not be bound to the localhost interface, but only to the external IP of the wireless adapter? If so, perhaps it would be best to spell it out to avoid ambiguity.

Correct.  I'll add an extra line to state this explicitly.

Comment 34

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2395/#review2193

> Authenticators.Prompt is not currently exposed outside this module.  External callers should all use |get()|.
> 
> The only (meaningful) use of |get()| is on the client side (see part 11) because the server's authentication mode is serialized into the advertisement.  So, "PROMPT" is serialized in the advertisement, and then used to look up the right authenticator for the client.
> 
> I am happy to go a different route, I was not really that happy with it as-is.

As discussed on IRC, this is approach seems okay, since the auth mode is serialized to the advertisement and the client looks it up by the mode.

> Same comment here. Without exposing a get() method, authenticator modes can already be retrieved by iterating over the Authenticators keys.

The mode names actually aren't used anywhere in this way, so I've removed this block.

Comment 35

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2397/#review2203

> Since this method uses the unusual method of throwing on failed validation instead of returning an appropriate value, perhaps it would be a good idea to spell this out in the comments?

Makes sense, added.

Comment 36

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2401/#review2221

> Better add a final catch() or then(null, ...) to make sure a broken allow() doesn't go undetected.

Good catch, fixed.

Comment 37

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2405/#review2223

> Is this supposed to be the "session" parameter? If not, can we add a comment about it?

Yes, it's the |session|.  I've made this more explicit.

> I don't think this is a net gain. I know I often look for the code that displays a string by first grepping for the displayed string and then grepping with the string's key. Finding where remoteIncomingPromptTitle is used now will not be that easy.

Yeah, I can see how it would be annoying to find later.  I've inlined the prefix again.

> Would it be clearer if we called it Endpoint instead of Address? I'm concerned that localhost:6000 is not an actual address.

Ah, good point.  I am using endpoint now.

Comment 38

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2415/#review2225

> Don't you need to resolve the deferred in onPacket?

Yes, this is eventually done in part 18.

> You probably mean "unverified certs" here?

Ah, good catch.  "self-signed" is the term I was aiming for.

Comment 39

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2419/#review2227

> Ah, this is why I was confused earlier: I assumed a single value would be transferred and it would be the product of a transformation of hash and k.

Ah okay.  I tweak the docs to be slighlty more explicit about this.

> Bonus points if you add the actual result to the message, for better debugging.

Added!

> Same comment here as before, about not having the entire string key.

Inlined, as before.

Comment 40

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2421/#review2235

> What's the point of this local variable? It's only used once.

I thought it slightly clarified the cert comparison, so that each cert has a more specific name, instead of one being called just |cert|.  I think I'll leave as-is.

Comment 41

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/2423/#review2237

> Nit: you aren't consistent about ending comments with a period in this file, but in this case I was actually confused as I assumed it was a single sentence :)

Haha, periods added.

Comment 42

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

All review comments addressed.

I've manually tested against the following servers and everything looks good:

* Local Simulator w/ WiFi enabled
* Simulator 2.2
* Flame via USB
* Browser Toolbox
* Fennec

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8620aa25f775

Comment 43

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Gij was failing in the previous try run, but now looks good after rebase:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d36ee46d915&exclusion_state=all

Comment 44

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://hg.mozilla.org/integration/fx-team/pushloghtml?changeset=fabdf209d813

Comment 45

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/projects/cypress/rev/3a36bc50398e
https://hg.mozilla.org/projects/cypress/rev/c3b4fc516509
https://hg.mozilla.org/projects/cypress/rev/b81f215db168
https://hg.mozilla.org/projects/cypress/rev/8698f3b5a1b5
https://hg.mozilla.org/projects/cypress/rev/2d11ab76ee6b
https://hg.mozilla.org/projects/cypress/rev/aa8f7229e2f5
https://hg.mozilla.org/projects/cypress/rev/6d131234e4d3
https://hg.mozilla.org/projects/cypress/rev/37e5dfcc7674
https://hg.mozilla.org/projects/cypress/rev/63a17819ae94
https://hg.mozilla.org/projects/cypress/rev/a40e2eeacf5a
https://hg.mozilla.org/projects/cypress/rev/5a2adc165563
https://hg.mozilla.org/projects/cypress/rev/dd5baaca1e27
https://hg.mozilla.org/projects/cypress/rev/869fb4fbada0
https://hg.mozilla.org/projects/cypress/rev/eba034ecc128
https://hg.mozilla.org/projects/cypress/rev/77d19984fec7
https://hg.mozilla.org/projects/cypress/rev/e815a6d601eb
https://hg.mozilla.org/projects/cypress/rev/b36599b3df09
https://hg.mozilla.org/projects/cypress/rev/c7e7edea57c0
https://hg.mozilla.org/projects/cypress/rev/d47db7a17a2e
https://hg.mozilla.org/projects/cypress/rev/182fc9215d27

Comment 46

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/3a36bc50398e
https://hg.mozilla.org/mozilla-central/rev/c3b4fc516509
https://hg.mozilla.org/mozilla-central/rev/b81f215db168
https://hg.mozilla.org/mozilla-central/rev/8698f3b5a1b5
https://hg.mozilla.org/mozilla-central/rev/2d11ab76ee6b
https://hg.mozilla.org/mozilla-central/rev/aa8f7229e2f5
https://hg.mozilla.org/mozilla-central/rev/6d131234e4d3
https://hg.mozilla.org/mozilla-central/rev/37e5dfcc7674
https://hg.mozilla.org/mozilla-central/rev/63a17819ae94
https://hg.mozilla.org/mozilla-central/rev/a40e2eeacf5a
https://hg.mozilla.org/mozilla-central/rev/5a2adc165563
https://hg.mozilla.org/mozilla-central/rev/dd5baaca1e27
https://hg.mozilla.org/mozilla-central/rev/869fb4fbada0
https://hg.mozilla.org/mozilla-central/rev/eba034ecc128
https://hg.mozilla.org/mozilla-central/rev/77d19984fec7
https://hg.mozilla.org/mozilla-central/rev/e815a6d601eb
https://hg.mozilla.org/mozilla-central/rev/b36599b3df09
https://hg.mozilla.org/mozilla-central/rev/c7e7edea57c0
https://hg.mozilla.org/mozilla-central/rev/d47db7a17a2e
https://hg.mozilla.org/mozilla-central/rev/182fc9215d27
https://hg.mozilla.org/mozilla-central/rev/fabdf209d813

Comment 48

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 5: Server: Create connection object on accept. r=past

Comment 49

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 18: Client: Validate random value, debugging begins. r=past

Comment 50

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 7: Server: Provide session to default prompt. r=past

Comment 51

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 8: Server: Use promises and results in allowConnection. r=past

Comment 52

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 9: Server: Require client cert, add cert to session. r=past

Comment 53

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 10: Server: Send pending auth message to client. r=past

Comment 54

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 21: Client: Close streams when connection refused. r=past

Comment 55

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 11: Client: Pass auth settings from advertisement. r=past

Comment 56

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 12: Client: Receive pending auth message from server. r=past

Comment 57

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 13: Client: Set cert when connecting. r=past

Comment 58

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 4: Server: Move default prompt to new file. r=past

Comment 59

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 14: Client: Send OOB data to server. r=past

Comment 60

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 15: Client: Compare server cert to advertisement. r=past

Comment 61

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 19: OOB cert auth tests. r=past

Comment 62

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 16: Server: Receive OOB data from client. r=past

Comment 63

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 1: Add WiFi auth design doc. r=past

Comment 64

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 6: Server: Move allowConnection to authenticator. r=past

Comment 65

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 2: Add authenticators for each auth mode. r=past

Comment 66

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 3: Server: Advertise cert for authentication. r=past

Comment 67

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 17: Server: Default receive OOB dialog. r=past

Comment 68

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1103120 - Part 20: B2G: Use OOB auth via WiFi. r=past