Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audit: log invalid wrapping token request/response #6541

Merged
merged 14 commits into from
Jul 5, 2019
2 changes: 1 addition & 1 deletion http/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func wrappingVerificationFunc(ctx context.Context, core *vault.Core, req *logica
return errwrap.Wrapf("error validating wrapping token: {{err}}", err)
}
if !valid {
return fmt.Errorf("wrapping token is not valid or does not exist")
return consts.ErrInvalidWrappingToken
}

return nil
Expand Down
3 changes: 2 additions & 1 deletion http/logical.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"time"

"github.com/hashicorp/errwrap"
uuid "github.com/hashicorp/go-uuid"
"github.com/hashicorp/go-uuid"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/vault"
Expand Down Expand Up @@ -208,6 +208,7 @@ func handleLogicalInternal(core *vault.Core, injectDataIntoTopLevel bool) http.H
}
}
switch req.Path {
// Route the token wrapping request to its respective sys NS
calvn marked this conversation as resolved.
Show resolved Hide resolved
case "sys/wrapping/lookup", "sys/wrapping/rewrap", "sys/wrapping/unwrap":
r = r.WithContext(newCtx)
if err := wrappingVerificationFunc(r.Context(), core, req); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should put this back, no? the logic feels cleaner here instead of in handleRequest and handleLoginRequest

Expand Down
90 changes: 90 additions & 0 deletions http/logical_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package http

import (
"bytes"
"context"
"encoding/json"
"io"
"io/ioutil"
Expand All @@ -16,6 +17,7 @@ import (
"github.com/go-test/deep"
log "github.com/hashicorp/go-hclog"

"github.com/hashicorp/vault/audit"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/sdk/helper/consts"
"github.com/hashicorp/vault/sdk/helper/logging"
Expand Down Expand Up @@ -347,3 +349,91 @@ func TestLogical_RespondWithStatusCode(t *testing.T) {
t.Fatalf("bad response: %s", string(bodyRaw[:]))
}
}

func TestLogical_Audit_invalidWrappingToken(t *testing.T) {
// Create a noop audit backend
var noop *vault.NoopAudit
c, _, root := vault.TestCoreUnsealedWithConfig(t, &vault.CoreConfig{
AuditBackends: map[string]audit.Factory{
"noop": func(ctx context.Context, config *audit.BackendConfig) (audit.Backend, error) {
noop = &vault.NoopAudit{
Config: config,
}
return noop, nil
},
},
})
ln, addr := TestServer(t, c)
defer ln.Close()

// Enable the audit backend

resp := testHttpPost(t, root, addr+"/v1/sys/audit/noop", map[string]interface{}{
"type": "noop",
})
testResponseStatus(t, resp, 204)

{
// Make a wrapping/unwrap request with an invalid token
resp := testHttpPost(t, root, addr+"/v1/sys/wrapping/unwrap", map[string]interface{}{
"token": "foo",
})
testResponseStatus(t, resp, 400)
body := map[string][]string{}
testResponseBody(t, resp, &body)
if body["errors"][0] != "wrapping token is not valid or does not exist" {
t.Fatal(body)
}

// Check the audit trail on request and response
if len(noop.ReqAuth) != 1 {
t.Fatalf("bad: %#v", noop)
}
auth := noop.ReqAuth[0]
if auth.ClientToken != root {
t.Fatalf("bad client token: %#v", auth)
}
if len(noop.Req) != 1 || noop.Req[0].Path != "sys/wrapping/unwrap" {
t.Fatalf("bad:\ngot:\n%#v", noop.Req[0])
}

if len(noop.ReqErrs) != 1 {
t.Fatalf("bad: %#v", noop.RespErrs)
}
if noop.ReqErrs[0] != consts.ErrInvalidWrappingToken {
t.Fatalf("bad: %#v", noop.ReqErrs)
}
}

{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we test the same thing twice? And should we test the happy path, i.e. that no audit record is written for a valid token?

// Make a wrapping/unwrap request with an invalid token
resp := testHttpPost(t, root, addr+"/v1/sys/wrapping/unwrap", map[string]interface{}{
"token": "foo",
})
testResponseStatus(t, resp, 400)
body := map[string][]string{}
testResponseBody(t, resp, &body)
if body["errors"][0] != "wrapping token is not valid or does not exist" {
t.Fatal(body)
}

// Check the audit trail on request and response
if len(noop.ReqAuth) != 2 {
t.Fatalf("bad: %#v", noop)
}
auth := noop.ReqAuth[1]
if auth.ClientToken != root {
t.Fatalf("bad client token: %#v", auth)
}
if len(noop.Req) != 2 || noop.Req[1].Path != "sys/wrapping/unwrap" {
t.Fatalf("bad:\ngot:\n%#v", noop.Req[1])
}

if len(noop.ReqErrs) != 2 {
t.Fatalf("bad: %#v", noop.RespErrs)
}
if noop.ReqErrs[1] != consts.ErrInvalidWrappingToken {
t.Fatalf("bad: %#v", noop.ReqErrs)
}
}
}
7 changes: 6 additions & 1 deletion sdk/helper/consts/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ var (
// No operation is expected to succeed until active.
ErrStandby = errors.New("Vault is in standby mode")

// Used when .. is used in a path
// ErrPathContainsParentReferences is returned when a path contains parent
// references.
ErrPathContainsParentReferences = errors.New("path cannot contain parent references")

// ErrInvalidWrappingToken is returned when checking for the validity of
// a wrapping token that turns out to be invalid.
ErrInvalidWrappingToken = errors.New("wrapping token is not valid or does not exist")
)
84 changes: 0 additions & 84 deletions vault/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"reflect"
"strings"
"sync"
"testing"
"time"

Expand All @@ -18,93 +17,10 @@ import (
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/sdk/helper/jsonutil"
"github.com/hashicorp/vault/sdk/helper/logging"
"github.com/hashicorp/vault/sdk/helper/salt"
"github.com/hashicorp/vault/sdk/logical"
"github.com/mitchellh/copystructure"
)

type NoopAudit struct {
Config *audit.BackendConfig
ReqErr error
ReqAuth []*logical.Auth
Req []*logical.Request
ReqHeaders []map[string][]string
ReqNonHMACKeys []string
ReqErrs []error

RespErr error
RespAuth []*logical.Auth
RespReq []*logical.Request
Resp []*logical.Response
RespNonHMACKeys []string
RespReqNonHMACKeys []string
RespErrs []error

salt *salt.Salt
saltMutex sync.RWMutex
}

func (n *NoopAudit) LogRequest(ctx context.Context, in *logical.LogInput) error {
n.ReqAuth = append(n.ReqAuth, in.Auth)
n.Req = append(n.Req, in.Request)
n.ReqHeaders = append(n.ReqHeaders, in.Request.Headers)
n.ReqNonHMACKeys = in.NonHMACReqDataKeys
n.ReqErrs = append(n.ReqErrs, in.OuterErr)
return n.ReqErr
}

func (n *NoopAudit) LogResponse(ctx context.Context, in *logical.LogInput) error {
n.RespAuth = append(n.RespAuth, in.Auth)
n.RespReq = append(n.RespReq, in.Request)
n.Resp = append(n.Resp, in.Response)
n.RespErrs = append(n.RespErrs, in.OuterErr)

if in.Response != nil {
n.RespNonHMACKeys = in.NonHMACRespDataKeys
n.RespReqNonHMACKeys = in.NonHMACReqDataKeys
}

return n.RespErr
}

func (n *NoopAudit) Salt(ctx context.Context) (*salt.Salt, error) {
n.saltMutex.RLock()
if n.salt != nil {
defer n.saltMutex.RUnlock()
return n.salt, nil
}
n.saltMutex.RUnlock()
n.saltMutex.Lock()
defer n.saltMutex.Unlock()
if n.salt != nil {
return n.salt, nil
}
salt, err := salt.NewSalt(ctx, n.Config.SaltView, n.Config.SaltConfig)
if err != nil {
return nil, err
}
n.salt = salt
return salt, nil
}

func (n *NoopAudit) GetHash(ctx context.Context, data string) (string, error) {
salt, err := n.Salt(ctx)
if err != nil {
return "", err
}
return salt.GetIdentifiedHMAC(data), nil
}

func (n *NoopAudit) Reload(ctx context.Context) error {
return nil
}

func (n *NoopAudit) Invalidate(ctx context.Context) {
n.saltMutex.Lock()
defer n.saltMutex.Unlock()
n.salt = nil
}

func TestAudit_ReadOnlyViewDuringMount(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)
c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig) (audit.Backend, error) {
Expand Down
3 changes: 1 addition & 2 deletions vault/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/go-test/deep"
"github.com/hashicorp/errwrap"
log "github.com/hashicorp/go-hclog"
uuid "github.com/hashicorp/go-uuid"
"github.com/hashicorp/go-uuid"
"github.com/hashicorp/vault/audit"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/sdk/helper/consts"
Expand Down Expand Up @@ -904,7 +904,6 @@ func TestCore_HandleRequest_AuditTrail_noHMACKeys(t *testing.T) {
}
}

// Ensure we get a client token
func TestCore_HandleLogin_AuditTrail(t *testing.T) {
// Create a badass credential backend that always logs in as armon
noop := &NoopAudit{}
Expand Down
82 changes: 82 additions & 0 deletions vault/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -1739,3 +1739,85 @@ func (m *mockBuiltinRegistry) Keys(pluginType consts.PluginType) []string {
func (m *mockBuiltinRegistry) Contains(name string, pluginType consts.PluginType) bool {
return false
}

type NoopAudit struct {
Config *audit.BackendConfig
ReqErr error
ReqAuth []*logical.Auth
Req []*logical.Request
ReqHeaders []map[string][]string
ReqNonHMACKeys []string
ReqErrs []error

RespErr error
RespAuth []*logical.Auth
RespReq []*logical.Request
Resp []*logical.Response
RespNonHMACKeys []string
RespReqNonHMACKeys []string
RespErrs []error

salt *salt.Salt
saltMutex sync.RWMutex
}

func (n *NoopAudit) LogRequest(ctx context.Context, in *logical.LogInput) error {
n.ReqAuth = append(n.ReqAuth, in.Auth)
n.Req = append(n.Req, in.Request)
n.ReqHeaders = append(n.ReqHeaders, in.Request.Headers)
n.ReqNonHMACKeys = in.NonHMACReqDataKeys
n.ReqErrs = append(n.ReqErrs, in.OuterErr)
return n.ReqErr
}

func (n *NoopAudit) LogResponse(ctx context.Context, in *logical.LogInput) error {
n.RespAuth = append(n.RespAuth, in.Auth)
n.RespReq = append(n.RespReq, in.Request)
n.Resp = append(n.Resp, in.Response)
n.RespErrs = append(n.RespErrs, in.OuterErr)

if in.Response != nil {
n.RespNonHMACKeys = in.NonHMACRespDataKeys
n.RespReqNonHMACKeys = in.NonHMACReqDataKeys
}

return n.RespErr
}

func (n *NoopAudit) Salt(ctx context.Context) (*salt.Salt, error) {
n.saltMutex.RLock()
if n.salt != nil {
defer n.saltMutex.RUnlock()
return n.salt, nil
}
n.saltMutex.RUnlock()
n.saltMutex.Lock()
defer n.saltMutex.Unlock()
if n.salt != nil {
return n.salt, nil
}
salt, err := salt.NewSalt(ctx, n.Config.SaltView, n.Config.SaltConfig)
if err != nil {
return nil, err
}
n.salt = salt
return salt, nil
}

func (n *NoopAudit) GetHash(ctx context.Context, data string) (string, error) {
salt, err := n.Salt(ctx)
if err != nil {
return "", err
}
return salt.GetIdentifiedHMAC(data), nil
}

func (n *NoopAudit) Reload(ctx context.Context) error {
return nil
}

func (n *NoopAudit) Invalidate(ctx context.Context) {
n.saltMutex.Lock()
defer n.saltMutex.Unlock()
n.salt = nil
}
Loading