Skip to content

Commit

Permalink
Log auth info on permission denied due to ACL (#2754)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bkrodgers @jefferai
bkrodgers authored and jefferai committed Jun 5, 2017
1 parent 043b3f8 commit d4fb262
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 40 deletions.
20 changes: 16 additions & 4 deletions audit/format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,17 @@ func (f *AuditFormatter) FormatRequest(

// Hash any sensitive information
if auth != nil {
// Cache and restore accessor in the auth
var authAccessor string
if !config.HMACAccessor && auth.Accessor != "" {
authAccessor = auth.Accessor
}
if err := Hash(salt, auth); err != nil {
return err
}
if authAccessor != "" {
auth.Accessor = authAccessor
}
}

// Cache and restore accessor in the request
Expand Down Expand Up @@ -110,6 +118,8 @@ func (f *AuditFormatter) FormatRequest(
Error: errString,

Auth: AuditAuth{
ClientToken: auth.ClientToken,
Accessor: auth.Accessor,
DisplayName: auth.DisplayName,
Policies: auth.Policies,
Metadata: auth.Metadata,
Expand Down Expand Up @@ -297,11 +307,13 @@ func (f *AuditFormatter) FormatResponse(
respEntry := &AuditResponseEntry{
Type: "response",
Error: errString,

Auth: AuditAuth{
DisplayName: auth.DisplayName,
Policies: auth.Policies,
Metadata: auth.Metadata,
ClientToken: auth.ClientToken,
Accessor: auth.Accessor,
DisplayName: auth.DisplayName,
Policies: auth.Policies,
Metadata: auth.Metadata,
RemainingUses: req.ClientTokenRemainingUses,
},

Request: AuditRequest{
Expand Down
33 changes: 20 additions & 13 deletions audit/format_json_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"errors"

"fmt"
"github.com/hashicorp/vault/helper/jsonutil"
"github.com/hashicorp/vault/helper/salt"
"github.com/hashicorp/vault/logical"
Expand All @@ -22,15 +23,18 @@ func TestFormatJSON_formatRequest(t *testing.T) {
saltFunc := func() (*salt.Salt, error) {
return salter, nil
}

expectedResultStr := fmt.Sprintf(testFormatJSONReqBasicStrFmt, salter.GetIdentifiedHMAC("foo"))

cases := map[string]struct {
Auth *logical.Auth
Req *logical.Request
Err error
Prefix string
Result string
Auth *logical.Auth
Req *logical.Request
Err error
Prefix string
ExpectedStr string
}{
"auth, request": {
&logical.Auth{ClientToken: "foo", Policies: []string{"root"}},
&logical.Auth{ClientToken: "foo", Accessor: "bar", DisplayName: "testtoken", Policies: []string{"root"}},
&logical.Request{
Operation: logical.UpdateOperation,
Path: "/foo",
Expand All @@ -46,10 +50,10 @@ func TestFormatJSON_formatRequest(t *testing.T) {
},
errors.New("this is an error"),
"",
testFormatJSONReqBasicStr,
expectedResultStr,
},
"auth, request with prefix": {
&logical.Auth{ClientToken: "foo", Policies: []string{"root"}},
&logical.Auth{ClientToken: "foo", Accessor: "bar", DisplayName: "testtoken", Policies: []string{"root"}},
&logical.Request{
Operation: logical.UpdateOperation,
Path: "/foo",
Expand All @@ -65,7 +69,7 @@ func TestFormatJSON_formatRequest(t *testing.T) {
},
errors.New("this is an error"),
"@cee: ",
testFormatJSONReqBasicStr,
expectedResultStr,
},
}

Expand All @@ -77,17 +81,20 @@ func TestFormatJSON_formatRequest(t *testing.T) {
SaltFunc: saltFunc,
},
}
config := FormatterConfig{}
config := FormatterConfig{
HMACAccessor: false,
}
if err := formatter.FormatRequest(&buf, config, tc.Auth, tc.Req, tc.Err); err != nil {
t.Fatalf("bad: %s\nerr: %s", name, err)
}

if !strings.HasPrefix(buf.String(), tc.Prefix) {
t.Fatalf("no prefix: %s \n log: %s\nprefix: %s", name, tc.Result, tc.Prefix)
t.Fatalf("no prefix: %s \n log: %s\nprefix: %s", name, expectedResultStr, tc.Prefix)
}

var expectedjson = new(AuditRequestEntry)
if err := jsonutil.DecodeJSON([]byte(tc.Result), &expectedjson); err != nil {

if err := jsonutil.DecodeJSON([]byte(expectedResultStr), &expectedjson); err != nil {
t.Fatalf("bad json: %s", err)
}

Expand All @@ -111,5 +118,5 @@ func TestFormatJSON_formatRequest(t *testing.T) {
}
}

const testFormatJSONReqBasicStr = `{"time":"2015-08-05T13:45:46Z","type":"request","auth":{"display_name":"","policies":["root"],"metadata":null},"request":{"operation":"update","path":"/foo","data":null,"wrap_ttl":60,"remote_address":"127.0.0.1","headers":{"foo":["bar"]}},"error":"this is an error"}
const testFormatJSONReqBasicStrFmt = `{"time":"2015-08-05T13:45:46Z","type":"request","auth":{"client_token":"%s","accessor":"bar","display_name":"testtoken","policies":["root"],"metadata":null},"request":{"operation":"update","path":"/foo","data":null,"wrap_ttl":60,"remote_address":"127.0.0.1","headers":{"foo":["bar"]}},"error":"this is an error"}
`
33 changes: 20 additions & 13 deletions audit/format_jsonx_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"errors"

"fmt"
"github.com/hashicorp/vault/helper/salt"
"github.com/hashicorp/vault/logical"
)
Expand All @@ -20,16 +21,19 @@ func TestFormatJSONx_formatRequest(t *testing.T) {
saltFunc := func() (*salt.Salt, error) {
return salter, nil
}

fooSalted := salter.GetIdentifiedHMAC("foo")

cases := map[string]struct {
Auth *logical.Auth
Req *logical.Request
Err error
Prefix string
Result string
Expected string
Auth *logical.Auth
Req *logical.Request
Err error
Prefix string
Result string
ExpectedStr string
}{
"auth, request": {
&logical.Auth{ClientToken: "foo", Policies: []string{"root"}},
&logical.Auth{ClientToken: "foo", Accessor: "bar", DisplayName: "testtoken", Policies: []string{"root"}},
&logical.Request{
Operation: logical.UpdateOperation,
Path: "/foo",
Expand All @@ -46,10 +50,11 @@ func TestFormatJSONx_formatRequest(t *testing.T) {
errors.New("this is an error"),
"",
"",
`<json:object name="auth"><json:string name="accessor"></json:string><json:string name="client_token"></json:string><json:string name="display_name"></json:string><json:null name="metadata" /><json:array name="policies"><json:string>root</json:string></json:array></json:object><json:string name="error">this is an error</json:string><json:object name="request"><json:string name="client_token"></json:string><json:string name="client_token_accessor"></json:string><json:null name="data" /><json:object name="headers"><json:array name="foo"><json:string>bar</json:string></json:array></json:object><json:string name="id"></json:string><json:string name="operation">update</json:string><json:string name="path">/foo</json:string><json:string name="remote_address">127.0.0.1</json:string><json:number name="wrap_ttl">60</json:number></json:object><json:string name="type">request</json:string>`,
fmt.Sprintf(`<json:object name="auth"><json:string name="accessor">bar</json:string><json:string name="client_token">%s</json:string><json:string name="display_name">testtoken</json:string><json:null name="metadata" /><json:array name="policies"><json:string>root</json:string></json:array></json:object><json:string name="error">this is an error</json:string><json:object name="request"><json:string name="client_token"></json:string><json:string name="client_token_accessor"></json:string><json:null name="data" /><json:object name="headers"><json:array name="foo"><json:string>bar</json:string></json:array></json:object><json:string name="id"></json:string><json:string name="operation">update</json:string><json:string name="path">/foo</json:string><json:string name="remote_address">127.0.0.1</json:string><json:number name="wrap_ttl">60</json:number></json:object><json:string name="type">request</json:string>`,
fooSalted),
},
"auth, request with prefix": {
&logical.Auth{ClientToken: "foo", Policies: []string{"root"}},
&logical.Auth{ClientToken: "foo", Accessor: "bar", DisplayName: "testtoken", Policies: []string{"root"}},
&logical.Request{
Operation: logical.UpdateOperation,
Path: "/foo",
Expand All @@ -66,7 +71,8 @@ func TestFormatJSONx_formatRequest(t *testing.T) {
errors.New("this is an error"),
"",
"@cee: ",
`<json:object name="auth"><json:string name="accessor"></json:string><json:string name="client_token"></json:string><json:string name="display_name"></json:string><json:null name="metadata" /><json:array name="policies"><json:string>root</json:string></json:array></json:object><json:string name="error">this is an error</json:string><json:object name="request"><json:string name="client_token"></json:string><json:string name="client_token_accessor"></json:string><json:null name="data" /><json:object name="headers"><json:array name="foo"><json:string>bar</json:string></json:array></json:object><json:string name="id"></json:string><json:string name="operation">update</json:string><json:string name="path">/foo</json:string><json:string name="remote_address">127.0.0.1</json:string><json:number name="wrap_ttl">60</json:number></json:object><json:string name="type">request</json:string>`,
fmt.Sprintf(`<json:object name="auth"><json:string name="accessor">bar</json:string><json:string name="client_token">%s</json:string><json:string name="display_name">testtoken</json:string><json:null name="metadata" /><json:array name="policies"><json:string>root</json:string></json:array></json:object><json:string name="error">this is an error</json:string><json:object name="request"><json:string name="client_token"></json:string><json:string name="client_token_accessor"></json:string><json:null name="data" /><json:object name="headers"><json:array name="foo"><json:string>bar</json:string></json:array></json:object><json:string name="id"></json:string><json:string name="operation">update</json:string><json:string name="path">/foo</json:string><json:string name="remote_address">127.0.0.1</json:string><json:number name="wrap_ttl">60</json:number></json:object><json:string name="type">request</json:string>`,
fooSalted),
},
}

Expand All @@ -79,7 +85,8 @@ func TestFormatJSONx_formatRequest(t *testing.T) {
},
}
config := FormatterConfig{
OmitTime: true,
OmitTime: true,
HMACAccessor: false,
}
if err := formatter.FormatRequest(&buf, config, tc.Auth, tc.Req, tc.Err); err != nil {
t.Fatalf("bad: %s\nerr: %s", name, err)
Expand All @@ -89,10 +96,10 @@ func TestFormatJSONx_formatRequest(t *testing.T) {
t.Fatalf("no prefix: %s \n log: %s\nprefix: %s", name, tc.Result, tc.Prefix)
}

if !strings.HasSuffix(strings.TrimSpace(buf.String()), string(tc.Expected)) {
if !strings.HasSuffix(strings.TrimSpace(buf.String()), string(tc.ExpectedStr)) {
t.Fatalf(
"bad: %s\nResult:\n\n'%s'\n\nExpected:\n\n'%s'",
name, strings.TrimSpace(buf.String()), string(tc.Expected))
name, strings.TrimSpace(buf.String()), string(tc.ExpectedStr))
}
}
}
21 changes: 12 additions & 9 deletions vault/core.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,24 +663,27 @@ func (c *Core) checkToken(req *logical.Request) (*logical.Auth, *TokenEntry, err
panic("unreachable code")
}
}
// Create the auth response
auth := &logical.Auth{
ClientToken: req.ClientToken,
Accessor: req.ClientTokenAccessor,
Policies: te.Policies,
Metadata: te.Meta,
DisplayName: te.DisplayName,
}

// Check the standard non-root ACLs. Return the token entry if it's not
// allowed so we can decrement the use count.
allowed, rootPrivs := acl.AllowOperation(req)
if !allowed {
return nil, te, logical.ErrPermissionDenied
// Return auth for audit logging even if not allowed
return auth, te, logical.ErrPermissionDenied
}
if rootPath && !rootPrivs {
return nil, te, logical.ErrPermissionDenied
// Return auth for audit logging even if not allowed
return auth, te, logical.ErrPermissionDenied
}

// Create the auth response
auth := &logical.Auth{
ClientToken: req.ClientToken,
Policies: te.Policies,
Metadata: te.Meta,
DisplayName: te.DisplayName,
}
return auth, te, nil
}

Expand Down
2 changes: 1 addition & 1 deletion vault/request_handling.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (c *Core) handleRequest(req *logical.Request) (retResp *logical.Response, r
if errType != nil {
retErr = multierror.Append(retErr, errType)
}
return logical.ErrorResponse(ctErr.Error()), nil, retErr
return logical.ErrorResponse(ctErr.Error()), auth, retErr
}

// Attach the display name
Expand Down

0 comments on commit d4fb262

Please sign in to comment.