kubernetes / ingress-nginx

Ingress-NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.26k stars 8.2k forks source link

How to increase max size for a CRL secret ? #5012

Closed zepouet closed 2 months ago

zepouet commented 4 years ago

Hello,

This is our config file.

apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
 name: ingress-client-auth
 namespace: develop
 annotations:
   kubernetes.io/ingress.class: "nginx"
   # Enable client certificate authentication
   nginx.ingress.kubernetes.io/auth-tls-verify-client: "on"
   # Create the secret containing the trusted ca certificates
   nginx.ingress.kubernetes.io/auth-tls-secret: "common/ca-secret"
   # Specify the verification depth in the client certificates chain
   nginx.ingress.kubernetes.io/auth-tls-verify-depth: "3"
   nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream: "true"
spec:
 tls:
   - secretName: api-cert-secret
     hosts:
       - dev-test-api.xxx.fr
 rules:
   - host: dev-test-api.xxx.fr
     http:
       paths:
         - backend:
             serviceName: xxx-api-svc
             servicePort: 9100

NGINX Ingress controller version: 0.26.1

Kubernetes version (use kubectl version): 1.15

Environment: GKE

Cloud provider or hardware configuration: GCP

What happened:

We reached the limit of large file for a secret about a CRL.

https://www.csoonline.com/article/3000574/the-sorry-state-of-certificate-revocation.html

What you expected to happen:

How to increase the limit of K8S Secret max file ? Is it possible to replace the secret usage with a classic mount with a volume ?

Thanks a lot. Best regards,

aledbf commented 4 years ago

How to increase the limit of K8S Secret max file ?

What's the size of the encoded content? The limit in k8s is about 1.5MB

Is it possible to replace the secret usage with a classic mount with a volume ?

No, because the secrets are handled in lua (memory), not files

zepouet commented 4 years ago

Thanks @aledbf

In this article, the author gives the information

https://www.csoonline.com/article/3000574/the-sorry-state-of-certificate-revocation.html

The median certificate revocation list size was 51KB; the max size was 76MB -- quite a variation!

In our case, we reach the 1.5 MB for a secret.

Thanks for your response about volume usage. So we cannot. I wanted to test this idea today. I think we are not the only one with this problem. Revocation list are a common use case.

So could we imagine to use more than one secrets and concat them in Lua in memory ? We will use a tag common.ca-secret associated to the secrets and will use a such annotation

nginx.ingress.kubernetes.io/auth-tls-secret.labels: "common.ca-secret"

The merge will be done with controller itselft.

Thanks for your response

aledbf commented 4 years ago

In our case, we reach the 1.5 MB for a secret.

Sorry, the limit is 1MB https://kubernetes.io/docs/concepts/configuration/secret/#details

So could we imagine to use more than one secrets and concat them in Lua in memory ?

No. We cannot diverge from k8s in this aspect. Secrets must be self-contained, and I think adding this logic will break something.

For your use case, certificate revocation, maybe we could add an annotation that receives a URL (where the certificate is located) and use that, bypassing the limit in k8s.

zepouet commented 4 years ago

Thanks @aledbf

The idea about an URL is great ! Do you want a PR about it or you foresee yourself ?

Thanks a lot. Best regards,

pdelorme commented 4 years ago

Hello,

CRLs don't have to be secret as they are publicly available. They should be stored in a regular ConfigMap. What do you think ?

Thanks, Regards

aledbf commented 4 years ago

@pdelorme configmaps have the same size limit

zepouet commented 4 years ago

It would great to have the feature with URL. I convinced you are right rather than labels. We could imagine a sidecar container into pod nginx-controler to gather it and purpose the CRL file through an URL.

Thanks for your answer.

kurbar commented 4 years ago

While integrating Estonian National ID card authentication I hit the same barrier. As nginx currently does not not support OCSP stapling on client certificates the only option was to opt for CRL. However since indeed there is a 1MB limit on secrets, my current CRL of 100MB is nowhere near the accepted limit.

Having an option to sideload this using URL's sounds really reasonable to me considering that some CRL's might become really large.

However if this approach is considered, I would suggest that it should support multiple URL's to CRL files. In my use case there are multiple CRL files (https://www.skidsolutions.eu/en/repository/CRL/) depending under what intermediate cert a given ID cards certificate was issued from. So if nginx ingress could then bundle them together, it would be awesome.

toonsevrin commented 4 years ago

I've proposed an extension to Kubernetes for this. Please take a look at the kubernetes issue #88709.

With this extension, you upload your configuration to s3 (or simply git) and reference it in a File resource. Finally the configmap references this file resource.

Feel free to upvote that proposal to raise awareness.

fejta-bot commented 4 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 4 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

chrko commented 4 years ago

/remove-lifecycle rotten

Still a problem if using client cert auth. Implementing side loading crl files by url I would call a very appreciating solution.

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

chrko commented 3 years ago

/remove-lifecycle stale

Still a problem if using client cert auth. Implementing side loading crl files by url I would call a very appreciating solution

YunziZhangAzure commented 3 years ago

Hi @aledbf Our team ran into a same size restriction. I'm wondering if there's any update about the "annotation that receives a URL" we think that would perfectly solve our problem.

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

chrko commented 3 years ago

/remove-lifecycle stale

Still a problem if using client cert auth. Implementing side loading crl files by url I would call a very appreciating solution

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

chrko commented 3 years ago

/remove-lifecycle stale

iamNoah1 commented 3 years ago

/label needs-priority

@rikatz @strongjz

strongjz commented 3 years ago

/triage accepted /priority longterm-backlog

k8s-ci-robot commented 3 years ago

@strongjz: The label(s) priority/longterm-backlog cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/5012#issuecomment-900463382): >/triage accepted >/priority longterm-backlog Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
strongjz commented 3 years ago

/priority backlog

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

rikatz commented 2 years ago

/lifecycle frozen

rikatz commented 2 years ago

@toonsevrin I've seen your proposal in k8s issue. So, my main concern with that approach here is Ingress is allowing users to download arbitrary files from the internet (not only CRLs) so a shared directory.

We've seen on really recent past that allowing users to configure arbitrary stuff on the running ingress controller may be bad: As an example, we may allow them to download some rogue certificate over the existing one and faking it's a CRL? And other examples.

Personally IF Kubernetes allowed bigger ConfigMaps I would do with that, but this is not possible. Downloading CRLs may be a possible solution here, but I really want some good security drawing/scenarios (like how to limit what can be downloaded, or even security levels that allows users or only the admin to point to a remote URL, etc) before we can jump into this implementation.

WDYT?

iamNoah1 commented 2 years ago

@toonsevrin friendly ping

k8s-triage-robot commented 1 year ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

strongjz commented 5 months ago

/priority/awaiting-more-evidence

rikatz commented 2 months ago

/close

k8s-ci-robot commented 2 months ago

@rikatz: Closing this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/5012#issuecomment-2227388712): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.