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
19 changes: 0 additions & 19 deletions http/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,25 +286,6 @@ func WrapForwardedForHandler(h http.Handler, authorizedAddrs []*sockaddr.SockAdd
})
}

// A lookup on a token that is about to expire returns nil, which means by the
// time we can validate a wrapping token lookup will return nil since it will
// be revoked after the call. So we have to do the validation here.
func wrappingVerificationFunc(ctx context.Context, core *vault.Core, req *logical.Request) error {
if req == nil {
return fmt.Errorf("invalid request")
}

valid, err := core.ValidateWrappingToken(ctx, req)
if err != nil {
return errwrap.Wrapf("error validating wrapping token: {{err}}", err)
}
if !valid {
return fmt.Errorf("wrapping token is not valid or does not exist")
}

return nil
}

// stripPrefix is a helper to strip a prefix from the path. It will
// return false from the second return value if it the prefix doesn't exist.
func stripPrefix(prefix, path string) (string, bool) {
Expand Down
11 changes: 2 additions & 9 deletions 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 @@ -189,16 +189,9 @@ 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

if errwrap.Contains(err, logical.ErrPermissionDenied.Error()) {
respondError(w, http.StatusForbidden, err)
} else {
respondError(w, http.StatusBadRequest, err)
}
return
}

// The -self paths have no meaning outside of the token NS, so
// requests for these paths always go to the token NS
Expand Down
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")
)
147 changes: 145 additions & 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,150 @@ func TestCore_HandleRequest_AuditTrail_noHMACKeys(t *testing.T) {
}
}

// Ensure we get a client token
func TestCore_HandleRequest_AuditTrail_invalidWrappingToken(t *testing.T) {
// Create a noop audit backend
var noop *NoopAudit
c, _, root := TestCoreUnsealed(t)
c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig) (audit.Backend, error) {
noop = &NoopAudit{
Config: config,
}
return noop, nil
}

// Enable the audit backend
req := logical.TestRequest(t, logical.UpdateOperation, "sys/audit/noop")
req.Data["type"] = "noop"
req.ClientToken = root
_, err := c.HandleRequest(namespace.RootContext(nil), req)
if err != nil {
t.Fatalf("err: %v", err)
}

{
// Make a wrapping/unwrap request with an invalid token
req := &logical.Request{
Operation: logical.UpdateOperation,
Path: "sys/wrapping/unwrap",
Data: map[string]interface{}{
"token": "foo",
},
ClientToken: root,
}
resp, err := c.HandleRequest(namespace.RootContext(nil), req)
if err == nil {
t.Fatal("expected invalid request error")
}

// 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 || !reflect.DeepEqual(req, noop.Req[0]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", req, noop.Req[0])
}

if len(noop.RespAuth) != 2 {
t.Fatalf("bad: %#v", noop)
}
if !reflect.DeepEqual(auth, noop.RespAuth[1]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", auth, noop.RespAuth[1])
}
if len(noop.RespReq) != 2 || !reflect.DeepEqual(req, noop.RespReq[1]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", req, noop.RespReq[1])
}
if len(noop.Resp) != 2 || !reflect.DeepEqual(resp, noop.Resp[1]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", resp, noop.Resp[1])
}
}

{
// Make a wrapping/rewrap request with an invalid token
req := &logical.Request{
Operation: logical.UpdateOperation,
Path: "sys/wrapping/rewrap",
Data: map[string]interface{}{
"token": "foo",
},
ClientToken: root,
}
resp, err := c.HandleRequest(namespace.RootContext(nil), req)
if err == nil {
t.Fatal("expected invalid request error")
}

// 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 || !reflect.DeepEqual(req, noop.Req[1]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", req, noop.Req[1])
}

if len(noop.RespAuth) != 3 {
t.Fatalf("bad: %#v", noop)
}
if !reflect.DeepEqual(auth, noop.RespAuth[2]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", auth, noop.RespAuth[2])
}
if len(noop.RespReq) != 3 || !reflect.DeepEqual(req, noop.RespReq[2]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", req, noop.RespReq[2])
}
if len(noop.Resp) != 3 || !reflect.DeepEqual(resp, noop.Resp[2]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", resp, noop.Resp[2])
}
}

{
// Make a wrapping/lookup request with an invalid token
req := &logical.Request{
Operation: logical.UpdateOperation,
Path: "sys/wrapping/lookup",
Data: map[string]interface{}{
"token": "foo",
},
ClientToken: root,
}
resp, err := c.HandleRequest(namespace.RootContext(nil), req)
if err == nil {
t.Fatal("expected invalid request error")
}

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

if len(noop.RespAuth) != 4 {
t.Fatalf("bad: %#v", noop)
}
if !reflect.DeepEqual(auth, noop.RespAuth[3]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", auth, noop.RespAuth[3])
}
if len(noop.RespReq) != 4 || !reflect.DeepEqual(req, noop.RespReq[3]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", req, noop.RespReq[3])
}
if len(noop.Resp) != 4 || !reflect.DeepEqual(resp, noop.Resp[3]) {
t.Fatalf("bad:\nexpected:\n%#v\ngot:\n%#v", resp, noop.Resp[3])
}
}
}

func TestCore_HandleLogin_AuditTrail(t *testing.T) {
// Create a badass credential backend that always logs in as armon
noop := &NoopAudit{}
Expand Down
76 changes: 74 additions & 2 deletions vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,42 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
return
}

// If this is a wrapping request that contains a wrapping token, check for
// token validity. We perform this before c.checkToken since wrapping token
// validation performs a token store lookup, and can potentially modify
// logical.Request's token-related fields.
switch req.Path {
case "sys/wrapping/rewrap", "sys/wrapping/unwrap":
calvn marked this conversation as resolved.
Show resolved Hide resolved
valid, err := c.validateWrappingToken(ctx, req)

// Perform audit logging before returning if there's an issue with checking
// the wrapping token
if err != nil || !valid {
// We log the Auth object like so here since the wrapping token can
// come from the header, which gets set as the ClientToken
auth := &logical.Auth{
ClientToken: req.ClientToken,
Accessor: req.ClientTokenAccessor,
}

logInput := &audit.LogInput{
Auth: auth,
Request: req,
NonHMACReqDataKeys: nonHMACReqDataKeys,
}
if err := c.auditBroker.LogRequest(ctx, logInput, c.auditedHeaders); err != nil {
c.logger.Error("failed to audit request", "path", req.Path, "error", err)
}

switch {
case err != nil:
return nil, auth, errwrap.Wrapf("error validating wrapping token: {{err}}", err)
case !valid:
return logical.ErrorResponse(consts.ErrInvalidWrappingToken.Error()), auth, logical.ErrInvalidRequest
}
}
}

// Validate the token
auth, te, ctErr := c.checkToken(ctx, req, false)
if ctErr == logical.ErrPerfStandbyPleaseForward {
Expand Down Expand Up @@ -912,8 +948,6 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (retResp *logical.Response, retAuth *logical.Auth, retErr error) {
defer metrics.MeasureSince([]string{"core", "handle_login_request"}, time.Now())

req.Unauthenticated = true

var nonHMACReqDataKeys []string
entry := c.router.MatchingMountEntry(ctx, req.Path)
if entry != nil {
Expand All @@ -923,6 +957,44 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
}
}

// If this is a wrapping token lookup, validate the token and produce an
// audit log. We perform this before c.checkToken since wrapping token
// validation performs a token store lookup, and can potentially modify
// logical.Request's token-related fields.
switch req.Path {
case "sys/wrapping/lookup":
valid, err := c.validateWrappingToken(ctx, req)
// Perform audit logging before returning if there's an issue with checking
// the wrapping token
if err != nil || !valid {
// We log the Auth object like so here since the wrapping token can
// come from the header, which gets set as the ClientToken
auth := &logical.Auth{
ClientToken: req.ClientToken,
Accessor: req.ClientTokenAccessor,
}

logInput := &audit.LogInput{
Auth: auth,
Request: req,
NonHMACReqDataKeys: nonHMACReqDataKeys,
}
if err := c.auditBroker.LogRequest(ctx, logInput, c.auditedHeaders); err != nil {
c.logger.Error("failed to audit request", "path", req.Path, "error", err)
}

switch {
case err != nil:
return nil, auth, errwrap.Wrapf("error validating wrapping token: {{err}}", err)
case !valid:
return logical.ErrorResponse(consts.ErrInvalidWrappingToken.Error()), auth, logical.ErrInvalidRequest

}
}
}
calvn marked this conversation as resolved.
Show resolved Hide resolved

req.Unauthenticated = true

// Do an unauth check. This will cause EGP policies to be checked
var auth *logical.Auth
var ctErr error
Expand Down
11 changes: 8 additions & 3 deletions vault/wrapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,16 +309,20 @@ DONELISTHANDLING:
return nil, nil
}

// ValidateWrappingToken checks whether a token is a wrapping token.
func (c *Core) ValidateWrappingToken(ctx context.Context, req *logical.Request) (bool, error) {
// validateWrappingToken checks whether a token is a wrapping token. The passed
// in logical request will be updated if the wrapping token was provided within
// a JWT token.
func (c *Core) validateWrappingToken(ctx context.Context, req *logical.Request) (bool, error) {
if req == nil {
return false, fmt.Errorf("invalid request")
}

var err error

var token string
var thirdParty bool

// Check if the wrapping token is coming from the request body, and if not
// assume that req.ClientToken is the wrapping token
if req.Data != nil && req.Data["token"] != nil {
thirdParty = true
if tokenStr, ok := req.Data["token"].(string); !ok {
Expand Down Expand Up @@ -365,6 +369,7 @@ func (c *Core) ValidateWrappingToken(ctx context.Context, req *logical.Request)
} else {
req.Data["token"] = claims.ID
}

token = claims.ID
}

Expand Down