From b3ef535b8f4d5c2051c6241276d5dc08201a21ee Mon Sep 17 00:00:00 2001 From: saroshali-dbx Date: Mon, 6 Jun 2022 11:26:37 -0500 Subject: [PATCH 01/17] add common-name verification Signed-off-by: saroshali-dbx Signed-off-by: chodges15 --- .../web_config.auth_client_common_name.good.yaml | 6 ++++++ .../web_config_auth_client_common_name.bad.yaml | 6 ++++++ web/tls_config.go | 15 ++++++++++++++- web/tls_config_test.go | 15 +++++++++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 web/testdata/web_config.auth_client_common_name.good.yaml create mode 100644 web/testdata/web_config_auth_client_common_name.bad.yaml diff --git a/web/testdata/web_config.auth_client_common_name.good.yaml b/web/testdata/web_config.auth_client_common_name.good.yaml new file mode 100644 index 00000000..b2892b1f --- /dev/null +++ b/web/testdata/web_config.auth_client_common_name.good.yaml @@ -0,0 +1,6 @@ +tls_server_config: + cert_file: "server.crt" + key_file: "server.key" + client_auth_type: "RequireAndVerifyClientCert" + client_ca_file: "client_selfsigned.pem", + client_cert_allowed_cn: "prometheus.example.com" diff --git a/web/testdata/web_config_auth_client_common_name.bad.yaml b/web/testdata/web_config_auth_client_common_name.bad.yaml new file mode 100644 index 00000000..5ddaecc3 --- /dev/null +++ b/web/testdata/web_config_auth_client_common_name.bad.yaml @@ -0,0 +1,6 @@ +tls_server_config: + cert_file: "server.crt" + key_file: "server.key" + client_auth_type: "RequireAndVerifyClientCert" + client_ca_file: "client_selfsigned.pem" + client_cert_allowed_cn: "bad.example.com" \ No newline at end of file diff --git a/web/tls_config.go b/web/tls_config.go index 47bbca17..1d39b888 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -28,7 +28,6 @@ import ( "github.com/go-kit/log/level" config_util "github.com/prometheus/common/config" "golang.org/x/sync/errgroup" - "gopkg.in/yaml.v2" ) var ( @@ -51,6 +50,7 @@ type TLSConfig struct { MinVersion TLSVersion `yaml:"min_version"` MaxVersion TLSVersion `yaml:"max_version"` PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites"` + ClientCertAllowedCN string `yaml:"client_cert_allowed_cn"` } type FlagConfig struct { @@ -163,6 +163,19 @@ func ConfigToTLSConfig(c *TLSConfig) (*tls.Config, error) { cfg.ClientCAs = clientCAPool } + if c.ClientCertAllowedCN != "" { + cfg.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + for _, chains := range verifiedChains { + if len(chains) != 0 { + if c.ClientCertAllowedCN == chains[0].Subject.CommonName { + return nil + } + } + } + return errors.New("CommonName authentication failed") + } + } + switch c.ClientAuth { case "RequestClientCert": cfg.ClientAuth = tls.RequestClientCert diff --git a/web/tls_config_test.go b/web/tls_config_test.go index ceb7f9b2..71324065 100644 --- a/web/tls_config_test.go +++ b/web/tls_config_test.go @@ -67,6 +67,7 @@ var ( "Bad certificate": regexp.MustCompile(`bad certificate`), "Invalid value": regexp.MustCompile(`invalid value for`), "Invalid header": regexp.MustCompile(`HTTP header ".*" can not be configured`), + "Invalid common-name": regexp.MustCompile(`CommonName authentication failed`), } ) @@ -347,6 +348,20 @@ func TestServerBehaviour(t *testing.T) { ClientCertificate: "client2_selfsigned", ExpectedError: ErrorMap["Bad certificate"], }, + { + Name: `valid tls config yml with all curves`, + YAMLConfigPath: "testdata/tls_config_noAuth.requireandverifyclientcert.good.yml", + UseTLSClient: true, + ClientCertificate: "client_selfsigned", + ExpectedError: nil, + }, + { + Name: `valid tls config yml and tls client with RequireAndVerifyClientCert (present wrong certificate)`, + YAMLConfigPath: "testdata/tls_config_noAuth.requireandverifyclientcert.good.yml", + UseTLSClient: true, + ClientCertificate: "client2_selfsigned", + ExpectedError: ErrorMap["Invalid common-name"], + }, } for _, testInputs := range testTables { t.Run(testInputs.Name, testInputs.Test) From 7c8fecd4f3bfbbf0aae86b32869b162c0d1123a6 Mon Sep 17 00:00:00 2001 From: saroshali-dbx Date: Mon, 6 Jun 2022 11:29:53 -0500 Subject: [PATCH 02/17] fix tests Signed-off-by: saroshali-dbx Signed-off-by: chodges15 --- web/tls_config_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/web/tls_config_test.go b/web/tls_config_test.go index 71324065..921f2514 100644 --- a/web/tls_config_test.go +++ b/web/tls_config_test.go @@ -349,17 +349,17 @@ func TestServerBehaviour(t *testing.T) { ExpectedError: ErrorMap["Bad certificate"], }, { - Name: `valid tls config yml with all curves`, - YAMLConfigPath: "testdata/tls_config_noAuth.requireandverifyclientcert.good.yml", + Name: `valid tls config yml and tls client with VerifyPeerCertificate (present good common-name)`, + YAMLConfigPath: "testdata/web_config_auth_client_common_name.good.yaml", UseTLSClient: true, ClientCertificate: "client_selfsigned", ExpectedError: nil, }, { - Name: `valid tls config yml and tls client with RequireAndVerifyClientCert (present wrong certificate)`, - YAMLConfigPath: "testdata/tls_config_noAuth.requireandverifyclientcert.good.yml", + Name: `valid tls config yml and tls client with VerifyPeerCertificate (present invalid common-name)`, + YAMLConfigPath: "testdata/web_config_auth_client_common_name.bad.yaml", UseTLSClient: true, - ClientCertificate: "client2_selfsigned", + ClientCertificate: "client_selfsigned", ExpectedError: ErrorMap["Invalid common-name"], }, } From a9384c4b419f7b07c4d4a94fdcb4878096cbeea7 Mon Sep 17 00:00:00 2001 From: saroshali-dbx Date: Mon, 6 Jun 2022 11:32:09 -0500 Subject: [PATCH 03/17] missing import Signed-off-by: saroshali-dbx Signed-off-by: chodges15 --- web/tls_config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/web/tls_config.go b/web/tls_config.go index 1d39b888..8b2cbce0 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -28,6 +28,7 @@ import ( "github.com/go-kit/log/level" config_util "github.com/prometheus/common/config" "golang.org/x/sync/errgroup" + "gopkg.in/yaml.v2" ) var ( From 1515dcade8c7689d16bb2bf8df86498702de886c Mon Sep 17 00:00:00 2001 From: saroshali-dbx Date: Thu, 9 Jun 2022 09:27:12 -0700 Subject: [PATCH 04/17] Fix tests Signed-off-by: saroshali-dbx Signed-off-by: chodges15 --- web/testdata/web_config_auth_client_common_name.bad.yaml | 2 +- ...good.yaml => web_config_auth_client_common_name.good.yaml} | 4 ++-- web/tls_config_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename web/testdata/{web_config.auth_client_common_name.good.yaml => web_config_auth_client_common_name.good.yaml} (55%) diff --git a/web/testdata/web_config_auth_client_common_name.bad.yaml b/web/testdata/web_config_auth_client_common_name.bad.yaml index 5ddaecc3..8ff979f3 100644 --- a/web/testdata/web_config_auth_client_common_name.bad.yaml +++ b/web/testdata/web_config_auth_client_common_name.bad.yaml @@ -3,4 +3,4 @@ tls_server_config: key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" client_ca_file: "client_selfsigned.pem" - client_cert_allowed_cn: "bad.example.com" \ No newline at end of file + client_cert_allowed_cn: "bad" \ No newline at end of file diff --git a/web/testdata/web_config.auth_client_common_name.good.yaml b/web/testdata/web_config_auth_client_common_name.good.yaml similarity index 55% rename from web/testdata/web_config.auth_client_common_name.good.yaml rename to web/testdata/web_config_auth_client_common_name.good.yaml index b2892b1f..54f636e2 100644 --- a/web/testdata/web_config.auth_client_common_name.good.yaml +++ b/web/testdata/web_config_auth_client_common_name.good.yaml @@ -2,5 +2,5 @@ tls_server_config: cert_file: "server.crt" key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" - client_ca_file: "client_selfsigned.pem", - client_cert_allowed_cn: "prometheus.example.com" + client_ca_file: "client_selfsigned.pem" + client_cert_allowed_cn: "test" \ No newline at end of file diff --git a/web/tls_config_test.go b/web/tls_config_test.go index 921f2514..b9a59451 100644 --- a/web/tls_config_test.go +++ b/web/tls_config_test.go @@ -67,7 +67,7 @@ var ( "Bad certificate": regexp.MustCompile(`bad certificate`), "Invalid value": regexp.MustCompile(`invalid value for`), "Invalid header": regexp.MustCompile(`HTTP header ".*" can not be configured`), - "Invalid common-name": regexp.MustCompile(`CommonName authentication failed`), + "Invalid common-name": regexp.MustCompile(`bad certificate`), } ) From 8d8bcbe2f2a94edf3928996767aa9e0366fd5d7b Mon Sep 17 00:00:00 2001 From: chodges15 Date: Tue, 10 Jan 2023 12:25:41 -0600 Subject: [PATCH 05/17] Add option to enforce a configured SAN DNS in the client TLS cert. The changed certs were updated based on the command @WGH- in #61, just with an added SAN DNS. They were generated with this command: openssl req -x509 -newkey ec:<(openssl ecparam -name secp384r1) -keyout client2_selfsigned.key -out client2_selfsigned.pem -nodes -subj '/CN=test3' -days 36500 -addext "subjectAltName = DNS:test3" -addext "extendedKeyUsage = clientAuth" Signed-off-by: chodges15 --- web/testdata/client2_selfsigned.key | 8 ++--- web/testdata/client2_selfsigned.pem | 20 +++++------ ...> web_config_auth_client_san_dns.bad.yaml} | 4 +-- ... web_config_auth_client_san_dns.good.yaml} | 4 +-- web/tls_config.go | 33 ++++++++++++------- web/tls_config_test.go | 16 ++++----- 6 files changed, 47 insertions(+), 38 deletions(-) rename web/testdata/{web_config_auth_client_common_name.bad.yaml => web_config_auth_client_san_dns.bad.yaml} (60%) rename web/testdata/{web_config_auth_client_common_name.good.yaml => web_config_auth_client_san_dns.good.yaml} (59%) diff --git a/web/testdata/client2_selfsigned.key b/web/testdata/client2_selfsigned.key index d4dad255..885a41a0 100644 --- a/web/testdata/client2_selfsigned.key +++ b/web/testdata/client2_selfsigned.key @@ -1,6 +1,6 @@ -----BEGIN PRIVATE KEY----- -MIG2AgEAMBAGByqGSM49AgEGBSuBBAAiBIGeMIGbAgEBBDC8CYtAwKp1uLWXLXFE -Ue2Bz6PijwHZcL7jAxtlk2dbW0GlRQ+rcalHCcnExIIKAAehZANiAATlPRxDnbJb -Zq9u+jh7DyEJumQZFqjIDFdFxfHtI6hwyMtlL6FIwpqn3z4uXs2wx6/NsD4XOChy -j/tXXKCHS/22+51TivjGA53c9bLgc4dK/uJJNSivp0kymbtA5vgKzJE= +MIG2AgEAMBAGByqGSM49AgEGBSuBBAAiBIGeMIGbAgEBBDCgZCrQEAUVqznwKRvu +dwwi8wutaRaHHHWDd/IpjJopLhcdvONT7Fv57X0foCvmYFOhZANiAAR/zAKpT17i +U9lmokwDicnziss91+vKhQjy2q4EAe1p7jJ9c/fPofP3Zd09pLhkAUONMu0myXjk +piLE1vvL121tWg3E3F0MLjLBqiSWqSkEZjQj0YSk3NoGWX/gMgm8ZyA= -----END PRIVATE KEY----- diff --git a/web/testdata/client2_selfsigned.pem b/web/testdata/client2_selfsigned.pem index 2bd61a7a..be1426c4 100644 --- a/web/testdata/client2_selfsigned.pem +++ b/web/testdata/client2_selfsigned.pem @@ -1,12 +1,12 @@ -----BEGIN CERTIFICATE----- -MIIByjCCAU+gAwIBAgIUYcG9p4RzCRdvUGa9BWvc6rB/wMYwCgYIKoZIzj0EAwIw -EDEOMAwGA1UEAwwFdGVzdDIwIBcNMjEwODIwMTUzMjE4WhgPMjEyMTA3MjcxNTMy -MThaMBAxDjAMBgNVBAMMBXRlc3QyMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE5T0c -Q52yW2avbvo4ew8hCbpkGRaoyAxXRcXx7SOocMjLZS+hSMKap98+Ll7NsMevzbA+ -Fzgoco/7V1ygh0v9tvudU4r4xgOd3PWy4HOHSv7iSTUor6dJMpm7QOb4CsyRo2gw -ZjAdBgNVHQ4EFgQUWpsZ2aWo6WEI2LiNQXoWKYr0rlkwHwYDVR0jBBgwFoAUWpsZ -2aWo6WEI2LiNQXoWKYr0rlkwDwYDVR0TAQH/BAUwAwEB/zATBgNVHSUEDDAKBggr -BgEFBQcDAjAKBggqhkjOPQQDAgNpADBmAjEA/Mv4OjCqVw8PzxQW4FJmZNyJB4ps -xkAUBRpDy75n64ICsWKX/Mille0bo+C8d63JAjEA3IH/y1O4oyCaawNpibfcwSZK -7ND9Z+WTJi50EumXUWKirmb/V59ToH5nc10x7NDX +MIIB3DCCAWGgAwIBAgIUJVN8KehL1MmccvLb/mHthSMfnnswCgYIKoZIzj0EAwIw +EDEOMAwGA1UEAwwFdGVzdDMwIBcNMjMwMTEwMTgxMTAwWhgPMjEyMjEyMTcxODEx +MDBaMBAxDjAMBgNVBAMMBXRlc3QzMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEf8wC +qU9e4lPZZqJMA4nJ84rLPdfryoUI8tquBAHtae4yfXP3z6Hz92XdPaS4ZAFDjTLt +Jsl45KYixNb7y9dtbVoNxNxdDC4ywaoklqkpBGY0I9GEpNzaBll/4DIJvGcgo3ow +eDAdBgNVHQ4EFgQUvyvu/TnJyRS7OGdujTbWM/W07yMwHwYDVR0jBBgwFoAUvyvu +/TnJyRS7OGdujTbWM/W07yMwDwYDVR0TAQH/BAUwAwEB/zAQBgNVHREECTAHggV0 +ZXN0MzATBgNVHSUEDDAKBggrBgEFBQcDAjAKBggqhkjOPQQDAgNpADBmAjEAt7HK +knE2MzwZ2B2dgn1/q3ikWDiO20Hbd97jo3tmv87FcF2vMqqJpHjcldJqplfsAjEA +sfAz49y6Sf6LNlNS+Fc/lbOOwcrlzC+J5GJ8OmNoQPsvvDvhzGbwFiVw1M2uMqtG -----END CERTIFICATE----- diff --git a/web/testdata/web_config_auth_client_common_name.bad.yaml b/web/testdata/web_config_auth_client_san_dns.bad.yaml similarity index 60% rename from web/testdata/web_config_auth_client_common_name.bad.yaml rename to web/testdata/web_config_auth_client_san_dns.bad.yaml index 8ff979f3..1462acb5 100644 --- a/web/testdata/web_config_auth_client_common_name.bad.yaml +++ b/web/testdata/web_config_auth_client_san_dns.bad.yaml @@ -2,5 +2,5 @@ tls_server_config: cert_file: "server.crt" key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" - client_ca_file: "client_selfsigned.pem" - client_cert_allowed_cn: "bad" \ No newline at end of file + client_ca_file: "client2_selfsigned.pem" + client_cert_allowed_san_dns: "bad" \ No newline at end of file diff --git a/web/testdata/web_config_auth_client_common_name.good.yaml b/web/testdata/web_config_auth_client_san_dns.good.yaml similarity index 59% rename from web/testdata/web_config_auth_client_common_name.good.yaml rename to web/testdata/web_config_auth_client_san_dns.good.yaml index 54f636e2..72425ae6 100644 --- a/web/testdata/web_config_auth_client_common_name.good.yaml +++ b/web/testdata/web_config_auth_client_san_dns.good.yaml @@ -2,5 +2,5 @@ tls_server_config: cert_file: "server.crt" key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" - client_ca_file: "client_selfsigned.pem" - client_cert_allowed_cn: "test" \ No newline at end of file + client_ca_file: "client2_selfsigned.pem" + client_cert_allowed_san_dns: "test3" \ No newline at end of file diff --git a/web/tls_config.go b/web/tls_config.go index 8b2cbce0..adbaf7b1 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -51,7 +51,7 @@ type TLSConfig struct { MinVersion TLSVersion `yaml:"min_version"` MaxVersion TLSVersion `yaml:"max_version"` PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites"` - ClientCertAllowedCN string `yaml:"client_cert_allowed_cn"` + ClientCertAllowedSanDns string `yaml:"client_cert_allowed_san_dns"` } type FlagConfig struct { @@ -67,6 +67,23 @@ func (t *TLSConfig) SetDirectory(dir string) { t.ClientCAs = config_util.JoinDir(dir, t.ClientCAs) } +// VerifyPeerCertificate will check the DNS SAN entries of the client cert if there is configuration for +func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + // sender cert comes first, see https://www.rfc-editor.org/rfc/rfc5246#section-7.4.2 + cert, err := x509.ParseCertificate(rawCerts[0]) + if err != nil { + return fmt.Errorf("error parsing client certificate: %s", err) + } + + for _, san := range cert.DNSNames { + if san == t.ClientCertAllowedSanDns { + return nil + } + } + + return fmt.Errorf("could not find configured SAN DNS in client cert: %s", t.ClientCertAllowedSanDns) +} + type HTTPConfig struct { HTTP2 bool `yaml:"http2"` Header map[string]string `yaml:"headers,omitempty"` @@ -164,17 +181,9 @@ func ConfigToTLSConfig(c *TLSConfig) (*tls.Config, error) { cfg.ClientCAs = clientCAPool } - if c.ClientCertAllowedCN != "" { - cfg.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { - for _, chains := range verifiedChains { - if len(chains) != 0 { - if c.ClientCertAllowedCN == chains[0].Subject.CommonName { - return nil - } - } - } - return errors.New("CommonName authentication failed") - } + if c.ClientCertAllowedSanDns != "" { + // verify that the client cert contains the allowed domain name + cfg.VerifyPeerCertificate = c.VerifyPeerCertificate } switch c.ClientAuth { diff --git a/web/tls_config_test.go b/web/tls_config_test.go index b9a59451..08faba01 100644 --- a/web/tls_config_test.go +++ b/web/tls_config_test.go @@ -67,7 +67,7 @@ var ( "Bad certificate": regexp.MustCompile(`bad certificate`), "Invalid value": regexp.MustCompile(`invalid value for`), "Invalid header": regexp.MustCompile(`HTTP header ".*" can not be configured`), - "Invalid common-name": regexp.MustCompile(`bad certificate`), + "Invalid client cert": regexp.MustCompile(`bad certificate`), } ) @@ -349,18 +349,18 @@ func TestServerBehaviour(t *testing.T) { ExpectedError: ErrorMap["Bad certificate"], }, { - Name: `valid tls config yml and tls client with VerifyPeerCertificate (present good common-name)`, - YAMLConfigPath: "testdata/web_config_auth_client_common_name.good.yaml", + Name: `valid tls config yml and tls client with VerifyPeerCertificate (present good SAN DNS entry)`, + YAMLConfigPath: "testdata/web_config_auth_client_san_dns.good.yaml", UseTLSClient: true, - ClientCertificate: "client_selfsigned", + ClientCertificate: "client2_selfsigned", ExpectedError: nil, }, { - Name: `valid tls config yml and tls client with VerifyPeerCertificate (present invalid common-name)`, - YAMLConfigPath: "testdata/web_config_auth_client_common_name.bad.yaml", + Name: `valid tls config yml and tls client with VerifyPeerCertificate (present invalid SAN DNS entries)`, + YAMLConfigPath: "testdata/web_config_auth_client_san_dns.bad.yaml", UseTLSClient: true, - ClientCertificate: "client_selfsigned", - ExpectedError: ErrorMap["Invalid common-name"], + ClientCertificate: "client2_selfsigned", + ExpectedError: ErrorMap["Invalid client cert"], }, } for _, testInputs := range testTables { From f8a6b39181dbec7e443e966b4c1b86275ebc5ff7 Mon Sep 17 00:00:00 2001 From: chodges15 Date: Mon, 30 Jan 2023 08:59:48 -0600 Subject: [PATCH 06/17] fix linter error Signed-off-by: chodges15 --- web/tls_config.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/tls_config.go b/web/tls_config.go index adbaf7b1..3d47869b 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -51,7 +51,7 @@ type TLSConfig struct { MinVersion TLSVersion `yaml:"min_version"` MaxVersion TLSVersion `yaml:"max_version"` PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites"` - ClientCertAllowedSanDns string `yaml:"client_cert_allowed_san_dns"` + ClientCertAllowedSanDNS string `yaml:"client_cert_allowed_san_dns"` } type FlagConfig struct { @@ -76,12 +76,12 @@ func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][] } for _, san := range cert.DNSNames { - if san == t.ClientCertAllowedSanDns { + if san == t.ClientCertAllowedSanDNS { return nil } } - return fmt.Errorf("could not find configured SAN DNS in client cert: %s", t.ClientCertAllowedSanDns) + return fmt.Errorf("could not find configured SAN DNS in client cert: %s", t.ClientCertAllowedSanDNS) } type HTTPConfig struct { @@ -181,7 +181,7 @@ func ConfigToTLSConfig(c *TLSConfig) (*tls.Config, error) { cfg.ClientCAs = clientCAPool } - if c.ClientCertAllowedSanDns != "" { + if c.ClientCertAllowedSanDNS != "" { // verify that the client cert contains the allowed domain name cfg.VerifyPeerCertificate = c.VerifyPeerCertificate } From 5b49cfea1ec0d7f368c469bb5f15e659b80b796b Mon Sep 17 00:00:00 2001 From: Chris Hodges Date: Wed, 1 Feb 2023 05:12:21 +0000 Subject: [PATCH 07/17] change config to regex Signed-off-by: chodges15 --- .../web_config_auth_client_san_dns.good.yaml | 2 +- .../web_config_auth_client_san_dns_regex.bad.yaml | 6 ++++++ .../web_config_auth_client_san_dns_regex.good.yaml | 6 ++++++ web/tls_config.go | 12 +++++++----- web/tls_config_test.go | 14 ++++++++++++++ 5 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 web/testdata/web_config_auth_client_san_dns_regex.bad.yaml create mode 100644 web/testdata/web_config_auth_client_san_dns_regex.good.yaml diff --git a/web/testdata/web_config_auth_client_san_dns.good.yaml b/web/testdata/web_config_auth_client_san_dns.good.yaml index 72425ae6..588664c6 100644 --- a/web/testdata/web_config_auth_client_san_dns.good.yaml +++ b/web/testdata/web_config_auth_client_san_dns.good.yaml @@ -3,4 +3,4 @@ tls_server_config: key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" client_ca_file: "client2_selfsigned.pem" - client_cert_allowed_san_dns: "test3" \ No newline at end of file + client_cert_allowed_san_dns: "test3" diff --git a/web/testdata/web_config_auth_client_san_dns_regex.bad.yaml b/web/testdata/web_config_auth_client_san_dns_regex.bad.yaml new file mode 100644 index 00000000..249da8c4 --- /dev/null +++ b/web/testdata/web_config_auth_client_san_dns_regex.bad.yaml @@ -0,0 +1,6 @@ +tls_server_config: + cert_file: "server.crt" + key_file: "server.key" + client_auth_type: "RequireAndVerifyClientCert" + client_ca_file: "client2_selfsigned.pem" + client_cert_allowed_san_dns: ".+test.+" \ No newline at end of file diff --git a/web/testdata/web_config_auth_client_san_dns_regex.good.yaml b/web/testdata/web_config_auth_client_san_dns_regex.good.yaml new file mode 100644 index 00000000..e7d7c7ca --- /dev/null +++ b/web/testdata/web_config_auth_client_san_dns_regex.good.yaml @@ -0,0 +1,6 @@ +tls_server_config: + cert_file: "server.crt" + key_file: "server.key" + client_auth_type: "RequireAndVerifyClientCert" + client_ca_file: "client2_selfsigned.pem" + client_cert_allowed_san_dns: (test\d|dns) diff --git a/web/tls_config.go b/web/tls_config.go index 3d47869b..d5f04bf1 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -22,6 +22,7 @@ import ( "net/http" "os" "path/filepath" + "regexp" "github.com/coreos/go-systemd/v22/activation" "github.com/go-kit/log" @@ -51,7 +52,8 @@ type TLSConfig struct { MinVersion TLSVersion `yaml:"min_version"` MaxVersion TLSVersion `yaml:"max_version"` PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites"` - ClientCertAllowedSanDNS string `yaml:"client_cert_allowed_san_dns"` + // regular expression to match the SAN DNS entries of the client cert + ClientCertAllowedSanDNSRegex string `yaml:"client_cert_allowed_san_dns"` } type FlagConfig struct { @@ -67,7 +69,7 @@ func (t *TLSConfig) SetDirectory(dir string) { t.ClientCAs = config_util.JoinDir(dir, t.ClientCAs) } -// VerifyPeerCertificate will check the DNS SAN entries of the client cert if there is configuration for +// VerifyPeerCertificate will check the DNS SAN entries of the client cert if there is configuration for it func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { // sender cert comes first, see https://www.rfc-editor.org/rfc/rfc5246#section-7.4.2 cert, err := x509.ParseCertificate(rawCerts[0]) @@ -76,12 +78,12 @@ func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][] } for _, san := range cert.DNSNames { - if san == t.ClientCertAllowedSanDNS { + if matched, _ := regexp.MatchString(t.ClientCertAllowedSanDNSRegex, san); matched { return nil } } - return fmt.Errorf("could not find configured SAN DNS in client cert: %s", t.ClientCertAllowedSanDNS) + return fmt.Errorf("could not find configured SAN DNS in client cert: %s", t.ClientCertAllowedSanDNSRegex) } type HTTPConfig struct { @@ -181,7 +183,7 @@ func ConfigToTLSConfig(c *TLSConfig) (*tls.Config, error) { cfg.ClientCAs = clientCAPool } - if c.ClientCertAllowedSanDNS != "" { + if c.ClientCertAllowedSanDNSRegex != "" { // verify that the client cert contains the allowed domain name cfg.VerifyPeerCertificate = c.VerifyPeerCertificate } diff --git a/web/tls_config_test.go b/web/tls_config_test.go index 08faba01..dcc60d37 100644 --- a/web/tls_config_test.go +++ b/web/tls_config_test.go @@ -362,6 +362,20 @@ func TestServerBehaviour(t *testing.T) { ClientCertificate: "client2_selfsigned", ExpectedError: ErrorMap["Invalid client cert"], }, + { + Name: `valid tls config yml and tls client with VerifyPeerCertificate (present SAN DNS entry that matches configured regex)`, + YAMLConfigPath: "testdata/web_config_auth_client_san_dns_regex.good.yaml", + UseTLSClient: true, + ClientCertificate: "client2_selfsigned", + ExpectedError: nil, + }, + { + Name: `valid tls config yml and tls client with VerifyPeerCertificate (present SAN DNS entry that does not match configured regex)`, + YAMLConfigPath: "testdata/web_config_auth_client_san_dns_regex.bad.yaml", + UseTLSClient: true, + ClientCertificate: "client2_selfsigned", + ExpectedError: ErrorMap["Invalid client cert"], + }, } for _, testInputs := range testTables { t.Run(testInputs.Name, testInputs.Test) From fd4ac69835b1d2b8ad64cdc9527838322fdd1318 Mon Sep 17 00:00:00 2001 From: Chris Hodges Date: Wed, 1 Feb 2023 06:39:10 -0600 Subject: [PATCH 08/17] check all SAN types against regex Signed-off-by: chodges15 --- ...ml => web_config_auth_client_san.bad.yaml} | 2 +- ...l => web_config_auth_client_san.good.yaml} | 2 +- ...web_config_auth_client_san_regex.bad.yaml} | 2 +- ...eb_config_auth_client_san_regex.good.yaml} | 2 +- web/tls_config.go | 26 ++++++++++++++----- web/tls_config_test.go | 8 +++--- 6 files changed, 27 insertions(+), 15 deletions(-) rename web/testdata/{web_config_auth_client_san_dns.bad.yaml => web_config_auth_client_san.bad.yaml} (81%) rename web/testdata/{web_config_auth_client_san_dns.good.yaml => web_config_auth_client_san.good.yaml} (80%) rename web/testdata/{web_config_auth_client_san_dns_regex.bad.yaml => web_config_auth_client_san_regex.bad.yaml} (79%) rename web/testdata/{web_config_auth_client_san_dns_regex.good.yaml => web_config_auth_client_san_regex.good.yaml} (78%) diff --git a/web/testdata/web_config_auth_client_san_dns.bad.yaml b/web/testdata/web_config_auth_client_san.bad.yaml similarity index 81% rename from web/testdata/web_config_auth_client_san_dns.bad.yaml rename to web/testdata/web_config_auth_client_san.bad.yaml index 1462acb5..9cf0640c 100644 --- a/web/testdata/web_config_auth_client_san_dns.bad.yaml +++ b/web/testdata/web_config_auth_client_san.bad.yaml @@ -3,4 +3,4 @@ tls_server_config: key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" client_ca_file: "client2_selfsigned.pem" - client_cert_allowed_san_dns: "bad" \ No newline at end of file + client_allowed_san_regex: "bad" diff --git a/web/testdata/web_config_auth_client_san_dns.good.yaml b/web/testdata/web_config_auth_client_san.good.yaml similarity index 80% rename from web/testdata/web_config_auth_client_san_dns.good.yaml rename to web/testdata/web_config_auth_client_san.good.yaml index 588664c6..ec05fb43 100644 --- a/web/testdata/web_config_auth_client_san_dns.good.yaml +++ b/web/testdata/web_config_auth_client_san.good.yaml @@ -3,4 +3,4 @@ tls_server_config: key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" client_ca_file: "client2_selfsigned.pem" - client_cert_allowed_san_dns: "test3" + client_allowed_san_regex: "test3" diff --git a/web/testdata/web_config_auth_client_san_dns_regex.bad.yaml b/web/testdata/web_config_auth_client_san_regex.bad.yaml similarity index 79% rename from web/testdata/web_config_auth_client_san_dns_regex.bad.yaml rename to web/testdata/web_config_auth_client_san_regex.bad.yaml index 249da8c4..7c6ec394 100644 --- a/web/testdata/web_config_auth_client_san_dns_regex.bad.yaml +++ b/web/testdata/web_config_auth_client_san_regex.bad.yaml @@ -3,4 +3,4 @@ tls_server_config: key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" client_ca_file: "client2_selfsigned.pem" - client_cert_allowed_san_dns: ".+test.+" \ No newline at end of file + client_allowed_san_regex: ".+test.+" diff --git a/web/testdata/web_config_auth_client_san_dns_regex.good.yaml b/web/testdata/web_config_auth_client_san_regex.good.yaml similarity index 78% rename from web/testdata/web_config_auth_client_san_dns_regex.good.yaml rename to web/testdata/web_config_auth_client_san_regex.good.yaml index e7d7c7ca..22d32f60 100644 --- a/web/testdata/web_config_auth_client_san_dns_regex.good.yaml +++ b/web/testdata/web_config_auth_client_san_regex.good.yaml @@ -3,4 +3,4 @@ tls_server_config: key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" client_ca_file: "client2_selfsigned.pem" - client_cert_allowed_san_dns: (test\d|dns) + client_allowed_san_regex: (test\d|dns) diff --git a/web/tls_config.go b/web/tls_config.go index d5f04bf1..4f9e2381 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -52,8 +52,7 @@ type TLSConfig struct { MinVersion TLSVersion `yaml:"min_version"` MaxVersion TLSVersion `yaml:"max_version"` PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites"` - // regular expression to match the SAN DNS entries of the client cert - ClientCertAllowedSanDNSRegex string `yaml:"client_cert_allowed_san_dns"` + ClientAllowedSanRegex string `yaml:"client_allowed_san_regex"` } type FlagConfig struct { @@ -69,7 +68,7 @@ func (t *TLSConfig) SetDirectory(dir string) { t.ClientCAs = config_util.JoinDir(dir, t.ClientCAs) } -// VerifyPeerCertificate will check the DNS SAN entries of the client cert if there is configuration for it +// VerifyPeerCertificate will check the SAN entries of the client cert if there is configuration for it func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { // sender cert comes first, see https://www.rfc-editor.org/rfc/rfc5246#section-7.4.2 cert, err := x509.ParseCertificate(rawCerts[0]) @@ -77,13 +76,26 @@ func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][] return fmt.Errorf("error parsing client certificate: %s", err) } - for _, san := range cert.DNSNames { - if matched, _ := regexp.MatchString(t.ClientCertAllowedSanDNSRegex, san); matched { + // Build up a slice of strings with all Subject Alternate Name values + sanValues := append(cert.DNSNames, cert.EmailAddresses...) + + for _, ip := range cert.IPAddresses { + sanValues = append(sanValues, ip.String()) + } + + for _, uri := range cert.URIs { + sanValues = append(sanValues, uri.String()) + } + + for _, sanValue := range sanValues { + if matched, _ := regexp.MatchString(t.ClientAllowedSanRegex, sanValue); matched { return nil } } - return fmt.Errorf("could not find configured SAN DNS in client cert: %s", t.ClientCertAllowedSanDNSRegex) + //todo: check other fields of the cert + + return fmt.Errorf("could not find configured SAN in client cert: %s", t.ClientAllowedSanRegex) } type HTTPConfig struct { @@ -183,7 +195,7 @@ func ConfigToTLSConfig(c *TLSConfig) (*tls.Config, error) { cfg.ClientCAs = clientCAPool } - if c.ClientCertAllowedSanDNSRegex != "" { + if c.ClientAllowedSanRegex != "" { // verify that the client cert contains the allowed domain name cfg.VerifyPeerCertificate = c.VerifyPeerCertificate } diff --git a/web/tls_config_test.go b/web/tls_config_test.go index dcc60d37..0237b860 100644 --- a/web/tls_config_test.go +++ b/web/tls_config_test.go @@ -350,28 +350,28 @@ func TestServerBehaviour(t *testing.T) { }, { Name: `valid tls config yml and tls client with VerifyPeerCertificate (present good SAN DNS entry)`, - YAMLConfigPath: "testdata/web_config_auth_client_san_dns.good.yaml", + YAMLConfigPath: "testdata/web_config_auth_client_san.good.yaml", UseTLSClient: true, ClientCertificate: "client2_selfsigned", ExpectedError: nil, }, { Name: `valid tls config yml and tls client with VerifyPeerCertificate (present invalid SAN DNS entries)`, - YAMLConfigPath: "testdata/web_config_auth_client_san_dns.bad.yaml", + YAMLConfigPath: "testdata/web_config_auth_client_san.bad.yaml", UseTLSClient: true, ClientCertificate: "client2_selfsigned", ExpectedError: ErrorMap["Invalid client cert"], }, { Name: `valid tls config yml and tls client with VerifyPeerCertificate (present SAN DNS entry that matches configured regex)`, - YAMLConfigPath: "testdata/web_config_auth_client_san_dns_regex.good.yaml", + YAMLConfigPath: "testdata/web_config_auth_client_san_regex.good.yaml", UseTLSClient: true, ClientCertificate: "client2_selfsigned", ExpectedError: nil, }, { Name: `valid tls config yml and tls client with VerifyPeerCertificate (present SAN DNS entry that does not match configured regex)`, - YAMLConfigPath: "testdata/web_config_auth_client_san_dns_regex.bad.yaml", + YAMLConfigPath: "testdata/web_config_auth_client_san_regex.bad.yaml", UseTLSClient: true, ClientCertificate: "client2_selfsigned", ExpectedError: ErrorMap["Invalid client cert"], From 0ed1d0e6f58eeb6712295e8e02be79baaff4db2d Mon Sep 17 00:00:00 2001 From: chodges15 Date: Wed, 1 Mar 2023 04:06:04 +0000 Subject: [PATCH 09/17] Add documentation Signed-off-by: chodges15 --- CHANGELOG.md | 4 ++++ VERSION | 2 +- docs/web-configuration.md | 5 +++++ web/tls_config.go | 2 -- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4340c49e..7cb5e927 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.8.2 / 2023-02-28 + +* [FEATURE] Add configuration for verifying the presence of an address in the Subject Alternate Name of the TLS cert provided by the client. #126 + ## 0.8.1 / 2022-10-21 * [BUGFIX] Fix systemd activation flag when using a custom kingpin app. #118 diff --git a/VERSION b/VERSION index 6f4eebdf..100435be 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.8.1 +0.8.2 diff --git a/docs/web-configuration.md b/docs/web-configuration.md index e461ffd8..f0ebb7c9 100644 --- a/docs/web-configuration.md +++ b/docs/web-configuration.md @@ -38,6 +38,11 @@ tls_server_config: # CA certificate for client certificate authentication to the server. [ client_ca_file: ] + # Verify that the client certificate has a Subject Alternate Name (SAN) which includes the following + # regex pattern, else terminate connection. SAN match can be one or multiple of the following: + # DNS, IP, e-mail, or URI address from https://pkg.go.dev/crypto/x509#Certificate. + [ client_allowed_san_regex: | default ""] + # Minimum TLS version that is acceptable. [ min_version: | default = "TLS12" ] diff --git a/web/tls_config.go b/web/tls_config.go index 4f9e2381..caa6eaf6 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -93,8 +93,6 @@ func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][] } } - //todo: check other fields of the cert - return fmt.Errorf("could not find configured SAN in client cert: %s", t.ClientAllowedSanRegex) } From 7b089c73fd776fb98555716bba9069762844d3a3 Mon Sep 17 00:00:00 2001 From: chodges15 Date: Wed, 1 Mar 2023 04:32:54 +0000 Subject: [PATCH 10/17] version the right way Signed-off-by: chodges15 --- CHANGELOG.md | 2 +- VERSION | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cb5e927..d1c78f36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 0.8.2 / 2023-02-28 +## master / unreleased * [FEATURE] Add configuration for verifying the presence of an address in the Subject Alternate Name of the TLS cert provided by the client. #126 diff --git a/VERSION b/VERSION index 100435be..6f4eebdf 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.8.2 +0.8.1 From 5f08414e223b9c2a087a9cddf02140e63fec4026 Mon Sep 17 00:00:00 2001 From: chodges15 Date: Tue, 28 Feb 2023 21:58:24 -0600 Subject: [PATCH 11/17] Add documentation Signed-off-by: chodges15 --- CHANGELOG.md | 2 -- VERSION | 2 +- docs/web-configuration.md | 5 +++++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1c78f36..f71b90c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,5 @@ ## master / unreleased -* [FEATURE] Add configuration for verifying the presence of an address in the Subject Alternate Name of the TLS cert provided by the client. #126 - ## 0.8.1 / 2022-10-21 * [BUGFIX] Fix systemd activation flag when using a custom kingpin app. #118 diff --git a/VERSION b/VERSION index 6f4eebdf..100435be 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.8.1 +0.8.2 diff --git a/docs/web-configuration.md b/docs/web-configuration.md index f0ebb7c9..97b60c5d 100644 --- a/docs/web-configuration.md +++ b/docs/web-configuration.md @@ -37,6 +37,11 @@ tls_server_config: # CA certificate for client certificate authentication to the server. [ client_ca_file: ] + + # Verify that the client certificate has a Subject Alternate Name (SAN) which includes the following + # regex pattern, else terminate connection. SAN match can be one or multiple of the following: + # DNS, IP, e-mail, or URI address from https://pkg.go.dev/crypto/x509#Certificate. + [ client_allowed_san_regex: | default ""] # Verify that the client certificate has a Subject Alternate Name (SAN) which includes the following # regex pattern, else terminate connection. SAN match can be one or multiple of the following: From 208cb5326edf9a183cd7347e4dfe0dcb405f9bff Mon Sep 17 00:00:00 2001 From: chodges15 Date: Wed, 8 Mar 2023 14:34:23 -0600 Subject: [PATCH 12/17] remove changelog Signed-off-by: chodges15 --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f71b90c3..4340c49e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,3 @@ -## master / unreleased - ## 0.8.1 / 2022-10-21 * [BUGFIX] Fix systemd activation flag when using a custom kingpin app. #118 From 3d91e48c349889693cf0663cf312868bda537b28 Mon Sep 17 00:00:00 2001 From: chodges15 Date: Wed, 8 Mar 2023 14:41:40 -0600 Subject: [PATCH 13/17] leave VERSION alone as well Signed-off-by: chodges15 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 100435be..6f4eebdf 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.8.2 +0.8.1 From b0e6f853443f48385ec3db250c04d2eab277dadc Mon Sep 17 00:00:00 2001 From: chodges15 Date: Thu, 23 Mar 2023 10:15:05 -0500 Subject: [PATCH 14/17] anchor configured regex Signed-off-by: chodges15 --- web/tls_config.go | 49 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/web/tls_config.go b/web/tls_config.go index caa6eaf6..a826b20b 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -52,7 +52,7 @@ type TLSConfig struct { MinVersion TLSVersion `yaml:"min_version"` MaxVersion TLSVersion `yaml:"max_version"` PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites"` - ClientAllowedSanRegex string `yaml:"client_allowed_san_regex"` + ClientAllowedSanRegex Regexp `yaml:"client_allowed_san_regex"` } type FlagConfig struct { @@ -69,7 +69,7 @@ func (t *TLSConfig) SetDirectory(dir string) { } // VerifyPeerCertificate will check the SAN entries of the client cert if there is configuration for it -func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { +func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, _ [][]*x509.Certificate) error { // sender cert comes first, see https://www.rfc-editor.org/rfc/rfc5246#section-7.4.2 cert, err := x509.ParseCertificate(rawCerts[0]) if err != nil { @@ -88,7 +88,7 @@ func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][] } for _, sanValue := range sanValues { - if matched, _ := regexp.MatchString(t.ClientAllowedSanRegex, sanValue); matched { + if t.ClientAllowedSanRegex.MatchString(sanValue) { return nil } } @@ -96,6 +96,47 @@ func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, verifiedChains [][] return fmt.Errorf("could not find configured SAN in client cert: %s", t.ClientAllowedSanRegex) } +// Regexp encapsulates a regexp.Regexp and makes it YAML marshalable. +type Regexp struct { + *regexp.Regexp +} + +// NewRegexp creates a new anchored Regexp and returns an error if the +// passed-in regular expression does not compile. +func NewRegexp(s string) (Regexp, error) { + regex, err := regexp.Compile("^(?:" + s + ")$") + return Regexp{Regexp: regex}, err +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface. +func (re *Regexp) UnmarshalYAML(unmarshal func(interface{}) error) error { + var s string + if err := unmarshal(&s); err != nil { + return err + } + r, err := NewRegexp(s) + if err != nil { + return err + } + *re = r + return nil +} + +// MarshalYAML implements the yaml.Marshaler interface. +func (re Regexp) MarshalYAML() (interface{}, error) { + if re.String() != "" { + return re.String(), nil + } + return nil, nil +} + +// String returns the original string used to compile the regular expression. +func (re Regexp) String() string { + str := re.Regexp.String() + // Trim the anchor `^(?:` prefix and `)$` suffix. + return str[4 : len(str)-2] +} + type HTTPConfig struct { HTTP2 bool `yaml:"http2"` Header map[string]string `yaml:"headers,omitempty"` @@ -193,7 +234,7 @@ func ConfigToTLSConfig(c *TLSConfig) (*tls.Config, error) { cfg.ClientCAs = clientCAPool } - if c.ClientAllowedSanRegex != "" { + if c.ClientAllowedSanRegex.Regexp != nil { // verify that the client cert contains the allowed domain name cfg.VerifyPeerCertificate = c.VerifyPeerCertificate } From b205f7a2300046ffd25993985e34754bd1958e48 Mon Sep 17 00:00:00 2001 From: chodges15 Date: Fri, 24 Mar 2023 09:42:52 -0500 Subject: [PATCH 15/17] update docs and remove duplicate entry Signed-off-by: chodges15 --- docs/web-configuration.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/docs/web-configuration.md b/docs/web-configuration.md index 97b60c5d..a0bd13a1 100644 --- a/docs/web-configuration.md +++ b/docs/web-configuration.md @@ -38,14 +38,10 @@ tls_server_config: # CA certificate for client certificate authentication to the server. [ client_ca_file: ] - # Verify that the client certificate has a Subject Alternate Name (SAN) which includes the following + # Verify that the client certificate has a Subject Alternate Name (SAN) which matches the following # regex pattern, else terminate connection. SAN match can be one or multiple of the following: - # DNS, IP, e-mail, or URI address from https://pkg.go.dev/crypto/x509#Certificate. - [ client_allowed_san_regex: | default ""] - - # Verify that the client certificate has a Subject Alternate Name (SAN) which includes the following - # regex pattern, else terminate connection. SAN match can be one or multiple of the following: - # DNS, IP, e-mail, or URI address from https://pkg.go.dev/crypto/x509#Certificate. + # DNS, IP, e-mail, or URI address from https://pkg.go.dev/crypto/x509#Certificate. Regex expression + # is anchored to avoid the confusing case of matching only a subset of the SAN instead of the full string. [ client_allowed_san_regex: | default ""] # Minimum TLS version that is acceptable. From 394edbb226dc52baab4680d7e2e1cedbe78ab502 Mon Sep 17 00:00:00 2001 From: chodges15 Date: Wed, 5 Apr 2023 21:29:09 -0500 Subject: [PATCH 16/17] regex -> list for SANs Signed-off-by: chodges15 --- docs/web-configuration.md | 13 ++-- .../web_config_auth_client_san.bad.yaml | 3 +- .../web_config_auth_client_san.good.yaml | 5 +- .../web_config_auth_client_san_regex.bad.yaml | 6 -- ...web_config_auth_client_san_regex.good.yaml | 6 -- web/tls_config.go | 65 ++++--------------- web/tls_config_test.go | 14 ---- 7 files changed, 25 insertions(+), 87 deletions(-) delete mode 100644 web/testdata/web_config_auth_client_san_regex.bad.yaml delete mode 100644 web/testdata/web_config_auth_client_san_regex.good.yaml diff --git a/docs/web-configuration.md b/docs/web-configuration.md index a0bd13a1..4042fd0a 100644 --- a/docs/web-configuration.md +++ b/docs/web-configuration.md @@ -38,12 +38,13 @@ tls_server_config: # CA certificate for client certificate authentication to the server. [ client_ca_file: ] - # Verify that the client certificate has a Subject Alternate Name (SAN) which matches the following - # regex pattern, else terminate connection. SAN match can be one or multiple of the following: - # DNS, IP, e-mail, or URI address from https://pkg.go.dev/crypto/x509#Certificate. Regex expression - # is anchored to avoid the confusing case of matching only a subset of the SAN instead of the full string. - [ client_allowed_san_regex: | default ""] - + # Verify that the client certificate has a Subject Alternate Name (SAN) + # which is an exact match to an entry in this list, else terminate the + # connection. SAN match can be one or multiple of the following: DNS, + # IP, e-mail, or URI address from https://pkg.go.dev/crypto/x509#Certificate. + [ client_allowed_sans: + [ - ] ] + # Minimum TLS version that is acceptable. [ min_version: | default = "TLS12" ] diff --git a/web/testdata/web_config_auth_client_san.bad.yaml b/web/testdata/web_config_auth_client_san.bad.yaml index 9cf0640c..3b0d9189 100644 --- a/web/testdata/web_config_auth_client_san.bad.yaml +++ b/web/testdata/web_config_auth_client_san.bad.yaml @@ -3,4 +3,5 @@ tls_server_config: key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" client_ca_file: "client2_selfsigned.pem" - client_allowed_san_regex: "bad" + client_allowed_sans: + - "bad" diff --git a/web/testdata/web_config_auth_client_san.good.yaml b/web/testdata/web_config_auth_client_san.good.yaml index ec05fb43..88eaad1e 100644 --- a/web/testdata/web_config_auth_client_san.good.yaml +++ b/web/testdata/web_config_auth_client_san.good.yaml @@ -3,4 +3,7 @@ tls_server_config: key_file: "server.key" client_auth_type: "RequireAndVerifyClientCert" client_ca_file: "client2_selfsigned.pem" - client_allowed_san_regex: "test3" + client_allowed_sans: + - "one" + - "test3" + - "two" diff --git a/web/testdata/web_config_auth_client_san_regex.bad.yaml b/web/testdata/web_config_auth_client_san_regex.bad.yaml deleted file mode 100644 index 7c6ec394..00000000 --- a/web/testdata/web_config_auth_client_san_regex.bad.yaml +++ /dev/null @@ -1,6 +0,0 @@ -tls_server_config: - cert_file: "server.crt" - key_file: "server.key" - client_auth_type: "RequireAndVerifyClientCert" - client_ca_file: "client2_selfsigned.pem" - client_allowed_san_regex: ".+test.+" diff --git a/web/testdata/web_config_auth_client_san_regex.good.yaml b/web/testdata/web_config_auth_client_san_regex.good.yaml deleted file mode 100644 index 22d32f60..00000000 --- a/web/testdata/web_config_auth_client_san_regex.good.yaml +++ /dev/null @@ -1,6 +0,0 @@ -tls_server_config: - cert_file: "server.crt" - key_file: "server.key" - client_auth_type: "RequireAndVerifyClientCert" - client_ca_file: "client2_selfsigned.pem" - client_allowed_san_regex: (test\d|dns) diff --git a/web/tls_config.go b/web/tls_config.go index a826b20b..56ca0368 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -18,18 +18,16 @@ import ( "crypto/x509" "errors" "fmt" - "net" - "net/http" - "os" - "path/filepath" - "regexp" - "github.com/coreos/go-systemd/v22/activation" "github.com/go-kit/log" "github.com/go-kit/log/level" config_util "github.com/prometheus/common/config" "golang.org/x/sync/errgroup" "gopkg.in/yaml.v2" + "net" + "net/http" + "os" + "path/filepath" ) var ( @@ -52,7 +50,7 @@ type TLSConfig struct { MinVersion TLSVersion `yaml:"min_version"` MaxVersion TLSVersion `yaml:"max_version"` PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites"` - ClientAllowedSanRegex Regexp `yaml:"client_allowed_san_regex"` + ClientAllowedSans []string `yaml:"client_allowed_sans"` } type FlagConfig struct { @@ -88,53 +86,14 @@ func (t *TLSConfig) VerifyPeerCertificate(rawCerts [][]byte, _ [][]*x509.Certifi } for _, sanValue := range sanValues { - if t.ClientAllowedSanRegex.MatchString(sanValue) { - return nil + for _, allowedSan := range t.ClientAllowedSans { + if sanValue == allowedSan { + return nil + } } } - return fmt.Errorf("could not find configured SAN in client cert: %s", t.ClientAllowedSanRegex) -} - -// Regexp encapsulates a regexp.Regexp and makes it YAML marshalable. -type Regexp struct { - *regexp.Regexp -} - -// NewRegexp creates a new anchored Regexp and returns an error if the -// passed-in regular expression does not compile. -func NewRegexp(s string) (Regexp, error) { - regex, err := regexp.Compile("^(?:" + s + ")$") - return Regexp{Regexp: regex}, err -} - -// UnmarshalYAML implements the yaml.Unmarshaler interface. -func (re *Regexp) UnmarshalYAML(unmarshal func(interface{}) error) error { - var s string - if err := unmarshal(&s); err != nil { - return err - } - r, err := NewRegexp(s) - if err != nil { - return err - } - *re = r - return nil -} - -// MarshalYAML implements the yaml.Marshaler interface. -func (re Regexp) MarshalYAML() (interface{}, error) { - if re.String() != "" { - return re.String(), nil - } - return nil, nil -} - -// String returns the original string used to compile the regular expression. -func (re Regexp) String() string { - str := re.Regexp.String() - // Trim the anchor `^(?:` prefix and `)$` suffix. - return str[4 : len(str)-2] + return fmt.Errorf("could not find allowed SANs in client cert, found: %v", t.ClientAllowedSans) } type HTTPConfig struct { @@ -234,8 +193,8 @@ func ConfigToTLSConfig(c *TLSConfig) (*tls.Config, error) { cfg.ClientCAs = clientCAPool } - if c.ClientAllowedSanRegex.Regexp != nil { - // verify that the client cert contains the allowed domain name + if c.ClientAllowedSans != nil { + // verify that the client cert contains an allowed SAN cfg.VerifyPeerCertificate = c.VerifyPeerCertificate } diff --git a/web/tls_config_test.go b/web/tls_config_test.go index 0237b860..b2479338 100644 --- a/web/tls_config_test.go +++ b/web/tls_config_test.go @@ -362,20 +362,6 @@ func TestServerBehaviour(t *testing.T) { ClientCertificate: "client2_selfsigned", ExpectedError: ErrorMap["Invalid client cert"], }, - { - Name: `valid tls config yml and tls client with VerifyPeerCertificate (present SAN DNS entry that matches configured regex)`, - YAMLConfigPath: "testdata/web_config_auth_client_san_regex.good.yaml", - UseTLSClient: true, - ClientCertificate: "client2_selfsigned", - ExpectedError: nil, - }, - { - Name: `valid tls config yml and tls client with VerifyPeerCertificate (present SAN DNS entry that does not match configured regex)`, - YAMLConfigPath: "testdata/web_config_auth_client_san_regex.bad.yaml", - UseTLSClient: true, - ClientCertificate: "client2_selfsigned", - ExpectedError: ErrorMap["Invalid client cert"], - }, } for _, testInputs := range testTables { t.Run(testInputs.Name, testInputs.Test) From 06d5a6abfad50a9d4348fffa2e5d0465e2e42ab6 Mon Sep 17 00:00:00 2001 From: chodges15 Date: Thu, 6 Apr 2023 07:21:36 -0500 Subject: [PATCH 17/17] fix linter error Signed-off-by: chodges15 --- web/tls_config.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/web/tls_config.go b/web/tls_config.go index 56ca0368..29d16371 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -18,16 +18,17 @@ import ( "crypto/x509" "errors" "fmt" + "net" + "net/http" + "os" + "path/filepath" + "github.com/coreos/go-systemd/v22/activation" "github.com/go-kit/log" "github.com/go-kit/log/level" config_util "github.com/prometheus/common/config" "golang.org/x/sync/errgroup" "gopkg.in/yaml.v2" - "net" - "net/http" - "os" - "path/filepath" ) var (