gatus icon indicating copy to clipboard operation
gatus copied to clipboard

feat(web): Support TLS encryption

Open chr1st1ank opened this issue 2 years ago • 1 comments

Summary

Implementing #253 (HTTPS/TLS support).

This to run the server on HTTPS instead of plain HTTP.

Two considerations are worth mentioning:

  1. The options are just the minimum to have TLS. There are many more options and one could add a full section for configuration of TLS. (See e.g. here: https://github.com/PowerDNS/go-tlsconfig/blob/master/config.go). But for my purposes the simple setup is more than enough. If people want more options, they can also use a reverse proxy or load balancer.
  2. At first I wanted to add tests. But then I saw that the serving part of the controller is already untested now. I would have introduced a lot of additional complexity into the testing suite (create certificates, actually run the server, run queries against it) with possibly additional dependencies for creating the certificate. If we instead add hardcoded test certificates, most security scanners would remark that there are credentials in the repo. And all of this to test just a few calls into the standard library. So I'd like to ask first whether such a test should be added or rather be left out.

How to test it:

Create a certificate pair:

openssl ecparam -genkey -name secp384r1 -out server.key
openssl req -new -x509 -sha256 -key server.key -out server.crt -days 3650

Configure gatus to use the keypair in the config.yaml:

web:
  port: 4443
  cert_file: "server.crt"
  key_file: "server.key"
endpoints:
  - name: example
    url: https://example.org
    interval: 30s
    conditions:
      - "[STATUS] == 200"

Run gatus.

Checklist

  • [X] Tested and/or added tests to validate that the changes work as intended, if applicable. (I tested it locally)
  • [X] Added the documentation in README.md, if applicable.

chr1st1ank avatar Aug 27 '22 10:08 chr1st1ank

@TwiN Any chance of including this? I tried to make it as consistent as possible with the existing code

chr1st1ank avatar Sep 02 '22 18:09 chr1st1ank

Could you add some tests?

It isn't straight-forward with the certificates. What way would you prefer:

  • Add hardcoded test certificates to the repository (which is simple, but linters ususally don't like it, e.g. see here: https://github.com/tornadoweb/tornado/issues/3107)
  • Generate a self-signed certificate on the fly with Go (which might need additional dependencies?). Here is an example of such code: https://go.dev/src/crypto/tls/generate_cert.go
  • Generate a self-signed certificate locally on the dev environment & on github actions before running the test suite

chr1st1ank avatar Sep 21 '22 09:09 chr1st1ank

Generate a self-signed certificate on the fly with Go (which might need additional dependencies?). Here is an example of such code: https://go.dev/src/crypto/tls/generate_cert.go

I personally prefer this one.

Based on what I can see in the example you linked, the dependency is only used for building anyways:

//go:build ignore

TwiN avatar Sep 26 '22 22:09 TwiN

Alright. Now the code is almost fully tested with certificates which are generated on the fly. It was actually not as complicated as I expected. I didn't know that go has all the necessary tools already in the standard library.

I'm looking forward to see the CI do its job. On my machine two tests in "client" and "core" are failing. But they don't seem to have any connection with my changes.

chr1st1ank avatar Sep 30 '22 18:09 chr1st1ank

Codecov Report

Base: 82.80% // Head: 82.72% // Decreases project coverage by -0.08% :warning:

Coverage data is based on head (66d885d) compared to base (be88af5). Patch coverage: 65.21% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #322      +/-   ##
==========================================
- Coverage   82.80%   82.72%   -0.09%     
==========================================
  Files          54       54              
  Lines        3902     3924      +22     
==========================================
+ Hits         3231     3246      +15     
- Misses        521      527       +6     
- Partials      150      151       +1     
Impacted Files Coverage Δ
controller/controller.go 82.75% <37.50%> (-12.70%) :arrow_down:
config/web/web.go 90.00% <80.00%> (-10.00%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Oct 02 '22 16:10 codecov-commenter

@TwiN I added tests and updated from master again.

chr1st1ank avatar Oct 21 '22 22:10 chr1st1ank

@chr1st1ank @TwiN Any updates on this? I would rather have TLS directly with gatus than adding NGINX

gaby avatar Apr 18 '23 03:04 gaby

@chr1st1ank I noticed there's no support for ClientCert auth, which is very useful for enterprise environments.

It basically requires adding a field for a CA, and adding a boolean option to verify that the incoming certs are signed by the same CA as the server.

This could probably be added later.

gaby avatar Apr 18 '23 03:04 gaby

I can resolve the conflicts to master and make it mergeable again, but only if there is an interest for this PR. @gaby the client certificates I'd really rather take separately

chr1st1ank avatar Apr 18 '23 06:04 chr1st1ank

@chr1st1ank If you can resolve the conflicts, I'll merge it. Sorry for the trouble 😔

TwiN avatar Apr 19 '23 02:04 TwiN

@chr1st1ank If you can resolve the conflicts, I'll merge it. Sorry for the trouble pensive

Alright, no problem. I've merged master into my branch again and tested it locally. Looks ready to go unless you find any remaining flaws.

chr1st1ank avatar Apr 19 '23 16:04 chr1st1ank

@chr1st1ank Thank you so much for your contribution (and patience 😂), I appreciate it!

TwiN avatar Apr 22 '23 16:04 TwiN