From e7a534d0a311d9fa75b5981879c755281c4c9fba Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Sun, 11 Feb 2024 13:30:14 -0500 Subject: [PATCH 1/4] caddyfile: Reject long heredoc markers (#6098) Co-authored-by: Mohammed Al Sahaf --- caddyconfig/caddyfile/formatter.go | 5 ++ caddyconfig/caddyfile/formatter_test.go | 45 +++++++++++++----- caddyconfig/caddyfile/lexer.go | 4 ++ ...ase-minimized-fuzz-format-5806400649363456 | Bin 0 -> 139348 bytes 4 files changed, 42 insertions(+), 12 deletions(-) create mode 100644 caddyconfig/caddyfile/testdata/clusterfuzz-testcase-minimized-fuzz-format-5806400649363456 diff --git a/caddyconfig/caddyfile/formatter.go b/caddyconfig/caddyfile/formatter.go index 764f7911802..423de542a22 100644 --- a/caddyconfig/caddyfile/formatter.go +++ b/caddyconfig/caddyfile/formatter.go @@ -16,6 +16,7 @@ package caddyfile import ( "bytes" + "fmt" "io" "unicode" @@ -118,6 +119,10 @@ func Format(input []byte) []byte { heredoc = heredocClosed } else { heredocMarker = append(heredocMarker, ch) + if len(heredocMarker) > 32 { + errorString := fmt.Sprintf("heredoc marker too long: <<%s", string(heredocMarker)) + panic(errorString) + } write(ch) continue } diff --git a/caddyconfig/caddyfile/formatter_test.go b/caddyconfig/caddyfile/formatter_test.go index 6eec822fe59..5ea29c33576 100644 --- a/caddyconfig/caddyfile/formatter_test.go +++ b/caddyconfig/caddyfile/formatter_test.go @@ -15,6 +15,8 @@ package caddyfile import ( + "fmt" + "os" "strings" "testing" ) @@ -24,6 +26,7 @@ func TestFormatter(t *testing.T) { description string input string expect string + panics bool }{ { description: "very simple", @@ -434,18 +437,36 @@ block2 { } `, }, + { + description: "very long heredoc from fuzzer", + input: func() string { + bs, _ := os.ReadFile("testdata/clusterfuzz-testcase-minimized-fuzz-format-5806400649363456") + return string(bs) + }(), + panics: true, + }, } { - // the formatter should output a trailing newline, - // even if the tests aren't written to expect that - if !strings.HasSuffix(tc.expect, "\n") { - tc.expect += "\n" - } - - actual := Format([]byte(tc.input)) - - if string(actual) != tc.expect { - t.Errorf("\n[TEST %d: %s]\n====== EXPECTED ======\n%s\n====== ACTUAL ======\n%s^^^^^^^^^^^^^^^^^^^^^", - i, tc.description, string(tc.expect), string(actual)) - } + t.Run(fmt.Sprintf("test case %d: %s", i, tc.description), func(t *testing.T) { + if tc.panics { + defer func() { + if r := recover(); r == nil { + t.Errorf("[TEST %d: %s] Expected panic, but got none", i, tc.description) + } + }() + } + + // the formatter should output a trailing newline, + // even if the tests aren't written to expect that + if !strings.HasSuffix(tc.expect, "\n") { + tc.expect += "\n" + } + + actual := Format([]byte(tc.input)) + + if !tc.panics && string(actual) != tc.expect { + t.Errorf("\n[TEST %d: %s]\n====== EXPECTED ======\n%s\n====== ACTUAL ======\n%s^^^^^^^^^^^^^^^^^^^^^", + i, tc.description, string(tc.expect), string(actual)) + } + }) } } diff --git a/caddyconfig/caddyfile/lexer.go b/caddyconfig/caddyfile/lexer.go index 4db63749b5b..a59f0fc46de 100644 --- a/caddyconfig/caddyfile/lexer.go +++ b/caddyconfig/caddyfile/lexer.go @@ -149,6 +149,10 @@ func (l *lexer) next() (bool, error) { continue } + if len(val) > 32 { + return false, fmt.Errorf("heredoc marker too long on line #%d: %s", l.line, string(val)) + } + // after hitting a newline, we know that the heredoc marker // is the characters after the two << and the newline. // we reset the val because the heredoc is syntax we don't diff --git a/caddyconfig/caddyfile/testdata/clusterfuzz-testcase-minimized-fuzz-format-5806400649363456 b/caddyconfig/caddyfile/testdata/clusterfuzz-testcase-minimized-fuzz-format-5806400649363456 new file mode 100644 index 0000000000000000000000000000000000000000..94b70919c4b59df0f1fa3740aa6f20577ac3d74a GIT binary patch literal 139348 zcmeI*J8s)R5CBlt3tz!U2*CY_Tn2lSRH=Og-NivJ=_ZB3k7N;!3pYj}>!}!xM%>xm zr(V-qd<>zr{9}prPk4D~Ep<;Vuam6 zN$ReZeZDn}ySDo+mbK^ZYx6n(=|zA50RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXFd|81ITC2w;=3QJXU-r%} zMt}eT0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZ;7$a7JwN}xldHNW0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZ;3D8J87~sv2@oJafB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ;HCw>-}LDno&W&? z1PBlyK!5-N0-q)jLTmN7#Jr1JWsBt)T4?q4!?NHMULIOY-P7CUsk>J8`F@LC^G~~$ z#smluAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBngF#*#fZ|nziTml3L5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5O`m}^vL&JR0$9uK!5-N0t5&UxO;)?u*d%~J@P!ba+@ca*7234 zrm2=VPN(H)^XSTLTIKQ<$3B)AYc4}yhAHJZ<~Y&mRpW4=LUEhwt z{J;#%T>4Z~9_F3;Dc3U0`{q2(n~!DQdZ;PJav0bC|4zz$ec-S@@U7M%ma!kEG7UAZ Jm+QOG{RP*)&zS%K literal 0 HcmV?d00001 From 91ec75441ab5b95deec4ee6794f00b3880ec6336 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 12 Feb 2024 12:15:35 -0500 Subject: [PATCH 2/4] logging: Inline Caddyfile syntax for `ip_mask` filter (#6094) --- .../caddyfile_adapt/log_filters.txt | 6 ++++++ modules/logging/filters.go | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/caddytest/integration/caddyfile_adapt/log_filters.txt b/caddytest/integration/caddyfile_adapt/log_filters.txt index 776fa68d34c..28524a34660 100644 --- a/caddytest/integration/caddyfile_adapt/log_filters.txt +++ b/caddytest/integration/caddyfile_adapt/log_filters.txt @@ -21,6 +21,7 @@ log { ipv4 24 ipv6 32 } + request>client_ip ip_mask 16 32 request>headers>Regexp regexp secret REDACTED request>headers>Hash hash } @@ -41,6 +42,11 @@ log { }, "encoder": { "fields": { + "request\u003eclient_ip": { + "filter": "ip_mask", + "ipv4_cidr": 16, + "ipv6_cidr": 32 + }, "request\u003eheaders\u003eAuthorization": { "filter": "replace", "value": "REDACTED" diff --git a/modules/logging/filters.go b/modules/logging/filters.go index f38f8c185c1..79d908fca63 100644 --- a/modules/logging/filters.go +++ b/modules/logging/filters.go @@ -169,6 +169,27 @@ func (IPMaskFilter) CaddyModule() caddy.ModuleInfo { // UnmarshalCaddyfile sets up the module from Caddyfile tokens. func (m *IPMaskFilter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { d.Next() // consume filter name + + args := d.RemainingArgs() + if len(args) > 2 { + return d.Errf("too many arguments") + } + if len(args) > 0 { + val, err := strconv.Atoi(args[0]) + if err != nil { + return d.Errf("error parsing %s: %v", args[0], err) + } + m.IPv4MaskRaw = val + + if len(args) > 1 { + val, err := strconv.Atoi(args[1]) + if err != nil { + return d.Errf("error parsing %s: %v", args[1], err) + } + m.IPv6MaskRaw = val + } + } + for d.NextBlock(0) { switch d.Val() { case "ipv4": From f9e11158bc139294804cba99e9fea408f1fb00d6 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 12 Feb 2024 12:34:23 -0500 Subject: [PATCH 3/4] caddyauth: Rename `basicauth` to `basic_auth` (#6092) --- caddyconfig/caddyfile/parse_test.go | 8 ++++---- caddyconfig/httpcaddyfile/directives.go | 3 ++- modules/caddyhttp/caddyauth/caddyfile.go | 10 ++++++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/caddyconfig/caddyfile/parse_test.go b/caddyconfig/caddyfile/parse_test.go index 90f5095d4f9..eec94e3afe7 100644 --- a/caddyconfig/caddyfile/parse_test.go +++ b/caddyconfig/caddyfile/parse_test.go @@ -801,7 +801,7 @@ func TestImportedFilesIgnoreNonDirectiveImportTokens(t *testing.T) { fileName := writeStringToTempFileOrDie(t, ` http://example.com { # This isn't an import directive, it's just an arg with value 'import' - basicauth / import password + basic_auth / import password } `) // Parse the root file that imports the other one. @@ -812,12 +812,12 @@ func TestImportedFilesIgnoreNonDirectiveImportTokens(t *testing.T) { } auth := blocks[0].Segments[0] line := auth[0].Text + " " + auth[1].Text + " " + auth[2].Text + " " + auth[3].Text - if line != "basicauth / import password" { + if line != "basic_auth / import password" { // Previously, it would be changed to: - // basicauth / import /path/to/test/dir/password + // basic_auth / import /path/to/test/dir/password // referencing a file that (probably) doesn't exist and changing the // password! - t.Errorf("Expected basicauth tokens to be 'basicauth / import password' but got %#q", line) + t.Errorf("Expected basic_auth tokens to be 'basic_auth / import password' but got %#q", line) } } diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go index 6e5241c7f7e..9a549a18e9f 100644 --- a/caddyconfig/httpcaddyfile/directives.go +++ b/caddyconfig/httpcaddyfile/directives.go @@ -58,7 +58,8 @@ var directiveOrder = []string{ "try_files", // middleware handlers; some wrap responses - "basicauth", + "basicauth", // TODO: deprecated, renamed to basic_auth + "basic_auth", "forward_auth", "request_header", "encode", diff --git a/modules/caddyhttp/caddyauth/caddyfile.go b/modules/caddyhttp/caddyauth/caddyfile.go index 66201dd941d..d46a2a8814f 100644 --- a/modules/caddyhttp/caddyauth/caddyfile.go +++ b/modules/caddyhttp/caddyauth/caddyfile.go @@ -22,12 +22,13 @@ import ( ) func init() { - httpcaddyfile.RegisterHandlerDirective("basicauth", parseCaddyfile) + httpcaddyfile.RegisterHandlerDirective("basicauth", parseCaddyfile) // deprecated + httpcaddyfile.RegisterHandlerDirective("basic_auth", parseCaddyfile) } // parseCaddyfile sets up the handler from Caddyfile tokens. Syntax: // -// basicauth [] [ []] { +// basic_auth [] [ []] { // [] // ... // } @@ -36,6 +37,11 @@ func init() { func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) { h.Next() // consume directive name + // "basicauth" is deprecated, replaced by "basic_auth" + if h.Val() == "basicauth" { + caddy.Log().Named("config.adapter.caddyfile").Warn("the 'basicauth' directive is deprecated, please use 'basic_auth' instead!") + } + var ba HTTPBasicAuth ba.HashCache = new(Cache) From 21744b6c4cabbdeaa81f645c3068c92dffa856e0 Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Mon, 12 Feb 2024 21:06:22 +0300 Subject: [PATCH 4/4] Revert "caddyfile: Reject long heredoc markers (#6098)" (#6100) This reverts commit e7a534d0a311d9fa75b5981879c755281c4c9fba. --- caddyconfig/caddyfile/formatter.go | 5 -- caddyconfig/caddyfile/formatter_test.go | 45 +++++------------- caddyconfig/caddyfile/lexer.go | 4 -- ...ase-minimized-fuzz-format-5806400649363456 | Bin 139348 -> 0 bytes 4 files changed, 12 insertions(+), 42 deletions(-) delete mode 100644 caddyconfig/caddyfile/testdata/clusterfuzz-testcase-minimized-fuzz-format-5806400649363456 diff --git a/caddyconfig/caddyfile/formatter.go b/caddyconfig/caddyfile/formatter.go index 423de542a22..764f7911802 100644 --- a/caddyconfig/caddyfile/formatter.go +++ b/caddyconfig/caddyfile/formatter.go @@ -16,7 +16,6 @@ package caddyfile import ( "bytes" - "fmt" "io" "unicode" @@ -119,10 +118,6 @@ func Format(input []byte) []byte { heredoc = heredocClosed } else { heredocMarker = append(heredocMarker, ch) - if len(heredocMarker) > 32 { - errorString := fmt.Sprintf("heredoc marker too long: <<%s", string(heredocMarker)) - panic(errorString) - } write(ch) continue } diff --git a/caddyconfig/caddyfile/formatter_test.go b/caddyconfig/caddyfile/formatter_test.go index 5ea29c33576..6eec822fe59 100644 --- a/caddyconfig/caddyfile/formatter_test.go +++ b/caddyconfig/caddyfile/formatter_test.go @@ -15,8 +15,6 @@ package caddyfile import ( - "fmt" - "os" "strings" "testing" ) @@ -26,7 +24,6 @@ func TestFormatter(t *testing.T) { description string input string expect string - panics bool }{ { description: "very simple", @@ -437,36 +434,18 @@ block2 { } `, }, - { - description: "very long heredoc from fuzzer", - input: func() string { - bs, _ := os.ReadFile("testdata/clusterfuzz-testcase-minimized-fuzz-format-5806400649363456") - return string(bs) - }(), - panics: true, - }, } { - t.Run(fmt.Sprintf("test case %d: %s", i, tc.description), func(t *testing.T) { - if tc.panics { - defer func() { - if r := recover(); r == nil { - t.Errorf("[TEST %d: %s] Expected panic, but got none", i, tc.description) - } - }() - } - - // the formatter should output a trailing newline, - // even if the tests aren't written to expect that - if !strings.HasSuffix(tc.expect, "\n") { - tc.expect += "\n" - } - - actual := Format([]byte(tc.input)) - - if !tc.panics && string(actual) != tc.expect { - t.Errorf("\n[TEST %d: %s]\n====== EXPECTED ======\n%s\n====== ACTUAL ======\n%s^^^^^^^^^^^^^^^^^^^^^", - i, tc.description, string(tc.expect), string(actual)) - } - }) + // the formatter should output a trailing newline, + // even if the tests aren't written to expect that + if !strings.HasSuffix(tc.expect, "\n") { + tc.expect += "\n" + } + + actual := Format([]byte(tc.input)) + + if string(actual) != tc.expect { + t.Errorf("\n[TEST %d: %s]\n====== EXPECTED ======\n%s\n====== ACTUAL ======\n%s^^^^^^^^^^^^^^^^^^^^^", + i, tc.description, string(tc.expect), string(actual)) + } } } diff --git a/caddyconfig/caddyfile/lexer.go b/caddyconfig/caddyfile/lexer.go index a59f0fc46de..4db63749b5b 100644 --- a/caddyconfig/caddyfile/lexer.go +++ b/caddyconfig/caddyfile/lexer.go @@ -149,10 +149,6 @@ func (l *lexer) next() (bool, error) { continue } - if len(val) > 32 { - return false, fmt.Errorf("heredoc marker too long on line #%d: %s", l.line, string(val)) - } - // after hitting a newline, we know that the heredoc marker // is the characters after the two << and the newline. // we reset the val because the heredoc is syntax we don't diff --git a/caddyconfig/caddyfile/testdata/clusterfuzz-testcase-minimized-fuzz-format-5806400649363456 b/caddyconfig/caddyfile/testdata/clusterfuzz-testcase-minimized-fuzz-format-5806400649363456 deleted file mode 100644 index 94b70919c4b59df0f1fa3740aa6f20577ac3d74a..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 139348 zcmeI*J8s)R5CBlt3tz!U2*CY_Tn2lSRH=Og-NivJ=_ZB3k7N;!3pYj}>!}!xM%>xm zr(V-qd<>zr{9}prPk4D~Ep<;Vuam6 zN$ReZeZDn}ySDo+mbK^ZYx6n(=|zA50RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXFd|81ITC2w;=3QJXU-r%} zMt}eT0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZ;7$a7JwN}xldHNW0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZ;3D8J87~sv2@oJafB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ;HCw>-}LDno&W&? z1PBlyK!5-N0-q)jLTmN7#Jr1JWsBt)T4?q4!?NHMULIOY-P7CUsk>J8`F@LC^G~~$ z#smluAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBngF#*#fZ|nziTml3L5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5O`m}^vL&JR0$9uK!5-N0t5&UxO;)?u*d%~J@P!ba+@ca*7234 zrm2=VPN(H)^XSTLTIKQ<$3B)AYc4}yhAHJZ<~Y&mRpW4=LUEhwt z{J;#%T>4Z~9_F3;Dc3U0`{q2(n~!DQdZ;PJav0bC|4zz$ec-S@@U7M%ma!kEG7UAZ Jm+QOG{RP*)&zS%K