Add annotation validation for fcgi

This commit is contained in:
Ricardo Katz 2023-06-16 12:41:17 +00:00
parent f8eb6b1879
commit b435ab13cf
3 changed files with 182 additions and 7 deletions

View file

@ -19,17 +19,49 @@ package fastcgi
import (
"fmt"
"reflect"
"regexp"
networking "k8s.io/api/networking/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
ing_errors "k8s.io/ingress-nginx/internal/ingress/errors"
"k8s.io/ingress-nginx/internal/ingress/resolver"
)
const (
fastCGIIndexAnnotation = "fastcgi-index"
fastCGIParamsAnnotation = "fastcgi-params-configmap"
)
var (
// fast-cgi valid parameters is just a single file name (like index.php)
regexValidIndexAnnotationAndKey = regexp.MustCompile(`^[A-Za-z0-9\.\-\_]+$`)
)
var fastCGIAnnotations = parser.Annotation{
Group: "fastcgi",
Annotations: parser.AnnotationFields{
fastCGIIndexAnnotation: {
Validator: parser.ValidateRegex(*regexValidIndexAnnotationAndKey, true),
Scope: parser.AnnotationScopeLocation,
Risk: parser.AnnotationRiskMedium,
Documentation: `This annotation can be used to specify an index file`,
},
fastCGIParamsAnnotation: {
Validator: parser.ValidateRegex(*parser.BasicCharsRegex, true),
Scope: parser.AnnotationScopeLocation,
Risk: parser.AnnotationRiskMedium,
Documentation: `This annotation can be used to specify a ConfigMap containing the fastcgi parameters as a key/value.
Only ConfigMaps on the same namespace of ingress can be used. They key and value from ConfigMap are validated for unauthorized characters.`,
},
},
}
type fastcgi struct {
r resolver.Resolver
r resolver.Resolver
annotationConfig parser.Annotation
}
// Config describes the per location fastcgi config
@ -57,7 +89,10 @@ func (l1 *Config) Equal(l2 *Config) bool {
// NewParser creates a new fastcgiConfig protocol annotation parser
func NewParser(r resolver.Resolver) parser.IngressAnnotation {
return fastcgi{r}
return fastcgi{
r: r,
annotationConfig: fastCGIAnnotations,
}
}
// ParseAnnotations parses the annotations contained in the ingress
@ -70,14 +105,21 @@ func (a fastcgi) Parse(ing *networking.Ingress) (interface{}, error) {
return fcgiConfig, nil
}
index, err := parser.GetStringAnnotation("fastcgi-index", ing)
index, err := parser.GetStringAnnotation(fastCGIIndexAnnotation, ing, a.annotationConfig.Annotations)
if err != nil {
if ing_errors.IsValidationError(err) {
return fcgiConfig, err
}
index = ""
}
fcgiConfig.Index = index
cm, err := parser.GetStringAnnotation("fastcgi-params-configmap", ing)
cm, err := parser.GetStringAnnotation(fastCGIParamsAnnotation, ing, a.annotationConfig.Annotations)
if err != nil {
if ing_errors.IsValidationError(err) {
return fcgiConfig, err
}
return fcgiConfig, nil
}
@ -100,7 +142,19 @@ func (a fastcgi) Parse(ing *networking.Ingress) (interface{}, error) {
}
}
for k, v := range cmap.Data {
if !regexValidIndexAnnotationAndKey.MatchString(k) || !parser.NGINXVariable.MatchString(v) {
klog.ErrorS(fmt.Errorf("fcgi contains invalid key or value"), "fcgi annotation error", "configmap", cmap.Name, "namespace", cmap.Namespace, "key", k, "value", v)
return fcgiConfig, ing_errors.NewValidationError(fastCGIParamsAnnotation)
}
}
fcgiConfig.Index = index
fcgiConfig.Params = cmap.Data
return fcgiConfig, nil
}
func (a fastcgi) GetDocumentation() parser.AnnotationFields {
return a.annotationConfig.Annotations
}

View file

@ -18,6 +18,7 @@ package fastcgi
import (
"fmt"
"reflect"
"testing"
api "k8s.io/api/core/v1"
@ -49,10 +50,16 @@ func buildIngress() *networking.Ingress {
type mockConfigMap struct {
resolver.Mock
extraConfigMap map[string]map[string]string
}
func (m mockConfigMap) GetConfigMap(name string) (*api.ConfigMap, error) {
if name != "default/demo-configmap" && name != "otherns/demo-configmap" {
if m.extraConfigMap == nil {
m.extraConfigMap = make(map[string]map[string]string)
}
cmdata, ok := m.extraConfigMap[name]
if name != "default/demo-configmap" && name != "otherns/demo-configmap" && !ok {
return nil, fmt.Errorf("there is no configmap with name %v", name)
}
@ -61,12 +68,17 @@ func (m mockConfigMap) GetConfigMap(name string) (*api.ConfigMap, error) {
return nil, fmt.Errorf("invalid configmap name")
}
data := map[string]string{"REDIRECT_STATUS": "200", "SERVER_NAME": "$server_name"}
if ok {
data = cmdata
}
return &api.ConfigMap{
ObjectMeta: meta_v1.ObjectMeta{
Namespace: cmns,
Name: cmn,
},
Data: map[string]string{"REDIRECT_STATUS": "200", "SERVER_NAME": "$server_name"},
Data: data,
}, nil
}
@ -283,3 +295,111 @@ func TestConfigEquality(t *testing.T) {
t.Errorf("config4 should be equal to config")
}
}
func Test_fastcgi_Parse(t *testing.T) {
tests := []struct {
name string
index string
configmapname string
configmap map[string]string
want interface{}
wantErr bool
}{
{
name: "valid configuration",
index: "indexxpto-92123.php",
configmapname: "default/fcgiconfig",
configmap: map[string]string{
"REQUEST_METHOD": "$request_method",
"SCRIPT_FILENAME": "$document_root$fastcgi_script_name",
},
want: Config{
Index: "indexxpto-92123.php",
Params: map[string]string{
"REQUEST_METHOD": "$request_method",
"SCRIPT_FILENAME": "$document_root$fastcgi_script_name",
},
},
},
{
name: "invalid index name",
index: "indexxpto-92123$xx.php",
configmapname: "default/fcgiconfig",
configmap: map[string]string{
"REQUEST_METHOD": "$request_method",
"SCRIPT_FILENAME": "$document_root$fastcgi_script_name",
},
want: Config{},
wantErr: true,
},
{
name: "invalid configmap namespace",
index: "indexxpto-92123.php",
configmapname: "otherns/fcgiconfig",
configmap: map[string]string{
"REQUEST_METHOD": "$request_method",
"SCRIPT_FILENAME": "$document_root$fastcgi_script_name",
},
want: Config{Index: "indexxpto-92123.php"},
wantErr: true,
},
{
name: "invalid configmap namespace name",
index: "indexxpto-92123.php",
configmapname: "otherns/fcgicon;{fig",
configmap: map[string]string{
"REQUEST_METHOD": "$request_method",
"SCRIPT_FILENAME": "$document_root$fastcgi_script_name",
},
want: Config{Index: "indexxpto-92123.php"},
wantErr: true,
},
{
name: "invalid configmap values key",
index: "indexxpto-92123.php",
configmapname: "default/fcgiconfig",
configmap: map[string]string{
"REQUEST_METHOD$XPTO": "$request_method",
},
want: Config{Index: "indexxpto-92123.php"},
wantErr: true,
},
{
name: "invalid configmap values val",
index: "indexxpto-92123.php",
configmapname: "default/fcgiconfig",
configmap: map[string]string{
"REQUEST_METHOD_XPTO": "$request_method{test};a",
},
want: Config{Index: "indexxpto-92123.php"},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ing := buildIngress()
data := map[string]string{}
data[parser.GetAnnotationWithPrefix("fastcgi-index")] = tt.index
data[parser.GetAnnotationWithPrefix("fastcgi-params-configmap")] = tt.configmapname
ing.SetAnnotations(data)
m := &mockConfigMap{
extraConfigMap: map[string]map[string]string{
tt.configmapname: tt.configmap,
},
}
got, err := NewParser(m).Parse(ing)
if (err != nil) != tt.wantErr {
t.Errorf("fastcgi.Parse() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("fastcgi.Parse() = %v, want %v", got, tt.want)
}
})
}
}

View file

@ -59,7 +59,8 @@ var SizeRegex = regexp.MustCompile("^(?i)[0-9]+[bkmg]?$")
var (
// URLIsValidRegex is used on full URLs, containing query strings (:, ? and &)
URLIsValidRegex = regexp.MustCompile("^[" + alphaNumericChars + urlEnabledChars + "]*$")
// BasicChars is alphanumeric and ".", "-", "_", "~" and ":", usually used on simple host:port/path composition
// BasicChars is alphanumeric and ".", "-", "_", "~" and ":", usually used on simple host:port/path composition.
// This combination can also be used on fields that may contain characters like / (as ns/name)
BasicCharsRegex = regexp.MustCompile("^[/" + alphaNumericChars + "]*$")
// ExtendedChars is alphanumeric and ".", "-", "_", "~" and ":" plus "," and spaces, usually used on simple host:port/path composition
ExtendedChars = regexp.MustCompile("^[/" + extendedAlphaNumeric + "]*$")