From 02c8b5d10fb55099a3a6dada896586bfee4844ac Mon Sep 17 00:00:00 2001 From: hghaf099 <83242695+hghaf099@users.noreply.github.com> Date: Wed, 16 Feb 2022 14:27:58 -0500 Subject: [PATCH] preventing replay attack on MFA passcodes (#14056) * preventing replay attack on MFA passcodes * using %w instead of %s for error --- .../identity/login_mfa_totp_test.go | 30 +++++++++++++++- vault/login_mfa.go | 34 ++++++++++++++++++- vault/request_handling.go | 1 + 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/vault/external_tests/identity/login_mfa_totp_test.go b/vault/external_tests/identity/login_mfa_totp_test.go index ebb17c2d7203..dd7b25628ea5 100644 --- a/vault/external_tests/identity/login_mfa_totp_test.go +++ b/vault/external_tests/identity/login_mfa_totp_test.go @@ -2,7 +2,9 @@ package identity import ( "fmt" + "strings" "testing" + "time" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/builtin/credential/userpass" @@ -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, @@ -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") } @@ -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{ @@ -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, diff --git a/vault/login_mfa.go b/vault/login_mfa.go index abc7a4b57e93..f2c363e6eae6 100644 --- a/vault/login_mfa.go +++ b/vault/login_mfa.go @@ -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" ) @@ -99,6 +100,7 @@ type MFABackend struct { mfaLogger hclog.Logger namespacer Namespacer methodTable string + usedCodes *cache.Cache } type LoginMFABackend struct { @@ -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), } } @@ -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 } } @@ -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 { @@ -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 } @@ -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") @@ -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 } diff --git a/vault/request_handling.go b/vault/request_handling.go index 7b8eeab4d684..c22495a2c98f 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -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