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
3 changes: 3 additions & 0 deletions helper/consts/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,7 @@ var (

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

// ErrInvalidWrappingToken
ErrInvalidWrappingToken = errors.New("wrapping token is not valid or does not exist")
)
19 changes: 0 additions & 19 deletions http/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,25 +284,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
15 changes: 6 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/logical"
"github.com/hashicorp/vault/vault"
Expand Down Expand Up @@ -187,14 +187,11 @@ func handleLogicalInternal(core *vault.Core, injectDataIntoTopLevel bool) http.H
switch req.Path {
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
}
// We perform validation but don't check the error here yet so
// that the request/response can be logged once we're in the
// logical layer. This is more for logical.Request modification
// depending on how the wrapping token was provided.
_, _ = core.ValidateWrappingToken(r.Context(), req, true)

// The -self paths have no meaning outside of the token NS, so
// requests for these paths always go to the token NS
Expand Down
69 changes: 60 additions & 9 deletions vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,32 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
return nil, nil, ctErr
}

// If this is a wrapping request, check for token validity before routing
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, false)
// Perform audit logging before returning if there's an issue with checking
// the wrapping token
if err != nil || !valid {
logInput := &audit.LogInput{
Auth: auth,
Request: req,
OuterErr: ctErr,
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)
}
}

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

// We run this logic first because we want to decrement the use count even
// in the case of an error (assuming we can successfully look up; if we
// need to forward, we exit before now)
Expand Down Expand Up @@ -901,6 +927,15 @@ 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())

var nonHMACReqDataKeys []string
entry := c.router.MatchingMountEntry(ctx, req.Path)
if entry != nil {
// Get and set ignored HMAC'd value.
if rawVals, ok := entry.synthesizedConfigCache.Load("audit_non_hmac_request_keys"); ok {
nonHMACReqDataKeys = rawVals.([]string)
}
}

req.Unauthenticated = true

var auth *logical.Auth
Expand All @@ -922,15 +957,6 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
errType = logical.ErrInvalidRequest
}

var nonHMACReqDataKeys []string
entry := c.router.MatchingMountEntry(ctx, req.Path)
if entry != nil {
// Get and set ignored HMAC'd value.
if rawVals, ok := entry.synthesizedConfigCache.Load("audit_non_hmac_request_keys"); ok {
nonHMACReqDataKeys = rawVals.([]string)
}
}

logInput := &audit.LogInput{
Auth: auth,
Request: req,
Expand All @@ -951,6 +977,31 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
return logical.ErrorResponse(ctErr.Error()), auth, retErr
}

// If this is a wrapping token lookup, validate the token and produce an audit log
switch req.Path {
case "sys/wrapping/lookup":
valid, err := c.ValidateWrappingToken(ctx, req, false)
// Perform audit logging before returning if there's an issue with checking
// the wrapping token
if err != nil || !valid {
logInput := &audit.LogInput{
Auth: auth,
Request: req,
OuterErr: ctErr,
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)
}
}
if err != nil {
return nil, nil, errwrap.Wrapf("error validating wrapping token: {{err}}", err)
}
if !valid {
return logical.ErrorResponse(consts.ErrInvalidWrappingToken.Error()), nil, logical.ErrInvalidRequest
}
}

// Create an audit trail of the request. Attach auth if it was returned,
// e.g. if a token was provided.
logInput := &audit.LogInput{
Expand Down
22 changes: 14 additions & 8 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 can be optionally updated if the wrapping token was
// provided within a JWT token.
func (c *Core) ValidateWrappingToken(ctx context.Context, req *logical.Request, updateRequest bool) (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 it not
calvn marked this conversation as resolved.
Show resolved Hide resolved
// 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 @@ -360,10 +364,12 @@ func (c *Core) ValidateWrappingToken(ctx context.Context, req *logical.Request)
if typeClaim != "wrapping" {
return false, errors.New("unexpected type claim")
}
if !thirdParty {
req.ClientToken = claims.ID
} else {
req.Data["token"] = claims.ID
if updateRequest {
if !thirdParty {
req.ClientToken = claims.ID
} else {
req.Data["token"] = claims.ID
}
}
token = claims.ID
}
Expand Down Expand Up @@ -398,7 +404,7 @@ func (c *Core) ValidateWrappingToken(ctx context.Context, req *logical.Request)
return false, nil
}

if !thirdParty {
if !thirdParty && updateRequest {
req.ClientTokenAccessor = te.Accessor
req.ClientTokenRemainingUses = te.NumUses
req.SetTokenEntry(te)
Expand Down