diff --git a/.github/SECURITY.md b/.github/SECURITY.md index 52f997149..2f5c3cb3a 100644 --- a/.github/SECURITY.md +++ b/.github/SECURITY.md @@ -49,7 +49,7 @@ We'll need enough information to verify the bug and make a patch. To speed thing Please DO NOT use containers, VMs, cloud instances or services, or any other complex infrastructure in your steps. Always prefer `curl -v` instead of web browsers. -We consider publicly-registered domain names to be public information. This necessary in order to maintain the integrity of certificate transparency, public DNS, and other public trust systems. Do not redact domain names from your reports. The actual content of your domain name affects Caddy's behavior, so we need the exact domain name(s) to reproduce with, or your report will be ignored. +We consider publicly-registered domain names to be public information. This is necessary in order to maintain the integrity of certificate transparency, public DNS, and other public trust systems. Do not redact domain names from your reports. The actual content of your domain name affects Caddy's behavior, so we need the exact domain name(s) to reproduce with, or your report will be ignored. It will speed things up if you suggest a working patch, such as a code diff, and explain why and how it works. Reports that are not actionable, do not contain enough information, are too pushy/demanding, or are not able to convince us that it is a viable and practical attack on the web server itself may be deferred to a later time or possibly ignored, depending on available resources. Priority will be given to credible, responsible reports that are constructive, specific, and actionable. (We get a lot of invalid reports.) Thank you for understanding. diff --git a/caddytest/integration/reverseproxy_test.go b/caddytest/integration/reverseproxy_test.go index 28af7c367..a602784d7 100644 --- a/caddytest/integration/reverseproxy_test.go +++ b/caddytest/integration/reverseproxy_test.go @@ -199,7 +199,7 @@ func TestReverseProxyWithPlaceholderDialAddress(t *testing.T) { ], "handle": [ { - + "handler": "reverse_proxy", "upstreams": [ { @@ -293,7 +293,7 @@ func TestReverseProxyWithPlaceholderTCPDialAddress(t *testing.T) { ], "handle": [ { - + "handler": "reverse_proxy", "upstreams": [ { @@ -374,7 +374,7 @@ func TestReverseProxyHealthCheck(t *testing.T) { http://localhost:9080 { reverse_proxy { to localhost:2020 - + health_uri /health health_port 2021 health_interval 10ms @@ -495,7 +495,7 @@ func TestReverseProxyHealthCheckUnixSocket(t *testing.T) { http://localhost:9080 { reverse_proxy { to unix/%s - + health_uri /health health_port 2021 health_interval 2s @@ -553,7 +553,7 @@ func TestReverseProxyHealthCheckUnixSocketWithoutPort(t *testing.T) { http://localhost:9080 { reverse_proxy { to unix/%s - + health_uri /health health_interval 2s health_timeout 5s @@ -831,3 +831,65 @@ func TestReverseProxySNIPlaceHolder(t *testing.T) { tester.AssertResponse(req, 200, "example.com") } } + +func TestWeightedRoundRobinSelectionValidation(t *testing.T) { + configTemplate := ` + { + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [":18080"], + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "load_balancing": { + "selection_policy": { + "policy": "weighted_round_robin", + "weights": %s + } + }, + "upstreams": [ + {"dial": "localhost:18081"}, + {"dial": "localhost:18082"} + ] + } + ] + } + ] + } + } + } + } + }` + + tests := []struct { + name string + weights string + errMsg string + }{ + { + name: "negative weight", + weights: "[-1, 2]", + errMsg: "weight of an upstream cannot be negative", + }, + { + name: "zero total weight", + weights: "[0, 0]", + errMsg: "requires at least one upstream with a positive weight", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + caddytest.AssertLoadError( + t, + fmt.Sprintf(configTemplate, tc.weights), + "json", + tc.errMsg, + ) + }) + } +} diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index 648edcf76..cc42b7a3b 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -127,6 +127,19 @@ func (r *WeightedRoundRobinSelection) Provision(ctx caddy.Context) error { return nil } +// Validate ensures that r's configuration is valid +func (r *WeightedRoundRobinSelection) Validate() error { + if r.totalWeight <= 0 { + return fmt.Errorf("weighted_round_robin requires at least one upstream with a positive weight") + } + for _, weight := range r.Weights { + if weight < 0 { + return fmt.Errorf("weight of an upstream cannot be negative") + } + } + return nil +} + // Select returns an available host, if any. func (r *WeightedRoundRobinSelection) Select(pool UpstreamPool, _ *http.Request, _ http.ResponseWriter) *Upstream { if len(pool) == 0 { @@ -891,6 +904,7 @@ var ( _ Selector = (*CookieHashSelection)(nil) _ caddy.Validator = (*RandomChoiceSelection)(nil) + _ caddy.Validator = (*WeightedRoundRobinSelection)(nil) _ caddy.Provisioner = (*RandomChoiceSelection)(nil) _ caddy.Provisioner = (*WeightedRoundRobinSelection)(nil) diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go index f915b1467..ce1d502df 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go @@ -131,6 +131,48 @@ func TestWeightedRoundRobinPolicy(t *testing.T) { } } +func TestWeightedRoundRobinSelection_Validate(t *testing.T) { + tests := []struct { + name string + weights []int + wantErr bool + }{ + { + name: "Valid 0 2 1 case", + weights: []int{0, 2, 1}, + wantErr: false, + }, + { + name: "Invalid 0 case (single)", + weights: []int{0}, + wantErr: true, + }, + { + name: "Invalid 0 0 case (multiple)", + weights: []int{0, 0}, + wantErr: true, + }, + { + name: "Valid weights", + weights: []int{1, 1, 1}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &WeightedRoundRobinSelection{ + Weights: tt.weights, + } + _ = s.Provision(caddy.Context{}) + err := s.Validate() + if (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + func TestWeightedRoundRobinPolicyWithZeroWeight(t *testing.T) { pool := testPool() wrrPolicy := WeightedRoundRobinSelection{