-
-
Notifications
You must be signed in to change notification settings - Fork 56
added support for certificates and insecure #96
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
base: master
Are you sure you want to change the base?
Conversation
|
As I understand it, this PR adds support for client TLS authentication when talking to the Squid manager endpoint, i.e. squid-exporter will communicate securely with Squid and identify itself with a client cert (if configured). Is that correct? If so, then presumably you would need to configure an ACL on Squid to verify one or more attributes of the client's (i.e., squid-exporter) certificate that it presents in the TLS handshake. Can you perhaps elaborate on your use case? |
|
I guess this is a misunderstanding. This pull request is not about supporting client certificates, but supporting self signed certificates by adding a custom trust store. This way, the squid exporter can trust the certificate presented by squid without relying on the system trust database. |
|
Ok, but loading certificates into the When using the Note that you don't have to use the host system's CA bundle. So if you wanted to support (and verify) a self-signed cert being used by Squid, you could simply add that cert to your There is no reason for the client (i.e. squid-exporter) to present a TLS cert, unless you're trying to implement some kind of mutual TLS - in which case you would likely still want to refer to a custom root CA, used to sign the (distinct) certs on both sides of the connection. |
af487ac to
8f78989
Compare
| func (ch *connectionHandlerImpl) connect() (net.Conn, error) { | ||
| return net.Dial("tcp", fmt.Sprintf("%s:%d", ch.hostname, ch.port)) | ||
| if ch.certificate != "" && ch.key != "" { | ||
| cert, err := tls.LoadX509KeyPair(ch.certificate, ch.key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR, I think this is a good addition. however I'm not sure if squid server supports client certs for this work, does it?
If you meant to support when the cache manager port is behind a secure port as the insecure flag implies then I think the right approach might be what's explained here: https://gist.github.com/denji/12b3a568f092ab951456?permalink_comment_id=1695690#gistcomment-1695690
Please let me know if I misunderstood anything :)
I added TLS support if squid runs on secure ports and also an option for insecure connections.