*: disable CommonName auth for gRPC-gateway

CHANGELOG-3.3.md

@@ -26,6 +26,10 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.3.10...v3.3.11) an
 
 - Add [`etcd gateway --discovery-srv-name`](https://github.com/etcd-io/etcd/pull/10250) flag.
 
+### Security, Authentication
+
+- Disable [CommonName authentication for gRPC-gateway](https://github.com/etcd-io/etcd/pull/10366) gRPC-gateway proxy requests to etcd server use the etcd client server TLS certificate. If that certificate contains CommonName we do not want to use that for authentication as it could lead to permission escalation.
+
 ### Go
 
 - Compile with [*Go 1.10.7*](https://golang.org/doc/devel/release.html#go1.10).

Documentation/dev-guide/api_grpc_gateway.md

@@ -18,6 +18,8 @@ gRPC gateway endpoint has changed since etcd v3.3:
 - etcd v3.5 or later uses only `[CLIENT-URL]/v3/*`.
   - **`[CLIENT-URL]/v3beta/*` is deprecated**.
 
+gRPC-gateway does not support authentication using TLS Common Name.
+
 ### Put and get keys
 
 Use the `/v3/kv/range` and `/v3/kv/put` services to read and write keys:

Documentation/op-guide/authentication.md

@@ -167,7 +167,8 @@ $ etcdctl --user user --password password get foo
 Otherwise, all `etcdctl` commands remain the same. Users and roles can still be created and modified, but require authentication by a user with the root role.
 
 ## Using TLS Common Name
-As of version v3.2 if an etcd server is launched with the option `--client-cert-auth=true`, the field of Common Name (CN) in the client's TLS cert will be used as an etcd user. In this case, the common name authenticates the user and the client does not need a password. Note that if both of 1. `--client-cert-auth=true` is passed and CN is provided by the client, and 2. username and password are provided by the client, the username and password based authentication is prioritized.
+As of version v3.2 if an etcd server is launched with the option `--client-cert-auth=true`, the field of Common Name (CN) in the client's TLS cert will be used as an etcd user. In this case, the common name authenticates the user and the client does not need a password. Note that if both of 1. `--client-cert-auth=true` is passed and CN is provided by the client, and 2. username and password are provided by the client, the username and password based authentication is prioritized. gRPC-gateway does not support authentication using TLS Common Name.
 
 As of version v3.3 if an etcd server is launched with the option `--peer-cert-allowed-cn` filtering of CN inter-peer connections is enabled.  Nodes can only join the etcd cluster if their CN match the allowed one.
 See [etcd security page](https://github.com/etcd-io/etcd/blob/master/Documentation/op-guide/security.md) for more details.
+

Documentation/op-guide/configuration.md

@@ -261,6 +261,7 @@ The security flags help to [build a secure etcd cluster][security].
 + Enable client cert authentication.
 + default: false
 + env variable: ETCD_CLIENT_CERT_AUTH
++ CN authentication is not supported by gRPC-gateway.
 
 ### --client-crl-file
 + Path to the client certificate revocation list file.

auth/store.go

@@ -1166,6 +1166,27 @@ func (as *authStore) AuthInfoFromTLS(ctx context.Context) (ai *AuthInfo) {
 			Username: chains[0].Subject.CommonName,
 			Revision: as.Revision(),
 		}
+		md, ok := metadata.FromIncomingContext(ctx)
+		if !ok {
+			return nil
+		}
+
+		// gRPC-gateway proxy request to etcd server includes Grpcgateway-Accept
+		// header. The proxy uses etcd client server certificate. If the certificate
+		// has a CommonName we should never use this for authentication.
+		if gw := md["grpcgateway-accept"]; len(gw) > 0 {
+			if as.lg != nil {
+				as.lg.Warn(
+					"ignoring common name in gRPC-gateway proxy request",
+					zap.String("common-name", ai.Username),
+					zap.String("user-name", ai.Username),
+					zap.Uint64("revision", ai.Revision),
+				)
+			} else {
+				plog.Warningf("ignoring common name in gRPC-gateway proxy request %s", ai.Username)
+			}
+			return nil
+		}
 		if as.lg != nil {
 			as.lg.Debug(
 				"found command name",

tests/e2e/v3_curl_test.go

@@ -73,6 +73,11 @@ func TestV3CurlAuth(t *testing.T) {
 		testCtl(t, testV3CurlAuth, withApiPrefix(p))
 	}
 }
+func TestV3CurlAuthClientTLSCertAuth(t *testing.T) {
+	for _, p := range apiPrefix {
+		testCtl(t, testV3CurlAuth, withApiPrefix(p), withCfg(configClientTLSCertAuth))
+	}
+}
 
 func testV3CurlPutGet(cx ctlCtx) {
 	var (
@@ -179,12 +184,21 @@ func testV3CurlTxn(cx ctlCtx) {
 }
 
 func testV3CurlAuth(cx ctlCtx) {
+	p := cx.apiPrefix
+
 	// create root user
-	userreq, err := json.Marshal(&pb.AuthUserAddRequest{Name: string("root"), Password: string("toor")})
+	rootuser, err := json.Marshal(&pb.AuthUserAddRequest{Name: string("root"), Password: string("toor")})
 	testutil.AssertNil(cx.t, err)
 
-	p := cx.apiPrefix
-	if err = cURLPost(cx.epc, cURLReq{endpoint: path.Join(p, "/auth/user/add"), value: string(userreq), expected: "revision"}); err != nil {
+	if err = cURLPost(cx.epc, cURLReq{endpoint: path.Join(p, "/auth/user/add"), value: string(rootuser), expected: "revision"}); err != nil {
+		cx.t.Fatalf("failed testV3CurlAuth add user with curl (%v)", err)
+	}
+
+	// create non root user
+	nonrootuser, err := json.Marshal(&pb.AuthUserAddRequest{Name: string("example.com"), Password: string("example")})
+	testutil.AssertNil(cx.t, err)
+
+	if err = cURLPost(cx.epc, cURLReq{endpoint: path.Join(p, "/auth/user/add"), value: string(nonrootuser), expected: "revision"}); err != nil {
 		cx.t.Fatalf("failed testV3CurlAuth add user with curl (%v)", err)
 	}
 
@@ -197,10 +211,18 @@ func testV3CurlAuth(cx ctlCtx) {
 	}
 
 	// grant root role
-	grantrolereq, err := json.Marshal(&pb.AuthUserGrantRoleRequest{User: string("root"), Role: string("root")})
+	grantroleroot, err := json.Marshal(&pb.AuthUserGrantRoleRequest{User: string("root"), Role: string("root")})
+	testutil.AssertNil(cx.t, err)
+
+	if err = cURLPost(cx.epc, cURLReq{endpoint: path.Join(p, "/auth/user/grant"), value: string(grantroleroot), expected: "revision"}); err != nil {
+		cx.t.Fatalf("failed testV3CurlAuth grant role with curl using prefix (%s) (%v)", p, err)
+	}
+
+	// grant non root user root role
+	grantrole, err := json.Marshal(&pb.AuthUserGrantRoleRequest{User: string("example.com"), Role: string("root")})
 	testutil.AssertNil(cx.t, err)
 
-	if err = cURLPost(cx.epc, cURLReq{endpoint: path.Join(p, "/auth/user/grant"), value: string(grantrolereq), expected: "revision"}); err != nil {
+	if err = cURLPost(cx.epc, cURLReq{endpoint: path.Join(p, "/auth/user/grant"), value: string(grantrole), expected: "revision"}); err != nil {
 		cx.t.Fatalf("failed testV3CurlAuth grant role with curl using prefix (%s) (%v)", p, err)
 	}
 
@@ -243,7 +265,7 @@ func testV3CurlAuth(cx ctlCtx) {
 		cx.t.Fatalf("failed invalid token in authenticate response with curl")
 	}
 
-	authHeader = "Authorization : " + token
+	authHeader = "Authorization: " + token
 
 	// put with auth
 	if err = cURLPost(cx.epc, cURLReq{endpoint: path.Join(p, "/kv/put"), value: string(putreq), header: authHeader, expected: "revision"}); err != nil {