From 801bb4c6dff1847dda5490a1e6f61e5af80929a2 Mon Sep 17 00:00:00 2001 From: Benjamin Wang Date: Thu, 6 Apr 2023 12:58:01 +0800 Subject: [PATCH 1/2] test: add an e2e test to reproduce https://nvd.nist.gov/vuln/detail/CVE-2021-28235 Signed-off-by: Benjamin Wang --- tests/e2e/cmux_test.go | 9 ----- tests/e2e/ctl_v3_auth_security_test.go | 56 ++++++++++++++++++++++++++ tests/e2e/ctl_v3_test.go | 6 +++ tests/e2e/utils.go | 10 +++++ 4 files changed, 72 insertions(+), 9 deletions(-) create mode 100644 tests/e2e/ctl_v3_auth_security_test.go diff --git a/tests/e2e/cmux_test.go b/tests/e2e/cmux_test.go index ba1a73e49..4f97c2bf2 100644 --- a/tests/e2e/cmux_test.go +++ b/tests/e2e/cmux_test.go @@ -208,12 +208,3 @@ func fetchDebugVars(endpoint string, httpVersion string, connType e2e.ClientConn var resp map[string]interface{} return json.Unmarshal([]byte(respData), &resp) } - -func curl(endpoint string, method string, curlReq e2e.CURLReq, connType e2e.ClientConnType) (string, error) { - args := e2e.CURLPrefixArgs(endpoint, e2e.ClientConfig{ConnectionType: connType}, false, method, curlReq) - lines, err := e2e.RunUtilCompletion(args, nil) - if err != nil { - return "", err - } - return strings.Join(lines, "\n"), nil -} diff --git a/tests/e2e/ctl_v3_auth_security_test.go b/tests/e2e/ctl_v3_auth_security_test.go new file mode 100644 index 000000000..789c9b3cb --- /dev/null +++ b/tests/e2e/ctl_v3_auth_security_test.go @@ -0,0 +1,56 @@ +// Copyright 2023 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !cluster_proxy + +package e2e + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" + "go.etcd.io/etcd/tests/v3/framework/e2e" +) + +// TestAuth_CVE_2021_28235 verifies https://nvd.nist.gov/vuln/detail/CVE-2021-28235 +func TestAuth_CVE_2021_28235(t *testing.T) { + testCtl(t, authTest_CVE_2021_28235, withCfg(*e2e.NewConfigNoTLS()), withLogLevel("debug")) +} + +func authTest_CVE_2021_28235(cx ctlCtx) { + // create root user with root role + rootPass := "changeme123" + err := ctlV3User(cx, []string{"add", "root", "--interactive=false"}, "User root created", []string{rootPass}) + require.NoError(cx.t, err) + err = ctlV3User(cx, []string{"grant-role", "root", "root"}, "Role root is granted to user root", nil) + require.NoError(cx.t, err) + err = ctlV3AuthEnable(cx) + require.NoError(cx.t, err) + + // issue a put request + cx.user, cx.pass = "root", rootPass + err = ctlV3Put(cx, "foo", "bar", "") + require.NoError(cx.t, err) + + // GET /debug/requests + httpEndpoint := cx.epc.Procs[0].EndpointsHTTP()[0] + req := e2e.CURLReq{Endpoint: "/debug/requests?fam=grpc.Recv.etcdserverpb.Auth&b=0&exp=1", Timeout: 5} + respData, err := curl(httpEndpoint, "GET", req, e2e.ClientNonTLS) + require.NoError(cx.t, err) + + if strings.Contains(respData, rootPass) { + cx.t.Errorf("The root password is included in the request.\n %s", respData) + } +} diff --git a/tests/e2e/ctl_v3_test.go b/tests/e2e/ctl_v3_test.go index e4ce43f0a..a775ad3a7 100644 --- a/tests/e2e/ctl_v3_test.go +++ b/tests/e2e/ctl_v3_test.go @@ -202,6 +202,12 @@ func withMaxConcurrentStreams(streams uint32) ctlOption { } } +func withLogLevel(logLevel string) ctlOption { + return func(cx *ctlCtx) { + cx.cfg.LogLevel = logLevel + } +} + func testCtl(t *testing.T, testFunc func(ctlCtx), opts ...ctlOption) { testCtlWithOffline(t, testFunc, nil, opts...) } diff --git a/tests/e2e/utils.go b/tests/e2e/utils.go index 26a4c6086..581ff8c27 100644 --- a/tests/e2e/utils.go +++ b/tests/e2e/utils.go @@ -17,6 +17,7 @@ package e2e import ( "context" "fmt" + "strings" "testing" "time" @@ -97,3 +98,12 @@ func fillEtcdWithData(ctx context.Context, c *clientv3.Client, dbSize int) error } return g.Wait() } + +func curl(endpoint string, method string, curlReq e2e.CURLReq, connType e2e.ClientConnType) (string, error) { + args := e2e.CURLPrefixArgs(endpoint, e2e.ClientConfig{ConnectionType: connType}, false, method, curlReq) + lines, err := e2e.RunUtilCompletion(args, nil) + if err != nil { + return "", err + } + return strings.Join(lines, "\n"), nil +} From 8b1cd036ffa83204b29f53bd74fae190e9187781 Mon Sep 17 00:00:00 2001 From: Benjamin Wang Date: Thu, 6 Apr 2023 16:48:57 +0800 Subject: [PATCH 2/2] security: remove password after authenticating the user fix https://nvd.nist.gov/vuln/detail/CVE-2021-28235 Signed-off-by: Benjamin Wang --- server/etcdserver/v3_server.go | 7 +++++++ tests/e2e/ctl_v3_auth_security_test.go | 1 + 2 files changed, 8 insertions(+) diff --git a/server/etcdserver/v3_server.go b/server/etcdserver/v3_server.go index 4f1cd6b13..6fcc1b4a3 100644 --- a/server/etcdserver/v3_server.go +++ b/server/etcdserver/v3_server.go @@ -445,6 +445,13 @@ func (s *EtcdServer) Authenticate(ctx context.Context, r *pb.AuthenticateRequest lg := s.Logger() + // fix https://nvd.nist.gov/vuln/detail/CVE-2021-28235 + defer func() { + if r != nil { + r.Password = "" + } + }() + var resp proto.Message for { checkedRevision, err := s.AuthStore().CheckPassword(r.Name, r.Password) diff --git a/tests/e2e/ctl_v3_auth_security_test.go b/tests/e2e/ctl_v3_auth_security_test.go index 789c9b3cb..754fa4bc1 100644 --- a/tests/e2e/ctl_v3_auth_security_test.go +++ b/tests/e2e/ctl_v3_auth_security_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.etcd.io/etcd/tests/v3/framework/e2e" )