diff --git a/client/v3/retry_interceptor.go b/client/v3/retry_interceptor.go index 04f157a1dcb..7dc5ddae0fd 100644 --- a/client/v3/retry_interceptor.go +++ b/client/v3/retry_interceptor.go @@ -74,13 +74,7 @@ func (c *Client) unaryClientInterceptor(optFuncs ...retryOption) grpc.UnaryClien continue } if c.shouldRefreshToken(lastErr, callOpts) { - // clear auth token before refreshing it. - // call c.Auth.Authenticate with an invalid token will always fail the auth check on the server-side, - // if the server has not apply the patch of pr #12165 (https://github.com/etcd-io/etcd/pull/12165) - // and a rpctypes.ErrInvalidAuthToken will recursively call c.getToken until system run out of resource. - c.authTokenBundle.UpdateAuthToken("") - - gterr := c.getToken(ctx) + gterr := c.refreshToken(ctx) if gterr != nil { c.GetLogger().Warn( "retrying of unary invoker failed to fetch new auth token", @@ -161,6 +155,24 @@ func (c *Client) shouldRefreshToken(err error, callOpts *options) bool { (rpctypes.Error(err) == rpctypes.ErrInvalidAuthToken || rpctypes.Error(err) == rpctypes.ErrAuthOldRevision) } +func (c *Client) refreshToken(ctx context.Context) error { + if c.authTokenBundle == nil { + // c.authTokenBundle will be initialized only when + // c.Username != "" && c.Password != "". + // + // When users use the TLS CommonName based authentication, the + // authTokenBundle is always nil. But it's possible for the clients + // to get `rpctypes.ErrAuthOldRevision` response when the clients + // concurrently modify auth data (e.g, addUser, deleteUser etc.). + // In this case, there is no need to refresh the token; instead the + // clients just need to retry the operations (e.g. Put, Delete etc). + return nil + } + // clear auth token before refreshing it. + c.authTokenBundle.UpdateAuthToken("") + return c.getToken(ctx) +} + // type serverStreamingRetryingStream is the implementation of grpc.ClientStream that acts as a // proxy to the underlying call. If any of the RecvMsg() calls fail, it will try to reestablish // a new ClientStream according to the retry policy. @@ -259,10 +271,7 @@ func (s *serverStreamingRetryingStream) receiveMsgAndIndicateRetry(m interface{} return true, err } if s.client.shouldRefreshToken(err, s.callOpts) { - // clear auth token to avoid failure when call getToken - s.client.authTokenBundle.UpdateAuthToken("") - - gterr := s.client.getToken(s.ctx) + gterr := s.client.refreshToken(s.ctx) if gterr != nil { s.client.lg.Warn("retry failed to fetch new auth token", zap.Error(gterr)) return false, err // return the original error for simplicity diff --git a/tests/e2e/ctl_v3_auth_no_proxy_test.go b/tests/e2e/ctl_v3_auth_no_proxy_test.go index 0b4807c8304..17b4ecd486d 100644 --- a/tests/e2e/ctl_v3_auth_no_proxy_test.go +++ b/tests/e2e/ctl_v3_auth_no_proxy_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// These tests depends on certificate-based authentication that is NOT supported +// These tests depend on certificate-based authentication that is NOT supported // by gRPC proxy. //go:build !cluster_proxy // +build !cluster_proxy @@ -20,7 +20,10 @@ package e2e import ( + "fmt" + "sync" "testing" + "time" ) func TestCtlV3AuthCertCN(t *testing.T) { @@ -32,3 +35,106 @@ func TestCtlV3AuthCertCNAndUsername(t *testing.T) { func TestCtlV3AuthCertCNAndUsernameNoPassword(t *testing.T) { testCtl(t, authTestCertCNAndUsernameNoPassword, withCfg(*newConfigClientTLSCertAuth())) } + +func TestCtlV3AuthCertCNWithWithConcurrentOperation(t *testing.T) { + BeforeTest(t) + + // apply the certificate which has `root` CommonName, + // and reset the setting when the test case finishes. + // TODO(ahrtr): enhance the e2e test framework to support + // certificates with CommonName. + t.Log("Apply certificate with root CommonName") + resetCert := applyTLSWithRootCommonName() + defer resetCert() + + t.Log("Create an etcd cluster") + cx := getDefaultCtlCtx(t) + cx.cfg = etcdProcessClusterConfig{ + clusterSize: 1, + clientTLS: clientTLS, + clientCertAuthEnabled: true, + initialToken: "new", + } + + epc, err := newEtcdProcessCluster(t, &cx.cfg) + if err != nil { + t.Fatalf("Failed to start etcd cluster: %v", err) + } + cx.epc = epc + cx.dataDir = epc.procs[0].Config().dataDirPath + + defer func() { + if err := epc.Close(); err != nil { + t.Fatalf("could not close test cluster (%v)", err) + } + }() + + t.Log("Enable auth") + authEnableTest(cx) + + // Create two goroutines, one goroutine keeps creating & deleting users, + // and the other goroutine keeps writing & deleting K/V entries. + var wg sync.WaitGroup + wg.Add(2) + errs := make(chan error, 2) + donec := make(chan struct{}) + + // Create the first goroutine to create & delete users + t.Log("Create the first goroutine to create & delete users") + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + user := fmt.Sprintf("testuser-%d", i) + pass := fmt.Sprintf("testpass-%d", i) + + if err := ctlV3User(cx, []string{"add", user, "--interactive=false"}, fmt.Sprintf("User %s created", user), []string{pass}); err != nil { + errs <- fmt.Errorf("failed to create user %q: %w", user, err) + break + } + + err := ctlV3User(cx, []string{"delete", user}, fmt.Sprintf("User %s deleted", user), []string{}) + if err != nil { + errs <- fmt.Errorf("failed to delete user %q: %w", user, err) + break + } + } + t.Log("The first goroutine finished") + }() + + // Create the second goroutine to write & delete K/V entries + t.Log("Create the second goroutine to write & delete K/V entries") + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + key := fmt.Sprintf("key-%d", i) + value := fmt.Sprintf("value-%d", i) + + if err := ctlV3Put(cx, key, value, ""); err != nil { + errs <- fmt.Errorf("failed to put key %q: %w", key, err) + break + } + + if err := ctlV3Del(cx, []string{key}, 1); err != nil { + errs <- fmt.Errorf("failed to delete key %q: %w", key, err) + break + } + } + t.Log("The second goroutine finished") + }() + + t.Log("Waiting for the two goroutines to complete") + go func() { + wg.Wait() + close(donec) + }() + + t.Log("Waiting for test result") + select { + case err := <-errs: + t.Fatalf("Unexpected error: %v", err) + case <-donec: + t.Log("All done!") + case <-time.After(60 * time.Second): + t.Fatal("Test case timeout after 60 seconds") + } +} diff --git a/tests/e2e/ctl_v3_auth_test.go b/tests/e2e/ctl_v3_auth_test.go index 9d937c38154..0792087e062 100644 --- a/tests/e2e/ctl_v3_auth_test.go +++ b/tests/e2e/ctl_v3_auth_test.go @@ -19,6 +19,7 @@ import ( "fmt" "net/url" "os" + "path/filepath" "syscall" "testing" "time" @@ -100,6 +101,28 @@ func authEnable(cx ctlCtx) error { return nil } +func applyTLSWithRootCommonName() func() { + var ( + oldCertPath = certPath + oldPrivateKeyPath = privateKeyPath + oldCaPath = caPath + + newCertPath = filepath.Join(fixturesDir, "CommonName-root.crt") + newPrivateKeyPath = filepath.Join(fixturesDir, "CommonName-root.key") + newCaPath = filepath.Join(fixturesDir, "CommonName-root.crt") + ) + + certPath = newCertPath + privateKeyPath = newPrivateKeyPath + caPath = newCaPath + + return func() { + certPath = oldCertPath + privateKeyPath = oldPrivateKeyPath + caPath = oldCaPath + } +} + func ctlV3AuthEnable(cx ctlCtx) error { cmdArgs := append(cx.PrefixArgs(), "auth", "enable") return spawnWithExpectWithEnv(cmdArgs, cx.envMap, "Authentication Enabled") diff --git a/tests/fixtures/CommonName-root.crt b/tests/fixtures/CommonName-root.crt new file mode 100644 index 00000000000..d786c80d668 --- /dev/null +++ b/tests/fixtures/CommonName-root.crt @@ -0,0 +1,29 @@ +-----BEGIN CERTIFICATE----- +MIIE5zCCA8+gAwIBAgIJAKooGDZuR2mMMA0GCSqGSIb3DQEBCwUAMH8xCzAJBgNV +BAYTAkNOMRAwDgYDVQQIDAdCZWlqaW5nMRAwDgYDVQQHDAdCZWlqaW5nMQ0wCwYD +VQQKDAREZW1vMQ0wCwYDVQQLDAREZW1vMQ0wCwYDVQQDDARyb290MR8wHQYJKoZI +hvcNAQkBFhB0ZXN0QGV4YW1wbGUuY29tMB4XDTIyMTExNjA2NTI1M1oXDTMyMTEx +MzA2NTI1M1owfzELMAkGA1UEBhMCQ04xEDAOBgNVBAgMB0JlaWppbmcxEDAOBgNV +BAcMB0JlaWppbmcxDTALBgNVBAoMBERlbW8xDTALBgNVBAsMBERlbW8xDTALBgNV +BAMMBHJvb3QxHzAdBgkqhkiG9w0BCQEWEHRlc3RAZXhhbXBsZS5jb20wggEiMA0G +CSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDEAKcjzhtOG3hWbAUCbudE1gPOeteT +0INk2ngN2uCMYjYSZmaGhW/GZk3EvV7wKVuhdTyrh36E5Iajng9d2t1iOU/8jROU ++uAyrS3C/S5P/urq8VBUrt3VG/44bhwTEdafNnAWQ6ojYfmK0tRqoQn1Ftm30l8I +nWof5Jm3loNA2WdNdvAp/D+6OpjUdqGdMkFd0NhkuQODMnycBMw6btUTj5SnmrMk +q7V1aasx4BqN5C4DciZF0pyyR/TT8MoQ5Vcit8rHvQUyz42Lj8+28RkDoi4prJ1i +tLaCt2egDp58vXlYQZTd50inMhnBIapKNdGpg3flW/8AFul1tCTqd8NfAgMBAAGj +ggFkMIIBYDAdBgNVHQ4EFgQUpwwvEqXjA/ArJu1Jnpw7+/sttOAwgbMGA1UdIwSB +qzCBqIAUpwwvEqXjA/ArJu1Jnpw7+/sttOChgYSkgYEwfzELMAkGA1UEBhMCQ04x +EDAOBgNVBAgMB0JlaWppbmcxEDAOBgNVBAcMB0JlaWppbmcxDTALBgNVBAoMBERl +bW8xDTALBgNVBAsMBERlbW8xDTALBgNVBAMMBHJvb3QxHzAdBgkqhkiG9w0BCQEW +EHRlc3RAZXhhbXBsZS5jb22CCQCqKBg2bkdpjDAMBgNVHRMEBTADAQH/MAsGA1Ud +DwQEAwIC/DA2BgNVHREELzAtggtleGFtcGxlLmNvbYINKi5leGFtcGxlLmNvbYIJ +bG9jYWxob3N0hwR/AAABMDYGA1UdEgQvMC2CC2V4YW1wbGUuY29tgg0qLmV4YW1w +bGUuY29tgglsb2NhbGhvc3SHBH8AAAEwDQYJKoZIhvcNAQELBQADggEBAGi48ntm +8cn08FrsCDWapsck7a56/dyFyzLg10c0blu396tzC3ZDCAwQYzHjeXVdeWHyGO+f +KSFlmR6IA0jq6pFhUyJtgaAUJ91jW6s68GTVhlLoFhtYjy6EvhQ0lo+7GWh4qB2s +LI0mJPjaLZY1teAC4TswzwMDVD8QsB06/aFBlA65VjgZiVH+aMwWJ88gKfVGp0Pv +AApsy5MvwQn8WZ2L6foSY04OzXtmAg2gCl0PyDNgieqFDcM1g7mklHNgWl2Gvtte +G6+TiB3gGUUlTsdy0+LS2psL71RS5Jv7g/7XGmSKBPqRmYyQ2t7m2kLPwWKtL5tE +63c0FPtpV0FzKdU= +-----END CERTIFICATE----- diff --git a/tests/fixtures/CommonName-root.key b/tests/fixtures/CommonName-root.key new file mode 100644 index 00000000000..046b4a58fbd --- /dev/null +++ b/tests/fixtures/CommonName-root.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAxACnI84bTht4VmwFAm7nRNYDznrXk9CDZNp4DdrgjGI2EmZm +hoVvxmZNxL1e8ClboXU8q4d+hOSGo54PXdrdYjlP/I0TlPrgMq0twv0uT/7q6vFQ +VK7d1Rv+OG4cExHWnzZwFkOqI2H5itLUaqEJ9RbZt9JfCJ1qH+SZt5aDQNlnTXbw +Kfw/ujqY1HahnTJBXdDYZLkDgzJ8nATMOm7VE4+Up5qzJKu1dWmrMeAajeQuA3Im +RdKcskf00/DKEOVXIrfKx70FMs+Ni4/PtvEZA6IuKaydYrS2grdnoA6efL15WEGU +3edIpzIZwSGqSjXRqYN35Vv/ABbpdbQk6nfDXwIDAQABAoIBAA5AMebTjH6wVp6J ++g9EOwJxQROZMOVparRBgisXt+3dEitiUKAFQaw+MfdVAXsatrPVj1S1ZEiLSRLK +YjmjuSb0HdGx/DN/zh9BIiukNuLQGQp+AyY1FKHzCBfYQahNSrqGvb2Qq+UosXkb +fSBHly6/u5K28/vvXhD1kQudIOvtAc9tOg8LZnM6N3J4E0GtLqWimRZ4jNK4APu1 +YsLIg87Eam+7x25+phz9xc22tZ1H4WY9FnOGprPnievqiV7mgcNGAklTB93C6yX1 +EI+QxQnPg0P732C4EJZFDPqhVRA4E7BUb5uTIXCJBA/FFuRIx9ppyLZKt9vjTchM +8YWIEsECgYEA/5DRR9FkIWJZb0Pv3SCc53PMPT/xpYB6lH2lGtG+u+L71dJNDiPt +da3dPXSBy+aF7BbmRDawRvyOLGArlWiSsoEUVlES8BYzQ1MmfDf+MJooJoBE6/g6 +2OyyNnPde1GqyxsxgNTITvJCTjYH64lxKVRYfMgMAASK49SjYiEgGn8CgYEAxFXs +Oe0sUcc3P1cQ9pJfSVKpSczZq/OGAxqlniqRHvoWgFfKOWB6F9PN0rd8G2aMlfGS +BjyiPe770gtpX8Z4G4lrtkJD8NvGoVC8yX78HbrXL2RA4lPjQfrveUnwXIRbRKWa +6D/GAYPOuNvJmwF4hY/orWyIqvpNczIjTjs1JyECgYEAvhuNAn6JnKfbXYBM+tIa +xbWHFXzula2IAdOhMN0bpApKSZmBxmYFa0elTuTO9M2Li77RFacU5AlU/T+gzCiZ +D34jkb4Hd18cTRWaiEbiqGbUPSennVzu8ZTJUOZJuEVc5m9ZGLuwMcHWfvWEWLrJ +2fOrS09IVe8LHkV8MC/yAKMCgYBmDUdhgK9Fvqgv60Cs+b4/rZDDBJCsOUOSP3qQ +sQ2HrXSet4MsucIcuoJEog0HbRFsKwm85i1qxdrs/fOCzfXGUnLDZMRN4N7pIL9Q +eQnxJhoNzy2Otw3sUNPDFrSyUjXig7X2PJfeV7XPDqdHQ8dynS/TXRPY04wIcao6 +Uro5IQKBgFUz2GjAxI6uc7ihmRv/GYTuXYOlO0IN7MFwQDd0pVnWHkLNZscO9L9/ +ALV4g1p/75CewlQfyC8ynOJJWcDeHHFNsSMsOzAxUOVtVenaF/dgwk95wpXj6Rx6 +4kvQqnJg97fRBbyzvQcdL36kL8+pbmHNoqHPwxbuigYShB74d6/h +-----END RSA PRIVATE KEY-----