gatus
gatus copied to clipboard
feat(web): Support TLS encryption
Summary
Implementing #253 (HTTPS/TLS support).
This to run the server on HTTPS instead of plain HTTP.
Two considerations are worth mentioning:
- 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.
- 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.
@TwiN Any chance of including this? I tried to make it as consistent as possible with the existing code
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
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
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.
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.
@TwiN I added tests and updated from master again.
@chr1st1ank @TwiN Any updates on this? I would rather have TLS directly with gatus than adding NGINX
@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.
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 If you can resolve the conflicts, I'll merge it. Sorry for the trouble 😔
@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 Thank you so much for your contribution (and patience 😂), I appreciate it!