Merge pull request #6903 from mitake/auth-member
protect membership change RPCs with authrelease-3.1
commit
35fd5dc9fc
|
@ -29,6 +29,11 @@ func TestCtlV3AuthUserDeleteDuringOps(t *testing.T) { testCtl(t, authUserDeleteD
|
||||||
func TestCtlV3AuthRoleRevokeDuringOps(t *testing.T) { testCtl(t, authRoleRevokeDuringOpsTest) }
|
func TestCtlV3AuthRoleRevokeDuringOps(t *testing.T) { testCtl(t, authRoleRevokeDuringOpsTest) }
|
||||||
func TestCtlV3AuthTxn(t *testing.T) { testCtl(t, authTestTxn) }
|
func TestCtlV3AuthTxn(t *testing.T) { testCtl(t, authTestTxn) }
|
||||||
func TestCtlV3AuthPerfixPerm(t *testing.T) { testCtl(t, authTestPrefixPerm) }
|
func TestCtlV3AuthPerfixPerm(t *testing.T) { testCtl(t, authTestPrefixPerm) }
|
||||||
|
func TestCtlV3AuthMemberAdd(t *testing.T) { testCtl(t, authTestMemberAdd) }
|
||||||
|
func TestCtlV3AuthMemberRemove(t *testing.T) {
|
||||||
|
testCtl(t, authTestMemberRemove, withQuorum(), withNoStrictReconfig())
|
||||||
|
}
|
||||||
|
func TestCtlV3AuthMemberUpdate(t *testing.T) { testCtl(t, authTestMemberUpdate) }
|
||||||
|
|
||||||
func authEnableTest(cx ctlCtx) {
|
func authEnableTest(cx ctlCtx) {
|
||||||
if err := authEnable(cx); err != nil {
|
if err := authEnable(cx); err != nil {
|
||||||
|
@ -450,3 +455,91 @@ func authTestPrefixPerm(cx ctlCtx) {
|
||||||
cx.t.Fatal(err)
|
cx.t.Fatal(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func authTestMemberAdd(cx ctlCtx) {
|
||||||
|
if err := authEnable(cx); err != nil {
|
||||||
|
cx.t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
cx.user, cx.pass = "root", "root"
|
||||||
|
authSetupTestUser(cx)
|
||||||
|
|
||||||
|
peerURL := fmt.Sprintf("http://localhost:%d", etcdProcessBasePort+11)
|
||||||
|
// ordinal user cannot add a new member
|
||||||
|
cx.user, cx.pass = "test-user", "pass"
|
||||||
|
if err := ctlV3MemberAdd(cx, peerURL); err == nil {
|
||||||
|
cx.t.Fatalf("ordinal user must not be allowed to add a member")
|
||||||
|
}
|
||||||
|
|
||||||
|
// root can add a new member
|
||||||
|
cx.user, cx.pass = "root", "root"
|
||||||
|
if err := ctlV3MemberAdd(cx, peerURL); err != nil {
|
||||||
|
cx.t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func authTestMemberRemove(cx ctlCtx) {
|
||||||
|
if err := authEnable(cx); err != nil {
|
||||||
|
cx.t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
cx.user, cx.pass = "root", "root"
|
||||||
|
authSetupTestUser(cx)
|
||||||
|
|
||||||
|
n1 := cx.cfg.clusterSize
|
||||||
|
if n1 < 2 {
|
||||||
|
cx.t.Fatalf("%d-node is too small to test 'member remove'", n1)
|
||||||
|
}
|
||||||
|
resp, err := getMemberList(cx)
|
||||||
|
if err != nil {
|
||||||
|
cx.t.Fatal(err)
|
||||||
|
}
|
||||||
|
if n1 != len(resp.Members) {
|
||||||
|
cx.t.Fatalf("expected %d, got %d", n1, len(resp.Members))
|
||||||
|
}
|
||||||
|
|
||||||
|
var (
|
||||||
|
memIDToRemove = fmt.Sprintf("%x", resp.Header.MemberId)
|
||||||
|
clusterID = fmt.Sprintf("%x", resp.Header.ClusterId)
|
||||||
|
)
|
||||||
|
|
||||||
|
// ordinal user cannot remove a member
|
||||||
|
cx.user, cx.pass = "test-user", "pass"
|
||||||
|
if err = ctlV3MemberRemove(cx, memIDToRemove, clusterID); err == nil {
|
||||||
|
cx.t.Fatalf("ordinal user must not be allowed to remove a member")
|
||||||
|
}
|
||||||
|
|
||||||
|
// root can remove a member
|
||||||
|
cx.user, cx.pass = "root", "root"
|
||||||
|
if err = ctlV3MemberRemove(cx, memIDToRemove, clusterID); err != nil {
|
||||||
|
cx.t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func authTestMemberUpdate(cx ctlCtx) {
|
||||||
|
if err := authEnable(cx); err != nil {
|
||||||
|
cx.t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
cx.user, cx.pass = "root", "root"
|
||||||
|
authSetupTestUser(cx)
|
||||||
|
|
||||||
|
mr, err := getMemberList(cx)
|
||||||
|
if err != nil {
|
||||||
|
cx.t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// ordinal user cannot update a member
|
||||||
|
cx.user, cx.pass = "test-user", "pass"
|
||||||
|
peerURL := fmt.Sprintf("http://localhost:%d", etcdProcessBasePort+11)
|
||||||
|
memberID := fmt.Sprintf("%x", mr.Members[0].ID)
|
||||||
|
if err = ctlV3MemberUpdate(cx, memberID, peerURL); err == nil {
|
||||||
|
cx.t.Fatalf("ordinal user must not be allowed to update a member")
|
||||||
|
}
|
||||||
|
|
||||||
|
// root can update a member
|
||||||
|
cx.user, cx.pass = "root", "root"
|
||||||
|
if err = ctlV3MemberUpdate(cx, memberID, peerURL); err != nil {
|
||||||
|
cx.t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
@ -98,13 +98,16 @@ func ctlV3MemberRemove(cx ctlCtx, memberID, clusterID string) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
func memberAddTest(cx ctlCtx) {
|
func memberAddTest(cx ctlCtx) {
|
||||||
peerURL := fmt.Sprintf("http://localhost:%d", etcdProcessBasePort+11)
|
if err := ctlV3MemberAdd(cx, fmt.Sprintf("http://localhost:%d", etcdProcessBasePort+11)); err != nil {
|
||||||
cmdArgs := append(cx.PrefixArgs(), "member", "add", "newmember", fmt.Sprintf("--peer-urls=%s", peerURL))
|
|
||||||
if err := spawnWithExpect(cmdArgs, " added to cluster "); err != nil {
|
|
||||||
cx.t.Fatal(err)
|
cx.t.Fatal(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func ctlV3MemberAdd(cx ctlCtx, peerURL string) error {
|
||||||
|
cmdArgs := append(cx.PrefixArgs(), "member", "add", "newmember", fmt.Sprintf("--peer-urls=%s", peerURL))
|
||||||
|
return spawnWithExpect(cmdArgs, " added to cluster ")
|
||||||
|
}
|
||||||
|
|
||||||
func memberUpdateTest(cx ctlCtx) {
|
func memberUpdateTest(cx ctlCtx) {
|
||||||
mr, err := getMemberList(cx)
|
mr, err := getMemberList(cx)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
@ -112,8 +115,13 @@ func memberUpdateTest(cx ctlCtx) {
|
||||||
}
|
}
|
||||||
|
|
||||||
peerURL := fmt.Sprintf("http://localhost:%d", etcdProcessBasePort+11)
|
peerURL := fmt.Sprintf("http://localhost:%d", etcdProcessBasePort+11)
|
||||||
cmdArgs := append(cx.PrefixArgs(), "member", "update", fmt.Sprintf("%x", mr.Members[0].ID), fmt.Sprintf("--peer-urls=%s", peerURL))
|
memberID := fmt.Sprintf("%x", mr.Members[0].ID)
|
||||||
if err = spawnWithExpect(cmdArgs, " updated in cluster "); err != nil {
|
if err = ctlV3MemberUpdate(cx, memberID, peerURL); err != nil {
|
||||||
cx.t.Fatal(err)
|
cx.t.Fatal(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func ctlV3MemberUpdate(cx ctlCtx, memberID, peerURL string) error {
|
||||||
|
cmdArgs := append(cx.PrefixArgs(), "member", "update", memberID, fmt.Sprintf("--peer-urls=%s", peerURL))
|
||||||
|
return spawnWithExpect(cmdArgs, " updated in cluster ")
|
||||||
|
}
|
||||||
|
|
|
@ -1007,7 +1007,32 @@ func (s *EtcdServer) LeaderStats() []byte {
|
||||||
|
|
||||||
func (s *EtcdServer) StoreStats() []byte { return s.store.JsonStats() }
|
func (s *EtcdServer) StoreStats() []byte { return s.store.JsonStats() }
|
||||||
|
|
||||||
|
func (s *EtcdServer) checkMembershipOperationPermission(ctx context.Context) error {
|
||||||
|
if s.authStore == nil {
|
||||||
|
// In the context of ordinal etcd process, s.authStore will never be nil.
|
||||||
|
// This branch is for handling cases in server_test.go
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Note that this permission check is done in the API layer,
|
||||||
|
// so TOCTOU problem can be caused potentially in a schedule like this:
|
||||||
|
// update membership with user A -> revoke root role of A -> apply membership change
|
||||||
|
// in the state machine layer
|
||||||
|
// However, both of membership change and role management requires the root privilege.
|
||||||
|
// So careful operation by admins can prevent the problem.
|
||||||
|
authInfo, err := s.authInfoFromCtx(ctx)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
return s.AuthStore().IsAdminPermitted(authInfo)
|
||||||
|
}
|
||||||
|
|
||||||
func (s *EtcdServer) AddMember(ctx context.Context, memb membership.Member) error {
|
func (s *EtcdServer) AddMember(ctx context.Context, memb membership.Member) error {
|
||||||
|
if err := s.checkMembershipOperationPermission(ctx); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
if s.Cfg.StrictReconfigCheck {
|
if s.Cfg.StrictReconfigCheck {
|
||||||
// by default StrictReconfigCheck is enabled; reject new members if unhealthy
|
// by default StrictReconfigCheck is enabled; reject new members if unhealthy
|
||||||
if !s.cluster.IsReadyToAddNewMember() {
|
if !s.cluster.IsReadyToAddNewMember() {
|
||||||
|
@ -1034,6 +1059,10 @@ func (s *EtcdServer) AddMember(ctx context.Context, memb membership.Member) erro
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *EtcdServer) RemoveMember(ctx context.Context, id uint64) error {
|
func (s *EtcdServer) RemoveMember(ctx context.Context, id uint64) error {
|
||||||
|
if err := s.checkMembershipOperationPermission(ctx); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// by default StrictReconfigCheck is enabled; reject removal if leads to quorum loss
|
// by default StrictReconfigCheck is enabled; reject removal if leads to quorum loss
|
||||||
if err := s.mayRemoveMember(types.ID(id)); err != nil {
|
if err := s.mayRemoveMember(types.ID(id)); err != nil {
|
||||||
return err
|
return err
|
||||||
|
@ -1073,8 +1102,12 @@ func (s *EtcdServer) mayRemoveMember(id types.ID) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *EtcdServer) UpdateMember(ctx context.Context, memb membership.Member) error {
|
func (s *EtcdServer) UpdateMember(ctx context.Context, memb membership.Member) error {
|
||||||
b, err := json.Marshal(memb)
|
b, merr := json.Marshal(memb)
|
||||||
if err != nil {
|
if merr != nil {
|
||||||
|
return merr
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := s.checkMembershipOperationPermission(ctx); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
cc := raftpb.ConfChange{
|
cc := raftpb.ConfChange{
|
||||||
|
|
Loading…
Reference in New Issue