From 288216e1fbf25ebe11fcea7c71be526c4cd0dcce Mon Sep 17 00:00:00 2001 From: Karun Agarwal <113603846+singhalkarun@users.noreply.github.com> Date: Sat, 19 Aug 2023 16:58:25 +0530 Subject: [PATCH] httpcaddyfile: Stricter errors for site and upstream address schemes (#5757) Co-authored-by: Mohammed Al Sahaf Co-authored-by: Francis Lavoie --- caddyconfig/httpcaddyfile/addresses.go | 11 + caddytest/integration/caddyfile_test.go | 349 ++++++++++++++++++++ modules/caddyhttp/reverseproxy/caddyfile.go | 16 +- 3 files changed, 374 insertions(+), 2 deletions(-) diff --git a/caddyconfig/httpcaddyfile/addresses.go b/caddyconfig/httpcaddyfile/addresses.go index b6a8ac072..658da48ed 100644 --- a/caddyconfig/httpcaddyfile/addresses.go +++ b/caddyconfig/httpcaddyfile/addresses.go @@ -197,6 +197,17 @@ func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key str } addr = addr.Normalize() + switch addr.Scheme { + case "wss": + return nil, fmt.Errorf("the scheme wss:// is only supported in browsers; use https:// instead") + case "ws": + return nil, fmt.Errorf("the scheme ws:// is only supported in browsers; use http:// instead") + case "https", "http", "": + // Do nothing or handle the valid schemes + default: + return nil, fmt.Errorf("unsupported URL scheme %s://", addr.Scheme) + } + // figure out the HTTP and HTTPS ports; either // use defaults, or override with user config httpPort, httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPPort), strconv.Itoa(caddyhttp.DefaultHTTPSPort) diff --git a/caddytest/integration/caddyfile_test.go b/caddytest/integration/caddyfile_test.go index 3f3ba0ae2..205bc5b91 100644 --- a/caddytest/integration/caddyfile_test.go +++ b/caddytest/integration/caddyfile_test.go @@ -135,3 +135,352 @@ func TestReplIndex(t *testing.T) { // act and assert tester.AssertGetResponse("http://localhost:9080/", 200, "") } + +func TestInvalidPrefix(t *testing.T) { + type testCase struct { + config, expectedError string + } + + failureCases := []testCase{ + { + config: `wss://localhost`, + expectedError: `the scheme wss:// is only supported in browsers; use https:// instead`, + }, + { + config: `ws://localhost`, + expectedError: `the scheme ws:// is only supported in browsers; use http:// instead`, + }, + { + config: `someInvalidPrefix://localhost`, + expectedError: "unsupported URL scheme someinvalidprefix://", + }, + { + config: `h2c://localhost`, + expectedError: `unsupported URL scheme h2c://`, + }, + { + config: `localhost, wss://localhost`, + expectedError: `the scheme wss:// is only supported in browsers; use https:// instead`, + }, + { + config: `localhost { + reverse_proxy ws://localhost" + }`, + expectedError: `the scheme ws:// is only supported in browsers; use http:// instead`, + }, + { + config: `localhost { + reverse_proxy someInvalidPrefix://localhost" + }`, + expectedError: `unsupported URL scheme someinvalidprefix://`, + }, + } + + for _, failureCase := range failureCases { + caddytest.AssertLoadError(t, failureCase.config, "caddyfile", failureCase.expectedError) + } +} + +func TestValidPrefix(t *testing.T) { + type testCase struct { + rawConfig, expectedResponse string + } + + successCases := []testCase{ + { + "localhost", + `{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "terminal": true + } + ] + } + } + } + } +}`, + }, + { + "https://localhost", + `{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "terminal": true + } + ] + } + } + } + } +}`, + }, + { + "http://localhost", + `{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":80" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "terminal": true + } + ] + } + } + } + } +}`, + }, + { + `localhost { + reverse_proxy http://localhost:3000 + }`, + `{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "upstreams": [ + { + "dial": "localhost:3000" + } + ] + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + } + } +}`, + }, + { + `localhost { + reverse_proxy https://localhost:3000 + }`, + `{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "transport": { + "protocol": "http", + "tls": {} + }, + "upstreams": [ + { + "dial": "localhost:3000" + } + ] + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + } + } +}`, + }, + { + `localhost { + reverse_proxy h2c://localhost:3000 + }`, + `{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "transport": { + "protocol": "http", + "versions": [ + "h2c", + "2" + ] + }, + "upstreams": [ + { + "dial": "localhost:3000" + } + ] + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + } + } +}`, + }, + { + `localhost { + reverse_proxy localhost:3000 + }`, + `{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "upstreams": [ + { + "dial": "localhost:3000" + } + ] + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + } + } +}`, + }, + } + + for _, successCase := range successCases { + caddytest.AssertAdapt(t, successCase.rawConfig, "caddyfile", successCase.expectedResponse) + } +} diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index a986f89f2..674776f87 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -155,6 +155,18 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // the underlying JSON does not yet support different // transports (protocols or schemes) to each backend, // so we remember the last one we see and compare them + + switch pa.scheme { + case "wss": + return d.Errf("the scheme wss:// is only supported in browsers; use https:// instead") + case "ws": + return d.Errf("the scheme ws:// is only supported in browsers; use http:// instead") + case "https", "http", "h2c", "": + // Do nothing or handle the valid schemes + default: + return d.Errf("unsupported URL scheme %s://", pa.scheme) + } + if commonScheme != "" && pa.scheme != commonScheme { return d.Errf("for now, all proxy upstreams must use the same scheme (transport protocol); expecting '%s://' but got '%s://'", commonScheme, pa.scheme) @@ -193,7 +205,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for _, up := range d.RemainingArgs() { err := appendUpstream(up) if err != nil { - return err + return fmt.Errorf("parsing upstream '%s': %w", up, err) } } @@ -217,7 +229,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for _, up := range args { err := appendUpstream(up) if err != nil { - return err + return fmt.Errorf("parsing upstream '%s': %w", up, err) } }