Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTPS/TLS Support in tusd #418

Closed
joey-coleman opened this issue Aug 31, 2020 · 6 comments · Fixed by #423
Closed

HTTPS/TLS Support in tusd #418

joey-coleman opened this issue Aug 31, 2020 · 6 comments · Fixed by #423

Comments

@joey-coleman
Copy link
Contributor

joey-coleman commented Aug 31, 2020

Is your feature request related to a problem? Please describe.

We (Kira Systems) would like to use tusd as a component in our application, and although it will actually would be proxied (and load balanced, and so forth), our problem is that we cannot have a “gap” between the proxy and tusd. Using apache/nginx/haproxy/etc as a TLS-terminator leaves the connection between the proxy and tusd as completely unprotected plaintext. Our particular network design requirements (which come, in part, from our clients) are such that unencrypted connections between components are not allowed, and forbidding unencrypted connections allows us to create a network that prevents data leakage even in scenarios where the attacker can capture all of the traffic.

So, the problem is that tusd does not natively support encrypted connections.

Describe the solution you'd like

I'd like to add HTTPS/TLS support to tusd.

Describe alternatives you've considered

We've considered TLS termination, but as described above, it does not satisfy our needs.

Can you provide help with implementing this feature?

I'm happy to provide the code and necessary documentation.

Additional context

Originally I started an email conversation with @Acconut, as I don't know if this is a sensitive topic in the community; he ultimately directed me to open an issue (so here I am). The original email exchange follows, then some follow-up.

I hope you are the correct person to contact about this; please redirect me if not.

I understand that tusd doesn’t support HTTPS at present, having read https://github.com/tus/tusd/blob/master/docs/faq.md#how-can-i-access-tusd-using-https and the discussion in issue #86. And I do understand the desire to keep the scope narrow — there are more interesting things to work on.

That said, would you be open to accepting a pull request for tusd that adds support for HTTPS? I believe that adding support is “just” a matter of adding a couple of command-line flags to specify the certificate key and chain files, then modifying the call to http.Serve() (here: https://github.com/tus/tusd/blob/master/cmd/tusd/cli/serve.go#L95) to be switched to a call to http.ServeTLS() in the case a certificate has been provided. Is there more that would be required by the project, or is the possibility of adding HTTPS just a "hard no”?

and

Hi again — apologies for the continuation before you had a chance to respond to my original email.

I was curious, so worked up the PR — see https://github.com/joey-coleman/tusd/pull/1 — and it seems to work, at least so far as a simple test using the command line upload client at jackhftang/tusc goes. I’d be happy to address any concerns you’d have about adding this and create a PR against the main tusd repository.

Not long after, @Acconut replied

thanks for your interest and reaching out to me! It's great that you created a POC but I think there are a few things to point out:

  • I am not sure how production-ready the TLS/SSL implementations in Go are. nginx and apache have been battle-tested for decades but I am not sure where Go stands here. This can probably be found out using a quick search but I didn't do it.
  • Supporting HTTPS natively in tusd is a bit more involved than just setting the certificate. Users will probably then want to configure protocols and ciphers which makes the code more complex. nginx also supports many other options (see https://github.com/joey-coleman/tusd/blob/master/examples/nginx.conf#L8-L39). Do we need to support them then also? I would rather not include a feature at all over including a half-baked feature.
  • Right now, if users ask about HTTPS, we simply point to nginx or apache. This is easy for us as the maintainers since we don't have to provide support for configuration problems/questions regarding nginx/apache. If tusd contains flags for HTTPS, I fear that our support load may increase because people cannot find help on tusd's HTTPS settings.

I hope this puts some more perspective on this topic. If you want to continue the discussion, please open an issue on GitHub, so we can discuss this in public and not in private e-mails :)

@Acconut's reply does provide some perspective, but the usual solution won't work for us.

I would like to address a couple points here:

  • Regarding "production-readiness": I suspect that the golang crypto/tls library is about as sound as any out there — it’s actively maintained by google engineers, and likely is the implementation of TLS in nearly every application written in golang. This includes things like Kubernetes, Hashicorp’s Vault, and the Caddy webserver — granted, that’s social proof, but golang’s TLS implementation is being used in production.

  • "more than just setting the certificate": I definitely agree that full support, with every possible option, is more involved than certificates — https://github.com/denji/golang-tls#perfect-ssl-labs-score-with-go gives a good example of the level of extra configuration that must be set. I concede that going for full flexibility is not a trivial task. However, I have to seriously argue that even hard-coding support to TLSv1.2/TLSv1.3 only and not providing any options to users is a significant improvement over nothing.

  • "pointing to nginx/apache": even with basic support of TLS, directing users to put a proxy in front of a TLS-enabled tusd in case they need more granular control is consistent with the current stance on this topic.

Ultimately, I'm hoping for an incremental improvement over the current "nothing", and as noted above, I'm happy to do the heavy lifting.

@Acconut
Copy link
Member

Acconut commented Sep 7, 2020

Our particular network design requirements (which come, in part, from our clients) are such that unencrypted connections between components are not allowed

Interesting requirement. tusd can also listen on a UNIX socket, so no TCP network is used. Does that help in your case?

If not, please open a PR. Please also research if we should supply a custom TLS configuration for ServeTLS (such as in https://github.com/denji/golang-tls#perfect-ssl-labs-score-with-go) or if the default values are sensible.

@joey-coleman
Copy link
Contributor Author

The UNIX socket solution would only work sometimes for us — I don't have the details to hand, but I recall there was a hook there. It's certainly easier for us to contemplate contributing code ;).

I'll work up a full PR and get it posted soon. It will likely have a few options for TLS configuration: I had been thinking in the intervening time that, as far as as options go, disabling ≤TLSv1.1 by default would be a good idea.

@Acconut
Copy link
Member

Acconut commented Sep 8, 2020

Sounds good 👍 Looking forward to your contributions :)

@joey-coleman
Copy link
Contributor Author

PR opened :).

The one open item on my end is that I'm still researching the consequences of including the tls.TLS_RSA_WITH_AES_256_GCM_SHA384 and tls.TLS_RSA_WITH_AES_128_GCM_SHA256 ciphersuites (which use RSA-based key exchange instead of ECDHE). The testssl.sh checker says it's not vulnerable to the "ROBOT" attack, and SSL Labs is ok with it (per denji's link, above); that said, Mozilla's configurator omits them. Going by the testssl.sh simulator output, the only pragmatic loss is that IE 11 on Windows 7/8.1 would be unable to connect — I am leaning towards removing those ciphersuites, but would appreciate opinions.

@joey-coleman
Copy link
Contributor Author

PR updated; the RSA-based key transport ciphersuites removed; Mozilla excludes them because they don't support Forward Secrecy.

@Acconut
Copy link
Member

Acconut commented Sep 15, 2020

the RSA-based key transport ciphersuites removed

I am fine with that move. We don't have to maintain backwards compatibility here so let's not include ciphers which should not be used. Thank you very much for the efforts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants