luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Adds missing token redact in the GET path.

This commit is contained in:
James Phillips 2016-02-26 15:59:00 -08:00
parent 0180e877de
commit f0150ff5ce
4 changed files with 145 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
consul/acl.go
View File

@ -370,14 +370,37 @@ func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) {
*dump = nd
}
// redactPreparedQueryTokens will redact any tokens unless the client has a
// management token. This eases the transition to delegated authority over
// prepared queries, since it was easy to capture management tokens in Consul
// 0.6.3 and earlier, and we don't want to willy-nilly show those. This does
// have the limitation of preventing delegated non-management users from seeing
// captured tokens, but they can at least see whether or not a token is set.
func (f *aclFilter) redactPreparedQueryTokens(query **structs.PreparedQuery) {
// Management tokens can see everything with no filtering.
if f.acl.ACLList() {
return
}
// Let the user see if there's a blank token, otherwise we need
// to redact it, since we know they don't have a management
// token.
if (*query).Token != "" {
// Redact the token, using a copy of the query structure
// since we could be pointed at a live instance from the
// state store so it's not safe to modify it. Note that
// this clone will still point to things like underlying
// arrays in the original, but for modifying just the
// token it will be safe to use.
clone := *(*query)
clone.Token = redactedToken
*query = &clone
}
}
// filterPreparedQueries is used to filter prepared queries based on ACL rules.
// We prune entries the user doesn't have access to, and we redact any tokens
// unless the client has a management token. This eases the transition to
// delegated authority over prepared queries, since it was easy to capture
// management tokens in Consul 0.6.3 and earlier, and we don't want to
// willy-nilly show those. This does have the limitation of preventing delegated
// non-management users from seeing captured tokens, but they can at least see
// that they are set.
// if the user doesn't have a management token.
func (f *aclFilter) filterPreparedQueries(queries *structs.PreparedQueries) {
// Management tokens can see everything with no filtering.
if f.acl.ACLList() {
@ -396,22 +419,11 @@ func (f *aclFilter) filterPreparedQueries(queries *structs.PreparedQueries) {
continue
}
// Let the user see if there's a blank token, otherwise we need
// to redact it, since we know they don't have a management
// token.
if query.Token == "" {
ret = append(ret, query)
} else {
// Redact the token, using a copy of the query structure
// since we could be pointed at a live instance from the
// state store so it's not safe to modify it. Note that
// this clone will still point to things like underlying
// arrays in the original, but for modifying just the
// token it will be safe to use.
clone := *query
clone.Token = redactedToken
ret = append(ret, &clone)
}
// Redact any tokens if necessary. We make a copy of just the
// pointer so we don't mess with the caller's slice.
final := query
f.redactPreparedQueryTokens(&final)
ret = append(ret, final)
}
*queries = ret
}
@ -461,6 +473,9 @@ func (s *Server) filterACL(token string, subj interface{}) error {
case *structs.IndexedPreparedQueries:
filt.filterPreparedQueries(&v.Queries)
case **structs.PreparedQuery:
filt.redactPreparedQueryTokens(v)
default:
panic(fmt.Errorf("Unhandled type passed to ACL filter: %#v", subj))
}

38
consul/acl_test.go
View File

@ -862,6 +862,44 @@ func TestACL_filterNodeDump(t *testing.T) {
}
}
func TestACL_redactPreparedQueryTokens(t *testing.T) {
query := &structs.PreparedQuery{
ID: "f004177f-2c28-83b7-4229-eacc25fe55d1",
Token: "root",
}
expected := &structs.PreparedQuery{
ID: "f004177f-2c28-83b7-4229-eacc25fe55d1",
Token: "root",
}
// Try permissive filtering with a management token. This will allow the
// embedded token to be seen.
filt := newAclFilter(acl.ManageAll(), nil)
filt.redactPreparedQueryTokens(&query)
if !reflect.DeepEqual(query, expected) {
t.Fatalf("bad: %#v", &query)
}
// Hang on to the entry with a token, which needs to survive the next
// operation.
original := query
// Now try permissive filtering with a client token, which should cause
// the embedded token to get redacted.
filt = newAclFilter(acl.AllowAll(), nil)
filt.redactPreparedQueryTokens(&query)
expected.Token = redactedToken
if !reflect.DeepEqual(query, expected) {
t.Fatalf("bad: %#v", *query)
}
// Make sure that the original object didn't lose its token.
if original.Token != "root" {
t.Fatalf("bad token: %s", original.Token)
}
}
func TestACL_filterPreparedQueries(t *testing.T) {
queries := structs.PreparedQueries{
&structs.PreparedQuery{

5
consul/prepared_query_endpoint.go
View File

@ -219,11 +219,12 @@ func (p *PreparedQuery) Get(args *structs.PreparedQuerySpecificRequest,
}
// If no prefix ACL applies to this query, then they are
// always allowed to see it if they have the ID.
// always allowed to see it if they have the ID. We still
// have to filter the remaining object for tokens.
reply.Index = index
reply.Queries = structs.PreparedQueries{query}
if _, ok := query.GetACLPrefix(); !ok {
return nil
return p.srv.filterACL(args.Token, &reply.Queries[0])
}
// Otherwise, attempt to filter it the usual way.

69
consul/prepared_query_endpoint_test.go
View File

@ -711,7 +711,6 @@ func TestPreparedQuery_Get(t *testing.T) {
// Try again with no token, this should work since this query is only
// managed by an ID (no name) so no ACLs apply to it.
query.Query.ID = reply
{
req := &structs.PreparedQuerySpecificRequest{
Datacenter: "dc1",
@ -736,6 +735,65 @@ func TestPreparedQuery_Get(t *testing.T) {
}
}
// Capture a token.
query.Op = structs.PreparedQueryUpdate
query.Query.Token = "le-token"
if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil {
t.Fatalf("err: %v", err)
}
// This should get redacted when we read it back without a token.
query.Query.Token = redactedToken
{
req := &structs.PreparedQuerySpecificRequest{
Datacenter: "dc1",
QueryID: query.Query.ID,
QueryOptions: structs.QueryOptions{Token: ""},
}
var resp structs.IndexedPreparedQueries
if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil {
t.Fatalf("err: %v", err)
}
if len(resp.Queries) != 1 {
t.Fatalf("bad: %v", resp)
}
actual := resp.Queries[0]
if resp.Index != actual.ModifyIndex {
t.Fatalf("bad index: %d", resp.Index)
}
actual.CreateIndex, actual.ModifyIndex = 0, 0
if !reflect.DeepEqual(actual, query.Query) {
t.Fatalf("bad: %v", actual)
}
}
// But a management token should be able to see it.
query.Query.Token = "le-token"
{
req := &structs.PreparedQuerySpecificRequest{
Datacenter: "dc1",
QueryID: query.Query.ID,
QueryOptions: structs.QueryOptions{Token: "root"},
}
var resp structs.IndexedPreparedQueries
if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Get", req, &resp); err != nil {
t.Fatalf("err: %v", err)
}
if len(resp.Queries) != 1 {
t.Fatalf("bad: %v", resp)
}
actual := resp.Queries[0]
if resp.Index != actual.ModifyIndex {
t.Fatalf("bad index: %d", resp.Index)
}
actual.CreateIndex, actual.ModifyIndex = 0, 0
if !reflect.DeepEqual(actual, query.Query) {
t.Fatalf("bad: %v", actual)
}
}
// Try to get an unknown ID.
{
req := &structs.PreparedQuerySpecificRequest{
@ -815,6 +873,7 @@ func TestPreparedQuery_List(t *testing.T) {
Op: structs.PreparedQueryCreate,
Query: &structs.PreparedQuery{
Name: "redis-master",
Token: "le-token",
Service: structs.ServiceQuery{
Service: "the-redis",
},
@ -826,8 +885,10 @@ func TestPreparedQuery_List(t *testing.T) {
t.Fatalf("err: %v", err)
}
// Capture the ID and read back the query to verify.
// Capture the ID and read back the query to verify. We also make sure
// the captured token gets redacted.
query.Query.ID = reply
query.Query.Token = redactedToken
{
req := &structs.DCSpecificRequest{
Datacenter: "dc1",
@ -868,7 +929,9 @@ func TestPreparedQuery_List(t *testing.T) {
}
}
// But a management token should work.
// But a management token should work, and be able to see the captured
// token.
query.Query.Token = "le-token"
{
req := &structs.DCSpecificRequest{
Datacenter: "dc1",