Skip to content

Commit 367c72b

Browse files
committed
fix(security): validate haproxy.acl.path + haproxy.acl.header at parse time
The audit's P2-10 framing was "drop the dead ACLPath/ACLHeader fields," but those labels are documented in ubuntu-ha-proxy-install/docs/architecture.md and ubuntu-ha-proxy-install/docs/example-docker-compose.yml as supported forward-looking features ("Path-based routing", "Custom ACL conditions"). Deleting the BackendConfig fields would break that documented API. Instead, validate them now so the latent injection class (same shape as P0-1 BackendName and P0-2 ACLIP) is closed before any future PR wires them into a generated directive like: acl req_path path_beg <path> acl req_hdr_cnt(<header>) gt 0 Two new validators in parser.go: - validACLPath: leading slash + URL-path-safe charset (`^/[a-zA-Z0-9/_.\-~%]*$`). Rejects embedded newlines, semicolons, whitespace, query strings, fragments. - validACLHeader: RFC 7230 HTTP header token characters (`^[a-zA-Z0-9!#$%&'*+\-.^_`+"`"+`|~]+$`). Rejects newlines, colons, slashes, spaces. extractBackendConfig now rejects the whole backend if either field has a non-empty malformed value. Empty values (the overwhelmingly common case) skip validation as today. Tests cover the legitimate-shape pass list, the injection class (newline + HAProxy directive payload), and the common-injection- character reject list. P2-10 from the 2026-05 security audit.
1 parent 509bf6a commit 367c72b

2 files changed

Lines changed: 140 additions & 2 deletions

File tree

gearbox-agent/internal/framework/services/compose/parser.go

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,18 @@ var (
4343
// newline / directive injection from a Docker Compose label into the
4444
// generated haproxy.cfg (see 2026-05 security audit, P0-1).
4545
validBackendName = regexp.MustCompile(`^[a-zA-Z0-9_][a-zA-Z0-9_.\-]{0,62}$`)
46+
// validACLPath matches a URL path component for an HAProxy path-based
47+
// ACL: leading `/`, then a safe URL-path character set. Used for the
48+
// `haproxy.acl.path` label, which is currently parsed into BackendConfig
49+
// but not yet wired into the generator. Validating now closes the
50+
// latent injection class — if a future PR wires this into a generated
51+
// `acl req_path path_beg %s` directive, no flag/newline can slip
52+
// through. See 2026-05 security audit, P2-10.
53+
validACLPath = regexp.MustCompile(`^/[a-zA-Z0-9/_.\-~%]*$`)
54+
// validACLHeader matches an HTTP header name (RFC 7230 token chars).
55+
// Same forward-looking rationale as validACLPath: parsed but not yet
56+
// generated, validated now.
57+
validACLHeader = regexp.MustCompile(`^[a-zA-Z0-9!#$%&'*+\-.^_` + "`" + `|~]+$`)
4658
)
4759

4860
// validateACLIPList validates a comma-separated list of IPs/CIDRs against
@@ -312,6 +324,31 @@ func (p *Parser) extractBackendConfig(labels map[string]string, appName, service
312324
return nil
313325
}
314326

327+
// 2026-05 audit P2-10: haproxy.acl.path and haproxy.acl.header are
328+
// documented labels (see ubuntu-ha-proxy-install/docs/) that gearbox-
329+
// agent parses today but does NOT yet emit into the generated config.
330+
// Validate them here so the latent injection class is closed BEFORE
331+
// any future PR wires them into a directive like
332+
// `acl req_path path_beg <path>` or `acl req_hdr_cnt(<header>) gt 0`.
333+
// Empty is accepted (the common case); non-empty must match the
334+
// allow-list pattern, otherwise the backend is rejected.
335+
aclPath := labels[LabelPrefix+"acl.path"]
336+
if aclPath != "" && !validACLPath.MatchString(aclPath) {
337+
p.logger.Warn("Invalid haproxy.acl.path value; rejecting backend",
338+
"app", appName,
339+
"service", serviceName,
340+
)
341+
return nil
342+
}
343+
aclHeader := labels[LabelPrefix+"acl.header"]
344+
if aclHeader != "" && !validACLHeader.MatchString(aclHeader) {
345+
p.logger.Warn("Invalid haproxy.acl.header value; rejecting backend",
346+
"app", appName,
347+
"service", serviceName,
348+
)
349+
return nil
350+
}
351+
315352
// Get and validate configurable values with safe defaults
316353
mode := getValidatedOrDefault(labels, LabelPrefix+"backend.mode", "http", validMode)
317354
balance := getValidatedOrDefault(labels, LabelPrefix+"backend.balance", "roundrobin", validBalance)
@@ -334,8 +371,8 @@ func (p *Parser) extractBackendConfig(labels map[string]string, appName, service
334371
CheckFall: checkFall,
335372
CheckRise: checkRise,
336373
SSLRedirect: labels[LabelPrefix+"ssl.redirect"] != "false",
337-
ACLPath: labels[LabelPrefix+"acl.path"],
338-
ACLHeader: labels[LabelPrefix+"acl.header"],
374+
ACLPath: aclPath,
375+
ACLHeader: aclHeader,
339376
Public: labels[LabelPrefix+"public"] == "true",
340377
RateLimit: rateLimit,
341378
ACLIP: aclIP,

gearbox-agent/internal/framework/services/compose/parser_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,3 +757,104 @@ func TestParseFile_InjectionViaACLIP(t *testing.T) {
757757
t.Errorf("ParseFile() returned %d backend(s) for an injection payload; want 0 (rejected)", len(backends))
758758
}
759759
}
760+
761+
// 2026-05 audit P2-10: haproxy.acl.path and haproxy.acl.header are
762+
// validated at parse time even though no current generator emits them.
763+
// Closes the latent injection class before a future PR wires them in.
764+
func TestValidACLPath(t *testing.T) {
765+
tests := []struct {
766+
name string
767+
input string
768+
want bool
769+
}{
770+
{"empty", "", true}, // empty handled by call-site short-circuit
771+
{"root", "/", true},
772+
{"single-segment", "/api", true},
773+
{"multi-segment", "/api/v1/users", true},
774+
{"trailing-slash", "/api/", true},
775+
{"with-dash-underscore", "/api/_v2-beta", true},
776+
{"url-encoded-allowed", "/api/%20space", true},
777+
778+
{"no-leading-slash", "api", false},
779+
{"newline-injection", "/api\n http-request deny", false},
780+
{"semicolon", "/api; default-src *", false},
781+
{"space", "/api /v1", false},
782+
{"backslash", "/api\\v1", false},
783+
{"query-string", "/api?x=1", false},
784+
{"fragment", "/api#section", false},
785+
}
786+
for _, tt := range tests {
787+
t.Run(tt.name, func(t *testing.T) {
788+
if tt.input == "" {
789+
return // call-site skips validation for empty
790+
}
791+
if got := validACLPath.MatchString(tt.input); got != tt.want {
792+
t.Errorf("validACLPath.MatchString(%q) = %v, want %v", tt.input, got, tt.want)
793+
}
794+
})
795+
}
796+
}
797+
798+
func TestValidACLHeader(t *testing.T) {
799+
tests := []struct {
800+
name string
801+
input string
802+
want bool
803+
}{
804+
{"simple", "X-Custom-Header", true},
805+
{"all-lower", "authorization", true},
806+
{"underscore", "X_Custom", true},
807+
{"dot-allowed-by-rfc", "X.Forwarded.For", true},
808+
809+
{"empty", "", false},
810+
{"space", "X Custom", false},
811+
{"newline-injection", "X-Custom\nhttp-request deny", false},
812+
{"semicolon", "X-Custom;evil", false},
813+
{"colon", "X-Custom:value", false},
814+
{"slash", "X-Custom/extra", false},
815+
}
816+
for _, tt := range tests {
817+
t.Run(tt.name, func(t *testing.T) {
818+
if got := validACLHeader.MatchString(tt.input); got != tt.want {
819+
t.Errorf("validACLHeader.MatchString(%q) = %v, want %v", tt.input, got, tt.want)
820+
}
821+
})
822+
}
823+
}
824+
825+
func TestParseFile_InjectionViaACLPath(t *testing.T) {
826+
// 2026-05 audit P2-10: even though the generator doesn't emit
827+
// haproxy.acl.path today, a malicious value must not be stored on
828+
// BackendConfig — otherwise a future generator wire-up would re-open
829+
// the P0-1/P0-2 class of injection.
830+
tmpDir := t.TempDir()
831+
appDir := filepath.Join(tmpDir, "aclpath")
832+
if err := os.MkdirAll(appDir, 0755); err != nil {
833+
t.Fatalf("MkdirAll: %v", err)
834+
}
835+
836+
composeContent := "services:\n" +
837+
" web:\n" +
838+
" image: nginx\n" +
839+
" labels:\n" +
840+
" haproxy.enable: \"true\"\n" +
841+
" haproxy.hostname: \"evil.example.com\"\n" +
842+
" haproxy.backend.server: \"10.0.0.1:80\"\n" +
843+
" haproxy.acl.path: |\n" +
844+
" /api\n" +
845+
" http-request deny\n"
846+
847+
composePath := filepath.Join(appDir, "docker-compose.yml")
848+
if err := os.WriteFile(composePath, []byte(composeContent), 0644); err != nil {
849+
t.Fatalf("WriteFile: %v", err)
850+
}
851+
852+
parser := NewParser(tmpDir, testLogger())
853+
backends, err := parser.ParseFile(composePath)
854+
if err != nil {
855+
t.Fatalf("ParseFile: %v", err)
856+
}
857+
if len(backends) != 0 {
858+
t.Errorf("ParseFile returned %d backend(s); want 0 (rejected on aclpath injection)", len(backends))
859+
}
860+
}

0 commit comments

Comments
 (0)