[v14] Always verify the old password when changing it

lib/auth/methods.go

@@ -371,6 +371,11 @@ func (a *Server) authenticateUserInternal(ctx context.Context, req AuthenticateU
 		authErr = authenticateHeadlessError
 	case req.Webauthn != nil:
 		authenticateFn = func() (*types.MFADevice, error) {
+			if req.Pass != nil {
+				if err = a.checkPasswordWOToken(user, req.Pass.Password); err != nil {
+					return nil, trace.Wrap(err)
+				}
+			}
 			mfaResponse := &proto.MFAAuthenticateResponse{
 				Response: &proto.MFAAuthenticateResponse_Webauthn{
 					Webauthn: wantypes.CredentialAssertionResponseToProto(req.Webauthn),

lib/auth/password.go

@@ -125,10 +125,8 @@ func (a *Server) ChangePassword(ctx context.Context, req *proto.ChangePasswordRe
 		Username: user,
 		Webauthn: wantypes.CredentialAssertionResponseFromProto(req.Webauthn),
 	}
-	if len(req.OldPassword) > 0 {
-		authReq.Pass = &PassCreds{
-			Password: req.OldPassword,
-		}
+	authReq.Pass = &PassCreds{
+		Password: req.OldPassword,
 	}
 	if req.SecondFactorToken != "" {
 		authReq.OTP = &OTPCreds{

lib/auth/password_test.go

@@ -27,11 +27,13 @@ import (
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
 	"github.com/pquerna/otp/totp"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/constants"
+	mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1"
 	"github.com/gravitational/teleport/api/types"
 	apievents "github.com/gravitational/teleport/api/types/events"
 	wanpb "github.com/gravitational/teleport/api/types/webauthn"
@@ -258,6 +260,55 @@ func TestServer_ChangePassword(t *testing.T) {
 	}
 }
 
+// This test asserts that an attacker is unable to change password without
+// providing the old one if they take over a user's web session and use a
+// different type of WebAuthn challenge that would be normally requested by the
+// Web UI. This is a regression test for
+// https://github.com/gravitational/teleport-private/issues/1369.
+func TestServer_ChangePassword_FailsWithoutOldPassword(t *testing.T) {
+	t.Parallel()
+
+	server := newTestTLSServer(t)
+	mfa := configureForMFA(t, server)
+	authServer := server.Auth()
+	ctx := context.Background()
+
+	username := mfa.User
+	newPass := []byte("capybarasarecool123")
+
+	userClient, err := server.NewClient(TestUser(username))
+	require.NoError(t, err)
+	defer userClient.Close()
+
+	// Acquire and solve an MFA challenge.
+	mfaChallenge, err := userClient.CreateAuthenticateChallenge(ctx, &proto.CreateAuthenticateChallengeRequest{
+		Request: &proto.CreateAuthenticateChallengeRequest_ContextUser{
+			ContextUser: &proto.ContextUser{},
+		},
+		ChallengeExtensions: &mfav1.ChallengeExtensions{
+			AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO,
+		},
+	})
+	require.NoError(t, err, "creating challenge")
+	mfaResp, err := mfa.WebDev.SolveAuthn(mfaChallenge)
+	require.NoError(t, err, "solving challenge with device")
+
+	// Change password.
+	req := &proto.ChangePasswordRequest{
+		User:        username,
+		NewPassword: newPass,
+		Webauthn:    mfaResp.GetWebauthn(),
+	}
+	err = authServer.ChangePassword(ctx, req)
+	assert.True(t,
+		trace.IsAccessDenied(err),
+		"ChangePassword error mismatch, want=AccessDenied, got=%v (%T)",
+		err, trace.Unwrap(err))
+
+	// Did the password change take effect?
+	assert.Error(t, authServer.checkPasswordWOToken(username, newPass), "password was changed")
+}
+
 func TestChangeUserAuthentication(t *testing.T) {
 	t.Parallel()