From 8e8e44e065bafbc51162487b9d460bdf70258867 Mon Sep 17 00:00:00 2001 From: Ricardo Katz Date: Mon, 19 Jun 2023 00:28:27 +0000 Subject: [PATCH] Add risk, flag for validation, tests --- internal/ingress/annotations/alias/main.go | 7 +- internal/ingress/annotations/annotations.go | 3 + internal/ingress/annotations/auth/main.go | 5 ++ internal/ingress/annotations/authreq/main.go | 5 ++ .../ingress/annotations/authreqglobal/main.go | 5 ++ internal/ingress/annotations/authtls/main.go | 5 ++ .../annotations/backendprotocol/main.go | 5 ++ internal/ingress/annotations/canary/main.go | 5 ++ .../annotations/clientbodybuffersize/main.go | 5 ++ .../ingress/annotations/connection/main.go | 5 ++ internal/ingress/annotations/cors/main.go | 5 ++ .../annotations/customhttperrors/main.go | 7 +- .../annotations/defaultbackend/main.go | 5 ++ internal/ingress/annotations/fastcgi/main.go | 5 ++ .../annotations/globalratelimit/main.go | 5 ++ .../annotations/http2pushpreload/main.go | 5 ++ .../ingress/annotations/ipallowlist/main.go | 5 ++ .../ingress/annotations/ipdenylist/main.go | 5 ++ .../ingress/annotations/loadbalancing/main.go | 5 ++ internal/ingress/annotations/log/main.go | 5 ++ internal/ingress/annotations/mirror/main.go | 5 ++ .../ingress/annotations/modsecurity/main.go | 5 ++ .../ingress/annotations/opentelemetry/main.go | 5 ++ .../ingress/annotations/opentracing/main.go | 5 ++ internal/ingress/annotations/parser/main.go | 23 ++++- .../ingress/annotations/parser/validators.go | 22 +++-- .../annotations/parser/validators_test.go | 87 +++++++++++++++++++ .../annotations/portinredirect/main.go | 5 ++ internal/ingress/annotations/proxy/main.go | 5 ++ internal/ingress/annotations/proxyssl/main.go | 5 ++ .../ingress/annotations/ratelimit/main.go | 5 ++ .../ingress/annotations/redirect/redirect.go | 5 ++ internal/ingress/annotations/rewrite/main.go | 5 ++ internal/ingress/annotations/satisfy/main.go | 5 ++ .../ingress/annotations/serversnippet/main.go | 5 ++ .../annotations/serviceupstream/main.go | 5 ++ .../annotations/sessionaffinity/main.go | 5 ++ internal/ingress/annotations/snippet/main.go | 5 ++ .../ingress/annotations/sslcipher/main.go | 5 ++ .../annotations/sslpassthrough/main.go | 5 ++ .../ingress/annotations/streamsnippet/main.go | 5 ++ .../annotations/upstreamhashby/main.go | 5 ++ .../ingress/annotations/upstreamvhost/main.go | 5 ++ .../annotations/xforwardedprefix/main.go | 5 ++ 44 files changed, 331 insertions(+), 8 deletions(-) diff --git a/internal/ingress/annotations/alias/main.go b/internal/ingress/annotations/alias/main.go index 902478af0..409ed7a77 100644 --- a/internal/ingress/annotations/alias/main.go +++ b/internal/ingress/annotations/alias/main.go @@ -37,7 +37,7 @@ var aliasAnnotation = parser.Annotation{ serverAliasAnnotation: { Validator: parser.ValidateArrayOfServerName, Scope: parser.AnnotationScopeIngress, - Risk: parser.AnnotationRiskLow, // Low, as it allows regexes but on a very limited set + Risk: parser.AnnotationRiskHigh, // High as this allows regex chars Documentation: `this annotation can be used to define additional server aliases for this Ingress`, }, @@ -86,3 +86,8 @@ func (a alias) Parse(ing *networking.Ingress) (interface{}, error) { return l, nil } + +func (a alias) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, aliasAnnotation.Annotations) +} diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index 5a9cbd4f9..5371e6eb7 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -180,6 +180,9 @@ func (e Extractor) Extract(ing *networking.Ingress) (*Ingress, error) { data := make(map[string]interface{}) for name, annotationParser := range e.annotations { + if err := annotationParser.Validate(ing.GetAnnotations()); err != nil { + return nil, errors.NewRiskyAnnotations(name) + } val, err := annotationParser.Parse(ing) klog.V(5).InfoS("Parsing Ingress annotation", "name", name, "ingress", klog.KObj(ing), "value", val) if err != nil { diff --git a/internal/ingress/annotations/auth/main.go b/internal/ingress/annotations/auth/main.go index e7c0ddaa6..37c6a0d31 100644 --- a/internal/ingress/annotations/auth/main.go +++ b/internal/ingress/annotations/auth/main.go @@ -275,3 +275,8 @@ func dumpSecretAuthMap(filename string, secret *api.Secret) error { func (a auth) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a auth) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, authSecretAnnotations.Annotations) +} diff --git a/internal/ingress/annotations/authreq/main.go b/internal/ingress/annotations/authreq/main.go index 2b4330b6f..099e1ec6d 100644 --- a/internal/ingress/annotations/authreq/main.go +++ b/internal/ingress/annotations/authreq/main.go @@ -502,3 +502,8 @@ func ParseStringToCacheDurations(input string) ([]string, error) { func (a authReq) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a authReq) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, authReqAnnotations.Annotations) +} diff --git a/internal/ingress/annotations/authreqglobal/main.go b/internal/ingress/annotations/authreqglobal/main.go index 03e0b88b3..a931d0bbd 100644 --- a/internal/ingress/annotations/authreqglobal/main.go +++ b/internal/ingress/annotations/authreqglobal/main.go @@ -67,3 +67,8 @@ func (a authReqGlobal) Parse(ing *networking.Ingress) (interface{}, error) { func (a authReqGlobal) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a authReqGlobal) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, globalAuthAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/authtls/main.go b/internal/ingress/annotations/authtls/main.go index cc383004e..2c47a14ff 100644 --- a/internal/ingress/annotations/authtls/main.go +++ b/internal/ingress/annotations/authtls/main.go @@ -215,3 +215,8 @@ func (a authTLS) Parse(ing *networking.Ingress) (interface{}, error) { func (a authTLS) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a authTLS) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, authTLSAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/backendprotocol/main.go b/internal/ingress/annotations/backendprotocol/main.go index a8f7d144a..f5d287f02 100644 --- a/internal/ingress/annotations/backendprotocol/main.go +++ b/internal/ingress/annotations/backendprotocol/main.go @@ -81,3 +81,8 @@ func (a backendProtocol) Parse(ing *networking.Ingress) (interface{}, error) { return proto, nil } + +func (a backendProtocol) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, backendProtocolConfig.Annotations) +} diff --git a/internal/ingress/annotations/canary/main.go b/internal/ingress/annotations/canary/main.go index e99901803..866f3a834 100644 --- a/internal/ingress/annotations/canary/main.go +++ b/internal/ingress/annotations/canary/main.go @@ -188,3 +188,8 @@ func (c canary) Parse(ing *networking.Ingress) (interface{}, error) { func (c canary) GetDocumentation() parser.AnnotationFields { return c.annotationConfig.Annotations } + +func (a canary) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, CanaryAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/clientbodybuffersize/main.go b/internal/ingress/annotations/clientbodybuffersize/main.go index fd82e85b0..025db96f1 100644 --- a/internal/ingress/annotations/clientbodybuffersize/main.go +++ b/internal/ingress/annotations/clientbodybuffersize/main.go @@ -64,3 +64,8 @@ func (cbbs clientBodyBufferSize) GetDocumentation() parser.AnnotationFields { func (cbbs clientBodyBufferSize) Parse(ing *networking.Ingress) (interface{}, error) { return parser.GetStringAnnotation(clientBodyBufferSizeAnnotation, ing, cbbs.annotationConfig.Annotations) } + +func (a clientBodyBufferSize) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, clientBodyBufferSizeConfig.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/connection/main.go b/internal/ingress/annotations/connection/main.go index 8b10145a6..4cca6a94f 100644 --- a/internal/ingress/annotations/connection/main.go +++ b/internal/ingress/annotations/connection/main.go @@ -100,3 +100,8 @@ func (r1 *Config) Equal(r2 *Config) bool { func (a connection) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a connection) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, connectionHeadersAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/cors/main.go b/internal/ingress/annotations/cors/main.go index 8ee0b4f38..8698588d4 100644 --- a/internal/ingress/annotations/cors/main.go +++ b/internal/ingress/annotations/cors/main.go @@ -259,3 +259,8 @@ func (c cors) Parse(ing *networking.Ingress) (interface{}, error) { func (c cors) GetDocumentation() parser.AnnotationFields { return c.annotationConfig.Annotations } + +func (a cors) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, corsAnnotation.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/customhttperrors/main.go b/internal/ingress/annotations/customhttperrors/main.go index 2eb19ed8b..b2623b023 100644 --- a/internal/ingress/annotations/customhttperrors/main.go +++ b/internal/ingress/annotations/customhttperrors/main.go @@ -66,7 +66,7 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // Parse parses the annotations contained in the ingress to use // custom http errors func (e customhttperrors) Parse(ing *networking.Ingress) (interface{}, error) { - c, err := parser.GetStringAnnotation("custom-http-errors", ing, e.annotationConfig.Annotations) + c, err := parser.GetStringAnnotation(customHTTPErrorsAnnotation, ing, e.annotationConfig.Annotations) if err != nil { return nil, err } @@ -87,3 +87,8 @@ func (e customhttperrors) Parse(ing *networking.Ingress) (interface{}, error) { func (e customhttperrors) GetDocumentation() parser.AnnotationFields { return e.annotationConfig.Annotations } + +func (a customhttperrors) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, customHTTPErrorsAnnotations.Annotations) +} diff --git a/internal/ingress/annotations/defaultbackend/main.go b/internal/ingress/annotations/defaultbackend/main.go index d73b7fa94..f413b4bab 100644 --- a/internal/ingress/annotations/defaultbackend/main.go +++ b/internal/ingress/annotations/defaultbackend/main.go @@ -75,3 +75,8 @@ func (db backend) Parse(ing *networking.Ingress) (interface{}, error) { func (db backend) GetDocumentation() parser.AnnotationFields { return db.annotationConfig.Annotations } + +func (a backend) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, defaultBackendAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/fastcgi/main.go b/internal/ingress/annotations/fastcgi/main.go index 7fa61e3db..2cc20444c 100644 --- a/internal/ingress/annotations/fastcgi/main.go +++ b/internal/ingress/annotations/fastcgi/main.go @@ -160,3 +160,8 @@ func (a fastcgi) Parse(ing *networking.Ingress) (interface{}, error) { func (a fastcgi) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a fastcgi) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, fastCGIAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/globalratelimit/main.go b/internal/ingress/annotations/globalratelimit/main.go index 97e820edf..0bd9dad9c 100644 --- a/internal/ingress/annotations/globalratelimit/main.go +++ b/internal/ingress/annotations/globalratelimit/main.go @@ -173,3 +173,8 @@ func (a globalratelimit) Parse(ing *networking.Ingress) (interface{}, error) { func (a globalratelimit) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a globalratelimit) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, globalRateLimitAnnotationConfig.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/http2pushpreload/main.go b/internal/ingress/annotations/http2pushpreload/main.go index 263d7a7b2..f9bc2f0f8 100644 --- a/internal/ingress/annotations/http2pushpreload/main.go +++ b/internal/ingress/annotations/http2pushpreload/main.go @@ -61,3 +61,8 @@ func (h2pp http2PushPreload) Parse(ing *networking.Ingress) (interface{}, error) func (h2pp http2PushPreload) GetDocumentation() parser.AnnotationFields { return h2pp.annotationConfig.Annotations } + +func (a http2PushPreload) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, http2PushPreloadAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/ipallowlist/main.go b/internal/ingress/annotations/ipallowlist/main.go index 94ea0a7f1..270e265c0 100644 --- a/internal/ingress/annotations/ipallowlist/main.go +++ b/internal/ingress/annotations/ipallowlist/main.go @@ -126,3 +126,8 @@ func (a ipallowlist) Parse(ing *networking.Ingress) (interface{}, error) { func (a ipallowlist) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a ipallowlist) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, allowlistAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/ipdenylist/main.go b/internal/ingress/annotations/ipdenylist/main.go index 6d09578be..0d8885401 100644 --- a/internal/ingress/annotations/ipdenylist/main.go +++ b/internal/ingress/annotations/ipdenylist/main.go @@ -123,3 +123,8 @@ func (a ipdenylist) Parse(ing *networking.Ingress) (interface{}, error) { func (a ipdenylist) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a ipdenylist) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, denylistAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/loadbalancing/main.go b/internal/ingress/annotations/loadbalancing/main.go index 4555def4d..80bbdb489 100644 --- a/internal/ingress/annotations/loadbalancing/main.go +++ b/internal/ingress/annotations/loadbalancing/main.go @@ -67,3 +67,8 @@ func (a loadbalancing) Parse(ing *networking.Ingress) (interface{}, error) { func (a loadbalancing) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a loadbalancing) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, loadBalanceAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/log/main.go b/internal/ingress/annotations/log/main.go index e42de2bd1..97779475e 100644 --- a/internal/ingress/annotations/log/main.go +++ b/internal/ingress/annotations/log/main.go @@ -100,3 +100,8 @@ func (l log) Parse(ing *networking.Ingress) (interface{}, error) { func (l log) GetDocumentation() parser.AnnotationFields { return l.annotationConfig.Annotations } + +func (a log) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, logAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/mirror/main.go b/internal/ingress/annotations/mirror/main.go index 33133bedc..bd4effa71 100644 --- a/internal/ingress/annotations/mirror/main.go +++ b/internal/ingress/annotations/mirror/main.go @@ -161,3 +161,8 @@ func (a mirror) Parse(ing *networking.Ingress) (interface{}, error) { func (a mirror) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a mirror) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, mirrorAnnotation.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/modsecurity/main.go b/internal/ingress/annotations/modsecurity/main.go index 607d98d46..004b9ae6e 100644 --- a/internal/ingress/annotations/modsecurity/main.go +++ b/internal/ingress/annotations/modsecurity/main.go @@ -153,3 +153,8 @@ func (a modSecurity) Parse(ing *networking.Ingress) (interface{}, error) { func (a modSecurity) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a modSecurity) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, modsecurityAnnotation.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/opentelemetry/main.go b/internal/ingress/annotations/opentelemetry/main.go index 89ab4b3c8..3add51a34 100644 --- a/internal/ingress/annotations/opentelemetry/main.go +++ b/internal/ingress/annotations/opentelemetry/main.go @@ -149,3 +149,8 @@ func (c opentelemetry) Parse(ing *networking.Ingress) (interface{}, error) { func (c opentelemetry) GetDocumentation() parser.AnnotationFields { return c.annotationConfig.Annotations } + +func (a opentelemetry) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, otelAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/opentracing/main.go b/internal/ingress/annotations/opentracing/main.go index 4723a0e35..4bce34cba 100644 --- a/internal/ingress/annotations/opentracing/main.go +++ b/internal/ingress/annotations/opentracing/main.go @@ -106,3 +106,8 @@ func (s opentracing) Parse(ing *networking.Ingress) (interface{}, error) { func (s opentracing) GetDocumentation() parser.AnnotationFields { return s.annotationConfig.Annotations } + +func (a opentracing) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, opentracingAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/parser/main.go b/internal/ingress/annotations/parser/main.go index af19b6420..e10c373f1 100644 --- a/internal/ingress/annotations/parser/main.go +++ b/internal/ingress/annotations/parser/main.go @@ -30,12 +30,15 @@ import ( // DefaultAnnotationsPrefix defines the common prefix used in the nginx ingress controller const ( - DefaultAnnotationsPrefix = "nginx.ingress.kubernetes.io" + DefaultAnnotationsPrefix = "nginx.ingress.kubernetes.io" + DefaultDisableAnnotationValidation = true ) var ( // AnnotationsPrefix is the mutable attribute that the controller explicitly refers to AnnotationsPrefix = DefaultAnnotationsPrefix + // DisableAnnotationValidation is the mutable attribute for enabling or disabling the validation functions + DisableAnnotationValidation = DefaultDisableAnnotationValidation ) // AnnotationGroup defines the group that this annotation may belong @@ -92,6 +95,7 @@ type Annotation struct { type IngressAnnotation interface { Parse(ing *networking.Ingress) (interface{}, error) GetDocumentation() AnnotationFields + Validate(anns map[string]string) error } type ingAnnotations map[string]string @@ -189,6 +193,23 @@ func GetAnnotationWithPrefix(suffix string) string { return fmt.Sprintf("%v/%v", AnnotationsPrefix, suffix) } +func TrimAnnotationPrefix(annotation string) string { + return strings.TrimPrefix(annotation, AnnotationsPrefix+"/") +} + +func StringRiskToRisk(risk string) AnnotationRisk { + switch strings.ToLower(risk) { + case "critical": + return AnnotationRiskCritical + case "high": + return AnnotationRiskHigh + case "medium": + return AnnotationRiskMedium + default: + return AnnotationRiskLow + } +} + func normalizeString(input string) string { trimmedContent := []string{} for _, line := range strings.Split(input, "\n") { diff --git a/internal/ingress/annotations/parser/validators.go b/internal/ingress/annotations/parser/validators.go index 45ce6911e..f9ac5f6d0 100644 --- a/internal/ingress/annotations/parser/validators.go +++ b/internal/ingress/annotations/parser/validators.go @@ -17,6 +17,7 @@ limitations under the License. package parser import ( + "errors" "fmt" "regexp" "strconv" @@ -25,7 +26,7 @@ import ( networking "k8s.io/api/networking/v1" machineryvalidation "k8s.io/apimachinery/pkg/api/validation" - "k8s.io/ingress-nginx/internal/ingress/errors" + ing_errors "k8s.io/ingress-nginx/internal/ingress/errors" "k8s.io/ingress-nginx/internal/net" "k8s.io/klog/v2" ) @@ -189,12 +190,12 @@ func checkAnnotation(name string, ing *networking.Ingress, fields AnnotationFiel } if ing == nil || len(ing.GetAnnotations()) == 0 { - return "", errors.ErrMissingAnnotations + return "", ing_errors.ErrMissingAnnotations } annotationFullName := GetAnnotationWithPrefix(name) if annotationFullName == "" { - return "", errors.ErrInvalidAnnotationName + return "", ing_errors.ErrInvalidAnnotationName } annotationValue := ing.GetAnnotations()[annotationFullName] @@ -213,13 +214,24 @@ func checkAnnotation(name string, ing *networking.Ingress, fields AnnotationFiel } } // We don't run validation against empty values - if annotationValue != "" { + if annotationValue != "" && !DisableAnnotationValidation { if err := validateFunc(annotationValue); err != nil { klog.Warningf("validation error on ingress %s/%s: annotation %s contains invalid value %s", ing.GetNamespace(), ing.GetName(), name, annotationValue) - return "", errors.NewValidationError(annotationFullName) + return "", ing_errors.NewValidationError(annotationFullName) } } } return annotationFullName, nil } + +func CheckAnnotationRisk(annotations map[string]string, maxrisk AnnotationRisk, config AnnotationFields) error { + var err error + for annotation := range annotations { + annPure := TrimAnnotationPrefix(annotation) + if cfg, ok := config[annPure]; ok && cfg.Risk > maxrisk { + err = errors.Join(err, fmt.Errorf("annotation %s is too risky for environment", annotation)) + } + } + return err +} diff --git a/internal/ingress/annotations/parser/validators_test.go b/internal/ingress/annotations/parser/validators_test.go index d37f83b1e..2aa6cec37 100644 --- a/internal/ingress/annotations/parser/validators_test.go +++ b/internal/ingress/annotations/parser/validators_test.go @@ -221,3 +221,90 @@ func Test_checkAnnotation(t *testing.T) { }) } } + +func TestCheckAnnotationRisk(t *testing.T) { + + tests := []struct { + name string + annotations map[string]string + maxrisk AnnotationRisk + config AnnotationFields + wantErr bool + }{ + { + name: "high risk should not be accepted with maximum medium", + maxrisk: AnnotationRiskMedium, + annotations: map[string]string{ + "nginx.ingress.kubernetes.io/bla": "blo", + "nginx.ingress.kubernetes.io/bli": "bl3", + }, + config: AnnotationFields{ + "bla": { + Risk: AnnotationRiskHigh, + }, + "bli": { + Risk: AnnotationRiskMedium, + }, + }, + wantErr: true, + }, + { + name: "high risk should be accepted with maximum critical", + maxrisk: AnnotationRiskCritical, + annotations: map[string]string{ + "nginx.ingress.kubernetes.io/bla": "blo", + "nginx.ingress.kubernetes.io/bli": "bl3", + }, + config: AnnotationFields{ + "bla": { + Risk: AnnotationRiskHigh, + }, + "bli": { + Risk: AnnotationRiskMedium, + }, + }, + wantErr: false, + }, + { + name: "low risk should be accepted with maximum low", + maxrisk: AnnotationRiskLow, + annotations: map[string]string{ + "nginx.ingress.kubernetes.io/bla": "blo", + "nginx.ingress.kubernetes.io/bli": "bl3", + }, + config: AnnotationFields{ + "bla": { + Risk: AnnotationRiskLow, + }, + "bli": { + Risk: AnnotationRiskLow, + }, + }, + wantErr: false, + }, + { + name: "critical risk should be accepted with maximum critical", + maxrisk: AnnotationRiskCritical, + annotations: map[string]string{ + "nginx.ingress.kubernetes.io/bla": "blo", + "nginx.ingress.kubernetes.io/bli": "bl3", + }, + config: AnnotationFields{ + "bla": { + Risk: AnnotationRiskCritical, + }, + "bli": { + Risk: AnnotationRiskCritical, + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := CheckAnnotationRisk(tt.annotations, tt.maxrisk, tt.config); (err != nil) != tt.wantErr { + t.Errorf("CheckAnnotationRisk() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/ingress/annotations/portinredirect/main.go b/internal/ingress/annotations/portinredirect/main.go index a8f8dc2bf..10f2c3d79 100644 --- a/internal/ingress/annotations/portinredirect/main.go +++ b/internal/ingress/annotations/portinredirect/main.go @@ -66,3 +66,8 @@ func (a portInRedirect) Parse(ing *networking.Ingress) (interface{}, error) { func (a portInRedirect) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a portInRedirect) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, portsInRedirectAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/proxy/main.go b/internal/ingress/annotations/proxy/main.go index f24f352c7..fe7f66cbe 100644 --- a/internal/ingress/annotations/proxy/main.go +++ b/internal/ingress/annotations/proxy/main.go @@ -357,3 +357,8 @@ func (a proxy) Parse(ing *networking.Ingress) (interface{}, error) { func (a proxy) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a proxy) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, proxyAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/proxyssl/main.go b/internal/ingress/annotations/proxyssl/main.go index 411dc66e0..07b5dbcfc 100644 --- a/internal/ingress/annotations/proxyssl/main.go +++ b/internal/ingress/annotations/proxyssl/main.go @@ -259,3 +259,8 @@ func (p proxySSL) Parse(ing *networking.Ingress) (interface{}, error) { func (p proxySSL) GetDocumentation() parser.AnnotationFields { return p.annotationConfig.Annotations } + +func (a proxySSL) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, proxySSLAnnotation.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/ratelimit/main.go b/internal/ingress/annotations/ratelimit/main.go index 628343407..12b426eb7 100644 --- a/internal/ingress/annotations/ratelimit/main.go +++ b/internal/ingress/annotations/ratelimit/main.go @@ -294,3 +294,8 @@ func encode(s string) string { func (a ratelimit) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a ratelimit) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, rateLimitAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/redirect/redirect.go b/internal/ingress/annotations/redirect/redirect.go index a7081da2f..1cbcc4e8c 100644 --- a/internal/ingress/annotations/redirect/redirect.go +++ b/internal/ingress/annotations/redirect/redirect.go @@ -177,3 +177,8 @@ func isValidURL(s string) error { func (a redirect) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a redirect) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, redirectAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/rewrite/main.go b/internal/ingress/annotations/rewrite/main.go index b430a2218..debd6e0f7 100644 --- a/internal/ingress/annotations/rewrite/main.go +++ b/internal/ingress/annotations/rewrite/main.go @@ -208,3 +208,8 @@ func (a rewrite) Parse(ing *networking.Ingress) (interface{}, error) { func (a rewrite) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a rewrite) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, rewriteAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/satisfy/main.go b/internal/ingress/annotations/satisfy/main.go index e9eefa7b0..dc0b25a6a 100644 --- a/internal/ingress/annotations/satisfy/main.go +++ b/internal/ingress/annotations/satisfy/main.go @@ -68,3 +68,8 @@ func (s satisfy) Parse(ing *networking.Ingress) (interface{}, error) { func (s satisfy) GetDocumentation() parser.AnnotationFields { return s.annotationConfig.Annotations } + +func (a satisfy) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, satisfyAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/serversnippet/main.go b/internal/ingress/annotations/serversnippet/main.go index e44675f35..3360a7dc3 100644 --- a/internal/ingress/annotations/serversnippet/main.go +++ b/internal/ingress/annotations/serversnippet/main.go @@ -62,3 +62,8 @@ func (a serverSnippet) Parse(ing *networking.Ingress) (interface{}, error) { func (a serverSnippet) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a serverSnippet) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, serverSnippetAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/serviceupstream/main.go b/internal/ingress/annotations/serviceupstream/main.go index 9e5d00b35..ca069dc1c 100644 --- a/internal/ingress/annotations/serviceupstream/main.go +++ b/internal/ingress/annotations/serviceupstream/main.go @@ -68,3 +68,8 @@ func (s serviceUpstream) Parse(ing *networking.Ingress) (interface{}, error) { func (s serviceUpstream) GetDocumentation() parser.AnnotationFields { return s.annotationConfig.Annotations } + +func (a serviceUpstream) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, serviceUpstreamAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/sessionaffinity/main.go b/internal/ingress/annotations/sessionaffinity/main.go index fc2dccad9..ea9fe9924 100644 --- a/internal/ingress/annotations/sessionaffinity/main.go +++ b/internal/ingress/annotations/sessionaffinity/main.go @@ -297,3 +297,8 @@ func (a affinity) Parse(ing *networking.Ingress) (interface{}, error) { func (a affinity) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a affinity) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, sessionAffinityAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/snippet/main.go b/internal/ingress/annotations/snippet/main.go index 0dd948969..851962544 100644 --- a/internal/ingress/annotations/snippet/main.go +++ b/internal/ingress/annotations/snippet/main.go @@ -62,3 +62,8 @@ func (a snippet) Parse(ing *networking.Ingress) (interface{}, error) { func (a snippet) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a snippet) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, configurationSnippetAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/sslcipher/main.go b/internal/ingress/annotations/sslcipher/main.go index ee391f92c..30f9cf822 100644 --- a/internal/ingress/annotations/sslcipher/main.go +++ b/internal/ingress/annotations/sslcipher/main.go @@ -103,3 +103,8 @@ func (sc sslCipher) Parse(ing *networking.Ingress) (interface{}, error) { func (sc sslCipher) GetDocumentation() parser.AnnotationFields { return sc.annotationConfig.Annotations } + +func (a sslCipher) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, sslCipherAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/sslpassthrough/main.go b/internal/ingress/annotations/sslpassthrough/main.go index ebeb4c405..2635136ed 100644 --- a/internal/ingress/annotations/sslpassthrough/main.go +++ b/internal/ingress/annotations/sslpassthrough/main.go @@ -65,3 +65,8 @@ func (a sslpt) Parse(ing *networking.Ingress) (interface{}, error) { func (a sslpt) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a sslpt) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, sslPassthroughAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/streamsnippet/main.go b/internal/ingress/annotations/streamsnippet/main.go index 680397749..2d69c4b2c 100644 --- a/internal/ingress/annotations/streamsnippet/main.go +++ b/internal/ingress/annotations/streamsnippet/main.go @@ -62,3 +62,8 @@ func (a streamSnippet) Parse(ing *networking.Ingress) (interface{}, error) { func (a streamSnippet) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a streamSnippet) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, streamSnippetAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/upstreamhashby/main.go b/internal/ingress/annotations/upstreamhashby/main.go index f3ac382ac..a111ffd41 100644 --- a/internal/ingress/annotations/upstreamhashby/main.go +++ b/internal/ingress/annotations/upstreamhashby/main.go @@ -107,3 +107,8 @@ func (a upstreamhashby) Parse(ing *networking.Ingress) (interface{}, error) { func (a upstreamhashby) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a upstreamhashby) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, upstreamHashByAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/upstreamvhost/main.go b/internal/ingress/annotations/upstreamvhost/main.go index 71090716c..1c779366d 100644 --- a/internal/ingress/annotations/upstreamvhost/main.go +++ b/internal/ingress/annotations/upstreamvhost/main.go @@ -63,3 +63,8 @@ func (a upstreamVhost) Parse(ing *networking.Ingress) (interface{}, error) { func (a upstreamVhost) GetDocumentation() parser.AnnotationFields { return a.annotationConfig.Annotations } + +func (a upstreamVhost) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, upstreamVhostAnnotations.Annotations) +} \ No newline at end of file diff --git a/internal/ingress/annotations/xforwardedprefix/main.go b/internal/ingress/annotations/xforwardedprefix/main.go index b45fb5c17..2b60cb3cf 100644 --- a/internal/ingress/annotations/xforwardedprefix/main.go +++ b/internal/ingress/annotations/xforwardedprefix/main.go @@ -61,3 +61,8 @@ func (cbbs xforwardedprefix) Parse(ing *networking.Ingress) (interface{}, error) func (cbbs xforwardedprefix) GetDocumentation() parser.AnnotationFields { return cbbs.annotationConfig.Annotations } + +func (a xforwardedprefix) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(a.r.GetSecurityConfiguration().AnnotationsRisk) + return parser.CheckAnnotationRisk(anns, maxrisk, xForwardedForAnnotations.Annotations) +} \ No newline at end of file