DevFW-CICD/ingress-nginx-helm
16
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add flag to disable cross namespace validation

Browse source
This commit is contained in:
Ricardo Katz 2023-06-18 21:40:25 +00:00
parent cdbf699b9f
commit 10b7baafe8
16 changed files with 225 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
internal/ingress/annotations/auth/main.go
View file

@ -182,8 +182,9 @@ func (a auth) Parse(ing *networking.Ingress) (interface{}, error) {
if sns == "" {
sns = ing.Namespace
}
secCfg := a.r.GetSecurityConfiguration()
// We don't accept different namespaces for secrets.
if sns != ing.Namespace {
if !secCfg.AllowCrossNamespaceResources && sns != ing.Namespace {
return nil, ing_errors.LocationDenied{
Reason: fmt.Errorf("cross namespace usage of secrets is not allowed"),
}

29
internal/ingress/annotations/auth/main_test.go
View file

@ -26,6 +26,7 @@ import (
api "k8s.io/api/core/v1"
networking "k8s.io/api/networking/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
ing_errors "k8s.io/ingress-nginx/internal/ingress/errors"
"k8s.io/ingress-nginx/internal/ingress/resolver"
@ -79,13 +80,18 @@ type mockSecret struct {
}
func (m mockSecret) GetSecret(name string) (*api.Secret, error) {
if name != "default/demo-secret" {
if name != "default/demo-secret" && name != "otherns/demo-secret" {
return nil, fmt.Errorf("there is no secret with name %v", name)
}
ns, _, err := cache.SplitMetaNamespaceKey(name)
if err != nil {
return nil, err
}
return &api.Secret{
ObjectMeta: meta_v1.ObjectMeta{
Namespace: api.NamespaceDefault,
Namespace: ns,
Name: "demo-secret",
},
Data: map[string][]byte{"auth": []byte("foo:$apr1$OFG3Xybp$ckL0FHDAkoXYIlH9.cysT0")},
@ -158,6 +164,25 @@ func TestIngressInvalidDifferentNamespace(t *testing.T) {
}
}
func TestIngressInvalidDifferentNamespaceAllowed(t *testing.T) {
ing := buildIngress()
data := map[string]string{}
data[parser.GetAnnotationWithPrefix(authTypeAnnotation)] = "basic"
data[parser.GetAnnotationWithPrefix(AuthSecretAnnotation)] = "otherns/demo-secret"
ing.SetAnnotations(data)
_, dir, _ := dummySecretContent(t)
defer os.RemoveAll(dir)
r := mockSecret{}
r.AllowCrossNamespace = true
_, err := NewParser(dir, r).Parse(ing)
if err != nil {
t.Errorf("not expecting an error")
}
}
func TestIngressInvalidSecretName(t *testing.T) {
ing := buildIngress()

4
internal/ingress/annotations/authreq/main.go
View file

@ -419,8 +419,10 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
if cns == "" {
cns = ing.Namespace
}
secCfg := a.r.GetSecurityConfiguration()
// We don't accept different namespaces for secrets.
if cns != ing.Namespace {
if !secCfg.AllowCrossNamespaceResources && cns != ing.Namespace {
return nil, ing_errors.LocationDenied{
Reason: fmt.Errorf("cross namespace usage of secrets is not allowed"),
}

4
internal/ingress/annotations/authtls/main.go
View file

@ -160,7 +160,9 @@ func (a authTLS) Parse(ing *networking.Ingress) (interface{}, error) {
if ns == "" {
ns = ing.Namespace
}
if ns != ing.Namespace {
secCfg := a.r.GetSecurityConfiguration()
// We don't accept different namespaces for secrets.
if !secCfg.AllowCrossNamespaceResources && ns != ing.Namespace {
return &Config{}, ing_errors.NewLocationDenied("cross namespace secrets are not supported")
}

4
internal/ingress/annotations/fastcgi/main.go
View file

@ -129,8 +129,10 @@ func (a fastcgi) Parse(ing *networking.Ingress) (interface{}, error) {
Reason: fmt.Errorf("error reading configmap name from annotation: %w", err),
}
}
secCfg := a.r.GetSecurityConfiguration()
if cmns != "" && cmns != ing.Namespace {
// We don't accept different namespaces for secrets.
if cmns != "" && !secCfg.AllowCrossNamespaceResources && cmns != ing.Namespace {
return fcgiConfig, fmt.Errorf("different namespace is not supported on fast_cgi param configmap")
}

2
internal/ingress/annotations/parser/main.go
View file

@ -54,7 +54,7 @@ var (
// AnnotationRisk is a subset of risk that an annotation may represent.
// Based on the Risk, the admin will be able to allow or disallow users to set it
// on their ingress objects
type AnnotationRisk string
type AnnotationRisk int
type AnnotationFields map[string]AnnotationConfig

8
internal/ingress/annotations/parser/validators.go
View file

@ -33,10 +33,10 @@ import (
type AnnotationValidator func(string) error
const (
AnnotationRiskLow AnnotationRisk = "Low"
AnnotationRiskMedium AnnotationRisk = "Medium"
AnnotationRiskHigh AnnotationRisk = "High"
AnnotationRiskCritical AnnotationRisk = "Critical"
AnnotationRiskLow AnnotationRisk = iota
AnnotationRiskMedium
AnnotationRiskHigh
AnnotationRiskCritical
)
var (

4
internal/ingress/annotations/proxyssl/main.go
View file

@ -199,7 +199,9 @@ func (p proxySSL) Parse(ing *networking.Ingress) (interface{}, error) {
return &Config{}, ing_errors.NewLocationDenied(err.Error())
}
if ns != ing.Namespace {
secCfg := p.r.GetSecurityConfiguration()
// We don't accept different namespaces for secrets.
if !secCfg.AllowCrossNamespaceResources && ns != ing.Namespace {
return &Config{}, ing_errors.NewLocationDenied("cross namespace secrets are not supported")
}

13
internal/ingress/controller/config/config.go
View file

@ -97,6 +97,17 @@ type Configuration struct {
// If disabled, only snippets added via ConfigMap are added to ingress.
AllowSnippetAnnotations bool `json:"allow-snippet-annotations"`
// AllowCrossNamespaceResources enables users to consume cross namespace resource on annotations
// Case disabled, attempts to use secrets or configmaps from a namespace different from Ingress will
// be denied
// This valid will default to `false` on future releases
AllowCrossNamespaceResources bool `json:"allow-cross-namespace-resources"`
// AnnotationsRisk represents the risk accepted on an annotation. If the risk is, for instance `Medium`, annotations
// with risk High and Critical will not be accepted.
// Default Risk is Critical by default, but this may be changed in future releases
AnnotationsRisk string `json:"annotations-risk"`
// AnnotationValueWordBlocklist defines words that should not be part of an user annotation value
// (can be used to run arbitrary code or configs, for example) and that should be dropped.
// This list should be separated by "," character
@ -853,8 +864,10 @@ func NewDefault() Configuration {
cfg := Configuration{
AllowSnippetAnnotations: true,
AllowCrossNamespaceResources: true,
AllowBackendServerHeader: false,
AnnotationValueWordBlocklist: "",
AnnotationsRisk: "Critical",
AccessLogPath: "/var/log/nginx/access.log",
AccessLogParams: "",
EnableAccessLogForDefaultBackend: false,

7
internal/ingress/controller/controller_test.go
View file

@ -73,6 +73,13 @@ func (fis fakeIngressStore) GetBackendConfiguration() ngx_config.Configuration {
return fis.configuration
}
func (fis fakeIngressStore) GetSecurityConfiguration() defaults.SecurityConfiguration {
return defaults.SecurityConfiguration{
AnnotationsRisk: fis.configuration.AnnotationsRisk,
AllowCrossNamespaceResources: fis.configuration.AllowCrossNamespaceResources,
}
}
func (fakeIngressStore) GetConfigMap(key string) (*corev1.ConfigMap, error) {
return nil, fmt.Errorf("test error")
}

21
internal/ingress/controller/store/store.go
View file

@ -69,6 +69,9 @@ type Storer interface {
// GetBackendConfiguration returns the nginx configuration stored in a configmap
GetBackendConfiguration() ngx_config.Configuration
// GetSecurityConfiguration returns the configuration options from Ingress
GetSecurityConfiguration() defaults.SecurityConfiguration
// GetConfigMap returns the ConfigMap matching key.
GetConfigMap(key string) (*corev1.ConfigMap, error)
@ -926,8 +929,9 @@ func (s *k8sStore) updateSecretIngressMap(ing *networkingv1.Ingress) {
"secure-verify-ca-secret",
}
secConfig := s.GetSecurityConfiguration().AllowCrossNamespaceResources
for _, ann := range secretAnnotations {
secrKey, err := objectRefAnnotationNsKey(ann, ing)
secrKey, err := objectRefAnnotationNsKey(ann, ing, secConfig)
if err != nil && !errors.IsMissingAnnotations(err) {
klog.Errorf("error reading secret reference in annotation %q: %s", ann, err)
continue
@ -943,7 +947,7 @@ func (s *k8sStore) updateSecretIngressMap(ing *networkingv1.Ingress) {
// objectRefAnnotationNsKey returns an object reference formatted as a
// 'namespace/name' key from the given annotation name.
func objectRefAnnotationNsKey(ann string, ing *networkingv1.Ingress) (string, error) {
func objectRefAnnotationNsKey(ann string, ing *networkingv1.Ingress, allowCrossNamespace bool) (string, error) {
// We pass nil fields, as this is an internal process and we don't need to validate it.
annValue, err := parser.GetStringAnnotation(ann, ing, nil)
if err != nil {
@ -958,7 +962,7 @@ func objectRefAnnotationNsKey(ann string, ing *networkingv1.Ingress) (string, er
if secrNs == "" {
return fmt.Sprintf("%v/%v", ing.Namespace, secrName), nil
}
if secrNs != ing.Namespace {
if !allowCrossNamespace && secrNs != ing.Namespace {
return "", fmt.Errorf("cross namespace secret is not supported")
}
return annValue, nil
@ -1135,6 +1139,17 @@ func (s *k8sStore) GetBackendConfiguration() ngx_config.Configuration {
return s.backendConfig
}
func (s *k8sStore) GetSecurityConfiguration() defaults.SecurityConfiguration {
s.backendConfigMu.RLock()
defer s.backendConfigMu.RUnlock()
secConfig := defaults.SecurityConfiguration{
AllowCrossNamespaceResources: s.backendConfig.AllowCrossNamespaceResources,
AnnotationsRisk: s.backendConfig.AnnotationsRisk,
}
return secConfig
}
func (s *k8sStore) setConfig(cmap *corev1.ConfigMap) {
s.backendConfigMu.Lock()
defer s.backendConfigMu.Unlock()

12
internal/ingress/defaults/main.go
View file

@ -170,3 +170,15 @@ type Backend struct {
// It disables that behavior and instead uses a single upstream in NGINX, the service's Cluster IP and port.
ServiceUpstream bool `json:"service-upstream"`
}
type SecurityConfiguration struct {
// AllowCrossNamespaceResources enables users to consume cross namespace resource on annotations
// Case disabled, attempts to use secrets or configmaps from a namespace different from Ingress will
// be denied
// This valid will default to `false` on future releases
AllowCrossNamespaceResources bool `json:"allow-cross-namespace-resources"`
// AnnotationsRisk represents the risk accepted on an annotation. If the risk is, for instance `Medium`, annotations
// with risk High and Critical will not be accepted
AnnotationsRisk string `json:"annotations-risk"`
}

3
internal/ingress/resolver/main.go
View file

@ -26,6 +26,9 @@ type Resolver interface {
// GetDefaultBackend returns the backend that must be used as default
GetDefaultBackend() defaults.Backend
// GetSecurityConfiguration returns the configuration options from Ingress
GetSecurityConfiguration() defaults.SecurityConfiguration
// GetConfigMap searches for configmap containing the namespace and name usting the character /
GetConfigMap(string) (*apiv1.ConfigMap, error)

15
internal/ingress/resolver/mock.go
View file

@ -26,7 +26,9 @@ import (
// Mock implements the Resolver interface
type Mock struct {
ConfigMaps map[string]*apiv1.ConfigMap
ConfigMaps map[string]*apiv1.ConfigMap
AnnotationRisk string
AllowCrossNamespace bool
}
// GetDefaultBackend returns the backend that must be used as default
@ -34,6 +36,17 @@ func (m Mock) GetDefaultBackend() defaults.Backend {
return defaults.Backend{}
}
func (m Mock) GetSecurityConfiguration() defaults.SecurityConfiguration {
defRisk := m.AnnotationRisk
if defRisk == "" {
defRisk = "Critical"
}
return defaults.SecurityConfiguration{
AnnotationsRisk: defRisk,
AllowCrossNamespaceResources: m.AllowCrossNamespace,
}
}
// GetSecret searches for secrets contenating the namespace and name using a the character /
func (m Mock) GetSecret(string) (*apiv1.Secret, error) {
return nil, nil

2
test/e2e/e2e.go
View file

@ -47,6 +47,8 @@ import (
_ "k8s.io/ingress-nginx/test/e2e/settings"
_ "k8s.io/ingress-nginx/test/e2e/settings/modsecurity"
_ "k8s.io/ingress-nginx/test/e2e/settings/ocsp"
// _ "k8s.io/ingress-nginx/test/e2e/settings/validations" // Test is not working, need to check the cross namespace stuff
_ "k8s.io/ingress-nginx/test/e2e/ssl"
_ "k8s.io/ingress-nginx/test/e2e/status"
_ "k8s.io/ingress-nginx/test/e2e/tcpudp"

110
test/e2e/settings/validations/validations.go Normal file
View file

@ -0,0 +1,110 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package annotations
import (
"context"
"fmt"
"net/http"
"strings"
"github.com/onsi/ginkgo/v2"
"github.com/stretchr/testify/assert"
"golang.org/x/crypto/bcrypt"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-nginx/test/e2e/framework"
)
func buildSecret(username, password, name, namespace string) *corev1.Secret {
//out, err := exec.Command("openssl", "passwd", "-crypt", password).CombinedOutput()
out, err := bcrypt.GenerateFromPassword([]byte(password), 14)
encpass := fmt.Sprintf("%v:%s\n", username, out)
assert.Nil(ginkgo.GinkgoT(), err)
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
DeletionGracePeriodSeconds: framework.NewInt64(1),
},
Data: map[string][]byte{
"auth": []byte(encpass),
},
Type: corev1.SecretTypeOpaque,
}
}
var _ = framework.DescribeAnnotation("annotation validations", func() {
f := framework.NewDefaultFramework("annotations-validations")
ginkgo.BeforeEach(func() {
f.NewEchoDeployment()
otherns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "otherns",
},
}
_, err := f.KubeClientSet.CoreV1().Namespaces().Create(context.Background(), otherns, metav1.CreateOptions{})
assert.Nil(ginkgo.GinkgoT(), err, "creating namespace")
})
ginkgo.AfterEach(func() {
err := f.KubeClientSet.CoreV1().Namespaces().Delete(context.Background(), "otherns", metav1.DeleteOptions{})
assert.Nil(ginkgo.GinkgoT(), err, "deleting namespace")
})
ginkgo.It("should return status code 401 when authentication is configured but Authorization header is not configured", func() {
host := "annotation-validations"
// Allow cross namespace consumption
f.UpdateNginxConfigMapData("allow-cross-namespace-resources", "true")
// Sleep a while just to guarantee that the configmap is applied
framework.Sleep()
s := f.EnsureSecret(buildSecret("foo", "bar", "test", "otherns"))
annotations := map[string]string{
"nginx.ingress.kubernetes.io/auth-type": "basic",
"nginx.ingress.kubernetes.io/auth-secret": fmt.Sprintf("%s/%s", s.Namespace, s.Name),
"nginx.ingress.kubernetes.io/auth-realm": "test auth",
}
ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, "server_name annotation-validations")
})
f.HTTPTestClient().
GET("/").
WithHeader("Host", host).
Expect().
Status(http.StatusUnauthorized).
Body().Contains("401 Authorization Required")
f.HTTPTestClient().
GET("/").
WithHeader("Host", host).
WithBasicAuth("foo", "bar").
Expect().
Status(http.StatusOK)
})
})