This repository was archived by the owner on Jun 23, 2022. It is now read-only.
etcd-signer-server: allow server to serve TLS based on SNI - #28
Merged
Merged
Conversation
abhinavdahiya
added a commit
to abhinavdahiya/installer
that referenced
this pull request
Apr 24, 2019
…i-int Requires etcd-signer-server to support multiple certs for serving [1] Currently the etcd-signer-server only serves request using `api-int` serving certificate which causes errors like, ```console time="2019-04-24T13:25:11-07:00" level=debug msg="Still waiting for the Kubernetes API: Get https://api.adahiya-0.tt.testing:6443/version?timeout=32s: x509: certificate is valid for api-int.adahiya-0.tt.testing, not api.adahiya-0.tt.testing" ``` as the installer is hitting the etcd-signer-server on api.$cluster_domain. Setting up etcd-signer-server to use multiple certs ie one for api and another for api-int allows both internal and external client to hot etcd-signer-server and not get x509 errors. [1]: coreos/kubecsr#28
wking
reviewed
Apr 24, 2019
| if cl, kl := len(serveOpts.sCrtFiles), len(serveOpts.sKeyFiles); cl == 0 || kl == 0 { | ||
| return errors.New("at least one pair of --servcrt and --servkey is required") | ||
| } else if cl != kl { | ||
| return errors.New("mismatch pairs of --servcrt and --servkey") |
Contributor
There was a problem hiding this comment.
This might possibly be read as "key A is not a valid key for cert A". Maybe rephrase to:
return errors.Errorf("%d --servercrt does not match %d --servkey", cl, kl)or something?
The previous cert used to test signing metrics certs expired on Apr 19 :( ``` openssl genrsa -out myCA.key 2048 openssl req -x509 -new -nodes -key myCA.key -sha256 -days 3650 -out myCA.pem You are about to be asked to enter information that will be incorporated into your certificate request. What you are about to enter is what is called a Distinguished Name or a DN. There are quite a few fields but you can leave some blank For some fields there will be a default value, If you enter '.', the field will be left blank. ----- Country Name (2 letter code) [XX]:US State or Province Name (full name) []:CA Locality Name (eg, city) [Default City]: Organization Name (eg, company) [Default Company Ltd]:fake-metrics-ca Organizational Unit Name (eg, section) []:fake-metrics-ca Common Name (eg, your name or your server's hostname) []: Email Address []: ``` The new one is valid for 10 years.
Currently the etcd-signer-server does not support serving TLS traffic based on SNI. Therefore, since openshift's kube-apiserver serves traffic on api.$cluster_domain and api-int.$cluster_domain [1] and because during bootstrapping when etcd-signer-server is mimic-ing the kube-apiserver to sign the etcd clients certificates it can only serve traffic on single domain, external clients trying to connect to `:6443` from api.$cluster_domain see errors like, ```console time="2019-04-24T13:25:11-07:00" level=debug msg="Still waiting for the Kubernetes API: Get https://api.adahiya-0.tt.testing:6443/version?timeout=32s: x509: certificate is valid for api-int.adahiya-0.tt.testing, not api.adahiya-0.tt.testing" ``` as the etcd-signer-server is using certs for `api-int.$cluster_domain` as internal clients ie etcd agent is contacting it on that domain, the external clients ie the installer hits the etcd-signer on `api.$cluster_domain` Allowing etcd-signer-server to accept multiple certs and serve TLS based on SNI allows it to correctly mimic the kube-apiserver's capability. [1]: openshift/installer#1633
wking
reviewed
Apr 25, 2019
| // ServerKeyFile is the file location of the server private key | ||
| ServerKeyFile string | ||
| // ServerCertKeys is a list of server certificates for serving on TLS based on SNI | ||
| ServerCertKeys []CertKey |
Contributor
There was a problem hiding this comment.
nit: Using []tls.Certificate here (and loading the cert/key pairs in cmd/kube-etcd-signer-server/serve.go) would let us simplify the pkg/certsigner API slightly by leaning on stdlib types instead of defining our own. But whatever, folks are unlikely to be using pkg/certsigner outside of this repository ;).
Contributor
|
/lgtm |
1 similar comment
Contributor
|
/lgtm |
Contributor
|
How do we get this picked up in openshift/kubecsr? |
Contributor
|
@wking I will take care of it in a few |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently the etcd-signer-server does not support serving TLS traffic based on SNI. Therefore, since openshift's kube-apiserver serves
traffic on api.$cluster_domain and api-int.$cluster_domain 1 and because during bootstrapping when etcd-signer-server is mimic-ing the kube-apiserver to
sign the etcd clients certificates it can only serve traffic on single domain, external clients trying to connect to
:6443from api.$cluster_domain see errors like,time="2019-04-24T13:25:11-07:00" level=debug msg="Still waiting for the Kubernetes API: Get https://api.adahiya-0.tt.testing:6443/version?timeout=32s: x509: certificate is valid for api-int.adahiya-0.tt.testing, not api.adahiya-0.tt.testing"as the etcd-signer-server is using certs for
api-int.$cluster_domainas internal clients ie etcd agent is contacting it on that domain, the external clients ie the installer hits the etcd-signer onapi.$cluster_domainAllowing etcd-signer-server to accept multiple certs and serve TLS based on SNI allows it to correctly mimic the kube-apiserver's capability.