Added authorization and config warning #5
Conversation
settings.d/shellhooks.yml.example
Outdated
| # Use of HTTPS only is strongly advised. Exposing shellhooks over http is | ||
| # technically possible but allows anyone to run things on your proxy. |
There was a problem hiding this comment.
Technically it doesn't allow anyone to run things, just those with a matching forward + reverse DNS that matches a name in trusted hosts. So if you have foreman.example.com, then when you connect from 192.0.2.1 then Smart Proxy will perform a reverse DNS lookup on that IP. If it finds foreman.example.com then it does a forward DNS lookup. If that finds 192.0.2.1, it is considered valid:
https://github.com/theforeman/smart-proxy/blob/fdeef1dc6febcfae22c8d3273cb18d6bdeb31a23/lib/sinatra/authorization.rb#L38
This calls remote_fqdn:
I would phrase it as
| # Use of HTTPS only is strongly advised. Exposing shellhooks over http is | |
| # technically possible but allows anyone to run things on your proxy. | |
| # Use https for production deployments. http and true only make sense in development |
Note that I explicitly used https in lower case since that's a valid value. I don't think HTTPS is accepted.
There was a problem hiding this comment.
@adamruzicka I suggested the wording so I'd appreciate it if you took a look as well.
|
I'm fine with it. @ehelms was originally concerned about the wording, might be worth getting his approval too |
|
Thanks all, that settles us with an improved message and also a security fix. I am merging and cutting a new release now. |
Authorization was completely missing.
Also adding a warning comment to the example setting file.
I am gonna cut a new release with this patch.