clientv3: Set authority used in cert checks to host of endpoint #11184

jpbetz · 2019-09-25T07:39:10Z

This is a workaround for the gPRC load balancer, which currently uses the "service name" (which the etcd client was setting to the 1st endpoint's hostname or IP) as authority for all credential checks for all the endpoints it load balances against, which does not work well for configurations where etcd servers use a Subject Alternative Names in their certs set to their hostname or IP. We previously added a workaround to fix this problem for IPs (db61ee1) but not hostnames.

The workaround keeps track of the endpoints the load balancer dials and overwrites the authority in ClientHandshake before the cert checks are performed so that the authority is always the host of the endpoint that the client was originally configured with. For example, if a client is configured with:

--endpoints=member1.etcd.xyz:2379,member2.etcd.xyz:2379,member3.etcd.xyz:2379"

The authorities of the endpoints will now be member1.etcd.xyz, member2.etcd.xyz and member3.etcd.xyz respectively. Previously, the authority of member1.etcd.xyz was used for all endpoints.

I've manually verified with etcdctl that this does result in the correct authority being used in DNS name SAN cert checks.

@gyuho @jingyih @wenjiaswe

clientv3/credentials/credentials.go

gyuho · 2019-09-25T21:59:49Z

clientv3/credentials/credentials.go

 	}
 }

 func (tc *transportCredential) OverrideServerName(serverNameOverride string) error {
 	return tc.gtc.OverrideServerName(serverNameOverride)
 }

+func (tc *transportCredential) Dialer(ctx context.Context, dialEp string) (net.Conn, error) {


So, gRPC first calls Dialer and by the time it calls, ClientHandshake, the remote address will always have been populated in addrToEndpoint, right?

Right, here's the dial call: https://github.com/grpc/grpc-go/blob/45bd2846a34b039c5f1e69b7202f118687156b34/internal/transport/http2_client.go#L167 and below it is the ClientHandshake call: https://github.com/grpc/grpc-go/blob/45bd2846a34b039c5f1e69b7202f118687156b34/internal/transport/http2_client.go#L212

And here's where the authority gets set: https://github.com/grpc/grpc-go/blob/230def769105c46b2e597578b23002a6ac159dde/clientconn.go#L1142.

We're basically just trying to get the authority set to an appropriate value for each endpoint instead of having a single value for the entire set of endpoints.

gyuho · 2019-09-25T22:13:18Z

clientv3/balancer/resolver/endpoint/endpoint.go

+
+// Dialer dials a endpoint using net.Dialer.
+// Context cancelation and timeout are supported.
+func Dialer(ctx context.Context, dialEp string) (net.Conn, error) {


Nice clean-up by moving code here!

gyuho

lgtm, thanks for the quick fix!

jpbetz · 2019-09-25T23:06:36Z

Great, let's hold off for just a bit before we merge to get feedback from the grpc team.

jingyih · 2019-09-25T23:20:04Z

Thanks for the quick fix! Implementation looks very clean.

jpbetz · 2019-10-03T17:22:17Z

Merging this to master. I'd like to improve test coverage before we backport to 3.4.

…-origin-release-3.4 Automated cherry pick of #11184

…-origin-release-3.3 Automated cherry pick of #11184

related etcd-io#11184 etcd-io#11211 etcd-io#11354

…after bumping grpc to 1.26.0.

This bumps the etcd dependency in order to pull in github.com/etcd-io/etcd/pull/11184 which is required to fix a gRPC load balancer issue when using a k8s service name to connect to etcd. Fixes: #9791 Signed-off-by: Thomas Graf <thomas@cilium.io>

[ upstream commit 26e3ca0 ] This bumps the etcd dependency in order to pull in github.com/etcd-io/etcd/pull/11184 which is required to fix a gRPC load balancer issue when using a k8s service name to connect to etcd. Fixes: #9791 Signed-off-by: Thomas Graf <thomas@cilium.io> Signed-off-by: Martynas Pumputis <m@lambda.lt>

…after bumping grpc to 1.26.0. Signed-off-by: Chao Chen <chaochn@amazon.com>

Backport [3.4] clientV3: simplify grpc dialer usage. Remove workaround #11184 after bumping grpc to 1.26.0.

gyuho added area/clientv3 backport/v3.4 labels Sep 25, 2019

gyuho self-assigned this Sep 25, 2019

jpbetz force-pushed the correct-authority-checks branch 2 times, most recently from a35b10a to 507f680 Compare September 25, 2019 21:40

jpbetz changed the title ~~WIP - clientv3: Set authority used in cert checks to host of endpoint~~ clientv3: Set authority used in cert checks to host of endpoint Sep 25, 2019

jpbetz commented Sep 25, 2019

View reviewed changes

clientv3/credentials/credentials.go Outdated Show resolved Hide resolved

gyuho reviewed Sep 25, 2019

View reviewed changes

jpbetz force-pushed the correct-authority-checks branch from 507f680 to c2702da Compare September 25, 2019 22:03

gyuho reviewed Sep 25, 2019

View reviewed changes

clientv3: Set authority used in cert checks to host of endpoint

97388ce

jpbetz force-pushed the correct-authority-checks branch from c2702da to 97388ce Compare September 25, 2019 22:14

jpbetz mentioned this pull request Sep 25, 2019

Use a custom authority for each connection in a single ClientConn grpc/grpc-go#3038

Closed

gyuho approved these changes Sep 25, 2019

View reviewed changes

jpbetz merged commit 6e74099 into etcd-io:master Oct 3, 2019

This was referenced Oct 3, 2019

Add integration TLS tests exercising SAN certs #11203

Closed

Automated cherry pick of #11184 #11214

Merged

Automated cherry pick of #11184 #11215

Merged

jpbetz added a commit that referenced this pull request Oct 9, 2019

Merge pull request #11214 from jpbetz/automated-cherry-pick-of-#11184…

99774d8

…-origin-release-3.4 Automated cherry pick of #11184

jpbetz added a commit that referenced this pull request Oct 9, 2019

Merge pull request #11215 from jpbetz/automated-cherry-pick-of-#11184…

ef61a56

…-origin-release-3.3 Automated cherry pick of #11184

This was referenced Oct 10, 2019

Upgrade to etcd 3.3.16 client kubernetes/kubernetes#83712

Closed

Update etcd client to 3.3.16 kubernetes/kubernetes#83735

Merged

Upgrade to etcd client 3.3.17 kubernetes/kubernetes#83801

Merged

jejer mentioned this pull request Oct 23, 2019

(error "EOF", ServerName "") error on etcd servers #10587

Closed

hexfusion mentioned this pull request Oct 23, 2019

[openshift-4.3]: bump etcd to v3.3.17 openshift/etcd#20

Merged

jingyih mentioned this pull request Nov 13, 2019

CHANGELOG: Add v3.4.3 #11354

Merged

hexfusion mentioned this pull request Nov 13, 2019

Bug 1772451: vendor: bump etcd, bbolt and gRPC-go openshift/origin#24149

Merged

spzala added a commit to spzala/etcd that referenced this pull request Dec 2, 2019

CHANGELOG: add missing changelogs

54edc8d

related etcd-io#11184 etcd-io#11211 etcd-io#11354

spzala mentioned this pull request Dec 2, 2019

CHANGELOG: add missing changelogs #11411

Merged

spzala added a commit to spzala/etcd that referenced this pull request Dec 2, 2019

CHANGELOG: add missing changelogs

a45b997

related etcd-io#11184 etcd-io#11211 etcd-io#11354

jpbetz mentioned this pull request Jan 14, 2020

Simplify grpc dialer usage in clientv3 #11519

Closed

YoyinZyc added a commit to YoyinZyc/etcd that referenced this pull request Jan 30, 2020

clientV3: simplify grpc dialer usage. Remove workaround etcd-io#11184 …

b90d801

…after bumping grpc to 1.26.0.

YoyinZyc mentioned this pull request Jan 31, 2020

clientv3: simplify grpc dialer usage. #11574

Merged

tgraf mentioned this pull request Feb 28, 2020

etcd: Fix gRPC load balancer issue cilium/cilium#10381

Merged

harshavardhana mentioned this pull request Dec 3, 2020

fix: etcd import paths again depend on v3.4.14 release minio/minio#11020

Merged

6 tasks

just1900 mentioned this pull request Mar 4, 2022

fix(cdc): failed to start when connects to pd with multiple endpoints and the 1st endpoint is not available pingcap/tiflow#4779

Merged

chaochn47 pushed a commit to chaochn47/etcd that referenced this pull request Oct 25, 2023

clientV3: simplify grpc dialer usage. Remove workaround etcd-io#11184 …

cbf8fbd

…after bumping grpc to 1.26.0. Signed-off-by: Chao Chen <chaochn@amazon.com>

chaochn47 pushed a commit to chaochn47/etcd that referenced this pull request Oct 27, 2023

clientV3: simplify grpc dialer usage. Remove workaround etcd-io#11184 …

0549566

…after bumping grpc to 1.26.0. Signed-off-by: Chao Chen <chaochn@amazon.com>

chaochn47 pushed a commit to chaochn47/etcd that referenced this pull request Oct 27, 2023

clientV3: simplify grpc dialer usage. Remove workaround etcd-io#11184 …

03f7900

…after bumping grpc to 1.26.0. Signed-off-by: Chao Chen <chaochn@amazon.com>

ahrtr added a commit that referenced this pull request Oct 27, 2023

Merge pull request #16842 from chaochn47/release-3.4-grpc-dialer

6100220

Backport [3.4] clientV3: simplify grpc dialer usage. Remove workaround #11184 after bumping grpc to 1.26.0.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clientv3: Set authority used in cert checks to host of endpoint #11184

clientv3: Set authority used in cert checks to host of endpoint #11184

jpbetz commented Sep 25, 2019 •

edited

Loading

gyuho Sep 25, 2019

jpbetz Sep 25, 2019

jpbetz Sep 25, 2019

gyuho Sep 25, 2019

gyuho left a comment

jpbetz commented Sep 25, 2019

jingyih commented Sep 25, 2019

jpbetz commented Oct 3, 2019

clientv3: Set authority used in cert checks to host of endpoint #11184

clientv3: Set authority used in cert checks to host of endpoint #11184

Conversation

jpbetz commented Sep 25, 2019 • edited Loading

gyuho Sep 25, 2019

Choose a reason for hiding this comment

jpbetz Sep 25, 2019

Choose a reason for hiding this comment

jpbetz Sep 25, 2019

Choose a reason for hiding this comment

gyuho Sep 25, 2019

Choose a reason for hiding this comment

gyuho left a comment

Choose a reason for hiding this comment

jpbetz commented Sep 25, 2019

jingyih commented Sep 25, 2019

jpbetz commented Oct 3, 2019

jpbetz commented Sep 25, 2019 •

edited

Loading