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
7 changes: 6 additions & 1 deletion 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")
)
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
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/logical"
"github.com/hashicorp/vault/vault"
Expand Down Expand Up @@ -185,16 +185,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
Contributor

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
78 changes: 67 additions & 11 deletions vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,35 @@ 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 {
logInput := &audit.LogInput{
Auth: req.Auth,
calvn marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}

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
}
}

// Validate the token
auth, te, ctErr := c.checkToken(ctx, req, false)
if ctErr == logical.ErrPerfStandbyPleaseForward {
Expand Down Expand Up @@ -901,11 +930,47 @@ 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 {
// Get and set ignored HMAC'd value.
if rawVals, ok := entry.synthesizedConfigCache.Load("audit_non_hmac_request_keys"); ok {
nonHMACReqDataKeys = rawVals.([]string)
}
}

var auth *logical.Auth
// 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 {

logInput := &audit.LogInput{
Auth: req.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)
}
}
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
}
}
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
auth, _, ctErr = c.checkToken(ctx, req, true)
if ctErr == logical.ErrPerfStandbyPleaseForward {
Expand All @@ -922,15 +987,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 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 can be optionally 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