search

kubernetes / ingress-nginx

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

CommonName not being checked during certificate hostname validation when Subject Alt Name is given #6905

Closed mattmattox closed 2 years ago

mattmattox commented 3 years ago

NGINX Ingress controller version: 0.35.0

Kubernetes version (use kubectl version): 1.19.7

Environment:

What happened:

controller.go:1146] Unexpected error validating SSL certificate "default/test-cert" for server "rancher.example.com": x509: certificate is valid for *.rancher.example.com, *.rancher.local, not rancher.example.com
controller.go:1147] Validating certificate against DNS names. This will be deprecated in a future version.
controller.go:1152] SSL certificate "default/test-cert" does not contain a Common Name or Subject Alternative Name for server "rancher.example.com": x509: certificate is valid for *.rancher.example.com, *.rancher.local, not rancher.example.com

What you expected to happen:

ingress-nginx should check the common name first for a match then check the Subject Alternative Names.

How to reproduce it:

Install minikube/kind

Install the ingress controller

kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/master/deploy/static/provider/baremetal/deploy.yaml

Install an application that will act as default backend (is just an echo app)

kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/master/docs/examples/http-svc.yaml

Create a self-signed cert

echo " [ req ] prompt = no distinguished_name = server_distinguished_name req_extensions = v3_req

[ server_distinguished_name ] commonName = rancher.example.com stateOrProvinceName = IL countryName = US emailAddress = support@rancher.com organizationName = Rancher Labs organizationalUnitName = Rancher Support

[ v3_req ] basicConstraints = CA:FALSE keyUsage = nonRepudiation, digitalSignature, keyEncipherment subjectAltName = @alt_names

[ alt_names ] DNS.0 = .rancher.example.com DNS.1 = .rancher.local

" > req.conf openssl req -x509 -nodes -days 730 -newkey rsa:2048 -keyout tls.key -out tls.crt -config req.conf -extensions 'v3_req' kubectl create secret tls test-cert --cert=tls.crt --key=tls.key

Create an ingress

echo " apiVersion: networking.k8s.io/v1beta1 kind: Ingress metadata: name: foo-bar spec: rules:

make a request

POD_NAME=$(k get pods -n ingress-nginx -l app.kubernetes.io/name=ingress-nginx -o NAME) kubectl exec -it -n ingress-nginx $POD_NAME -- curl -H 'Host: rancher.example.com' https://localhost

You'll get the fake cert

Anything else we need to know:

This caused by https://github.com/kubernetes/ingress-nginx/blob/a268ec493c6a1d81d0932f9aeedd1781df2fe7b1/internal/ingress/controller/certificate.go#L59

/kind bug

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

oxr463 commented 3 years ago

/remove-lifecycle stale

iamNoah1 commented 3 years ago

@rikatz could you have a look at this one. There is also a PR, but as far as I understand, we don't want to do this. Also it is not considered being a bug.

/remove-kind bug /kind design

iamNoah1 commented 3 years ago

/kind design

k8s-triage-robot commented 3 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

MadhavJivrajani commented 3 years ago

/remove-kind design /kind feature kind/design is migrated to kind/feature, see https://github.com/kubernetes/community/issues/6144 for more details

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active 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 rotten

oxr463 commented 2 years ago

/remove-lifecycle rotten

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

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active 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 rotten

theunrealgeek commented 2 years ago

This problem is generally solved by having whatever is in the CN be also part of the SAN list.

From a standards perspective, the SANs list is supposed to be preferred (ref)

If a subjectAltName extension of type dNSName is present, that MUST be used as the identity. Otherwise, the (most specific) Common Name field in the Subject field of the certificate MUST be used. Although the use of the Common Name is existing practice, it is deprecated and Certification Authorities are encouraged to use the dNSName instead.

longwuyuan commented 2 years ago

Change the curl to send host header as foo.rancher.example.com . The behaviour posted is expected as per config and that is evident from the error message.

longwuyuan commented 2 years ago

/close

k8s-ci-robot commented 2 years ago

@longwuyuan: Closing this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/6905#issuecomment-1072949105): >/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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.