Skip to content
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

Support TLS client authentication #264

Merged
merged 6 commits into from
Jun 21, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ redis-only-metrics | REDIS_EXPORTER_REDIS_ONLY_METRICS | Whether to also
include-system-metrics | REDIS_EXPORTER_INCL_SYSTEM_METRICS | Whether to include system metrics like `total_system_memory_bytes`, defaults to false.
is-tile38 | REDIS_EXPORTER_IS_TILE38 | Whether to scrape Tile38 specific metrics, defaults to false.
skip-tls-verification | REDIS_EXPORTER_SKIP_TLS_VERIFICATION | Whether to to skip TLS verification
tls-client-key-file | REDIS_EXPORTER_TLS_CLIENT_KEY_FILE | Full path to the client key file if the server requires TLS client authentication
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: your docs say full path to the client key file. Does that mean the expected parameter is a path (and the redis lib figures out what the file is) or is it the full name of the client key file?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, the text here is somewhat inconsistent with the --help output which says Optional client certificate file...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the path including the file name which is passed to tls.LoadX509KeyPair. I should make the doc consistent with the flag description. Which is less ambiguous to you?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, in that case maybe dbe a bit more explicit that it's a file name that's expected:
e.g. Name of the client key file (including full path) if the server requires TLS client authentication

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated both the README and flag descriptions!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woooooooot !

tls-client-cert-file | REDIS_EXPORTER_TLS_CLIENT_CERT_FILE | Full path to the client cert file if the server requires for TLS client authentication

Redis instance addresses can be tcp addresses: `redis://localhost:6379`, `redis.example.com:6379` or e.g. unix sockets: `unix:///tmp/redis.sock`.\
SSL is supported by using the `rediss://` schema, for example: `rediss://azure-ssl-enabled-host.redis.cache.windows.net:6380` (note that the port is required when connecting to a non-standard 6379 port, e.g. with Azure Redis instances).\
Expand Down
8 changes: 5 additions & 3 deletions exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type ExporterOptions struct {
ConfigCommandName string
CheckSingleKeys string
CheckKeys string
ClientCertificates []tls.Certificate
InclSystemMetrics bool
SkipTLSVerification bool
IsTile38 bool
Expand Down Expand Up @@ -970,14 +971,15 @@ func getKeysFromPatterns(c redis.Conn, keys []dbKeyPair) (expandedKeys []dbKeyPa
return expandedKeys, err
}

func (e *Exporter) connectToRedis(skipTLSVerification bool) (redis.Conn, error) {
func (e *Exporter) connectToRedis() (redis.Conn, error) {
options := []redis.DialOption{
redis.DialConnectTimeout(e.options.ConnectionTimeouts),
redis.DialReadTimeout(e.options.ConnectionTimeouts),
redis.DialWriteTimeout(e.options.ConnectionTimeouts),

redis.DialTLSConfig(&tls.Config{
InsecureSkipVerify: skipTLSVerification,
InsecureSkipVerify: e.options.SkipTLSVerification,
Certificates: e.options.ClientCertificates,
}),
}

Expand Down Expand Up @@ -1005,7 +1007,7 @@ func (e *Exporter) connectToRedis(skipTLSVerification bool) (redis.Conn, error)
}

func (e *Exporter) scrapeRedisHost(ch chan<- prometheus.Metric) error {
c, err := e.connectToRedis(e.options.SkipTLSVerification)
c, err := e.connectToRedis()
if err != nil {
log.Errorf("Couldn't connect to redis instance")
log.Debugf("connectToRedis( %s ) err: %s", e.redisAddr, err)
Expand Down
10 changes: 5 additions & 5 deletions exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,8 +401,8 @@ func TestHostVariations(t *testing.T) {
host := strings.ReplaceAll(os.Getenv("TEST_REDIS_URI"), "redis://", "")

for _, prefix := range []string{"", "redis://", "tcp://", ""} {
e, _ := NewRedisExporter(prefix+host, ExporterOptions{})
c, err := e.connectToRedis(true)
e, _ := NewRedisExporter(prefix+host, ExporterOptions{SkipTLSVerification: true})
c, err := e.connectToRedis()
if err != nil {
t.Errorf("connectToRedis() err: %s", err)
continue
Expand Down Expand Up @@ -586,7 +586,7 @@ func TestScanForKeys(t *testing.T) {

c, err := redis.DialURL(addr)
if err != nil {
t.Errorf("Couldn't connect to %#v: %#v", addr, err)
t.Fatalf("Couldn't connect to %#v: %#v", addr, err)
}
_, err = c.Do("SELECT", db)
if err != nil {
Expand Down Expand Up @@ -643,7 +643,7 @@ func TestGetKeysFromPatterns(t *testing.T) {

c, err := redis.DialURL(addr)
if err != nil {
t.Errorf("Couldn't connect to %#v: %#v", addr, err)
t.Fatalf("Couldn't connect to %#v: %#v", addr, err)
}

defer func() {
Expand Down Expand Up @@ -707,7 +707,7 @@ func TestGetKeyInfo(t *testing.T) {

c, err := redis.DialURL(addr)
if err != nil {
t.Errorf("Couldn't connect to %#v: %#v", addr, err)
t.Fatalf("Couldn't connect to %#v: %#v", addr, err)
}
_, err = c.Do("SELECT", db)
if err != nil {
Expand Down
16 changes: 16 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"crypto/tls"
"flag"
"io/ioutil"
"net/http"
Expand Down Expand Up @@ -48,6 +49,8 @@ func main() {
logFormat = flag.String("log-format", getEnv("REDIS_EXPORTER_LOG_FORMAT", "txt"), "Log format, valid options are txt and json")
configCommand = flag.String("config-command", getEnv("REDIS_EXPORTER_CONFIG_COMMAND", "CONFIG"), "What to use for the CONFIG command")
connectionTimeout = flag.String("connection-timeout", getEnv("REDIS_EXPORTER_CONNECTION_TIMEOUT", "15s"), "Timeout for connection to Redis instance")
tlsClientKeyFile = flag.String("tls-client-key-file", getEnv("REDIS_EXPORTER_TLS_CLIENT_KEY_FILE", ""), "Optional client key file if the Redis server requires")
tlsClientCertFile = flag.String("tls-client-cert-file", getEnv("REDIS_EXPORTER_TLS_CLIENT_CERT_FILE", ""), "Optional client certificate file if the Redis server requires")
isDebug = flag.Bool("debug", getEnvBool("REDIS_EXPORTER_DEBUG"), "Output verbose debug information")
isTile38 = flag.Bool("is-tile38", getEnvBool("REDIS_EXPORTER_IS_TILE38"), "Whether to scrape Tile38 specific metrics")
showVersion = flag.Bool("version", false, "Show version information and exit")
Expand Down Expand Up @@ -83,6 +86,18 @@ func main() {
log.Fatalf("Couldn't parse connection timeout duration, err: %s", err)
}

var tlsClientCertificates []tls.Certificate
if (*tlsClientKeyFile != "") != (*tlsClientCertFile != "") {
log.Fatal("TLS client key file and cert file should both be present")
}
if *tlsClientKeyFile != "" && *tlsClientCertFile != "" {
cert, err := tls.LoadX509KeyPair(*tlsClientCertFile, *tlsClientKeyFile)
if err != nil {
log.Fatalf("Couldn't load TLS client key pair, err: %s", err)
}
tlsClientCertificates = append(tlsClientCertificates, cert)
}

exp, err := NewRedisExporter(
*redisAddr,
ExporterOptions{
Expand All @@ -94,6 +109,7 @@ func main() {
InclSystemMetrics: *inclSystemMetrics,
IsTile38: *isTile38,
SkipTLSVerification: *skipTLSVerification,
ClientCertificates: tlsClientCertificates,
ConnectionTimeouts: to,
},
)
Expand Down