Security: Follow-up on recent changes. (#11874)

This commit is contained in:
Marco Ebert 2024-08-26 22:09:16 +02:00 committed by GitHub
parent bde6a6bc3e
commit e9f6c8e8f2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 19 additions and 27 deletions

View file

@ -141,7 +141,7 @@ jobs:
(needs.changes.outputs.kube-webhook-certgen == 'true') (needs.changes.outputs.kube-webhook-certgen == 'true')
strategy: strategy:
matrix: matrix:
k8s: [v1.26.15, v1.27.13, v1.28.9, v1.29.4, v1.30.0] k8s: [v1.28.13, v1.29.8, v1.30.4, v1.31.0]
steps: steps:
- name: Checkout - name: Checkout
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7

View file

@ -1,7 +1,7 @@
{{- define "ingress-nginx.params" -}} {{- define "ingress-nginx.params" -}}
- /nginx-ingress-controller - /nginx-ingress-controller
{{- if .Values.controller.enableAnnotationValidations }} {{- if not .Values.controller.enableAnnotationValidations }}
- --enable-annotation-validation=true - --enable-annotation-validation=false
{{- end }} {{- end }}
{{- if .Values.defaultBackend.enabled }} {{- if .Values.defaultBackend.enabled }}
- --default-backend-service=$(POD_NAMESPACE)/{{ include "ingress-nginx.defaultBackend.fullname" . }} - --default-backend-service=$(POD_NAMESPACE)/{{ include "ingress-nginx.defaultBackend.fullname" . }}

View file

@ -13,7 +13,9 @@ metadata:
name: {{ include "ingress-nginx.controller.fullname" . }} name: {{ include "ingress-nginx.controller.fullname" . }}
namespace: {{ include "ingress-nginx.namespace" . }} namespace: {{ include "ingress-nginx.namespace" . }}
data: data:
allow-snippet-annotations: "{{ .Values.controller.allowSnippetAnnotations }}" {{- if .Values.controller.allowSnippetAnnotations }}
allow-snippet-annotations: "true"
{{- end }}
{{- if .Values.controller.addHeaders }} {{- if .Values.controller.addHeaders }}
add-headers: {{ include "ingress-nginx.namespace" . }}/{{ include "ingress-nginx.fullname" . }}-custom-add-headers add-headers: {{ include "ingress-nginx.namespace" . }}/{{ include "ingress-nginx.fullname" . }}-custom-add-headers
{{- end }} {{- end }}

View file

@ -29,9 +29,9 @@ The following table shows a configuration option's name, type, and the default v
|:--------------------------------------------------------------------------------|:-------------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:------------------------------------------------------------------------------------| |:--------------------------------------------------------------------------------|:-------------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:------------------------------------------------------------------------------------|
| [add-headers](#add-headers) | string | "" | | | [add-headers](#add-headers) | string | "" | |
| [allow-backend-server-header](#allow-backend-server-header) | bool | "false" | | | [allow-backend-server-header](#allow-backend-server-header) | bool | "false" | |
| [allow-cross-namespace-resources](#allow-cross-namespace-resources) | bool | "true" | | | [allow-cross-namespace-resources](#allow-cross-namespace-resources) | bool | "false" | |
| [allow-snippet-annotations](#allow-snippet-annotations) | bool | "false" | | | [allow-snippet-annotations](#allow-snippet-annotations) | bool | "false" | |
| [annotations-risk-level](#annotations-risk-level) | string | Critical | | | [annotations-risk-level](#annotations-risk-level) | string | High | |
| [annotation-value-word-blocklist](#annotation-value-word-blocklist) | string array | "" | | | [annotation-value-word-blocklist](#annotation-value-word-blocklist) | string array | "" | |
| [hide-headers](#hide-headers) | string array | empty | | | [hide-headers](#hide-headers) | string array | empty | |
| [access-log-params](#access-log-params) | string | "" | | | [access-log-params](#access-log-params) | string | "" | |
@ -221,7 +221,7 @@ The following table shows a configuration option's name, type, and the default v
| [service-upstream](#service-upstream) | bool | "false" | | | [service-upstream](#service-upstream) | bool | "false" | |
| [ssl-reject-handshake](#ssl-reject-handshake) | bool | "false" | | | [ssl-reject-handshake](#ssl-reject-handshake) | bool | "false" | |
| [debug-connections](#debug-connections) | []string | "127.0.0.1,1.1.1.1/24" | | | [debug-connections](#debug-connections) | []string | "127.0.0.1,1.1.1.1/24" | |
| [strict-validate-path-type](#strict-validate-path-type) | bool | "false" (v1.7.x) | | | [strict-validate-path-type](#strict-validate-path-type) | bool | "true" | |
| [grpc-buffer-size-kb](#grpc-buffer-size-kb) | int | 0 | | | [grpc-buffer-size-kb](#grpc-buffer-size-kb) | int | 0 | |
## add-headers ## add-headers
@ -234,18 +234,16 @@ Enables the return of the header Server from the backend instead of the generic
## allow-cross-namespace-resources ## allow-cross-namespace-resources
Enables users to consume cross namespace resource on annotations, when was previously enabled . _**default:**_ true Enables users to consume cross namespace resource on annotations, when was previously enabled . _**default:**_ false
**Annotations that may be impacted with this change**: **Annotations that may be impacted with this change**:
* `auth-secret` * `auth-secret`
* `auth-proxy-set-header` * `auth-proxy-set-header`
* `auth-tls-secret` * `auth-tls-secret`
* `fastcgi-params-configmap` * `fastcgi-params-configmap`
* `proxy-ssl-secret` * `proxy-ssl-secret`
**This option will be defaulted to false in the next major release**
## allow-snippet-annotations ## allow-snippet-annotations
Enables Ingress to parse and add *-snippet annotations/directives created by the user. _**default:**_ `false` Enables Ingress to parse and add *-snippet annotations/directives created by the user. _**default:**_ `false`
@ -253,15 +251,13 @@ Enables Ingress to parse and add *-snippet annotations/directives created by the
Warning: We recommend enabling this option only if you TRUST users with permission to create Ingress objects, as this Warning: We recommend enabling this option only if you TRUST users with permission to create Ingress objects, as this
may allow a user to add restricted configurations to the final nginx.conf file may allow a user to add restricted configurations to the final nginx.conf file
**This option will be defaulted to false in the next major release**
## annotations-risk-level ## annotations-risk-level
Represents the risk accepted on an annotation. If the risk is, for instance `Medium`, annotations with risk High and Critical will not be accepted. Represents the risk accepted on an annotation. If the risk is, for instance `Medium`, annotations with risk High and Critical will not be accepted.
Accepted values are `Critical`, `High`, `Medium` and `Low`. Accepted values are `Critical`, `High`, `Medium` and `Low`.
Defaults to `Critical` but will be changed to `High` on the next minor release _**default:**_ `High`
## annotation-value-word-blocklist ## annotation-value-word-blocklist
@ -1364,6 +1360,7 @@ _References:_
[http://nginx.org/en/docs/ngx_core_module.html#debug_connection](http://nginx.org/en/docs/ngx_core_module.html#debug_connection) [http://nginx.org/en/docs/ngx_core_module.html#debug_connection](http://nginx.org/en/docs/ngx_core_module.html#debug_connection)
## strict-validate-path-type ## strict-validate-path-type
Ingress objects contains a field called pathType that defines the proxy behavior. It can be `Exact`, `Prefix` and `ImplementationSpecific`. Ingress objects contains a field called pathType that defines the proxy behavior. It can be `Exact`, `Prefix` and `ImplementationSpecific`.
When pathType is configured as `Exact` or `Prefix`, there should be a more strict validation, allowing only paths starting with "/" and When pathType is configured as `Exact` or `Prefix`, there should be a more strict validation, allowing only paths starting with "/" and
@ -1377,6 +1374,8 @@ This means that Ingress objects that rely on paths containing regex characters s
The cluster admin should establish validation rules using mechanisms like [Open Policy Agent](https://www.openpolicyagent.org/) to The cluster admin should establish validation rules using mechanisms like [Open Policy Agent](https://www.openpolicyagent.org/) to
validate that only authorized users can use `ImplementationSpecific` pathType and that only the authorized characters can be used. validate that only authorized users can use `ImplementationSpecific` pathType and that only the authorized characters can be used.
_**default:**_ "true"
## grpc-buffer-size-kb ## grpc-buffer-size-kb
Sets the configuration for the GRPC Buffer Size parameter. If not set it will use the default from NGINX. Sets the configuration for the GRPC Buffer Size parameter. If not set it will use the default from NGINX.

View file

@ -39,7 +39,7 @@ func (m Mock) GetDefaultBackend() defaults.Backend {
func (m Mock) GetSecurityConfiguration() defaults.SecurityConfiguration { func (m Mock) GetSecurityConfiguration() defaults.SecurityConfiguration {
defRisk := m.AnnotationsRiskLevel defRisk := m.AnnotationsRiskLevel
if defRisk == "" { if defRisk == "" {
defRisk = "Critical" defRisk = "High"
} }
return defaults.SecurityConfiguration{ return defaults.SecurityConfiguration{
AnnotationsRiskLevel: defRisk, AnnotationsRiskLevel: defRisk,

View file

@ -160,7 +160,7 @@ Requires the update-status parameter.`)
`Prefix of the Ingress annotations specific to the NGINX controller.`) `Prefix of the Ingress annotations specific to the NGINX controller.`)
enableAnnotationValidation = flags.Bool("enable-annotation-validation", true, enableAnnotationValidation = flags.Bool("enable-annotation-validation", true,
`If true, will enable the annotation validation feature. This value will be defaulted to true on a future release`) `If true, will enable the annotation validation feature. Defaults to true`)
enableSSLChainCompletion = flags.Bool("enable-ssl-chain-completion", false, enableSSLChainCompletion = flags.Bool("enable-ssl-chain-completion", false,
`Autocomplete SSL certificate chains with missing intermediate CA certificates. `Autocomplete SSL certificate chains with missing intermediate CA certificates.

View file

@ -58,16 +58,8 @@ var _ = framework.IngressNginxDescribe("Dynamic $proxy_host", func() {
}) })
ginkgo.It("should exist a proxy_host using the upstream-vhost annotation value", func() { ginkgo.It("should exist a proxy_host using the upstream-vhost annotation value", func() {
f.SetNginxConfigMapData(map[string]string{ disableSnippet := f.AllowSnippetConfiguration()
"allow-snippet-annotations": "true", defer disableSnippet()
"annotations-risk-level": "Critical", // To allow Configuration Snippet
})
defer func() {
f.SetNginxConfigMapData(map[string]string{
"allow-snippet-annotations": "false",
"annotations-risk-level": "High",
})
}()
upstreamName := fmt.Sprintf("%v-%v-80", f.Namespace, framework.EchoService) upstreamName := fmt.Sprintf("%v-%v-80", f.Namespace, framework.EchoService)
upstreamVHost := "different.host" upstreamVHost := "different.host"

View file

@ -58,7 +58,6 @@ else
# TODO: remove the need to use fullnameOverride # TODO: remove the need to use fullnameOverride
fullnameOverride: nginx-ingress fullnameOverride: nginx-ingress
controller: controller:
enableAnnotationValidations: true
image: image:
repository: ingress-controller/controller repository: ingress-controller/controller
chroot: ${IS_CHROOT} chroot: ${IS_CHROOT}