Merge pull request #11101 from hashicorp/dnephin/acl-legacy-remove-rpc-2

acl: remove legacy ACL.Apply RPC
This commit is contained in:
Daniel Nephin 2021-09-29 12:23:55 -04:00 committed by GitHub
commit 0330966315
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 81 additions and 442 deletions

View File

@ -2,141 +2,16 @@ package consul
import (
"fmt"
"time"
"github.com/armon/go-metrics"
"github.com/armon/go-metrics/prometheus"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib"
)
var ACLEndpointLegacySummaries = []prometheus.SummaryDefinition{
{
Name: []string{"acl", "apply"},
Help: "Measures the time it takes to complete an update to the ACL store.",
},
}
func (a *ACL) Bootstrap(*structs.DCSpecificRequest, *structs.ACL) error {
return fmt.Errorf("ACL.Bootstrap: the legacy ACL system has been removed")
}
// aclApplyInternal is used to apply an ACL request after it has been vetted that
// this is a valid operation. It is used when users are updating ACLs, in which
// case we check their token to make sure they have management privileges. It is
// also used for ACL replication. We want to run the replicated ACLs through the
// same checks on the change itself.
func aclApplyInternal(srv *Server, args *structs.ACLRequest, reply *string) error {
// All ACLs must have an ID by this point.
if args.ACL.ID == "" {
return fmt.Errorf("Missing ACL ID")
}
switch args.Op {
case structs.ACLSet:
// Verify the ACL type
switch args.ACL.Type {
case structs.ACLTokenTypeClient:
case structs.ACLTokenTypeManagement:
default:
return fmt.Errorf("Invalid ACL Type")
}
// No need to check expiration times as those did not exist in legacy tokens.
_, existing, _ := srv.fsm.State().ACLTokenGetBySecret(nil, args.ACL.ID, nil)
if existing != nil && existing.UsesNonLegacyFields() {
return fmt.Errorf("Cannot use legacy endpoint to modify a non-legacy token")
}
// Verify this is not a root ACL
if acl.RootAuthorizer(args.ACL.ID) != nil {
return acl.PermissionDeniedError{Cause: "Cannot modify root ACL"}
}
// Ensure that we allow more permissive rule formats for legacy tokens,
// but that we correct them on the way into the system.
//
// DEPRECATED (ACL-Legacy-Compat)
correctedRules := structs.SanitizeLegacyACLTokenRules(args.ACL.Rules)
if correctedRules != "" {
args.ACL.Rules = correctedRules
}
// Validate the rules compile
_, err := acl.NewPolicyFromSource("", 0, args.ACL.Rules, acl.SyntaxLegacy, srv.aclConfig, nil)
if err != nil {
return fmt.Errorf("ACL rule compilation failed: %v", err)
}
case structs.ACLDelete:
if args.ACL.ID == anonymousToken {
return acl.PermissionDeniedError{Cause: "Cannot delete anonymous token"}
}
default:
return fmt.Errorf("Invalid ACL Operation")
}
// Apply the update
resp, err := srv.raftApply(structs.ACLRequestType, args)
if err != nil {
return fmt.Errorf("raft apply failed: %w", err)
}
// Check if the return type is a string
if respString, ok := resp.(string); ok {
*reply = respString
}
return nil
}
// Apply is used to apply a modifying request to the data store. This should
// only be used for operations that modify the data
func (a *ACL) Apply(args *structs.ACLRequest, reply *string) error {
if done, err := a.srv.ForwardRPC("ACL.Apply", args, reply); done {
return err
}
defer metrics.MeasureSince([]string{"acl", "apply"}, time.Now())
// Verify we are allowed to serve this request
if !a.srv.config.ACLsEnabled {
return acl.ErrDisabled
}
// Verify token is permitted to modify ACLs
// NOTE: We will not support enterprise authorizer contexts with legacy ACLs
if authz, err := a.srv.ResolveToken(args.Token); err != nil {
return err
} else if authz.ACLWrite(nil) != acl.Allow {
return acl.ErrPermissionDenied
}
// If no ID is provided, generate a new ID. This must be done prior to
// appending to the Raft log, because the ID is not deterministic. Once
// the entry is in the log, the state update MUST be deterministic or
// the followers will not converge.
if args.Op == structs.ACLSet && args.ACL.ID == "" {
var err error
args.ACL.ID, err = lib.GenerateUUID(a.srv.checkTokenUUID)
if err != nil {
return err
}
}
// Do the apply now that this update is vetted.
if err := aclApplyInternal(a.srv, args, reply); err != nil {
return err
}
// Clear the cache if applicable
if args.ACL.ID != "" {
a.srv.acls.cache.RemoveIdentity(tokenSecretCacheID(args.ACL.ID))
}
return nil
func (a *ACL) Apply(*structs.ACLRequest, *string) error {
return fmt.Errorf("ACL.Apply: the legacy ACL system has been removed")
}
func (a *ACL) Get(*structs.ACLSpecificRequest, *structs.IndexedACLs) error {

View File

@ -74,252 +74,6 @@ func TestACLEndpoint_BootstrapTokens(t *testing.T) {
require.Equal(t, out.CreateIndex, out.ModifyIndex)
}
func TestACLEndpoint_Apply(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
_, srv, codec := testACLServerWithConfig(t, nil, false)
waitForLeaderEstablishment(t, srv)
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
Name: "User token",
Type: structs.ACLTokenTypeClient,
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var out string
err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
require.NoError(t, err)
id := out
// Verify
state := srv.fsm.State()
_, s, err := state.ACLTokenGetBySecret(nil, out, nil)
require.NoError(t, err)
require.NotNil(t, s)
require.Equal(t, out, s.SecretID)
require.Equal(t, "User token", s.Description)
// Do a delete
arg.Op = structs.ACLDelete
arg.ACL.ID = out
err = msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
require.NoError(t, err)
// Verify
_, s, err = state.ACLTokenGetBySecret(nil, id, nil)
require.NoError(t, err)
require.Nil(t, s)
}
func TestACLEndpoint_Update_PurgeCache(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
_, srv, codec := testACLServerWithConfig(t, nil, false)
waitForLeaderEstablishment(t, srv)
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
Name: "User token",
Type: structs.ACLTokenTypeClient,
Rules: `key "" { policy = "read"}`,
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var out string
err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
require.NoError(t, err)
id := out
// Resolve
acl1, err := srv.ResolveToken(id)
require.NoError(t, err)
require.NotNil(t, acl1)
require.Equal(t, acl.Allow, acl1.KeyRead("foo", nil))
// Do an update
arg.ACL.ID = out
arg.ACL.Rules = `{"key": {"": {"policy": "deny"}}}`
err = msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
require.NoError(t, err)
// Resolve again
acl2, err := srv.ResolveToken(id)
require.NoError(t, err)
require.NotNil(t, acl2)
require.NotSame(t, acl2, acl1)
require.NotEqual(t, acl.Allow, acl2.KeyRead("foo", nil))
// Do a delete
arg.Op = structs.ACLDelete
arg.ACL.Rules = ""
err = msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
require.NoError(t, err)
// Resolve again
acl3, err := srv.ResolveToken(id)
require.True(t, acl.IsErrNotFound(err), "Error %v is not acl.ErrNotFound", err)
require.Nil(t, acl3)
}
func TestACLEndpoint_Apply_CustomID(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
_, srv, codec := testACLServerWithConfig(t, nil, false)
waitForLeaderEstablishment(t, srv)
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
ID: "foobarbaz", // Specify custom ID, does not exist
Name: "User token",
Type: structs.ACLTokenTypeClient,
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var out string
err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
require.NoError(t, err)
require.Equal(t, "foobarbaz", out)
// Verify
state := srv.fsm.State()
_, s, err := state.ACLTokenGetBySecret(nil, out, nil)
require.NoError(t, err)
require.NotNil(t, s)
require.Equal(t, out, s.SecretID)
require.Equal(t, "User token", s.Description)
}
func TestACLEndpoint_Apply_Denied(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
_, srv, codec := testACLServerWithConfig(t, nil, false)
waitForLeaderEstablishment(t, srv)
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
Name: "User token",
Type: structs.ACLTokenTypeClient,
},
}
var out string
err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
require.True(t, acl.IsErrPermissionDenied(err), "Err %v is not acl.PermissionDenied", err)
}
func TestACLEndpoint_Apply_DeleteAnon(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
_, srv, codec := testACLServerWithConfig(t, nil, false)
waitForLeaderEstablishment(t, srv)
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLDelete,
ACL: structs.ACL{
ID: anonymousToken,
Name: "User token",
Type: structs.ACLTokenTypeClient,
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var out string
err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
testutil.RequireErrorContains(t, err, "delete anonymous")
}
func TestACLEndpoint_Apply_RootChange(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
_, srv, codec := testACLServerWithConfig(t, nil, false)
waitForLeaderEstablishment(t, srv)
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
ID: "manage",
Name: "User token",
Type: structs.ACLTokenTypeClient,
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var out string
err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
testutil.RequireErrorContains(t, err, "root ACL")
}
func TestACLEndpoint_GetPolicy(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
_, srv, codec := testACLServerWithConfig(t, nil, false)
waitForLeaderEstablishment(t, srv)
arg := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
Name: "User token",
Type: structs.ACLTokenTypeClient,
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var out string
err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out)
require.NoError(t, err)
getR := structs.ACLPolicyResolveLegacyRequest{
Datacenter: "dc1",
ACL: out,
}
var acls structs.ACLPolicyResolveLegacyResponse
retry.Run(t, func(r *retry.R) {
err := msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", &getR, &acls)
require.NoError(r, err)
require.NotNil(t, acls.Policy)
require.Equal(t, 30*time.Second, acls.TTL)
})
// Do a conditional lookup with etag
getR.ETag = acls.ETag
var out2 structs.ACLPolicyResolveLegacyResponse
require.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", &getR, &out2))
require.Nil(t, out2.Policy)
require.Equal(t, 30*time.Second, out2.TTL)
}
func TestACLEndpoint_GetPolicy_Management(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")

View File

@ -9,6 +9,8 @@ import (
"testing"
"time"
"github.com/hashicorp/go-uuid"
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
"github.com/mitchellh/copystructure"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -3771,3 +3773,80 @@ func TestACLResolver_ACLsEnabled(t *testing.T) {
}
}
func TestACLResolver_ResolveTokenToIdentityAndAuthorizer_UpdatesPurgeTheCache(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
_, srv, codec := testACLServerWithConfig(t, nil, false)
waitForLeaderEstablishment(t, srv)
reqPolicy := structs.ACLPolicySetRequest{
Datacenter: "dc1",
Policy: structs.ACLPolicy{
Name: "the-policy",
Rules: `key_prefix "" { policy = "read"}`,
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var respPolicy = structs.ACLPolicy{}
err := msgpackrpc.CallWithCodec(codec, "ACL.PolicySet", &reqPolicy, &respPolicy)
require.NoError(t, err)
token, err := uuid.GenerateUUID()
require.NoError(t, err)
reqToken := structs.ACLTokenSetRequest{
Datacenter: "dc1",
ACLToken: structs.ACLToken{
SecretID: token,
Policies: []structs.ACLTokenPolicyLink{{Name: "the-policy"}},
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var respToken structs.ACLToken
err = msgpackrpc.CallWithCodec(codec, "ACL.TokenSet", &reqToken, &respToken)
require.NoError(t, err)
runStep(t, "first resolve", func(t *testing.T) {
_, authz, err := srv.acls.ResolveTokenToIdentityAndAuthorizer(token)
require.NoError(t, err)
require.NotNil(t, authz)
require.Equal(t, acl.Allow, authz.KeyRead("foo", nil))
})
runStep(t, "update the policy and resolve again", func(t *testing.T) {
reqPolicy := structs.ACLPolicySetRequest{
Datacenter: "dc1",
Policy: structs.ACLPolicy{
ID: respPolicy.ID,
Name: "the-policy",
Rules: `{"key_prefix": {"": {"policy": "deny"}}}`,
},
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
err := msgpackrpc.CallWithCodec(codec, "ACL.PolicySet", &reqPolicy, &structs.ACLPolicy{})
require.NoError(t, err)
_, authz, err := srv.acls.ResolveTokenToIdentityAndAuthorizer(token)
require.NoError(t, err)
require.NotNil(t, authz)
require.Equal(t, acl.Deny, authz.KeyRead("foo", nil))
})
runStep(t, "delete the token", func(t *testing.T) {
req := structs.ACLTokenDeleteRequest{
Datacenter: "dc1",
TokenID: respToken.AccessorID,
WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken},
}
var resp string
err := msgpackrpc.CallWithCodec(codec, "ACL.TokenDelete", &req, &resp)
require.NoError(t, err)
_, _, err = srv.acls.ResolveTokenToIdentityAndAuthorizer(token)
require.True(t, acl.IsErrNotFound(err), "Error %v is not acl.ErrNotFound", err)
})
}

View File

@ -1208,73 +1208,6 @@ func TestLeader_ACL_Initialization(t *testing.T) {
}
}
func TestLeader_ACLUpgrade(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
t.Parallel()
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.ACLsEnabled = true
c.PrimaryDatacenter = "dc1"
c.ACLMasterToken = "root"
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
testrpc.WaitForTestAgent(t, s1.RPC, "dc1")
codec := rpcClient(t, s1)
defer codec.Close()
// create a legacy management ACL
mgmt := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
Name: "Management token",
Type: structs.ACLTokenTypeManagement,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
var mgmt_id string
require.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.Apply", &mgmt, &mgmt_id))
// wait for it to be upgraded
retry.Run(t, func(t *retry.R) {
_, token, err := s1.fsm.State().ACLTokenGetBySecret(nil, mgmt_id, nil)
require.NoError(t, err)
require.NotNil(t, token)
require.NotEqual(t, "", token.AccessorID)
require.Equal(t, structs.ACLTokenTypeManagement, token.Type)
require.Len(t, token.Policies, 1)
require.Equal(t, structs.ACLPolicyGlobalManagementID, token.Policies[0].ID)
})
// create a legacy management ACL
client := structs.ACLRequest{
Datacenter: "dc1",
Op: structs.ACLSet,
ACL: structs.ACL{
Name: "Management token",
Type: structs.ACLTokenTypeClient,
Rules: `node "" { policy = "read"}`,
},
WriteRequest: structs.WriteRequest{Token: "root"},
}
var client_id string
require.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.Apply", &client, &client_id))
// wait for it to be upgraded
retry.Run(t, func(t *retry.R) {
_, token, err := s1.fsm.State().ACLTokenGetBySecret(nil, client_id, nil)
require.NoError(t, err)
require.NotNil(t, token)
require.NotEqual(t, "", token.AccessorID)
require.Len(t, token.Policies, 0)
require.Equal(t, structs.ACLTokenTypeClient, token.Type)
require.Equal(t, client.ACL.Rules, token.Rules)
})
}
func TestLeader_ACLUpgrade_IsStickyEvenIfSerfTagsRegress(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")

View File

@ -296,7 +296,6 @@ func getPrometheusDefs(cfg lib.TelemetryConfig) ([]prometheus.GaugeDefinition, [
HTTPSummaries,
consul.ACLSummaries,
consul.ACLEndpointSummaries,
consul.ACLEndpointLegacySummaries,
consul.CatalogSummaries,
consul.FederationStateSummaries,
consul.IntentionSummaries,

View File

@ -329,7 +329,6 @@ These metrics are used to monitor the health of the Consul servers.
| Metric | Description | Unit | Type |
| --------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------- | ------- |
| `consul.acl.apply` | Measures the time it takes to complete an update to the ACL store. | ms | timer |
| `consul.acl.resolveTokenLegacy` | Measures the time it takes to resolve an ACL token using the legacy ACL system. | ms | timer |
| `consul.acl.ResolveToken` | Measures the time it takes to resolve an ACL token. | ms | timer |
| `consul.acl.ResolveTokenToIdentity` | Measures the time it takes to resolve an ACL token to an Identity. | ms | timer |