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

cherry-pick: etcd-signer-server: allow server to serve TLS based on SNI #14

Merged
merged 2 commits into from
Apr 26, 2019

Conversation

hexfusion
Copy link

This PR cherrypicks

added upstream via coreos#28

/cc @wking @abhinavdahiya

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
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 26, 2019
@wking
Copy link
Member

wking commented Apr 26, 2019

$ diff -u1 <(git show ccabab5) <(git show de9e5c8)
--- /dev/fd/63	2019-04-25 22:10:41.075006932 -0700
+++ /dev/fd/62	2019-04-25 22:10:41.075006932 -0700
@@ -1,2 +1,2 @@
-commit ccabab542d0584e9c806ace42def502b900c748d
+commit de9e5c8810b07d9631abae7168ce4b734d246883
 Author: Abhinav Dahiya <abhinav.dahiya@redhat.com>
@@ -76,3 +76,3 @@
 diff --git a/pkg/certsigner/signer.go b/pkg/certsigner/signer.go
-index ab45e22..2da6eb4 100644
+index 8f46dac..4a04fee 100644
 --- a/pkg/certsigner/signer.go
@@ -109,3 +109,3 @@
  	// EtcdMetricCertDuration
-@@ -439,7 +443,22 @@ func StartSignerServer(c Config) error {
+@@ -440,7 +444,22 @@ func StartSignerServer(c Config) error {
  	if err != nil {
$ diff -u1 <(git show ccabab5^) <(git show de9e5c8^)
--- /dev/fd/63	2019-04-25 22:11:11.521296304 -0700
+++ /dev/fd/62	2019-04-25 22:11:11.522296313 -0700
@@ -1,2 +1,2 @@
-commit 94cc8fcd669be58be394c89e668d2e4611aa7c78
+commit a832ebcf3340e77368c0a187e2602f9673494324
 Author: Abhinav Dahiya <abhinav.dahiya@redhat.com>

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 26, 2019
@abhinavdahiya
Copy link

/retest
/lgtm

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, hexfusion, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,hexfusion,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants