Closed LaurenceJJones closed 5 months ago
Just because I was curious if SSL over unix sockets actually did anything here are the results
TLDR: Yes
Does it actually encrypt the data passed through we can MITM with socat
sudo mv /var/run/test.sock /var/run/test.o.sock
sudo socat -t100 -x -v UNIX-LISTEN:/var/run/test.sock,mode=777,reuseaddr,fork UNIX-CONNECT:/var/run/test.o.sock
However, there is nio advantage to this as if they can already MITM the unix file, you already got bigger issues than this 😆
Implemented graceful shutdown for all server types. Put cleanup within a defer function which will return once we get a signal through chan
I want to make one more change, and that is moving the listener creation within the run functions
I do that tomorrow as its 11pm here
So might of gone out of scope abit, however, the redirect to https could of been a middleware within gin itself so I have now done this LMK if you think this is out of scope. The redirect function are now obsolete and now used I can reuse them within the gin
router itself 🤷🏻
We now only run one http server on two listeners, the functions chains are ensured that the letsscrypt
function runs if enabled then passes to gin
which was the functionality before**. The first middleware will check if http request needs to be upgrade and returns redirect.
One change in functionality is since gin is handling the upgrade http -> https the 302's are now logged within gin itself
** However, further testing this means on https
requests to /well-known/acme-challenge/
will be accepted by letsencrypt
function. However, I don't see any issue with this as gin would of responded with 404
anyways.
Are the http .well-known requests now redirected to https? They must go directly to the letsencrypt handler, otherwise no TLS can be obtained and https likely doesn't work.
I'd say this is all in scope, the change looks kinda complicated now but I've only took a quick glance at it (seems reasonable, at second glance (: ). Will review and test it on the weekend.
Are the http .well-known requests now redirected to https? They must go directly to the letsencrypt handler, otherwise no TLS can be obtained and https likely doesn't work.
I'd say this is all in scope,
the change looks kinda complicated now but I've only took a quick glance at it(seems reasonable, at second glance (: ). Will review and test it on the weekend.
I tested it just with it let's encrypt enabled flag and it worked on non https connection with redirect flag. Because the handler is wrapped the acme stuff happens first then the https upgrade is run afterwards if the url didn't pass the acme handler filter.
Implemented error handling now, so if user did supply http and tls unix sockets the other would be cleaned up if the other failed to bind. It best to avoid log.Fatal as we dont actually handle the error and nothing get cleaned up. Useful learning experience about the different waitgroups there are.
Attention: 16 lines
in your changes are missing coverage. Please review.
Comparison is base (
0bfa5ca
) 86.14% compared to head (8bd514a
) 86.89%.
Files | Patch % | Lines |
---|---|---|
router/router.go | 0.00% | 15 Missing and 1 partial :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Fixes #237
Tested http and https unix sockets both work, (Even thought ssl unix sockets dont make much sense 🤷🏻 )
Nginx work fine with both
Pushing early PR since we have to work on cli tool to interact with sockets, Feel free to comment look forward to hearing from you.