[WIP] Local HTTPS Support #77

samgiles · 2016-02-24T19:11:04Z

Configuring TLS:

Create a certs/ directory containing your cert's private key and certificate:
The private_key and cert must be X509 files named crt.pem and private_key.pem

For example:

certs/
certs/test.knilxof.org/crt.pem
certs/test.knixof.org/private_key.pem

The directory that certs are found in can be configured in the configuration file under: foxbox.certficate_directory, by default the software will look for the certs directory in your current working directory.

You can disable TLS entirely using the --disable-tls command line argument.

samgiles · 2016-03-01T19:23:48Z

@fabricedesre can I get an initial review? Or suggest someone else.

I'm still trying to figure out how best to plumb in the hostname and certs from the controller as we'll need them before the server is started with this PR - we can change that, but maybe in a later PR. Also, following this we'll want to use LetsEncrypt, and pre-generate, or generate the certs on demand.

fabricedesre · 2016-03-01T22:18:54Z

I took avery quick look, but could you have a CertificateManager that you pass to the HttpServer instead of your current code?

@ferjm do you mind taking a look?

ferjm · 2016-03-02T10:41:04Z

src/controller.rs

+    fn get_local_tls_key(&self)  -> PathBuf;
+    fn get_local_tls_crt(&self)  -> PathBuf;
+    fn get_remote_tls_key(&self) -> PathBuf;
+    fn get_remote_tls_crt(&self) -> PathBuf;


As Fabrice mentioned, it'd be nice to move all this to a CertificateManager.

ferjm · 2016-03-02T11:19:51Z

Great work Sam!

I think @michielbdejong has a more solid understanding of what's needed here. Michiel, Could you take a look, please? Thanks!

michielbdejong · 2016-03-03T17:09:42Z

side note: I got https with self-signed certs, including QR, mDNS, and HSS, working with a nodejs proxy in #129 - may be useful as a reference.

michielbdejong · 2016-03-14T09:19:32Z

Latest scripts for DNS/TLS are in #163. The way it works:

create a self-signed cert
set up DNS
register LetsEncrypt cert and generate QR code for the public-local URL.
Announce mDNS and generate QR code for the self-signed URL.
Serve https with the self-signed and the LetsEncrypt-signed certs.
(to do:) apart from local IP address, post all URLs to the registration server, something like:

{
  "self-signed": "https://ba25ea553e3ff05620224368284e4ae3.self-signed/a6450c08893/",
  "local": "https://dcaa0a22b76e30da2cfe0d10e94603ae.box.knilxof.org/",
  "mirrors": [
    "https://a.dcaa0a22b76e30da2cfe0d10e94603ae.box.knilxof.org/",
    "https://b.dcaa0a22b76e30da2cfe0d10e94603ae.box.knilxof.org/"
  ],
  "remote": "https://dcaa0a22b76e30da2cfe0d10e94603ae.box.knilxof.org/"
}

samgiles · 2016-03-14T15:27:36Z

This requires the API compatible fork of Iron to allow custom SslContext support, see: iron/iron#433

A few of our sub-dependencies also rely on Iron, it is not enough to simply replace iron with a fork because of Cargo's dependency resolution, see: rust-lang/cargo#2385

fabricedesre · 2016-03-14T16:36:55Z

@samgiles I guess the alternative is to fork all the sub-dependencies that depend on Iron?

samgiles · 2016-03-14T16:40:22Z

I just thought: I'm doing this locally by setting a .cargo/config with a path override. It overrides 'iron' with the implementation found at the configured path:

My .cargo/config looks like this:

paths = ["/Users/sam/code/rust/iron"]

I could add the iron into the repository and commit a config. @fabricedesre Does that sound reasonable as a work around?

fabricedesre · 2016-03-14T16:45:03Z

Yes, but can we override with a http or git url?

samgiles · 2016-03-14T16:51:10Z

No, it needs to be a local path.

fabricedesre · 2016-03-14T17:11:04Z

So, something like:
paths = ["./iron-fork"]

michielbdejong · 2016-03-15T17:02:47Z

.travis.yml

@@ -33,4 +33,4 @@ script:
    - $TRAVIS_BUILD_DIR/tools/execute-unit-tests-with-coverage
    - jshint static/main/js/*.js static/setup/js/*.js test/selenium/*.js test/integration/lib/*.js test/integration/test/*.js
    - npm run test-integration # starts foxbox and kills it within the test script
-    - (cargo run -- -l $BOX_LOCAL_NAME -p $BOX_PORT &> /dev/null &) ; npm run test-selenium
+    - (cargo run -- -l $BOX_LOCAL_NAME -p $BOX_PORT --disable-tls &> /dev/null &) ; npm run test-selenium


Why test with --disable-tls?

We don't have a solution for testing with self signed certs or real certs yet. We don't lose anything by disabling TLS for the tests for the time being.

In the future we need to figure out how to test properly (and cross platform) with certs. Maybe @JohanLorenzo or @npark-mozilla have some thoughts.

@samgiles Just run selenium with a Firefox profile that has the security exception for the certificate you're using in the tests, see e.g. https://blog.codecentric.de/en/2010/02/selenium-and-ssl-certificates/.

I don't agree we don't lose anything - currently, we have a http API with a test. Once we merge this, we will have a https API without a test. :(

If some upstream change in Iron causes our API server to break on https but not on http, we will not be able to see that in our tests. Right?

Can you create a follow-up ticket to repair the selenium test?

I created this issue (#221) to create https test. Would like to find out how can I store / use certificates, will check the link you provided first on above comment.

Looks like for nodejs test, I need the .pem format of the cert/key/ca of the generated certificate somehow

ferjm · 2016-03-16T08:54:38Z

src/tls/certificate_manager.rs

+        if self.context_provider.is_none() {
+           self.context_provider = Some(context_provider);
+           self.notify_provider();
+        }


Should we throw (or log an error) if the context provider is set more than once?

ferjm · 2016-03-16T09:19:55Z

This is an excellent job Sam!

I only added a few minor comments. Please, rebase and r=me with the comments addressed.

Local and remote HTTPS with SNI support

samgiles force-pushed the https-support branch 2 times, most recently from aac981b to 2639f1e Compare March 1, 2016 16:19

ferjm reviewed Mar 2, 2016
View reviewed changes

samgiles mentioned this pull request Mar 3, 2016

[WiP] Add QR code generation #129

Closed

2 tasks

samgiles force-pushed the https-support branch from 48c5ae4 to 623cf53 Compare March 14, 2016 13:23

michielbdejong mentioned this pull request Mar 14, 2016

Add tunnel url to registry. r=fabrice fxbox/registration_server#11

Merged

samgiles force-pushed the https-support branch 5 times, most recently from a8be92b to 24c8fa9 Compare March 15, 2016 15:39

samgiles force-pushed the https-support branch from 24c8fa9 to 133828f Compare March 15, 2016 16:02

michielbdejong reviewed Mar 15, 2016
View reviewed changes

samgiles force-pushed the https-support branch from 133828f to d776e5d Compare March 15, 2016 17:22

ferjm reviewed Mar 16, 2016
View reviewed changes

nojunpark mentioned this pull request Mar 16, 2016

Create integration test that uses https #221

Open

samgiles force-pushed the https-support branch from 3900879 to 6f61f70 Compare March 17, 2016 10:31

Support TLS with SNI via a CertificateManager r=ferjm

b35592c

samgiles force-pushed the https-support branch from 6f61f70 to b35592c Compare March 17, 2016 10:32

samgiles added a commit that referenced this pull request Mar 17, 2016

Merge pull request #77 from samgiles/https-support

d8ccbfa

Local and remote HTTPS with SNI support

samgiles merged commit d8ccbfa into fxbox:master Mar 17, 2016

samgiles deleted the https-support branch March 17, 2016 10:45

michielbdejong mentioned this pull request Mar 17, 2016

Self-signed cert #14

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Local HTTPS Support #77

[WIP] Local HTTPS Support #77

samgiles commented Feb 24, 2016

samgiles commented Mar 1, 2016

fabricedesre commented Mar 1, 2016

ferjm Mar 2, 2016

ferjm commented Mar 2, 2016

michielbdejong commented Mar 3, 2016

michielbdejong commented Mar 14, 2016

samgiles commented Mar 14, 2016

fabricedesre commented Mar 14, 2016

samgiles commented Mar 14, 2016

fabricedesre commented Mar 14, 2016

samgiles commented Mar 14, 2016

fabricedesre commented Mar 14, 2016

michielbdejong Mar 15, 2016

samgiles Mar 15, 2016

michielbdejong Mar 16, 2016

nojunpark Mar 16, 2016

ferjm Mar 16, 2016

ferjm commented Mar 16, 2016

[WIP] Local HTTPS Support #77

[WIP] Local HTTPS Support #77

Conversation

samgiles commented Feb 24, 2016

samgiles commented Mar 1, 2016

fabricedesre commented Mar 1, 2016

ferjm Mar 2, 2016

Choose a reason for hiding this comment

ferjm commented Mar 2, 2016

michielbdejong commented Mar 3, 2016

michielbdejong commented Mar 14, 2016

samgiles commented Mar 14, 2016

fabricedesre commented Mar 14, 2016

samgiles commented Mar 14, 2016

fabricedesre commented Mar 14, 2016

samgiles commented Mar 14, 2016

fabricedesre commented Mar 14, 2016

michielbdejong Mar 15, 2016

Choose a reason for hiding this comment

samgiles Mar 15, 2016

Choose a reason for hiding this comment

michielbdejong Mar 16, 2016

Choose a reason for hiding this comment

nojunpark Mar 16, 2016

Choose a reason for hiding this comment

ferjm Mar 16, 2016

Choose a reason for hiding this comment

ferjm commented Mar 16, 2016