10BC0 Add string sanitisation for proxy-pass-headers & proxy-hide-headers (… · nginx/kubernetes-ingress@f3d905b · GitHub
[go: up one dir, main page]
Skip to content

Commit f3d905b

Browse files
shaun-nx“shaun-nx”haywoodsh
authored
Add string sanitisation for proxy-pass-headers & proxy-hide-headers (#2730)
* Add example annotaiton validation function * Add validation for proxy-hide-headers * Create unit tests for proxy header validation * Update documentation * Revert example yaml files * Initialize allErrs field to zero value * Update function name Co-authored-by: “shaun-nx” <“s.odonovan@f5.com”> Co-authored-by: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com>
1 parent 6681e2c commit f3d905b

File tree

3 files changed

+160
-7
lines changed

3 files changed

+160
-7
lines changed

‎docs/content/configuration/ingress-resources/advanced-configuration-with-annotations.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,6 @@ Note how the events section includes a Warning event with the Rejected reason.
8484
The following Ingress annotations currently have limited or no validation:
8585
8686
- `nginx.org/server-tokens`,
87-
- `nginx.org/proxy-hide-headers`,
88-
- `nginx.org/proxy-pass-headers`,
8987
- `nginx.org/rewrites`,
9088
- `nginx.com/jwt-key`,
9189
- `nginx.com/jwt-realm`,

‎internal/k8s/validation.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ const (
6161
stickyCookieServicesAnnotation = "nginx.com/sticky-cookie-services"
6262
)
6363

64+
const (
65+
commaDelimiter = ","
66+
)
67+
6468
type annotationValidationContext struct {
6569
annotations map[string]string
6670
specServices map[string]bool
@@ -135,8 +139,12 @@ var (
135139
validateRequiredAnnotation,
136140
validateTimeAnnotation,
137141
},
138-
proxyHideHeadersAnnotation: {},
139-
proxyPassHeadersAnnotation: {},
142+
proxyHideHeadersAnnotation: {
143+
validateHTTPHeadersAnnotation,
144+
},
145+
proxyPassHeadersAnnotation: {
146+
validateHTTPHeadersAnnotation,
147+
},
140148
clientMaxBodySizeAnnotation: {
141149
validateRequiredAnnotation,
142150
validateOffsetAnnotation,
@@ -269,6 +277,19 @@ var (
269277
annotationNames = sortedAnnotationNames(annotationValidations)
270278
)
271279

280+
func validateHTTPHeadersAnnotation(context *annotationValidationContext) field.ErrorList {
281+
var allErrs field.ErrorList
282+
headers := strings.Split(context.value, commaDelimiter)
283+
284+
for _, header := range headers {
285+
header = strings.TrimSpace(header)
286+
for _, msg := range validation.IsHTTPHeaderName(header) {
287+
allErrs = append(allErrs, field.Invalid(context.fieldPath, header, msg))
288+
}
289+
}
290+
return allErrs
291+
}
292+
272293
func sortedAnnotationNames(annotationValidations annotationValidationConfig) []string {
273294
sortedNames := make([]string, 0)
274295
for annotationName := range annotationValidations {
@@ -540,7 +561,7 @@ func validateServiceListAnnotation(context *annotationValidationContext) field.E
540561
if len(unknownServices) > 0 {
541562
errorMsg := fmt.Sprintf(
542563
"must be a comma-separated list of services. The following services were not found: %s",
543-
strings.Join(unknownServices, ","),
564+
strings.Join(unknownServices, commaDelimiter),
544565
)
545566
return append(allErrs, field.Invalid(context.fieldPath, context.value, errorMsg))
546567
}

‎internal/k8s/validation_test.go‎

Lines changed: 136 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,74 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
722722
expectedErrors: nil,
723723
msg: "valid nginx.org/proxy-hide-headers annotation, multi-value",
724724
},
725-
725+
{
726+
annotations: map[string]string{
727+
"nginx.org/proxy-hide-headers": "header-1, header-2, header-3",
728+
},
729+
specServices: map[string]bool{},
730+
isPlus: false,
731+
appProtectEnabled: false,
732+
appProtectDosEnabled: false,
733+
internalRoutesEnabled: false,
734+
expectedErrors: nil,
735+
msg: "valid nginx.org/proxy-hide-headers annotation, multi-value with spaces",
736+
},
737+
{
738+
annotations: map[string]string{
739+
"nginx.org/proxy-hide-headers": "$header1",
740+
},
741+
specServices: map[string]bool{},
742+
isPlus: false,
743+
appProtectEnabled: false,
744+
appProtectDosEnabled: false,
745+
internalRoutesEnabled: false,
746+
expectedErrors: []string{
747+
`annotations.nginx.org/proxy-hide-headers: Invalid value: "$header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`,
748+
},
749+
msg: "invalid nginx.org/proxy-hide-headers annotation, single-value containing '$'",
750+
},
751+
{
752+
annotations: map[string]string{
753+
"nginx.org/proxy-hide-headers": "{header1",
754+
},
755+
specServices: map[string]bool{},
756+
isPlus: false,
757+
appProtectEnabled: false,
758+
appProtectDosEnabled: false,
759+
internalRoutesEnabled: false,
760+
expectedErrors: []string{
761+
`annotations.nginx.org/proxy-hide-headers: Invalid value: "{header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`,
762+
},
763+
msg: "invalid nginx.org/proxy-hide-headers annotation, single-value containing '{'",
764+
},
765+
{
766+
annotations: map[string]string{
767+
"nginx.org/proxy-hide-headers": "$header1,header2",
768+
},
769+
specServices: map[string]bool{},
770+
isPlus: false,
771+
appProtectEnabled: false,
772+
appProtectDosEnabled: false,
773+
internalRoutesEnabled: false,
774+
expectedErrors: []string{
775+
`annotations.nginx.org/proxy-hide-headers: Invalid value: "$header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`,
776+
},
777+
msg: "invalid nginx.org/proxy-hide-headers annotation, multi-value containing '$'",
778+
},
779+
{
780+
annotations: map[string]string{
781+
"nginx.org/proxy-hide-headers": "header1,$header2",
782+
},
783+
specServices: map[string]bool{},
784+
isPlus: false,
785+
appProtectEnabled: false,
786+
appProtectDosEnabled: false,
787+
internalRoutesEnabled: false,
788+
expectedErrors: []string{
789+
`annotations.nginx.org/proxy-hide-headers: Invalid value: "$header2": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`,
790+
},
791+
msg: "invalid nginx.org/proxy-hide-headers annotation, multi-value containing '$' after valid header",
792+
},
726793
{
727794
annotations: map[string]string{
728795
"nginx.org/proxy-pass-headers": "header-1",
@@ -747,7 +814,74 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
747814
expectedErrors: nil,
748815
msg: "valid nginx.org/proxy-pass-headers annotation, multi-value",
749816
},
750-
817+
{
818+
annotations: map[string]string{
819+
"nginx.org/proxy-pass-headers": "header-1, header-2, header-3",
820+
},
821+
specServices: map[string]bool{},
822+
isPlus: false,
823+
appProtectEnabled: false,
824+
appProtectDosEnabled: false,
825+
internalRoutesEnabled: false,
826+
expectedErrors: nil,
827+
msg: "valid nginx.org/proxy-pass-headers annotation, multi-value with spaces",
828+
},
829+
{
830+
annotations: map[string]string{
831+
"nginx.org/proxy-pass-headers": "$header1",
832+
},
833+
specServices: map[string]bool{},
834+
isPlus: false,
835+
appProtectEnabled: false,
836+
appProtectDosEnabled: false,
837+
internalRoutesEnabled: false,
838+
expectedErrors: []string{
839+
`annotations.nginx.org/proxy-pass-headers: Invalid value: "$header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`,
840+
},
841+
msg: "invalid nginx.org/proxy-pass-headers annotation, single-value containing '$'",
842+
},
843+
{
844+
annotations: map[string]string{
845+
"nginx.org/proxy-pass-headers": "{header1",
846+
},
847+
specServices: map[string]bool{},
848+
isPlus: false,
849+
appProtectEnabled: false,
850+
appProtectDosEnabled: false,
851+
internalRoutesEnabled: false,
852+
expectedErrors: []string{
853+
`annotations.nginx.org/proxy-pass-headers: Invalid value: "{header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`,
854+
},
855+
msg: "invalid nginx.org/proxy-pass-headers annotation, single-value containing '{'",
856+
},
857+
{
858+
annotations: map[string]string{
859+
"nginx.org/proxy-pass-headers": "$header1,header2",
860+
},
861+
specServices: map[string]bool{},
862+
isPlus: false,
863+
appProtectEnabled: false,
864+
appProtectDosEnabled: false,
865+
internalRoutesEnabled: false,
866+
expectedErrors: []string{
867+
`annotations.nginx.org/proxy-pass-headers: Invalid value: "$header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`,
868+
},
869+
msg: "invalid nginx.org/proxy-pass-headers annotation, multi-value containing '$'",
870+
},
871+
{
872+
annotations: map[string]string{
873+
"nginx.org/proxy-pass-headers": "header1,$header2",
874+
},
875+
specServices: map[string]bool{},
876+
isPlus: false,
877+
appProtectEnabled: false,
878+
appProtectDosEnabled: false,
879+
internalRoutesEnabled: false,
880+
expectedErrors: []string{
881+
`annotations.nginx.org/proxy-pass-headers: Invalid value: "$header2": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`,
882+
},
883+
msg: "invalid nginx.org/proxy-pass-headers annotation, multi-value containing '$' after valid header",
884+
},
751885
{
752886
annotations: map[string]string{
753887
"nginx.org/client-max-body-size": "16M",

0 commit comments

Comments
 (0)
0