Skip to content

Commit

Permalink
preventing replay attack on MFA passcodes (#14056)
Browse files Browse the repository at this point in the history
* preventing replay attack on MFA passcodes

* using %w instead of %s for error
  • Loading branch information
hghaf099 authored Feb 16, 2022
1 parent 0626c75 commit 02c8b5d
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 2 deletions.
30 changes: 29 additions & 1 deletion vault/external_tests/identity/login_mfa_totp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package identity

import (
"fmt"
"strings"
"testing"
"time"

"github.com/hashicorp/vault/api"
"github.com/hashicorp/vault/builtin/credential/userpass"
Expand Down Expand Up @@ -115,7 +117,7 @@ func TestLoginMfaGenerateTOTPRoleTest(t *testing.T) {
// create a config
resp1, err := client.Logical().Write("identity/mfa/method/totp", map[string]interface{}{
"issuer": "yCorp",
"period": 10000,
"period": 5,
"algorithm": "SHA1",
"digits": 6,
"skew": 1,
Expand Down Expand Up @@ -206,6 +208,10 @@ func TestLoginMfaGenerateTOTPRoleTest(t *testing.T) {
t.Fatalf("MFA failed: %v", err)
}

if len(secret.Warnings) == 0 || !strings.Contains(strings.Join(secret.Warnings, ""), "A login request was issued that is subject to MFA validation") {
t.Fatalf("first phase of login did not have a warning")
}

if secret.Auth == nil || secret.Auth.MFARequirement == nil {
t.Fatalf("two phase login returned nil MFARequirement")
}
Expand All @@ -229,6 +235,15 @@ func TestLoginMfaGenerateTOTPRoleTest(t *testing.T) {
}

// validation
// waiting for 5 seconds so that a fresh code could be generated
time.Sleep(5 * time.Second)
// getting a fresh totp passcode for the validation step
totpResp, err := client.Logical().Read("totp/code/loginMFA")
if err != nil {
t.Fatalf("failed to create totp passcode: %v", err)
}
totpPasscode = totpResp.Data["code"].(string)

secret, err = user2Client.Logical().Write("sys/mfa/validate", map[string]interface{}{
"mfa_request_id": secret.Auth.MFARequirement.MFARequestID,
"mfa_payload": map[string][]string{
Expand All @@ -251,6 +266,19 @@ func TestLoginMfaGenerateTOTPRoleTest(t *testing.T) {
t.Fatalf("two phase login returned nil MFARequirement")
}

_, err = user2Client.Logical().Write("sys/mfa/validate", map[string]interface{}{
"mfa_request_id": secret.Auth.MFARequirement.MFARequestID,
"mfa_payload": map[string][]string{
methodID: {totpPasscode},
},
})
if err == nil {
t.Fatalf("MFA succeeded with an already used passcode")
}
if !strings.Contains(err.Error(), "code already used") {
t.Fatalf("expected error message to mention code already used")
}

// Destroy the secret so that the token can self generate
_, err = userClient.Logical().Write(fmt.Sprintf("identity/mfa/method/totp/admin-destroy"), map[string]interface{}{
"entity_id": entityID,
Expand Down
34 changes: 33 additions & 1 deletion vault/login_mfa.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/vault/quotas"
"github.com/mitchellh/mapstructure"
"github.com/patrickmn/go-cache"
otplib "github.com/pquerna/otp"
totplib "github.com/pquerna/otp/totp"
)
Expand Down Expand Up @@ -99,6 +100,7 @@ type MFABackend struct {
mfaLogger hclog.Logger
namespacer Namespacer
methodTable string
usedCodes *cache.Cache
}

type LoginMFABackend struct {
Expand Down Expand Up @@ -138,6 +140,7 @@ func NewMFABackend(core *Core, logger hclog.Logger, prefix string, schemaFuncs [
mfaLogger: logger.Named("mfa"),
namespacer: core,
methodTable: prefix,
usedCodes: cache.New(0, 30*time.Second),
}
}

Expand Down Expand Up @@ -553,7 +556,7 @@ func (b *LoginMFABackend) handleMFALoginValidate(ctx context.Context, req *logic
for _, eConfig := range matchedMfaEnforcementList {
err = b.Core.validateLoginMFA(ctx, eConfig, entity, req.Connection.RemoteAddr, mfaCreds)
if err != nil {
return logical.ErrorResponse("failed to satisfy enforcement %s", eConfig.Name), logical.ErrPermissionDenied
return logical.ErrorResponse(fmt.Sprintf("failed to satisfy enforcement %s. error: %s", eConfig.Name, err.Error())), logical.ErrPermissionDenied
}
}

Expand Down Expand Up @@ -1566,9 +1569,16 @@ func (c *Core) validateDuo(ctx context.Context, creds []string, mConfig *mfa.Con
return fmt.Errorf("invalid response from Duo preauth: %q", preauth.Response.Result)
}

var usedName string
options := []func(*url.Values){}
factor := "push"
if passcode != "" {
usedName = fmt.Sprintf("%s_%s", mConfig.ID, passcode)
_, ok := c.loginMFABackend.usedCodes.Get(usedName)
if ok {
return fmt.Errorf("code already used; wait until the next time period")
}

factor = "passcode"
options = append(options, authapi.AuthPasscode(passcode))
} else {
Expand Down Expand Up @@ -1610,6 +1620,10 @@ func (c *Core) validateDuo(ctx context.Context, creds []string, mConfig *mfa.Con
case "deny":
return fmt.Errorf("duo authentication failed: %q", statusResult.Response.Status_Msg)
case "allow":
err = c.loginMFABackend.usedCodes.Add(usedName, nil, 30*time.Second)
if err != nil {
return fmt.Errorf("error adding code to used cache: %w", err)
}
return nil
}

Expand Down Expand Up @@ -1962,6 +1976,13 @@ func (c *Core) validateTOTP(ctx context.Context, creds []string, entityMethodSec
return fmt.Errorf("more than one TOTP passcode supplied")
}

usedName := fmt.Sprintf("%s_%s", configID, creds[0])

_, ok := c.loginMFABackend.usedCodes.Get(usedName)
if ok {
return fmt.Errorf("code already used; wait until the next time period")
}

totpSecret := entityMethodSecret.GetTOTPSecret()
if totpSecret == nil {
return fmt.Errorf("entity does not contain the TOTP secret")
Expand Down Expand Up @@ -1992,6 +2013,17 @@ func (c *Core) validateTOTP(ctx context.Context, creds []string, entityMethodSec
return fmt.Errorf("failed to validate TOTP passcode")
}

// Adding the used code to the cache
// Take the key skew, add two for behind and in front, and multiply that by
// the period to cover the full possibility of the validity of the key
err = c.loginMFABackend.usedCodes.Add(usedName, nil, time.Duration(
int64(time.Second)*
int64(totpSecret.Period)*
int64(2+totpSecret.Skew)))
if err != nil {
return fmt.Errorf("error adding code to used cache: %w", err)
}

return nil
}

Expand Down
1 change: 1 addition & 0 deletions vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,7 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
resp.Auth = &logical.Auth{
MFARequirement: mfaRequirement,
}
resp.AddWarning("A login request was issued that is subject to MFA validation. Please make sure to validate the login by sending another request to mfa/validate endpoint.")
// going to return early before generating the token
// the user receives the mfaRequirement, and need to use the
// login MFA validate endpoint to get the token
Expand Down

0 comments on commit 02c8b5d

Please sign in to comment.