Update nginx.org/ca secret type to support CRL & add crl field to Ing…

…ressMTLS (#3632)

* Update nginx.org/ca secret type to support CRL

* Add unit tests

* Update test message

* Update crt and crl file names

* Update documentation

* Allow crl file to be set in ingressMTLs polciy spec

* Add additional unit tests

* Update documentation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Remove CRL from examples

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add crl option to list of fields in document

* Add crl to helm policy crd

* Update CRDs

* Make ingressMTLS.crl in policy override ca.crl in secret when both are set

* Add new line

* Add tests for CRL

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add test data files for CRL

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* replace patch with delete and create

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update tests marks and remove debug prints

* Update warning message

* Update documentation

* Update pytest mark

* Change field name to crlFileName in ingressMTLS policy

* Update documentation

* Update documentation

* Fix warning message

* Update documentation

* Update documentation

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Venktesh <ve.patel@f5.com>
Co-authored-by: Ciara Stacke <18287516+ciarams87@users.noreply.github.com>

deployments/common/crds/k8s.nginx.org_policies.yaml

@@ -92,6 +92,8 @@ spec:
                   properties:
                     clientCertSecret:
                       type: string
+                    crlFileName:
+                      type: string
                     verifyClient:
                       type: string
                     verifyDepth:

deployments/helm-chart/crds/k8s.nginx.org_policies.yaml

@@ -92,6 +92,8 @@ spec:
                   properties:
                     clientCertSecret:
                       type: string
+                    crlFileName:
+                      type: string
                     verifyClient:
                       type: string
                     verifyDepth:

docs/content/configuration/policy-resource.md

@@ -262,6 +262,17 @@ ingressMTLS:
   verifyDepth: 1
 ```
 
+Below is an example of the `ingress-mtls-secret` using the secret type `nginx.org/ca`
+```yaml
+kind: Secret
+metadata:
+  name: ingress-mtls-secret
+apiVersion: v1
+type: nginx.org/ca
+data:
+  ca.crt: <base64encoded-certificate>
+```
+
 A VirtualServer that references an IngressMTLS policy must:
 * Enable [TLS termination](/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/#virtualservertls).
 * Reference the policy in the VirtualServer [`spec`](/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/#virtualserver-specification). It is not allowed to reference an IngressMTLS policy in a [`route `](/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/#virtualserverroute) or in a VirtualServerRoute [`subroute`](/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/#virtualserverroutesubroute).
@@ -284,12 +295,58 @@ We use the `requestHeaders` of the [Action.Proxy](/nginx-ingress-controller/conf
 
 > Note: The feature is implemented using the NGINX [ngx_http_ssl_module](https://nginx.org/en/docs/http/ngx_http_ssl_module.html).
 
+#### Using a Certificate Revocation List
+The IngressMTLS policy supports configuring at CRL for your policy.
+This can be done in one of two ways.
+
+> Note: Only one of these configurations options can be used at a time.
+
+1. Adding the `ca.crl` field to the `nginx.org/ca` secret type, which accepts a base64 encoded certificate revocation list (crl).
+   Example YAML:
+```yaml
+kind: Secret
+metadata:
+  name: ingress-mtls-secret
+apiVersion: v1
+type: nginx.org/ca
+data:
+  ca.crt: <base64encoded-certificate>
+  ca.crl: <base64encoded-crl>
+```
+
+2. Adding the `crlFileName` field to your IngressMTLS policy spec with the name of the CRL file.
+
+> Note: This configuration option should only be used when using a CRL that is larger than 1MiB
+> Otherwise we recommend using the `nginx.org/ca` secret type for managing your CRL.
+
+Example YAML:
+```yaml
+apiVersion: k8s.nginx.org/v1
+kind: Policy
+metadata:
+  name: ingress-mtls-policy
+spec:
+ingressMTLS:
+    clientCertSecret: ingress-mtls-secret
+    crlFileName: webapp.crl
+    verifyClient: "on"
+    verifyDepth: 1
+```
+
+**IMPORTANT NOTE**
+When configuring a CRL with the `ingressMTLS.crlFileName` field, there is additional context to keep in mind:
+1. The Ingress Controller will expect the CRL, in this case `webapp.crl`, will be in `/etc/nginx/secrets`. A volume mount will need to be added to the Ingress Controller deployment add your CRL to `/etc/nginx/secrets`
+2. When updating the content of your CRL (e.g a new certificate has been revoked), NGINX will need to be reloaded to pick up the latest changes. Depending on your environment this may require updating the name of your CRL and applying this update to your `ingress-mtls.yaml` policy to ensure NGINX picks up the latest CRL.
+
+Please refer to the Kubernetes documentation on [volumes](https://kubernetes.io/docs/concepts/storage/volumes/) to find the best implementation for your environment.
+
 {{% table %}}
 |Field | Description | Type | Required |
 | ---| ---| ---| --- |
 |``clientCertSecret`` | The name of the Kubernetes secret that stores the CA certificate. It must be in the same namespace as the Policy resource. The secret must be of the type ``nginx.org/ca``, and the certificate must be stored in the secret under the key ``ca.crt``, otherwise the secret will be rejected as invalid. | ``string`` | Yes |
 |``verifyClient`` | Verification for the client. Possible values are ``"on"``, ``"off"``, ``"optional"``, ``"optional_no_ca"``. The default is ``"on"``. | ``string`` | No |
 |``verifyDepth`` | Sets the verification depth in the client certificates chain. The default is ``1``. | ``int`` | No |
+|``crlFileName`` | The file name of the Certificate Revocation List. The Ingress Controller will look for this file in `/etc/nginx/secrets` | ``string`` | No |
 {{% /table %}}
 
 #### IngressMTLS Merging Behavior

examples/custom-resources/ingress-mtls/README.md

@@ -2,6 +2,9 @@
 
 In this example, we deploy a web application, configure load balancing for it via a VirtualServer, and apply an Ingress MTLS policy.
 
+> Note: The Ingress MTLS policy supports configuring a Certificate Revocation List (CRL).
+> See [Using a Certificate Revocation List](https://docs.nginx.com/nginx-ingress-controller/configuration/policy-resource/#using-a-certificate-revocation-list) for details on how to set this option.
+
 ## Prerequisites
 
 1. Follow the [installation](https://docs.nginx.com/nginx-ingress-controller/installation/installation-with-manifests/) instructions to deploy the Ingress Controller.

internal/configs/configurator.go

@@ -59,8 +59,11 @@ const JWTKeyKey = "jwk"
 // HtpasswdFileKey is the key of the data field of a Secret where the HTTP basic authorization list must be stored
 const HtpasswdFileKey = "htpasswd"
 
-// CAKey is the key of the data field of a Secret where the cert must be stored.
-const CAKey = "ca.crt"
+// CACrtKey is the key of the data field of a Secret where the cert must be stored.
+const CACrtKey = "ca.crt"
+
+// CACrlKey is the key of the data field of a Secret where the cert revocation list must be stored.
+const CACrlKey = "ca.crl"
 
 // ClientSecretKey is the key of the data field of a Secret where the OIDC client secret must be stored.
 const ClientSecretKey = "client-secret"
@@ -727,8 +730,12 @@ func generateTLSPassthroughHostsConfig(tlsPassthroughPairs map[string]tlsPassthr
 
 func (cnf *Configurator) addOrUpdateCASecret(secret *api_v1.Secret) string {
 	name := objectMetaToFileName(&secret.ObjectMeta)
-	data := GenerateCAFileContent(secret)
-	return cnf.nginxManager.CreateSecret(name, data, nginx.TLSSecretFileMode)
+	crtData, crlData := GenerateCAFileContent(secret)
+	crtSecretName := fmt.Sprintf("%s-%s", name, CACrtKey)
+	crlSecretName := fmt.Sprintf("%s-%s", name, CACrlKey)
+	crtFileName := cnf.nginxManager.CreateSecret(crtSecretName, crtData, nginx.TLSSecretFileMode)
+	crlFileName := cnf.nginxManager.CreateSecret(crlSecretName, crlData, nginx.TLSSecretFileMode)
+	return fmt.Sprintf("%s %s", crtFileName, crlFileName)
 }
 
 func (cnf *Configurator) addOrUpdateJWKSecret(secret *api_v1.Secret) string {
@@ -818,12 +825,14 @@ func GenerateCertAndKeyFileContent(secret *api_v1.Secret) []byte {
 }
 
 // GenerateCAFileContent generates a pem file content from the TLS secret.
-func GenerateCAFileContent(secret *api_v1.Secret) []byte {
-	var res bytes.Buffer
+func GenerateCAFileContent(secret *api_v1.Secret) ([]byte, []byte) {
+	var caKey bytes.Buffer
+	var caCrl bytes.Buffer
 
-	res.Write(secret.Data[CAKey])
+	caKey.Write(secret.Data[CACrtKey])
+	caCrl.Write(secret.Data[CACrlKey])
 
-	return res.Bytes()
+	return caKey.Bytes(), caCrl.Bytes()
 }
 
 // DeleteIngress deletes NGINX configuration for the Ingress resource.

internal/configs/version2/http.go

@@ -91,6 +91,7 @@ type SSL struct {
 // IngressMTLS defines TLS configuration for a server. This is a subset of TLS specifically for clients auth.
 type IngressMTLS struct {
 	ClientCert   string
+	ClientCrl    string
 	VerifyClient string
 	VerifyDepth  int
 }

internal/configs/version2/nginx-plus.virtualserver.tmpl

@@ -115,6 +115,9 @@ server {
 
     {{ with $s.IngressMTLS }}
     ssl_client_certificate {{ .ClientCert }};
+    {{ if .ClientCrl }}
+    ssl_crl {{ .ClientCrl }};
+    {{ end }}
     ssl_verify_client {{ .VerifyClient }};
     ssl_verify_depth {{ .VerifyDepth }};
     {{ end }}

internal/configs/version2/nginx.virtualserver.tmpl

@@ -70,6 +70,9 @@ server {
 
     {{ with $s.IngressMTLS }}
     ssl_client_certificate {{ .ClientCert }};
+    {{ if .ClientCrl }}
+    ssl_crl {{ .ClientCrl }};
+    {{ end }}
     ssl_verify_client {{ .VerifyClient }};
     ssl_verify_depth {{ .VerifyDepth }};
     {{ end }}

internal/configs/virtualserver.go

@@ -7,15 +7,14 @@ import (
 	"strings"
 
 	"github.com/golang/glog"
+	"github.com/nginxinc/kubernetes-ingress/internal/configs/version2"
 	"github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets"
 	"github.com/nginxinc/kubernetes-ingress/internal/nginx"
+	conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
 	api_v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
-
-	"github.com/nginxinc/kubernetes-ingress/internal/configs/version2"
-	conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
 )
 
 const (
@@ -908,10 +907,32 @@ func (p *policiesCfg) addIngressMTLSConfig(
 		verifyClient = ingressMTLS.VerifyClient
 	}
 
-	p.IngressMTLS = &version2.IngressMTLS{
-		ClientCert:   secretRef.Path,
-		VerifyClient: verifyClient,
-		VerifyDepth:  verifyDepth,
+	caFields := strings.Fields(secretRef.Path)
+
+	if _, hasCrlKey := secretRef.Secret.Data[CACrlKey]; hasCrlKey && ingressMTLS.CrlFileName != "" {
+		res.addWarningf("Both ca.crl in the Secret and ingressMTLS.crlFileName fields cannot be used. ca.crl in %s will be ignored and %s will be applied", secretKey, polKey)
+	}
+
+	if ingressMTLS.CrlFileName != "" {
+		p.IngressMTLS = &version2.IngressMTLS{
+			ClientCert:   caFields[0],
+			ClientCrl:    fmt.Sprintf("%s/%s", DefaultSecretPath, ingressMTLS.CrlFileName),
+			VerifyClient: verifyClient,
+			VerifyDepth:  verifyDepth,
+		}
+	} else if _, hasCrlKey := secretRef.Secret.Data[CACrlKey]; hasCrlKey {
+		p.IngressMTLS = &version2.IngressMTLS{
+			ClientCert:   caFields[0],
+			ClientCrl:    caFields[1],
+			VerifyClient: verifyClient,
+			VerifyDepth:  verifyDepth,
+		}
+	} else {
+		p.IngressMTLS = &version2.IngressMTLS{
+			ClientCert:   caFields[0],
+			VerifyClient: verifyClient,
+			VerifyDepth:  verifyDepth,
+		}
 	}
 	return res
 }