diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 967ac53dd..78544257f 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/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) diff --git a/modules/caddyhttp/reverseproxy/httptransport_test.go b/modules/caddyhttp/reverseproxy/httptransport_test.go index 599aee26b..56900fb4d 100644 --- a/modules/caddyhttp/reverseproxy/httptransport_test.go +++ b/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") + } +} diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go index 0dff51af8..182bed2cd 100644 --- a/modules/caddytls/connpolicy.go +++ b/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 diff --git a/modules/caddytls/connpolicy_test.go b/modules/caddytls/connpolicy_test.go index a90d639ed..798d476b9 100644 --- a/modules/caddytls/connpolicy_test.go +++ b/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) + } + }) + } +}