Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ingress configuration snippets disabled by default #6

Open
chuegel opened this issue Mar 19, 2024 · 17 comments
Open

ingress configuration snippets disabled by default #6

chuegel opened this issue Mar 19, 2024 · 17 comments

Comments

@chuegel
Copy link

chuegel commented Mar 19, 2024

Since nginx version 1.9 configuration snippets are disabled by default kubernetes/ingress-nginx#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
Copy link
Contributor

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
Copy link
Author

chuegel commented Mar 22, 2024

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
Copy link
Contributor

A PR would be great. Thank you!

@chuegel
Copy link
Author

chuegel commented Mar 22, 2024

Based on this CVE: kubernetes/kubernetes#126811 it's probably not a good idea to enable this feature.
I'll will look for another option.

@chuegel
Copy link
Author

chuegel commented Mar 24, 2024

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
Copy link
Contributor

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
Copy link
Author

chuegel commented Apr 6, 2024

Any update on this?

@svenska-primekey
Copy link
Contributor

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
Copy link
Author

chuegel commented Apr 7, 2024

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
Copy link
Contributor

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
Copy link
Author

chuegel commented May 17, 2024

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

@svenska-primekey
Copy link
Contributor

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
Copy link
Author

chuegel commented May 24, 2024

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 [email protected]

@svenska-primekey
Copy link
Contributor

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
Copy link
Author

chuegel commented May 24, 2024

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

@svenska-primekey
Copy link
Contributor

I updated the version now and made a new release.

@chuegel
Copy link
Author

chuegel commented May 28, 2024

Hi Sven,
thanks but the deployment has an issue: #23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants