Serve both HTTP and HTTPS

[nyarly]

Against v0.1.2, I was doing:

https://github.com/nyarly/d2tools/blob/0733e7f3b6f322f0d0ed0c91f77c605657ab71a5/src/server/mod.rs#L34-L54

pub fn start_https() -> Result<()> {
  let der = include_bytes!("../identity.p12");
  let cert = Pkcs12::from_der(der, "mypass")?;
  let tls_cx = TlsAcceptor::builder(cert)?.build()?;

  let proto = proto::Server::new(Http::new(), tls_cx);
  let addr = "127.0.0.1:8080".parse()?;

  configure_logging();
  info!("Listening on {}", addr);
  let srv = TcpServer::new(proto, addr);
  Ok(srv.serve(|| Ok(new_https_service())))
}

fn new_https_service() -> HTTPSService {
  HTTPSService { http: GothamService::new(router::new()) }
}

struct HTTPSService {
  http: GothamService<Router>,
}

That, however, relies on GothamService (at the time NewServiceHandler) which is now private.

I needed TLS to do oauth2 (the provider for this service requires an https endpoint), and it was very convenient to be able to develop with a "bare" application. In production, I'd likely put the Gotham app behind Apache or Nginx and let them terminate TLS - at least until static files are available, there's no other good way to handle ACME.

What I would like to do is run my Gotham app such that it provides both HTTP and HTTPS. Failing that, I'd like to configure with an environment variable.

In a perfect world, Gotham would provide convenience start and start_tls functions, as well as a way to get a tokio_service::Service so that it could be passed to other tokio consumers (especially hyper)

[smangelsdorf]

This might be solved along with the solution to #183 and #95. Need to evaluate after those are fixed to see if there's more we can do here.

Perhaps a start_tls convenience function which applies some best practice.

[bradleybeddoes]

I'm also very keen to ensure that we provide a detailed TLS support story as @smangelsdorf has recommended. This entire space is full of footguns.

Sensible, well researched, secure defaults based on current best practice is something that Gotham should continually strive for. We're actually already a ways down this road with create_response for example.

FWIW this kind of 'security included' approach vs what you get elsewhere is another reason why I hate micro-benchmarks so much, but that is a rant for a different ticket.

[crusty-dave]

How does one implement TLS with gotham? I cannot seem to find an example anywhere... Can one use the native_tls crate with gotham?

FYI: I have converted a test program from actix-web to gotham, not realizing that the same level of TLS support doesn't exist. Without TLS, gotham is useless to me. At this point I think I need to focus on actix-web where I know that TLS works, unless someone can enlighten me here?

Thanks,

[whitfin]

@crusty-dave as far as I know, this is still up for discussion. It should be pretty simple in terms of passing through to Hyper though; there's no reason specifically why it's not embedded. I can take a look tomorrow perhaps, I have some time set aside to work on some other Gotham stuff.

FWIW just because the server process itself doesn't bake TLS in does not make it useless; many (most?) people these days handle TLS in their proxying layer.

[crusty-dave]

If it can be incorporated in 0.3.*, that would be very useful to me.

The following is what I currently do with actix, it would be great if I could do something similar:

        let srv_res = match tls {
            true => {
                let tls_builder = self.apply_cert();
                server.bind_rustls(srv_uri, tls_builder)
            },
            _ => {
                server.bind(srv_uri)
            }
        };

Thanks!

[whitfin]

@crusty-dave yeah, I think perhaps we should do our best to include this with 0.4 if we're able to (which is what we're thinking of for the next release).

Do you have any thoughts @nyarly, @colinbankier and @secretfader? Since @nyarly opened this in the first place, maybe they have something lying around that might be useful.

[crusty-dave]

I would add that I really like StateMiddleware, which is why I would prefer to use gotham! :)

[crusty-dave]

@whitfin FYI: I hope it goes without saying that it needs to support TLS 1.3. Thanks again.

[secretfader]

Up until this point, I've recommended Gotham users front their HTTP services with nginx and terminate TLS connections at the proxy layer. However, it's clear this is becoming an issue for framework users.

tower-web, another web framework I've been investigating, provides this example for enabling TLS. At the very least, we should expose a method supporting a similar technique.

[nyarly]

Some of the motivation to convert to Tokio 0.12 was in service of being able to connect the Service to different HTTP servers, and therefore into a HTTPS server. The details have faded in my recollection somewhat over time. It should be straightforward to provide a start_tls (and maybe a start_http with an eye toward changing the default behavior). I'll jump on that this week unless anyone else feels a burning desire. I have a set of TLS defaults in mind, although it'll take a little hunting to find the documentation. If anyone has their own ideas I'm open to discussion though.

…

On Mon, Mar 11, 2019 at 7:53 AM Nicholas Young ***@***.***> wrote: Up until this point, I've recommended Gotham users front their HTTP services with nginx and terminate TLS connections at the proxy layer. However, it's clear this is becoming an issue for framework users. tower-web <https://github.com/carllerche/tower-web>, another web framework I've been investigating, provides this example <https://github.com/carllerche/tower-web/blob/master/examples/rustls/src/main.rs#L66-L86> for enabling TLS. At the very least, we should expose a method supporting a similar technique. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#182 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHyPCSsMphld56M7-mKSOdB3KG_mxUCks5vVm38gaJpZM4SunQS> .

[secretfader]

Does da2f7a2 resolve the basic need for TLS, as raised by the conversation above? If so, let's create an example we can merge before closing this issue.

[crusty-dave]

I like having the TLS as simply an Option, it seems like a clean solution.

[secretfader]

Regardless of whether this issue is resolved, da2f7a2 seems to have broken CI. I admit to feeling a bit frustrated when I saw the changes pushed to master directly, which I tried to brush off, but now feel justified in questioning whether pushing directly to master should be allowed at all.

With master in its current state, users can't depend on it while using the JWT middleware, a silly and frustrating trade-off when the middleware implementation is compatible with all other Gotham features and dependencies. While I mentioned that the JWT middleware incompatibility shouldn't hold up a release of the core framework, I also believe the work to include TLS in core should've been handled in a feature branch and compiled optionally. Bonus points if it was discussed in a PR before the decision to merge.

I know the goal was "secure by default," but sacrificing the master branch CI pipeline to a specific feature is a bit too much.

[nyarly]

That was my error, and not an intentional change to master I'd 👍 protecting master, if only from that.

[crusty-dave]

How long does it take for a feature like this to migrate from nightly to stable?

[joseluisq]

Any update on this?
Since #511 is merged ?

[msrd0]

@joseluisq I don't think anyone is working on this currently, so if you want to work on a PR, feel free to do so!