address comments; to be rebased after

Signed-off-by: David Luu <david@davidluu.info>
distribution · Feb 25, 2021 · 990b318 · 990b318
1 parent 44553eb
commit 990b318
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 38 deletions.
diff --git a/registry/registry.go b/registry/registry.go
@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"os"
 	"os/signal"
+	"strings"
 	"syscall"
 	"time"
 
@@ -67,6 +68,29 @@ var cipherSuites = map[string]uint16{
 	"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305": tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
 }
 
+// a list of default ciphersuites to utilize
+var defaultCipherSuites = []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_AES_128_GCM_SHA256,
+	tls.TLS_CHACHA20_POLY1305_SHA256,
+	tls.TLS_AES_256_GCM_SHA384,
+}
+
+// maps tls version strings to constants
+var defaultTLSVersionStr = "tls1.2"
+var tlsVersions = map[string]uint16{
+	// user specified values
+	"tls1.0": tls.VersionTLS10,
+	"tls1.1": tls.VersionTLS11,
+	"tls1.2": tls.VersionTLS12,
+	"tls1.3": tls.VersionTLS13,
+}
+
 // this channel gets notified when process receives signal. It is global to ease unit testing
 var quit = make(chan os.Signal, 1)
 
@@ -162,9 +186,10 @@ func NewRegistry(ctx context.Context, config *configuration.Configuration) (*Reg
 }
 
 // takes a list of cipher suites and converts it to a list of respective tls constants
+// if an empty list is provided, then the defaults will be used
 func getCipherSuites(names []string) ([]uint16, error) {
 	if len(names) == 0 {
-		return nil, nil
+		return defaultCipherSuites, nil
 	}
 	cipherSuiteConsts := make([]uint16, 0)
 	for _, name := range names {
@@ -177,6 +202,18 @@ func getCipherSuites(names []string) ([]uint16, error) {
 	return cipherSuiteConsts, nil
 }
 
+// takes a list of cipher suite ids and converts it to a list of respective names
+func getCipherSuiteNames(ids []uint16) []string {
+	if len(ids) == 0 {
+		return nil
+	}
+	var names []string
+	for _, id := range ids {
+		names = append(names, tls.CipherSuiteName(id))
+	}
+	return names
+}
+
 // ListenAndServe runs the registry's HTTP server.
 func (registry *Registry) ListenAndServe() error {
 	config := registry.config
@@ -187,43 +224,21 @@ func (registry *Registry) ListenAndServe() error {
 	}
 
 	if config.HTTP.TLS.Certificate != "" || config.HTTP.TLS.LetsEncrypt.CacheFile != "" {
-		var tlsMinVersion uint16
 		if config.HTTP.TLS.MinimumTLS == "" {
-			tlsMinVersion = tls.VersionTLS12
-		} else {
-			switch config.HTTP.TLS.MinimumTLS {
-			case "tls1.0":
-				tlsMinVersion = tls.VersionTLS10
-			case "tls1.1":
-				tlsMinVersion = tls.VersionTLS11
-			case "tls1.2":
-				tlsMinVersion = tls.VersionTLS12
-			case "tls1.3":
-				tlsMinVersion = tls.VersionTLS13
-			default:
-				return fmt.Errorf("unknown minimum TLS level '%s' specified for http.tls.minimumtls", config.HTTP.TLS.MinimumTLS)
-			}
-			dcontext.GetLogger(registry.app).Infof("restricting TLS to %s or higher", config.HTTP.TLS.MinimumTLS)
+			config.HTTP.TLS.MinimumTLS = defaultTLSVersionStr
 		}
-		var tlsCipherSuites []uint16
-		if len(config.HTTP.TLS.CipherSuites) == 0 {
-			tlsCipherSuites = []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_AES_128_GCM_SHA256,
-				tls.TLS_CHACHA20_POLY1305_SHA256,
-				tls.TLS_AES_256_GCM_SHA384,
-			}
-		} else {
-			tlsCipherSuites, err = getCipherSuites(config.HTTP.TLS.CipherSuites)
-			if err != nil {
-				return err
-			}
+		tlsMinVersion, ok := tlsVersions[config.HTTP.TLS.MinimumTLS]
+		if !ok {
+			return fmt.Errorf("unknown minimum TLS level '%s' specified for http.tls.minimumtls", config.HTTP.TLS.MinimumTLS)
 		}
+		dcontext.GetLogger(registry.app).Infof("restricting TLS version to %s or higher", config.HTTP.TLS.MinimumTLS)
+
+		tlsCipherSuites, err := getCipherSuites(config.HTTP.TLS.CipherSuites)
+		if err != nil {
+			return err
+		}
+		dcontext.GetLogger(registry.app).Infof("restricting TLS cipher suites to: %s", strings.Join(getCipherSuiteNames(tlsCipherSuites), ","))
+
 		tlsConf := &tls.Config{
 			ClientAuth:               tls.NoClientCert,
 			NextProtos:               nextProtos(config),

diff --git a/registry/registry_test.go b/registry/registry_test.go
@@ -20,6 +20,7 @@ import (
 	"os"
 	"path"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -125,18 +126,24 @@ func TestGracefulShutdown(t *testing.T) {
 func TestGetCipherSuite(t *testing.T) {
 	resp, err := getCipherSuites([]string{"TLS_RSA_WITH_AES_128_CBC_SHA"})
 	if err != nil || len(resp) != 1 || resp[0] != tls.TLS_RSA_WITH_AES_128_CBC_SHA {
-		t.Error("did not return expected cipher suite constant")
+		t.Errorf("expected cipher suite %q, got %q",
+			"TLS_RSA_WITH_AES_128_CBC_SHA",
+			strings.Join(getCipherSuiteNames(resp), ","),
+		)
 	}
 
 	resp, err = getCipherSuites([]string{"TLS_RSA_WITH_AES_128_CBC_SHA", "TLS_AES_128_GCM_SHA256"})
 	if err != nil || len(resp) != 2 ||
 		resp[0] != tls.TLS_RSA_WITH_AES_128_CBC_SHA || resp[1] != tls.TLS_AES_128_GCM_SHA256 {
-		t.Error("did not return expected cipher suite constant")
+		t.Errorf("expected cipher suites %q, got %q",
+			"TLS_RSA_WITH_AES_128_CBC_SHA,TLS_AES_128_GCM_SHA256",
+			strings.Join(getCipherSuiteNames(resp), ","),
+		)
 	}
 
 	_, err = getCipherSuites([]string{"TLS_RSA_WITH_AES_128_CBC_SHA", "bad_input"})
 	if err == nil {
-		t.Error("did not return expected error")
+		t.Error("did not return expected error about unknown cipher suite")
 	}
 }