Keyfactor / ejbca-community-helm

Helm chart for deploying EJBCA in Kubernetes
GNU Lesser General Public License v2.1
12 stars 7 forks source link

ingress configuration snippets disabled by default #6

Open chuegel opened 3 months ago

chuegel commented 3 months ago

Since nginx version 1.9 configuration snippets are disabled by default https://github.com/kubernetes/ingress-nginx/pull/10393

This leads to the error:

error: ingresses.networking.k8s.io "keyfactor-ejbca-community-helm" could not be patched: admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: nginx.ingress.kubernetes.io/configuration-snippet annotation cannot be used. Snippet directives are disabled by the Ingress administrator
svenska-primekey commented 3 months ago

The snippet is needed for the client certificate to be passed back to Wildfly in a header. Are you aware of another option to use instead of a configuration snippet annotation?

chuegel commented 3 months ago

Annotating the ingress controller with allowSnippetAnnotations: "true" works

Here is the reference: https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#allow-snippet-annotations Might be a good idea to have a hint in the documentation about it. If it's ok, I'll create a PR

svenska-primekey commented 3 months ago

A PR would be great. Thank you!

chuegel commented 3 months ago

Based on this CVE: https://github.com/kubernetes/ingress-nginx/issues/7837 it's probably not a good idea to enable this feature. I'll will look for another option.

chuegel commented 3 months ago

OK so I did a couple of tests with the latest nginx-ingress helm chart https://github.com/kubernetes/ingress-nginx/tree/main/charts/ingress-nginx The problem is the modified header ssl_client_cert

proxy_set_header SSL_CLIENT_CERT $ssl_client_cert;

I don't know why EJBCS expects the client certificate in that header (upper case). The annotation nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream: "true" will send following headers to the backend by default and doesn't need any configuration snippets:

    ssl-client-issuer-dn: The issuer information of the client certificate. Example: "CN=My CA"
    ssl-client-subject-dn: The subject information of the client certificate. Example: "CN=My Client"
    ssl-client-verify: The result of the client verification. Possible values: "SUCCESS", "FAILED: <description, why the verification failed>"
    ssl-client-cert: The full client certificate in PEM format. Will only be sent when nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream is set to "true". Example: -----BEGIN%20CERTIFICATE-----%0A...---END%20CERTIFICATE-----%0A

See: https://github.com/kubernetes/ingress-nginx/blob/main/docs/user-guide/nginx-configuration/annotations.md#client-certificate-authentication

So, if EJBCA will accept the client certificate in the ssl-client-cert header, no extra configuration-snippets are needed. That way you will also address the nasty CVE mentioned above.

svenska-primekey commented 3 months ago

I have to ask the dev on the header and if this is EJBCA or Wildfly. It would be best to not have to use a different header and keep this simple.

chuegel commented 3 months ago

Any update on this?

svenska-primekey commented 3 months ago

The header SSL_CLIENT_CERT is what Wildfly expects to see the certificate: https://docs.wildfly.org/26/wildscribe/subsystem/undertow/server/https-listener/index.html

This got me into looking at nginx and I learned that ssl_client_cert is deprecated and ssl_client_escaped_cert should be used instead.

https://nginx.org/en/docs/http/ngx_http_ssl_module.html#var_ssl_client_cert

I'm not a Java dev, however I think it would require a code change to wildfly to support using the nginx header in addition to the httpd header. https://github.com/wildfly/wildfly/blob/cdd3b79aec95df553c03acc00e68e033e0653fec/undertow/src/main/java/org/wildfly/extension/undertow/ExchangeAttributeDefinitions.java#L549

Otherwise the alternative path would be to run httpd or nginx in the same pod as EJBCA to map the header correctly and then pass the connection to the EJBCA container where Wildfly can access the SSL_CLIENT_CERT header.

chuegel commented 2 months ago

This got me into looking at nginx and I learned that ssl_client_cert is deprecated and ssl_client_escaped_cert should be used instead.

https://nginx.org/en/docs/http/ngx_http_ssl_module.html#var_ssl_client_cert

But we are still discussing the Kubernetes Ingress Nginx Controller

I'm not a Java dev either so maybe a dev can chip in and clarify what is the best solution. However, rewriting the ssl_client_cert header with configuration snippets is IMHO the least suitable solution due to the CVE I mentioned above.

svenska-primekey commented 1 month ago

One thing I have working now in the helm chart is a load balancer service and an nginx container sidecar for EJBCA. This seems to be working good. Would this option work for you?

chuegel commented 1 month ago

Thanks, I can give it a try. Is there a branch I can checkout?

svenska-primekey commented 1 month ago

It's merged into the main branch now. The values.yaml would look like this:

services:
  # not recommended, should only be used for debugging purpose
  directHttp:
    enabled: false
    type: NodePort
    httpPort: 30080
    httpsPort: 30443
  proxyAJP:
    enabled: false
    type: ClusterIP
    bindIP: 0.0.0.0
    port: 8009
    # recommended, use with nginx in pod with LoadBalancer service or ingress
  proxyHttp:
    enabled: true
    type: LoadBalancer
    bindIP: 0.0.0.0
    httpPort: 80
    httpsPort: 443
  # Extra sidecar ports to be added to the service, optionally used when sidecarContainers
  # are defined and need to expose ports
  sidecarPorts: []

# Requires proxyHttp service to be enabled
nginx:
  enabled: true
  # hostname used in the commonName of the TLS certificate issued for nginx
  host: "myejbcahost.gotsven.com"
  # The hostname used to proxy from nginx to EJBCA. When nginx is in the same pod as EJBCA use localhost
  proxy_url_host: localhost
  service:
    enabled: false
    type: NodePort
    httpPort: 30080
    httpsPort: 30443
chuegel commented 1 month ago

Greetings, thanks for the update. Is it possible that you forgot to bump the chart version ;) ?

flux get helmreleases -n apps ejbca
NAME    REVISION        SUSPENDED       READY   MESSAGE                                                                                    
ejbca   1.0.3           False           True    Helm upgrade succeeded for release apps/keyfactor.v2 with chart ejbca-community-helm@1.0.3
svenska-primekey commented 1 month ago

The chart should be 1.0.4 in the Chart.yml. Maybe I'm not doing something right that I need to figure out still

chuegel commented 1 month ago

I mean you need to tag the release: https://github.com/Keyfactor/ejbca-community-helm/releases

svenska-primekey commented 1 month ago

I updated the version now and made a new release.

chuegel commented 1 month ago

Hi Sven, thanks but the deployment has an issue: https://github.com/Keyfactor/ejbca-community-helm/issues/23