From f11862372a98dc0c28ef14fbc99950e6042edf3b Mon Sep 17 00:00:00 2001 From: Ben Ritcey Date: Tue, 22 Sep 2020 17:50:47 -0400 Subject: [PATCH 1/3] add client cert support Signed-off-by: Ben Ritcey --- cmd/jiralert/main.go | 98 +++++++++++++++++++++++++++++++++++++++++-- examples/jiralert.yml | 4 ++ pkg/config/config.go | 31 ++++++++++++-- 3 files changed, 126 insertions(+), 7 deletions(-) diff --git a/cmd/jiralert/main.go b/cmd/jiralert/main.go index 5fefa1b..072b821 100644 --- a/cmd/jiralert/main.go +++ b/cmd/jiralert/main.go @@ -14,9 +14,12 @@ package main import ( + "crypto/tls" + "crypto/x509" "encoding/json" "flag" "fmt" + "io/ioutil" "net/http" "os" "runtime" @@ -94,16 +97,27 @@ func main() { level.Debug(logger).Log("msg", " matched receiver", "receiver", conf.Name) // TODO: Consider reusing notifiers or just jira clients to reuse connections. - tp := jira.BasicAuthTransport{ - Username: conf.User, - Password: string(conf.Password), + + // if cert and key are specified, ignore username/password + // TODO can we (easily) support both client cert and username/password? + hc, err := httpClient(conf) + if err != nil { + errorHandler(w, http.StatusInternalServerError, err, conf.Name, &data, logger) + return } - client, err := jira.NewClient(tp.Client(), conf.APIURL) + + client, err := jira.NewClient(hc, conf.APIURL) if err != nil { errorHandler(w, http.StatusInternalServerError, err, conf.Name, &data, logger) return } + if conf.User != "" && conf.Password != "" { + // SetBasicAuth is marked as deprecated, but can't use BasicAuthTransport + // with custom TLS settings, like InsecureSkipVerify + client.Authentication.SetBasicAuth(conf.User, string(conf.Password)) + } + if retry, err := notify.NewReceiver(logger, conf, tmpl, client.Issue).Notify(&data); err != nil { var status int if retry { @@ -179,3 +193,79 @@ func setupLogger(lvl string, fmt string) (logger log.Logger) { logger = log.With(logger, "ts", log.DefaultTimestampUTC, "caller", log.DefaultCaller) return } + +func httpClient(conf *config.ReceiverConfig) (*http.Client, error) { + tlsConfig, err := newTLSConfig(conf) + if err != nil { + return nil, err + } + + hc := &http.Client{ + // Timeout: options.Timeout, + Transport: &http.Transport{ + TLSClientConfig: tlsConfig, + }, + } + + return hc, nil + +} + +func newTLSConfig(conf *config.ReceiverConfig) (*tls.Config, error) { + tlsConfig := &tls.Config{ + InsecureSkipVerify: conf.InsecureSkipVerify, + Renegotiation: tls.RenegotiateOnceAsClient} + + // If a CA cert is provided then let's read it in + if len(conf.CAFile) > 0 { + b, err := readCAFile(conf.CAFile) + if err != nil { + return nil, err + } + if !updateRootCA(tlsConfig, b) { + return nil, fmt.Errorf("unable to use specified CA cert %s", conf.CAFile) + } + } + + // If a client cert & key is provided then configure TLS config accordingly + if len(conf.CertFile) > 0 && len(conf.KeyFile) == 0 { + return nil, fmt.Errorf("client cert file %q specified without client key file", conf.CertFile) + } else if len(conf.KeyFile) > 0 && len(conf.CertFile) == 0 { + return nil, fmt.Errorf("client key file %q specified without client cert file", conf.KeyFile) + } else if len(conf.CertFile) > 0 && len(conf.KeyFile) > 0 { + cert, err := getClientCertificate(conf) + if err != nil { + return nil, err + } + tlsConfig.Certificates = []tls.Certificate{*cert} + } + + return tlsConfig, nil +} + +// readCAFile reads the CA cert file from disk. +func readCAFile(f string) ([]byte, error) { + data, err := ioutil.ReadFile(f) + if err != nil { + return nil, fmt.Errorf("unable to load specified CA cert %s: %s", f, err) + } + return data, nil +} + +func updateRootCA(cfg *tls.Config, b []byte) bool { + caCertPool := x509.NewCertPool() + if !caCertPool.AppendCertsFromPEM(b) { + return false + } + cfg.RootCAs = caCertPool + return true +} + +// getClientCertificate reads the pair of client cert and key from disk and returns a tls.Certificate. +func getClientCertificate(c *config.ReceiverConfig) (*tls.Certificate, error) { + cert, err := tls.LoadX509KeyPair(c.CertFile, c.KeyFile) + if err != nil { + return nil, fmt.Errorf("unable to use specified client cert (%s) and key (%s): %s", c.CertFile, c.KeyFile, err) + } + return &cert, nil +} diff --git a/examples/jiralert.yml b/examples/jiralert.yml index ef63418..b76d395 100644 --- a/examples/jiralert.yml +++ b/examples/jiralert.yml @@ -4,6 +4,10 @@ defaults: api_url: https://jiralert.atlassian.net user: jiralert password: 'JIRAlert' + # optional client cert + # cert_file: /path/to/cert.crt + # key_file: /path/to/cert.key + # insecure_skip_verify: false # The type of JIRA issue to create. Required. issue_type: Bug diff --git a/pkg/config/config.go b/pkg/config/config.go index 7359bc7..8f17dea 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -94,9 +94,13 @@ type ReceiverConfig struct { Name string `yaml:"name" json:"name"` // API access fields - APIURL string `yaml:"api_url" json:"api_url"` - User string `yaml:"user" json:"user"` - Password Secret `yaml:"password" json:"password"` + APIURL string `yaml:"api_url" json:"api_url"` + User string `yaml:"user" json:"user"` + Password Secret `yaml:"password" json:"password"` + CAFile string `yaml:"ca_file" json:"ca_file"` + CertFile string `yaml:"cert_file" json:"cert_file"` + KeyFile string `yaml:"key_file" json:"key_file"` + InsecureSkipVerify bool `yaml:"insecure_skip_verify" json:"insecure_skip_verify"` // Required issue fields Project string `yaml:"project" json:"project"` @@ -178,6 +182,8 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { if _, err := url.Parse(rc.APIURL); err != nil { return fmt.Errorf("invalid api_url %q in receiver %q: %s", rc.APIURL, rc.Name, err) } + // user/password isn't technically required if using client cert + // auth, but will be ignored if set, so leaving this as required if rc.User == "" { if c.Defaults.User == "" { return fmt.Errorf("missing user in receiver %q", rc.Name) @@ -191,6 +197,25 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { rc.Password = c.Defaults.Password } + // want to be able to override CAFile/KeyFile/CertFile in each receiver + // while still having a default value - use "none" to indicate no value + // set + if rc.CAFile == "none" { + rc.CAFile = "" + } else if rc.CAFile == "" && c.Defaults.CAFile != "" { + rc.CAFile = c.Defaults.CAFile + } + if rc.CertFile == "none" { + rc.CertFile = "" + } else if rc.CertFile == "" && c.Defaults.CertFile != "" { + rc.CertFile = c.Defaults.CertFile + } + if rc.KeyFile == "none" { + rc.KeyFile = "" + } else if rc.KeyFile == "" && c.Defaults.KeyFile != "" { + rc.KeyFile = c.Defaults.KeyFile + } + // Check required issue fields if rc.Project == "" { if c.Defaults.Project == "" { From 9154509bd8f547a48a339a55eb049501e0e01ef6 Mon Sep 17 00:00:00 2001 From: Ben Ritcey Date: Wed, 23 Sep 2020 14:56:55 -0400 Subject: [PATCH 2/3] updates based on PR feedback Signed-off-by: Ben Ritcey --- cmd/jiralert/main.go | 41 +++++++++++++++++++++-------------------- examples/jiralert.yml | 3 +-- pkg/config/config.go | 12 +++++++----- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/cmd/jiralert/main.go b/cmd/jiralert/main.go index 072b821..744fb13 100644 --- a/cmd/jiralert/main.go +++ b/cmd/jiralert/main.go @@ -97,10 +97,7 @@ func main() { level.Debug(logger).Log("msg", " matched receiver", "receiver", conf.Name) // TODO: Consider reusing notifiers or just jira clients to reuse connections. - - // if cert and key are specified, ignore username/password - // TODO can we (easily) support both client cert and username/password? - hc, err := httpClient(conf) + hc, err := createHTTPClient(conf) if err != nil { errorHandler(w, http.StatusInternalServerError, err, conf.Name, &data, logger) return @@ -113,8 +110,8 @@ func main() { } if conf.User != "" && conf.Password != "" { - // SetBasicAuth is marked as deprecated, but can't use BasicAuthTransport - // with custom TLS settings, like InsecureSkipVerify + //lint:ignore SA1019 SetBasicAuth is marked as deprecated but we can't use + // BasicAuthTransport with custom TLS settings, like client certs. client.Authentication.SetBasicAuth(conf.User, string(conf.Password)) } @@ -194,14 +191,13 @@ func setupLogger(lvl string, fmt string) (logger log.Logger) { return } -func httpClient(conf *config.ReceiverConfig) (*http.Client, error) { +func createHTTPClient(conf *config.ReceiverConfig) (*http.Client, error) { tlsConfig, err := newTLSConfig(conf) if err != nil { return nil, err } hc := &http.Client{ - // Timeout: options.Timeout, Transport: &http.Transport{ TLSClientConfig: tlsConfig, }, @@ -214,25 +210,30 @@ func httpClient(conf *config.ReceiverConfig) (*http.Client, error) { func newTLSConfig(conf *config.ReceiverConfig) (*tls.Config, error) { tlsConfig := &tls.Config{ InsecureSkipVerify: conf.InsecureSkipVerify, - Renegotiation: tls.RenegotiateOnceAsClient} + Renegotiation: tls.RenegotiateOnceAsClient, + } - // If a CA cert is provided then let's read it in + // Read in a CA certificate, if one is specified. if len(conf.CAFile) > 0 { b, err := readCAFile(conf.CAFile) if err != nil { return nil, err } if !updateRootCA(tlsConfig, b) { - return nil, fmt.Errorf("unable to use specified CA cert %s", conf.CAFile) + return nil, fmt.Errorf("unable to use specified CA certificate %s", conf.CAFile) } } - // If a client cert & key is provided then configure TLS config accordingly + // Configure TLS with a client certificate, if certificate and key files are specified. if len(conf.CertFile) > 0 && len(conf.KeyFile) == 0 { - return nil, fmt.Errorf("client cert file %q specified without client key file", conf.CertFile) - } else if len(conf.KeyFile) > 0 && len(conf.CertFile) == 0 { - return nil, fmt.Errorf("client key file %q specified without client cert file", conf.KeyFile) - } else if len(conf.CertFile) > 0 && len(conf.KeyFile) > 0 { + return nil, fmt.Errorf("client certificate file %q specified without client key file", conf.CertFile) + } + + if len(conf.KeyFile) > 0 && len(conf.CertFile) == 0 { + return nil, fmt.Errorf("client key file %q specified without client certificate file", conf.KeyFile) + } + + if len(conf.CertFile) > 0 && len(conf.KeyFile) > 0 { cert, err := getClientCertificate(conf) if err != nil { return nil, err @@ -243,11 +244,11 @@ func newTLSConfig(conf *config.ReceiverConfig) (*tls.Config, error) { return tlsConfig, nil } -// readCAFile reads the CA cert file from disk. +// readCAFile reads the CA certificate file from disk. func readCAFile(f string) ([]byte, error) { data, err := ioutil.ReadFile(f) if err != nil { - return nil, fmt.Errorf("unable to load specified CA cert %s: %s", f, err) + return nil, fmt.Errorf("unable to load specified CA certificate %s: %s", f, err) } return data, nil } @@ -261,11 +262,11 @@ func updateRootCA(cfg *tls.Config, b []byte) bool { return true } -// getClientCertificate reads the pair of client cert and key from disk and returns a tls.Certificate. +// getClientCertificate reads the pair of client certificate and key from disk and returns a tls.Certificate. func getClientCertificate(c *config.ReceiverConfig) (*tls.Certificate, error) { cert, err := tls.LoadX509KeyPair(c.CertFile, c.KeyFile) if err != nil { - return nil, fmt.Errorf("unable to use specified client cert (%s) and key (%s): %s", c.CertFile, c.KeyFile, err) + return nil, fmt.Errorf("unable to use specified client certificate (%s) and key (%s): %s", c.CertFile, c.KeyFile, err) } return &cert, nil } diff --git a/examples/jiralert.yml b/examples/jiralert.yml index b76d395..7ad482f 100644 --- a/examples/jiralert.yml +++ b/examples/jiralert.yml @@ -4,10 +4,9 @@ defaults: api_url: https://jiralert.atlassian.net user: jiralert password: 'JIRAlert' - # optional client cert + # Optional client certificate. # cert_file: /path/to/cert.crt # key_file: /path/to/cert.key - # insecure_skip_verify: false # The type of JIRA issue to create. Required. issue_type: Bug diff --git a/pkg/config/config.go b/pkg/config/config.go index 8f17dea..c214a5b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -182,8 +182,9 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { if _, err := url.Parse(rc.APIURL); err != nil { return fmt.Errorf("invalid api_url %q in receiver %q: %s", rc.APIURL, rc.Name, err) } - // user/password isn't technically required if using client cert - // auth, but will be ignored if set, so leaving this as required + // Username and password aren't necessarily required if using a client + // certificate for authentication, but they will (usually) be ignored in + // that situation, so leaving them as required. if rc.User == "" { if c.Defaults.User == "" { return fmt.Errorf("missing user in receiver %q", rc.Name) @@ -197,9 +198,10 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { rc.Password = c.Defaults.Password } - // want to be able to override CAFile/KeyFile/CertFile in each receiver - // while still having a default value - use "none" to indicate no value - // set + // We want to be able to override CAFile/KeyFile/CertFile in each receiver + // even if there is a default value set. Setting it to a blank string will + // cause the existing logic to just fall back to the default, so we use + // "none" to indicate no value set. if rc.CAFile == "none" { rc.CAFile = "" } else if rc.CAFile == "" && c.Defaults.CAFile != "" { From 9cf450d32cd5dcdbb1f5651bf52b9fb7178e05eb Mon Sep 17 00:00:00 2001 From: Ben Ritcey Date: Thu, 15 Oct 2020 16:23:46 -0400 Subject: [PATCH 3/3] return jira.BasicAuthTransport.Client if client certs/CAcert are not specified Signed-off-by: Ben Ritcey --- cmd/jiralert/main.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/cmd/jiralert/main.go b/cmd/jiralert/main.go index 744fb13..18002cf 100644 --- a/cmd/jiralert/main.go +++ b/cmd/jiralert/main.go @@ -109,12 +109,6 @@ func main() { return } - if conf.User != "" && conf.Password != "" { - //lint:ignore SA1019 SetBasicAuth is marked as deprecated but we can't use - // BasicAuthTransport with custom TLS settings, like client certs. - client.Authentication.SetBasicAuth(conf.User, string(conf.Password)) - } - if retry, err := notify.NewReceiver(logger, conf, tmpl, client.Issue).Notify(&data); err != nil { var status int if retry { @@ -191,7 +185,19 @@ func setupLogger(lvl string, fmt string) (logger log.Logger) { return } +// createHTTPClient returns a jira.BasicAuthTransport or http Client, depending +// on CAFile/client certificate options func createHTTPClient(conf *config.ReceiverConfig) (*http.Client, error) { + + // if CAFile, CertFile or KeyFile aren't specified, return BasicAuthTransport client + if len(conf.CAFile) == 0 && len(conf.CertFile) == 0 && len(conf.KeyFile) == 0 { + tp := jira.BasicAuthTransport{ + Username: conf.User, + Password: string(conf.Password), + } + return tp.Client(), nil + } + tlsConfig, err := newTLSConfig(conf) if err != nil { return nil, err @@ -204,7 +210,6 @@ func createHTTPClient(conf *config.ReceiverConfig) (*http.Client, error) { } return hc, nil - } func newTLSConfig(conf *config.ReceiverConfig) (*tls.Config, error) {