From ce66fdfb2fc6a5e65df42ea12504eed175c91e65 Mon Sep 17 00:00:00 2001 From: Gerardo Di Giacomo Date: Wed, 5 Feb 2020 16:58:27 -0800 Subject: [PATCH 1/4] adding support for TLS 1.3 for TCP listeners --- .../cassandra/path_config_connection.go | 2 +- command/server/listener_tcp_test.go | 84 +++++++++++++++++++ helper/listenerutil/listener.go | 2 +- physical/cassandra/cassandra.go | 4 +- sdk/helper/ldaputil/config.go | 8 +- sdk/helper/tlsutil/tlsutil.go | 1 + .../pages/api-docs/auth/kerberos/index.mdx | 4 +- website/pages/api-docs/auth/ldap/index.mdx | 4 +- .../pages/docs/configuration/listener/tcp.mdx | 2 +- .../service-registration/consul.mdx | 2 +- .../docs/configuration/storage/cassandra.mdx | 2 +- .../docs/configuration/storage/consul.mdx | 2 +- .../docs/configuration/storage/zookeeper.mdx | 2 +- 13 files changed, 103 insertions(+), 16 deletions(-) diff --git a/builtin/logical/cassandra/path_config_connection.go b/builtin/logical/cassandra/path_config_connection.go index 86d29d13f19d..a3852fad2c61 100644 --- a/builtin/logical/cassandra/path_config_connection.go +++ b/builtin/logical/cassandra/path_config_connection.go @@ -44,7 +44,7 @@ effect if a CA certificate is provided`, "tls_min_version": &framework.FieldSchema{ Type: framework.TypeString, Default: "tls12", - Description: "Minimum TLS version to use. Accepted values are 'tls10', 'tls11' or 'tls12'. Defaults to 'tls12'", + Description: "Minimum TLS version to use. Accepted values are 'tls10', 'tls11', 'tls12' or 'tls13'. Defaults to 'tls12'", }, "pem_bundle": &framework.FieldSchema{ diff --git a/command/server/listener_tcp_test.go b/command/server/listener_tcp_test.go index 89045da29538..44ef0770e224 100644 --- a/command/server/listener_tcp_test.go +++ b/command/server/listener_tcp_test.go @@ -113,3 +113,87 @@ func TestTCPListener_tls(t *testing.T) { testListenerImpl(t, ln, connFn(false), "foo.example.com") } + +func TestTCPListener_tls13(t *testing.T) { + wd, _ := os.Getwd() + wd += "/test-fixtures/reload/" + + td, err := ioutil.TempDir("", fmt.Sprintf("vault-test-%d", rand.New(rand.NewSource(time.Now().Unix())).Int63())) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(td) + + // Setup initial certs + inBytes, _ := ioutil.ReadFile(wd + "reload_ca.pem") + certPool := x509.NewCertPool() + ok := certPool.AppendCertsFromPEM(inBytes) + if !ok { + t.Fatal("not ok when appending CA cert") + } + + ln, _, _, err := tcpListenerFactory(map[string]interface{}{ + "address": "127.0.0.1:0", + "tls_cert_file": wd + "reload_foo.pem", + "tls_key_file": wd + "reload_foo.key", + "tls_require_and_verify_client_cert": "true", + "tls_client_ca_file": wd + "reload_ca.pem", + "tls_min_version": "tls13", + }, nil, cli.NewMockUi()) + if err != nil { + t.Fatalf("err: %s", err) + } + cwd, _ := os.Getwd() + + clientCert, _ := tls.LoadX509KeyPair( + cwd+"/test-fixtures/reload/reload_foo.pem", + cwd+"/test-fixtures/reload/reload_foo.key") + + connFn := func(clientCerts bool) func(net.Listener) (net.Conn, error) { + return func(lnReal net.Listener) (net.Conn, error) { + conf := &tls.Config{ + RootCAs: certPool, + } + if clientCerts { + conf.Certificates = []tls.Certificate{clientCert} + } + conn, err := tls.Dial("tcp", ln.Addr().String(), conf) + + if err != nil { + return nil, err + } + if err = conn.Handshake(); err != nil { + return nil, err + } + return conn, nil + } + } + + testListenerImpl(t, ln, connFn(true), "foo.example.com") + + ln, _, _, err = tcpListenerFactory(map[string]interface{}{ + "address": "127.0.0.1:0", + "tls_cert_file": wd + "reload_foo.pem", + "tls_key_file": wd + "reload_foo.key", + "tls_require_and_verify_client_cert": "true", + "tls_disable_client_certs": "true", + "tls_client_ca_file": wd + "reload_ca.pem", + "tls_min_version": "tls13", + }, nil, cli.NewMockUi()) + if err == nil { + t.Fatal("expected error due to mutually exclusive client cert options") + } + + ln, _, _, err = tcpListenerFactory(map[string]interface{}{ + "address": "127.0.0.1:0", + "tls_cert_file": wd + "reload_foo.pem", + "tls_key_file": wd + "reload_foo.key", + "tls_disable_client_certs": "true", + "tls_client_ca_file": wd + "reload_ca.pem", + }, nil, cli.NewMockUi()) + if err != nil { + t.Fatalf("err: %s", err) + } + + testListenerImpl(t, ln, connFn(false), "foo.example.com") +} diff --git a/helper/listenerutil/listener.go b/helper/listenerutil/listener.go index 8f17b723ad0b..4bf76ebb25f6 100644 --- a/helper/listenerutil/listener.go +++ b/helper/listenerutil/listener.go @@ -126,7 +126,7 @@ PASSPHRASECORRECT: tlsConf.NextProtos = []string{"h2", "http/1.1"} tlsConf.MinVersion, ok = tlsutil.TLSLookup[tlsvers] if !ok { - return nil, nil, nil, nil, fmt.Errorf("'tls_min_version' value %q not supported, please specify one of [tls10,tls11,tls12]", tlsvers) + return nil, nil, nil, nil, fmt.Errorf("'tls_min_version' value %q not supported, please specify one of [tls10,tls11,tls12,tls13]", tlsvers) } tlsConf.ClientAuth = tls.RequestClientCert diff --git a/physical/cassandra/cassandra.go b/physical/cassandra/cassandra.go index e1ca49ad75d4..6530bd204137 100644 --- a/physical/cassandra/cassandra.go +++ b/physical/cassandra/cassandra.go @@ -217,8 +217,10 @@ func setupCassandraTLS(conf map[string]string, cluster *gocql.ClusterConfig) err tlsConfig.MinVersion = tls.VersionTLS11 case "tls12": tlsConfig.MinVersion = tls.VersionTLS12 + case "tls13": + tlsConfig.MinVersion = tls.VersionTLS13 default: - return fmt.Errorf("'tls_min_version' must be one of `tls10`, `tls11` or `tls12`") + return fmt.Errorf("'tls_min_version' must be one of `tls10`, `tls11`, `tls12` or `tls13`") } } diff --git a/sdk/helper/ldaputil/config.go b/sdk/helper/ldaputil/config.go index 1ecceffc2775..47f3c042e535 100644 --- a/sdk/helper/ldaputil/config.go +++ b/sdk/helper/ldaputil/config.go @@ -134,21 +134,21 @@ Default: cn`, "tls_min_version": { Type: framework.TypeString, Default: "tls12", - Description: "Minimum TLS version to use. Accepted values are 'tls10', 'tls11' or 'tls12'. Defaults to 'tls12'", + Description: "Minimum TLS version to use. Accepted values are 'tls10', 'tls11', 'tls12' or 'tls13'. Defaults to 'tls12'", DisplayAttrs: &framework.DisplayAttributes{ Name: "Minimum TLS Version", }, - AllowedValues: []interface{}{"tls10", "tls11", "tls12"}, + AllowedValues: []interface{}{"tls10", "tls11", "tls12", "tls13"}, }, "tls_max_version": { Type: framework.TypeString, Default: "tls12", - Description: "Maximum TLS version to use. Accepted values are 'tls10', 'tls11' or 'tls12'. Defaults to 'tls12'", + Description: "Maximum TLS version to use. Accepted values are 'tls10', 'tls11', 'tls12' or 'tls13'. Defaults to 'tls12'", DisplayAttrs: &framework.DisplayAttributes{ Name: "Maximum TLS Version", }, - AllowedValues: []interface{}{"tls10", "tls11", "tls12"}, + AllowedValues: []interface{}{"tls10", "tls11", "tls12", "tls13"}, }, "deny_null_bind": { diff --git a/sdk/helper/tlsutil/tlsutil.go b/sdk/helper/tlsutil/tlsutil.go index a17e10cda8b1..8adc0040a154 100644 --- a/sdk/helper/tlsutil/tlsutil.go +++ b/sdk/helper/tlsutil/tlsutil.go @@ -21,6 +21,7 @@ var TLSLookup = map[string]uint16{ "tls10": tls.VersionTLS10, "tls11": tls.VersionTLS11, "tls12": tls.VersionTLS12, + "tls13": tls.VersionTLS13, } // cipherMap maps the cipher suite names to the internal cipher suite code. diff --git a/website/pages/api-docs/auth/kerberos/index.mdx b/website/pages/api-docs/auth/kerberos/index.mdx index 75221312bf74..aa157bd22e59 100644 --- a/website/pages/api-docs/auth/kerberos/index.mdx +++ b/website/pages/api-docs/auth/kerberos/index.mdx @@ -109,9 +109,9 @@ This endpoint configures LDAP in the Kerberos auth method. - `starttls` `(bool: false)` – If true, issues a `StartTLS` command after establishing an unencrypted connection. - `tls_min_version` `(string: tls12)` – Minimum TLS version to use. Accepted - values are `tls10`, `tls11` or `tls12`. + values are `tls10`, `tls11`, `tls12` or `tls13`. - `tls_max_version` `(string: tls12)` – Maximum TLS version to use. Accepted - values are `tls10`, `tls11` or `tls12`. + values are `tls10`, `tls11`, `tls12` or `tls13`. - `insecure_tls` `(bool: false)` – If true, skips LDAP server SSL certificate verification - insecure, use with caution! - `certificate` `(string: "")` – CA certificate to use when verifying LDAP server diff --git a/website/pages/api-docs/auth/ldap/index.mdx b/website/pages/api-docs/auth/ldap/index.mdx index ec69a5b1ed6e..71c44834b4cf 100644 --- a/website/pages/api-docs/auth/ldap/index.mdx +++ b/website/pages/api-docs/auth/ldap/index.mdx @@ -40,9 +40,9 @@ This endpoint configures the LDAP auth method. - `starttls` `(bool: false)` – If true, issues a `StartTLS` command after establishing an unencrypted connection. - `tls_min_version` `(string: tls12)` – Minimum TLS version to use. Accepted - values are `tls10`, `tls11` or `tls12`. + values are `tls10`, `tls11`, `tls12` or `tls13`. - `tls_max_version` `(string: tls12)` – Maximum TLS version to use. Accepted - values are `tls10`, `tls11` or `tls12`. + values are `tls10`, `tls11`, `tls12` or `tls13`. - `insecure_tls` `(bool: false)` – If true, skips LDAP server SSL certificate verification - insecure, use with caution! - `certificate` `(string: "")` – CA certificate to use when verifying LDAP server diff --git a/website/pages/docs/configuration/listener/tcp.mdx b/website/pages/docs/configuration/listener/tcp.mdx index 86af4e4fec0d..cb29ce802745 100644 --- a/website/pages/docs/configuration/listener/tcp.mdx +++ b/website/pages/docs/configuration/listener/tcp.mdx @@ -99,7 +99,7 @@ advertise the correct address to other nodes. while Vault is running will have no effect for `SIGHUP`s. - `tls_min_version` `(string: "tls12")` – Specifies the minimum supported - version of TLS. Accepted values are "tls10", "tls11" or "tls12". + version of TLS. Accepted values are "tls10", "tls11", "tls12" or "tls13". ~> **Warning**: TLS 1.1 and lower are generally considered insecure. diff --git a/website/pages/docs/configuration/service-registration/consul.mdx b/website/pages/docs/configuration/service-registration/consul.mdx index e397cb68a96a..31d7a0476c42 100644 --- a/website/pages/docs/configuration/service-registration/consul.mdx +++ b/website/pages/docs/configuration/service-registration/consul.mdx @@ -103,7 +103,7 @@ connection. You can read more about encrypting Consul connections on the in Consul. - `tls_min_version` `(string: "tls12")` – Specifies the minimum TLS version to - use. Accepted values are `"tls10"`, `"tls11"` or `"tls12"`. + use. Accepted values are `"tls10"`, `"tls11"`, `"tls12"` or `"tls13"`. - `tls_skip_verify` `(string: "false")` – Disable verification of TLS certificates. Using this option is highly discouraged. diff --git a/website/pages/docs/configuration/storage/cassandra.mdx b/website/pages/docs/configuration/storage/cassandra.mdx index f009948a0870..e2c91083951a 100644 --- a/website/pages/docs/configuration/storage/cassandra.mdx +++ b/website/pages/docs/configuration/storage/cassandra.mdx @@ -88,7 +88,7 @@ CREATE TABLE "vault"."entries" ( will be disabled for Cassandra. Defaults to `0`. - `tls_min_version` `(string: "tls12")` - Minimum TLS version to use. Accepted - values are `tls10`, `tls11` or `tls12`. Defaults to `tls12`. + values are `tls10`, `tls11`, `tls12` or `tls13`. Defaults to `tls12`. [cassandra]: http://cassandra.apache.org/ [replication-options]: https://docs.datastax.com/en/cassandra/2.1/cassandra/architecture/architectureDataDistributeReplication_c.html diff --git a/website/pages/docs/configuration/storage/consul.mdx b/website/pages/docs/configuration/storage/consul.mdx index 5c77f99a15cb..df275a7f316e 100644 --- a/website/pages/docs/configuration/storage/consul.mdx +++ b/website/pages/docs/configuration/storage/consul.mdx @@ -133,7 +133,7 @@ connection. You can read more about encrypting Consul connections on the in Consul. - `tls_min_version` `(string: "tls12")` – Specifies the minimum TLS version to - use. Accepted values are `"tls10"`, `"tls11"` or `"tls12"`. + use. Accepted values are `"tls10"`, `"tls11"`, `"tls12"` or `"tls13"`. - `tls_skip_verify` `(string: "false")` – Disable verification of TLS certificates. Using this option is highly discouraged. diff --git a/website/pages/docs/configuration/storage/zookeeper.mdx b/website/pages/docs/configuration/storage/zookeeper.mdx index 21624dfb83aa..3be8260a20b0 100644 --- a/website/pages/docs/configuration/storage/zookeeper.mdx +++ b/website/pages/docs/configuration/storage/zookeeper.mdx @@ -78,7 +78,7 @@ znodes and, potentially, take Vault out of service. Zookeeper communication. - `tls_min_version` `(string: "tls12")` – Specifies the minimum TLS version to - use. Accepted values are `"tls10"`, `"tls11"` or `"tls12"`. + use. Accepted values are `"tls10"`, `"tls11"`, `"tls12"` or `"tls13"`. - `tls_skip_verify` `(bool: false)` – Disable verification of TLS certificates. Using this option is highly discouraged. From df62488f17c4315e8cb987fd1ab5219c3c519988 Mon Sep 17 00:00:00 2001 From: gedigi <19227040+gedigi@users.noreply.github.com> Date: Wed, 5 Feb 2020 22:18:12 -0800 Subject: [PATCH 2/4] removed test as CI uses go 1.12 --- command/server/listener_tcp_test.go | 84 ----------------------------- 1 file changed, 84 deletions(-) diff --git a/command/server/listener_tcp_test.go b/command/server/listener_tcp_test.go index 44ef0770e224..89045da29538 100644 --- a/command/server/listener_tcp_test.go +++ b/command/server/listener_tcp_test.go @@ -113,87 +113,3 @@ func TestTCPListener_tls(t *testing.T) { testListenerImpl(t, ln, connFn(false), "foo.example.com") } - -func TestTCPListener_tls13(t *testing.T) { - wd, _ := os.Getwd() - wd += "/test-fixtures/reload/" - - td, err := ioutil.TempDir("", fmt.Sprintf("vault-test-%d", rand.New(rand.NewSource(time.Now().Unix())).Int63())) - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(td) - - // Setup initial certs - inBytes, _ := ioutil.ReadFile(wd + "reload_ca.pem") - certPool := x509.NewCertPool() - ok := certPool.AppendCertsFromPEM(inBytes) - if !ok { - t.Fatal("not ok when appending CA cert") - } - - ln, _, _, err := tcpListenerFactory(map[string]interface{}{ - "address": "127.0.0.1:0", - "tls_cert_file": wd + "reload_foo.pem", - "tls_key_file": wd + "reload_foo.key", - "tls_require_and_verify_client_cert": "true", - "tls_client_ca_file": wd + "reload_ca.pem", - "tls_min_version": "tls13", - }, nil, cli.NewMockUi()) - if err != nil { - t.Fatalf("err: %s", err) - } - cwd, _ := os.Getwd() - - clientCert, _ := tls.LoadX509KeyPair( - cwd+"/test-fixtures/reload/reload_foo.pem", - cwd+"/test-fixtures/reload/reload_foo.key") - - connFn := func(clientCerts bool) func(net.Listener) (net.Conn, error) { - return func(lnReal net.Listener) (net.Conn, error) { - conf := &tls.Config{ - RootCAs: certPool, - } - if clientCerts { - conf.Certificates = []tls.Certificate{clientCert} - } - conn, err := tls.Dial("tcp", ln.Addr().String(), conf) - - if err != nil { - return nil, err - } - if err = conn.Handshake(); err != nil { - return nil, err - } - return conn, nil - } - } - - testListenerImpl(t, ln, connFn(true), "foo.example.com") - - ln, _, _, err = tcpListenerFactory(map[string]interface{}{ - "address": "127.0.0.1:0", - "tls_cert_file": wd + "reload_foo.pem", - "tls_key_file": wd + "reload_foo.key", - "tls_require_and_verify_client_cert": "true", - "tls_disable_client_certs": "true", - "tls_client_ca_file": wd + "reload_ca.pem", - "tls_min_version": "tls13", - }, nil, cli.NewMockUi()) - if err == nil { - t.Fatal("expected error due to mutually exclusive client cert options") - } - - ln, _, _, err = tcpListenerFactory(map[string]interface{}{ - "address": "127.0.0.1:0", - "tls_cert_file": wd + "reload_foo.pem", - "tls_key_file": wd + "reload_foo.key", - "tls_disable_client_certs": "true", - "tls_client_ca_file": wd + "reload_ca.pem", - }, nil, cli.NewMockUi()) - if err != nil { - t.Fatalf("err: %s", err) - } - - testListenerImpl(t, ln, connFn(false), "foo.example.com") -} From d1dc2f70b9d2e224e5e7e7a6db71d4b41b5fcd38 Mon Sep 17 00:00:00 2001 From: Gerardo Di Giacomo Date: Thu, 13 Feb 2020 09:25:43 -0800 Subject: [PATCH 3/4] removed Cassandra support, added deprecation notice --- builtin/logical/cassandra/path_config_connection.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/logical/cassandra/path_config_connection.go b/builtin/logical/cassandra/path_config_connection.go index a3852fad2c61..f3e03a5b3ae7 100644 --- a/builtin/logical/cassandra/path_config_connection.go +++ b/builtin/logical/cassandra/path_config_connection.go @@ -41,10 +41,11 @@ set, this is automatically set to true`, effect if a CA certificate is provided`, }, + // TLS 1.3 is not supported as this engine is deprecated. Please switch to the Cassandra database secrets engine "tls_min_version": &framework.FieldSchema{ Type: framework.TypeString, Default: "tls12", - Description: "Minimum TLS version to use. Accepted values are 'tls10', 'tls11', 'tls12' or 'tls13'. Defaults to 'tls12'", + Description: "Minimum TLS version to use. Accepted values are 'tls10', 'tls11' or 'tls12'. Defaults to 'tls12'", }, "pem_bundle": &framework.FieldSchema{ From f1da4c6486d9caae3f5b2128c0abb44415bcd9df Mon Sep 17 00:00:00 2001 From: gedigi <19227040+gedigi@users.noreply.github.com> Date: Sat, 15 Feb 2020 10:47:23 -0800 Subject: [PATCH 4/4] re-added TestTCPListener_tls13 --- command/server/listener_tcp_test.go | 85 +++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/command/server/listener_tcp_test.go b/command/server/listener_tcp_test.go index 89045da29538..36c50a3e20c0 100644 --- a/command/server/listener_tcp_test.go +++ b/command/server/listener_tcp_test.go @@ -113,3 +113,88 @@ func TestTCPListener_tls(t *testing.T) { testListenerImpl(t, ln, connFn(false), "foo.example.com") } + +func TestTCPListener_tls13(t *testing.T) { + wd, _ := os.Getwd() + wd += "/test-fixtures/reload/" + + td, err := ioutil.TempDir("", fmt.Sprintf("vault-test-%d", rand.New(rand.NewSource(time.Now().Unix())).Int63())) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(td) + + // Setup initial certs + inBytes, _ := ioutil.ReadFile(wd + "reload_ca.pem") + certPool := x509.NewCertPool() + ok := certPool.AppendCertsFromPEM(inBytes) + if !ok { + t.Fatal("not ok when appending CA cert") + } + + ln, _, _, err := tcpListenerFactory(map[string]interface{}{ + "address": "127.0.0.1:0", + "tls_cert_file": wd + "reload_foo.pem", + "tls_key_file": wd + "reload_foo.key", + "tls_require_and_verify_client_cert": "true", + "tls_client_ca_file": wd + "reload_ca.pem", + "tls_min_version": "tls13", + }, nil, cli.NewMockUi()) + if err != nil { + t.Fatalf("err: %s", err) + } + cwd, _ := os.Getwd() + + clientCert, _ := tls.LoadX509KeyPair( + cwd+"/test-fixtures/reload/reload_foo.pem", + cwd+"/test-fixtures/reload/reload_foo.key") + + connFn := func(clientCerts bool) func(net.Listener) (net.Conn, error) { + return func(lnReal net.Listener) (net.Conn, error) { + conf := &tls.Config{ + RootCAs: certPool, + } + if clientCerts { + conf.Certificates = []tls.Certificate{clientCert} + } + conn, err := tls.Dial("tcp", ln.Addr().String(), conf) + + if err != nil { + return nil, err + } + if err = conn.Handshake(); err != nil { + return nil, err + } + return conn, nil + } + } + + testListenerImpl(t, ln, connFn(true), "foo.example.com") + + ln, _, _, err = tcpListenerFactory(map[string]interface{}{ + "address": "127.0.0.1:0", + "tls_cert_file": wd + "reload_foo.pem", + "tls_key_file": wd + "reload_foo.key", + "tls_require_and_verify_client_cert": "true", + "tls_disable_client_certs": "true", + "tls_client_ca_file": wd + "reload_ca.pem", + "tls_min_version": "tls13", + }, nil, cli.NewMockUi()) + if err == nil { + t.Fatal("expected error due to mutually exclusive client cert options") + } + + ln, _, _, err = tcpListenerFactory(map[string]interface{}{ + "address": "127.0.0.1:0", + "tls_cert_file": wd + "reload_foo.pem", + "tls_key_file": wd + "reload_foo.key", + "tls_disable_client_certs": "true", + "tls_client_ca_file": wd + "reload_ca.pem", + "tls_min_version": "tls13", + }, nil, cli.NewMockUi()) + if err != nil { + t.Fatalf("err: %s", err) + } + + testListenerImpl(t, ln, connFn(false), "foo.example.com") +}