acl: Prevent tokens from deleting themselves (#5210)
Fixes #4897 Also apparently token deletion could segfault in secondary DCs when attempting to delete non-existant tokens. For that reason both checks are wrapped within the non-nil check.
This commit is contained in:
parent
cc14db1c26
commit
29b4512120
|
@ -482,10 +482,16 @@ func (a *ACL) TokenDelete(args *structs.ACLTokenDeleteRequest, reply *string) er
|
|||
return err
|
||||
}
|
||||
|
||||
if token != nil {
|
||||
if args.Token == token.SecretID {
|
||||
return fmt.Errorf("Deletion of the request's authorization token is not permitted")
|
||||
}
|
||||
|
||||
if !a.srv.InACLDatacenter() && !token.Local {
|
||||
args.Datacenter = a.srv.config.ACLDatacenter
|
||||
return a.srv.forwardDC("ACL.TokenDelete", a.srv.config.ACLDatacenter, args, reply)
|
||||
}
|
||||
}
|
||||
|
||||
req := &structs.ACLTokenBatchDeleteRequest{
|
||||
TokenIDs: []string{args.TokenID},
|
||||
|
|
|
@ -873,10 +873,31 @@ func TestACLEndpoint_TokenDelete(t *testing.T) {
|
|||
|
||||
testrpc.WaitForLeader(t, s1.RPC, "dc1")
|
||||
|
||||
dir2, s2 := testServerWithConfig(t, func(c *Config) {
|
||||
c.ACLDatacenter = "dc1"
|
||||
c.ACLsEnabled = true
|
||||
c.Datacenter = "dc2"
|
||||
// token replication is required to test deleting non-local tokens in secondary dc
|
||||
c.ACLTokenReplication = true
|
||||
})
|
||||
defer os.RemoveAll(dir2)
|
||||
defer s2.Shutdown()
|
||||
codec2 := rpcClient(t, s2)
|
||||
defer codec2.Close()
|
||||
|
||||
s2.tokens.UpdateACLReplicationToken("root")
|
||||
|
||||
testrpc.WaitForLeader(t, s1.RPC, "dc1")
|
||||
testrpc.WaitForLeader(t, s2.RPC, "dc2")
|
||||
|
||||
// Try to join
|
||||
joinWAN(t, s2, s1)
|
||||
|
||||
existingToken, err := upsertTestToken(codec, "root", "dc1")
|
||||
assert.NoError(err)
|
||||
|
||||
acl := ACL{srv: s1}
|
||||
acl2 := ACL{srv: s2}
|
||||
|
||||
// deletes a token
|
||||
{
|
||||
|
@ -897,6 +918,32 @@ func TestACLEndpoint_TokenDelete(t *testing.T) {
|
|||
assert.NoError(err)
|
||||
}
|
||||
|
||||
// can't delete itself
|
||||
{
|
||||
readReq := structs.ACLTokenGetRequest{
|
||||
Datacenter: "dc1",
|
||||
TokenID: "root",
|
||||
TokenIDType: structs.ACLTokenSecret,
|
||||
QueryOptions: structs.QueryOptions{Token: "root"},
|
||||
}
|
||||
|
||||
var out structs.ACLTokenResponse
|
||||
|
||||
err := msgpackrpc.CallWithCodec(codec, "ACL.TokenRead", &readReq, &out)
|
||||
|
||||
assert.NoError(err)
|
||||
|
||||
req := structs.ACLTokenDeleteRequest{
|
||||
Datacenter: "dc1",
|
||||
TokenID: out.Token.AccessorID,
|
||||
WriteRequest: structs.WriteRequest{Token: "root"},
|
||||
}
|
||||
|
||||
var resp string
|
||||
err = acl.TokenDelete(&req, &resp)
|
||||
assert.EqualError(err, "Deletion of the request's authorization token is not permitted")
|
||||
}
|
||||
|
||||
// errors when token doesn't exist
|
||||
{
|
||||
fakeID, err := uuid.GenerateUUID()
|
||||
|
@ -918,6 +965,30 @@ func TestACLEndpoint_TokenDelete(t *testing.T) {
|
|||
assert.Nil(tokenResp.Token)
|
||||
assert.NoError(err)
|
||||
}
|
||||
|
||||
// don't segfault when attempting to delete non existant token in secondary dc
|
||||
{
|
||||
fakeID, err := uuid.GenerateUUID()
|
||||
assert.NoError(err)
|
||||
|
||||
req := structs.ACLTokenDeleteRequest{
|
||||
Datacenter: "dc2",
|
||||
TokenID: fakeID,
|
||||
WriteRequest: structs.WriteRequest{Token: "root"},
|
||||
}
|
||||
|
||||
var resp string
|
||||
|
||||
waitForNewACLs(t, s2)
|
||||
|
||||
err = acl2.TokenDelete(&req, &resp)
|
||||
assert.NoError(err)
|
||||
|
||||
// token should be nil
|
||||
tokenResp, err := retrieveTestToken(codec2, "root", "dc1", existingToken.AccessorID)
|
||||
assert.Nil(tokenResp.Token)
|
||||
assert.NoError(err)
|
||||
}
|
||||
}
|
||||
func TestACLEndpoint_TokenDelete_anon(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
|
|
@ -9,6 +9,7 @@ import (
|
|||
"github.com/hashicorp/consul/testutil/retry"
|
||||
"github.com/hashicorp/raft"
|
||||
"github.com/hashicorp/serf/serf"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func waitForLeader(servers ...*Server) error {
|
||||
|
@ -152,6 +153,14 @@ func joinWAN(t *testing.T, member, leader *Server) {
|
|||
}
|
||||
}
|
||||
|
||||
func waitForNewACLs(t *testing.T, server *Server) {
|
||||
retry.Run(t, func(r *retry.R) {
|
||||
require.False(r, server.UseLegacyACLs(), "Server cannot use new ACLs")
|
||||
})
|
||||
|
||||
require.False(t, server.UseLegacyACLs(), "Server cannot use new ACLs")
|
||||
}
|
||||
|
||||
func seeEachOther(a, b []serf.Member, addra, addrb string) bool {
|
||||
return serfMembersContains(a, addrb) && serfMembersContains(b, addra)
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue