caddytls: validate curve and protocol names, compare protocol IDs (#8266)

* caddytls: validate curve and protocol names, compare protocol IDs
* caddytls: Add final newline to tests and drop duplicate protocol comment

(cherry picked from commit 71c5812cfe)

modules/caddyhttp/reverseproxy/httptransport.go

@@ -896,7 +896,10 @@ func (t *TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error)
 
 	curvesAdded := make(map[tls.CurveID]struct{})
 	for _, curveName := range t.Curves {
-		curveID := caddytls.SupportedCurves[curveName]
+		curveID, ok := caddytls.SupportedCurves[curveName]
+		if !ok {
+			return nil, fmt.Errorf("unsupported curve: %s", curveName)
+		}
 		if _, ok := curvesAdded[curveID]; !ok {
 			curvesAdded[curveID] = struct{}{}
 			cfg.CurvePreferences = append(cfg.CurvePreferences, curveID)

modules/caddyhttp/reverseproxy/httptransport_test.go

@@ -406,3 +406,13 @@ func TestHTTPTransportH3UsesConfiguredTrustPool(t *testing.T) {
 		t.Fatal("HTTP/3 transport: TLS client config must be a clone, not the same pointer as the HTTP/1.1+2 transport's")
 	}
 }
+
+func TestTLSConfigRejectsUnknownCurve(t *testing.T) {
+	ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()})
+	defer cancel()
+
+	tc := &TLSConfig{Curves: []string{"X25519"}}
+	if _, err := tc.MakeTLSClientConfig(ctx); err == nil {
+		t.Fatal("expected an error for an unknown curve, got nil")
+	}
+}

modules/caddytls/connpolicy.go

@@ -363,7 +363,10 @@ func (p *ConnectionPolicy) buildStandardTLSConfig(ctx caddy.Context) error {
 	// add all the curve preferences in order, without duplicates
 	curvesAdded := make(map[tls.CurveID]struct{})
 	for _, curveName := range p.Curves {
-		curveID := SupportedCurves[curveName]
+		curveID, ok := SupportedCurves[curveName]
+		if !ok {
+			return fmt.Errorf("unsupported curve: %s", curveName)
+		}
 		if _, ok := curvesAdded[curveID]; !ok {
 			curvesAdded[curveID] = struct{}{}
 			cfg.CurvePreferences = append(cfg.CurvePreferences, curveID)
@@ -377,14 +380,23 @@ func (p *ConnectionPolicy) buildStandardTLSConfig(ctx caddy.Context) error {
 	}
 
 	// min and max protocol versions
-	if (p.ProtocolMin != "" && p.ProtocolMax != "") && p.ProtocolMin > p.ProtocolMax {
-		return fmt.Errorf("protocol min (%x) cannot be greater than protocol max (%x)", p.ProtocolMin, p.ProtocolMax)
-	}
 	if p.ProtocolMin != "" {
-		cfg.MinVersion = SupportedProtocols[p.ProtocolMin]
+		minVersion, ok := SupportedProtocols[p.ProtocolMin]
+		if !ok {
+			return fmt.Errorf("unsupported protocol_min: %s", p.ProtocolMin)
+		}
+		cfg.MinVersion = minVersion
 	}
 	if p.ProtocolMax != "" {
-		cfg.MaxVersion = SupportedProtocols[p.ProtocolMax]
+		maxVersion, ok := SupportedProtocols[p.ProtocolMax]
+		if !ok {
+			return fmt.Errorf("unsupported protocol_max: %s", p.ProtocolMax)
+		}
+		cfg.MaxVersion = maxVersion
+	}
+	if cfg.MinVersion > cfg.MaxVersion {
+		return fmt.Errorf("protocol min (%s) cannot be greater than protocol max (%s)",
+			ProtocolName(cfg.MinVersion), ProtocolName(cfg.MaxVersion))
 	}
 
 	// client authentication

modules/caddytls/connpolicy_test.go

@@ -418,3 +418,50 @@ func TestParseCaddyfileNestedMatcherSet(t *testing.T) {
 		}
 	}
 }
+
+func TestConnectionPolicyRejectsUnknownNames(t *testing.T) {
+	for _, tc := range []struct {
+		name   string
+		policy ConnectionPolicy
+	}{
+		{"unknown curve", ConnectionPolicy{Curves: []string{"X25519"}}},
+		{"unknown protocol_min", ConnectionPolicy{ProtocolMin: "TLS1.3"}},
+		{"unknown protocol_max", ConnectionPolicy{ProtocolMax: "tls13"}},
+		{"protocol_min greater than protocol_max", ConnectionPolicy{ProtocolMin: "tls1.3", ProtocolMax: "tls1.2"}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx, err := caddy.ProvisionContext(&caddy.Config{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			if err := (ConnectionPolicies{&tc.policy}).Provision(ctx); err == nil {
+				t.Fatal("expected an error, got nil")
+			}
+		})
+	}
+}
+
+func TestConnectionPolicyProtocolDefaults(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		policy   ConnectionPolicy
+		min, max uint16
+	}{
+		{"both empty", ConnectionPolicy{}, tls.VersionTLS12, tls.VersionTLS13},
+		{"min only", ConnectionPolicy{ProtocolMin: "tls1.3"}, tls.VersionTLS13, tls.VersionTLS13},
+		{"max only", ConnectionPolicy{ProtocolMax: "tls1.2"}, tls.VersionTLS12, tls.VersionTLS12},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx, err := caddy.ProvisionContext(&caddy.Config{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			if err := (ConnectionPolicies{&tc.policy}).Provision(ctx); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if got := tc.policy.TLSConfig; got.MinVersion != tc.min || got.MaxVersion != tc.max {
+				t.Fatalf("got min=%x max=%x, want min=%x max=%x", got.MinVersion, got.MaxVersion, tc.min, tc.max)
+			}
+		})
+	}
+}