NE-2744: UPSTREAM: 8227: fix(tls): use Go TLS defaults

plugin/etcd/README.md

@@ -55,6 +55,10 @@ etcd [ZONES...] {
     * three arguments - path to cert PEM file, path to client private key PEM file, path to CA PEM
       file - if the server certificate is not signed by a system-installed CA and client certificate
       is needed.
+
+CoreDNS sets the minimum TLS version to TLS 1.2. The maximum TLS version, TLS 1.2 cipher suites, and
+key exchange mechanisms use the Go `crypto/tls` defaults.
+
 * `min-lease-ttl` the minimum TTL for DNS records based on etcd lease duration. Accepts flexible time formats like '30', '30s', '5m', '1h', '2h30m'. Default: 30 seconds.
 * `max-lease-ttl` the maximum TTL for DNS records based on etcd lease duration. Accepts flexible time formats like '30', '30s', '5m', '1h', '2h30m'. Default: 24 hours.
 

plugin/forward/README.md

@@ -77,6 +77,9 @@ forward FROM TO... {
   * `tls` **CERT** **KEY**  **CA** - client authentication is used with the specified cert/key pair.
     The server certificate is verified using the specified CA file
 
+CoreDNS sets the minimum TLS version to TLS 1.2. The maximum TLS version, TLS 1.2 cipher suites, and
+key exchange mechanisms use the Go `crypto/tls` defaults.
+
 * `tls_servername` **NAME** allows you to set a server name in the TLS configuration; for instance 9.9.9.9
   needs this to be set to `dns.quad9.net`. Multiple upstreams are still allowed in this scenario,
   but they have to use the same `tls_servername`. E.g. mixing 9.9.9.9 (QuadDNS) with 1.1.1.1

plugin/grpc/README.md

@@ -50,6 +50,9 @@ grpc FROM TO... {
   * `tls` **CERT** **KEY**  **CA** - client authentication is used with the specified cert/key pair.
     The server certificate is verified using the specified CA file
 
+CoreDNS sets the minimum TLS version to TLS 1.2. The maximum TLS version, TLS 1.2 cipher suites, and
+key exchange mechanisms use the Go `crypto/tls` defaults.
+
 * `tls_servername` **NAME** allows you to set a server name in the TLS configuration; for instance 9.9.9.9
   needs this to be set to `dns.quad9.net`. Multiple upstreams are still allowed in this scenario,
   but they have to use the same `tls_servername`. E.g. mixing 9.9.9.9 (QuadDNS) with 1.1.1.1
@@ -158,4 +161,4 @@ example.org {
 ## Bugs
 
 The TLS config is global for the whole grpc proxy if you need a different `tls_servername` for
-different upstreams you're out of luck.
+different upstreams you're out of luck.

plugin/pkg/tls/tls.go

@@ -13,18 +13,6 @@ import (
 
 func setTLSDefaults(ctls *tls.Config) {
 	ctls.MinVersion = tls.VersionTLS12
-	ctls.MaxVersion = tls.VersionTLS13
-	ctls.CipherSuites = []uint16{
-		tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
-		tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
-		tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
-		tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
-		tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-		tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
-		tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
-		tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
-		tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
-	}
 }
 
 // NewTLSConfigFromArgs returns a TLS config based upon the passed
@@ -95,7 +83,11 @@ func NewTLSConfig(certPath, keyPath, caPath string) (*tls.Config, error) {
 		return nil, err
 	}
 
-	tlsConfig := &tls.Config{Certificates: []tls.Certificate{cert}, RootCAs: roots}
+	// #nosec G402 -- MinVersion is set in setTLSDefaults
+	tlsConfig := &tls.Config{
+		Certificates: []tls.Certificate{cert},
+		RootCAs:      roots,
+	}
 	setTLSDefaults(tlsConfig)
 
 	return tlsConfig, nil
@@ -109,7 +101,10 @@ func NewTLSClientConfig(caPath string) (*tls.Config, error) {
 		return nil, err
 	}
 
-	tlsConfig := &tls.Config{RootCAs: roots}
+	// #nosec G402 -- MinVersion is set in setTLSDefaults
+	tlsConfig := &tls.Config{
+		RootCAs: roots,
+	}
 	setTLSDefaults(tlsConfig)
 
 	return tlsConfig, nil

plugin/pkg/tls/tls_test.go

@@ -1,6 +1,7 @@
 package tls
 
 import (
+	"crypto/tls"
 	"path/filepath"
 	"testing"
 
@@ -21,35 +22,55 @@ func getPEMFiles(t *testing.T) (cert, key, ca string) {
 	return
 }
 
+func assertTLSDefaults(t *testing.T, c *tls.Config) {
+	t.Helper()
+	if c.MinVersion != tls.VersionTLS12 {
+		t.Errorf("MinVersion = %d, want %d", c.MinVersion, tls.VersionTLS12)
+	}
+	if c.MaxVersion != 0 {
+		t.Errorf("MaxVersion = %d, want 0 to use Go defaults", c.MaxVersion)
+	}
+	if c.CipherSuites != nil {
+		t.Errorf("CipherSuites = %v, want nil to use Go defaults", c.CipherSuites)
+	}
+	if c.CurvePreferences != nil {
+		t.Errorf("CurvePreferences = %v, want nil to use Go defaults", c.CurvePreferences)
+	}
+}
+
 func TestNewTLSConfig(t *testing.T) {
 	cert, key, ca := getPEMFiles(t)
-	_, err := NewTLSConfig(cert, key, ca)
+	c, err := NewTLSConfig(cert, key, ca)
 	if err != nil {
 		t.Errorf("Failed to create TLSConfig: %s", err)
 	}
+	assertTLSDefaults(t, c)
 }
 
 func TestNewTLSClientConfig(t *testing.T) {
 	_, _, ca := getPEMFiles(t)
 
-	_, err := NewTLSClientConfig(ca)
+	c, err := NewTLSClientConfig(ca)
 	if err != nil {
 		t.Errorf("Failed to create TLSConfig: %s", err)
 	}
+	assertTLSDefaults(t, c)
 }
 
 func TestNewTLSConfigFromArgs(t *testing.T) {
 	cert, key, ca := getPEMFiles(t)
 
-	_, err := NewTLSConfigFromArgs()
+	c, err := NewTLSConfigFromArgs()
 	if err != nil {
 		t.Errorf("Failed to create TLSConfig: %s", err)
 	}
+	assertTLSDefaults(t, c)
 
-	c, err := NewTLSConfigFromArgs(ca)
+	c, err = NewTLSConfigFromArgs(ca)
 	if err != nil {
 		t.Errorf("Failed to create TLSConfig: %s", err)
 	}
+	assertTLSDefaults(t, c)
 	if c.RootCAs == nil {
 		t.Error("RootCAs should not be nil when one arg passed")
 	}
@@ -58,6 +79,7 @@ func TestNewTLSConfigFromArgs(t *testing.T) {
 	if err != nil {
 		t.Errorf("Failed to create TLSConfig: %s", err)
 	}
+	assertTLSDefaults(t, c)
 	if c.RootCAs != nil {
 		t.Error("RootCAs should be nil when two args passed")
 	}
@@ -69,6 +91,7 @@ func TestNewTLSConfigFromArgs(t *testing.T) {
 	if err != nil {
 		t.Errorf("Failed to create TLSConfig: %s", err)
 	}
+	assertTLSDefaults(t, c)
 	if c.RootCAs == nil {
 		t.Error("RootCAs should not be nil when three args passed")
 	}
@@ -92,6 +115,7 @@ func TestNewTLSConfigFromArgsWithRoot(t *testing.T) {
 	if err != nil {
 		t.Errorf("Failed to create TLSConfig: %s", err)
 	}
+	assertTLSDefaults(t, c)
 	if c.RootCAs == nil {
 		t.Error("RootCAs should not be nil when three args passed")
 	}

plugin/tls/README.md

@@ -35,6 +35,10 @@ The option value corresponds to the [ClientAuthType values of the Go tls package
 The default is "nocert".  Note that it makes no sense to specify parameter CA unless this option is
 set to verify\_if\_given or require\_and\_verify.
 
+CoreDNS sets the minimum TLS version to TLS 1.2. The maximum TLS version, TLS 1.2 cipher suites, and
+key exchange mechanisms use the Go `crypto/tls` defaults.
+
+
 ## Examples
 
 Start a DNS-over-TLS server that picks up incoming DNS-over-TLS queries on port 5553 and uses the