Handle private key policy errors for the web UI

api/client/webclient/webconfig.go

@@ -16,7 +16,10 @@ limitations under the License.
 
 package webclient
 
-import "github.com/gravitational/teleport/api/constants"
+import (
+	"github.com/gravitational/teleport/api/constants"
+	"github.com/gravitational/teleport/api/utils/keys"
+)
 
 const (
 	// WebConfigAuthProviderOIDCType is OIDC provider type
@@ -82,4 +85,6 @@ type WebConfigAuthSettings struct {
 	PreferredLocalMFA constants.SecondFactorType `json:"preferredLocalMfa,omitempty"`
 	// LocalConnectorName is the name of the local connector.
 	LocalConnectorName string `json:"localConnectorName,omitempty"`
+	// PrivateKeyPolicy is the configured private key policy for the cluster.
+	PrivateKeyPolicy keys.PrivateKeyPolicy `json:"privateKeyPolicy,omitempty"`
 }

api/proto/teleport/legacy/client/proto/authservice.proto

@@ -1373,6 +1373,9 @@ message ChangeUserAuthenticationResponse {
   //  - cloud feature is enabled
   //  - username is in valid email format
   RecoveryCodes Recovery = 2 [(gogoproto.jsontag) = "recovery,omitempty"];
+  // PrivateKeyPolicyEnabled is a flag that when true means one of the private key policy was
+  // set in either through cluster config or through a user's assigned role.
+  bool PrivateKeyPolicyEnabled = 3 [(gogoproto.jsontag) = "private_key_policy_enabled,omitempty"];
 }
 
 // StartAccountRecoveryRequest defines a request to create a recovery start token for a user who is

api/utils/keys/policy.go

@@ -52,7 +52,7 @@ func (p PrivateKeyPolicy) VerifyPolicy(policy PrivateKeyPolicy) error {
 			return nil
 		}
 	}
-	return newPrivateKeyPolicyError(p)
+	return NewPrivateKeyPolicyError(p)
 }
 
 func (p PrivateKeyPolicy) validate() error {
@@ -65,7 +65,7 @@ func (p PrivateKeyPolicy) validate() error {
 
 var privateKeyPolicyErrRegex = regexp.MustCompile(`private key policy not met: (\w+)`)
 
-func newPrivateKeyPolicyError(p PrivateKeyPolicy) error {
+func NewPrivateKeyPolicyError(p PrivateKeyPolicy) error {
 	return trace.BadParameter(fmt.Sprintf("private key policy not met: %s", p))
 }
 

api/utils/keys/policy_test.go

@@ -58,32 +58,32 @@ func TestPrivateKeyPolicyError(t *testing.T) {
 			expectKeyPolicyErr: true,
 		}, {
 			desc:               "unknown_key_policy",
-			errIn:              newPrivateKeyPolicyError("unknown_key_policy"),
+			errIn:              NewPrivateKeyPolicyError("unknown_key_policy"),
 			expectIsKeyPolicy:  true,
 			expectKeyPolicyErr: true,
 		}, {
 			desc:              string(PrivateKeyPolicyNone),
-			errIn:             newPrivateKeyPolicyError(PrivateKeyPolicyNone),
+			errIn:             NewPrivateKeyPolicyError(PrivateKeyPolicyNone),
 			expectIsKeyPolicy: true,
 			expectKeyPolicy:   PrivateKeyPolicyNone,
 		}, {
 			desc:              string(PrivateKeyPolicyHardwareKey),
-			errIn:             newPrivateKeyPolicyError(PrivateKeyPolicyHardwareKey),
+			errIn:             NewPrivateKeyPolicyError(PrivateKeyPolicyHardwareKey),
 			expectIsKeyPolicy: true,
 			expectKeyPolicy:   PrivateKeyPolicyHardwareKey,
 		}, {
 			desc:              string(PrivateKeyPolicyHardwareKeyTouch),
-			errIn:             newPrivateKeyPolicyError(PrivateKeyPolicyHardwareKeyTouch),
+			errIn:             NewPrivateKeyPolicyError(PrivateKeyPolicyHardwareKeyTouch),
 			expectIsKeyPolicy: true,
 			expectKeyPolicy:   PrivateKeyPolicyHardwareKeyTouch,
 		}, {
 			desc:              "wrapped policy error",
-			errIn:             trace.Wrap(newPrivateKeyPolicyError(PrivateKeyPolicyHardwareKeyTouch), "wrapped err"),
+			errIn:             trace.Wrap(NewPrivateKeyPolicyError(PrivateKeyPolicyHardwareKeyTouch), "wrapped err"),
 			expectIsKeyPolicy: true,
 			expectKeyPolicy:   PrivateKeyPolicyHardwareKeyTouch,
 		}, {
 			desc:              "policy error string contained in error",
-			errIn:             trace.Errorf("ssh: rejected: administratively prohibited (%s)", newPrivateKeyPolicyError(PrivateKeyPolicyHardwareKeyTouch).Error()),
+			errIn:             trace.Errorf("ssh: rejected: administratively prohibited (%s)", NewPrivateKeyPolicyError(PrivateKeyPolicyHardwareKeyTouch).Error()),
 			expectIsKeyPolicy: true,
 			expectKeyPolicy:   PrivateKeyPolicyHardwareKeyTouch,
 		},

lib/auth/password.go

@@ -29,6 +29,7 @@ import (
 	"github.com/gravitational/teleport/api/constants"
 	"github.com/gravitational/teleport/api/types"
 	apievents "github.com/gravitational/teleport/api/types/events"
+	"github.com/gravitational/teleport/api/utils/keys"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/events"
 	"github.com/gravitational/teleport/lib/services"
@@ -62,6 +63,17 @@ func (s *Server) ChangeUserAuthentication(ctx context.Context, req *proto.Change
 
 	webSession, err := s.createUserWebSession(ctx, user)
 	if err != nil {
+		if keys.IsPrivateKeyPolicyError(err) {
+			// Do not return an error, otherwise
+			// the user won't be able to receive
+			// recovery codes. Even with no recovery codes
+			// this positive response indicates the user
+			// has successfully reset/registered their account.
+			return &proto.ChangeUserAuthenticationResponse{
+				Recovery:                newRecovery,
+				PrivateKeyPolicyEnabled: true,
+			}, nil
+		}
 		return nil, trace.Wrap(err)
 	}
 

lib/modules/test.go

@@ -39,6 +39,8 @@ type TestModules struct {
 	TestFeatures Features
 
 	defaultModules
+
+	MockAttestHardwareKey func(_ context.Context, _ interface{}, policy keys.PrivateKeyPolicy, _ *keys.AttestationStatement, _ crypto.PublicKey, _ time.Duration) (keys.PrivateKeyPolicy, error)
 }
 
 // SetTestModules sets the value returned from GetModules to testModules
@@ -84,6 +86,9 @@ func (m *TestModules) BuildType() string {
 }
 
 // AttestHardwareKey attests a hardware key.
-func (m *TestModules) AttestHardwareKey(_ context.Context, _ interface{}, policy keys.PrivateKeyPolicy, _ *keys.AttestationStatement, _ crypto.PublicKey, _ time.Duration) (keys.PrivateKeyPolicy, error) {
+func (m *TestModules) AttestHardwareKey(ctx context.Context, obj interface{}, policy keys.PrivateKeyPolicy, as *keys.AttestationStatement, pk crypto.PublicKey, d time.Duration) (keys.PrivateKeyPolicy, error) {
+	if m.MockAttestHardwareKey != nil {
+		return m.MockAttestHardwareKey(ctx, obj, policy, as, pk, d)
+	}
 	return policy, nil
 }

lib/web/apiserver.go

@@ -44,6 +44,7 @@ import (
 	apievents "github.com/gravitational/teleport/api/types/events"
 	"github.com/gravitational/teleport/api/types/installers"
 	apiutils "github.com/gravitational/teleport/api/utils"
+	"github.com/gravitational/teleport/api/utils/keys"
 	apisshutils "github.com/gravitational/teleport/api/utils/sshutils"
 	"github.com/gravitational/teleport/lib/auth"
 	wanlib "github.com/gravitational/teleport/lib/auth/webauthn"
@@ -1115,6 +1116,7 @@ func (h *Handler) getWebConfig(w http.ResponseWriter, r *http.Request, p httprou
 			AuthType:           authType,
 			PreferredLocalMFA:  cap.GetPreferredLocalMFA(),
 			LocalConnectorName: localConnectorName,
+			PrivateKeyPolicy:   cap.GetPrivateKeyPolicy(),
 		}
 	}
 
@@ -1710,6 +1712,12 @@ func (h *Handler) createWebSession(w http.ResponseWriter, r *http.Request, p htt
 	}
 	if err != nil {
 		h.log.WithError(err).Warnf("Access attempt denied for user %q.", req.User)
+		// Since checking for private key policy meant that they passed authn,
+		// return policy error as is to help direct user.
+		if keys.IsPrivateKeyPolicyError(err) {
+			return nil, trace.Wrap(err)
+		}
+		// Obscure all other errors.
 		return nil, trace.AccessDenied("invalid credentials")
 	}
 
@@ -1861,6 +1869,21 @@ func (h *Handler) changeUserAuthentication(w http.ResponseWriter, r *http.Reques
 		return nil, trace.Wrap(err)
 	}
 
+	if res.PrivateKeyPolicyEnabled {
+		if res.GetRecovery() == nil {
+			return &ui.ChangedUserAuthn{
+				PrivateKeyPolicyEnabled: res.PrivateKeyPolicyEnabled,
+			}, nil
+		}
+		return &ui.ChangedUserAuthn{
+			Recovery: ui.RecoveryCodes{
+				Codes:   res.GetRecovery().GetCodes(),
+				Created: &res.GetRecovery().Created,
+			},
+			PrivateKeyPolicyEnabled: res.PrivateKeyPolicyEnabled,
+		}, nil
+	}
+
 	sess := res.WebSession
 	ctx, err := h.auth.newSessionContext(r.Context(), sess.GetUser(), sess.GetName())
 	if err != nil {
@@ -1877,12 +1900,14 @@ func (h *Handler) changeUserAuthentication(w http.ResponseWriter, r *http.Reques
 	}
 
 	if res.GetRecovery() == nil {
-		return &ui.RecoveryCodes{}, nil
+		return &ui.ChangedUserAuthn{}, nil
 	}
 
-	return &ui.RecoveryCodes{
-		Codes:   res.Recovery.Codes,
-		Created: &res.Recovery.Created,
+	return &ui.ChangedUserAuthn{
+		Recovery: ui.RecoveryCodes{
+			Codes:   res.GetRecovery().GetCodes(),
+			Created: &res.GetRecovery().Created,
+		},
 	}, nil
 }
 
@@ -2036,6 +2061,12 @@ func (h *Handler) mfaLoginFinishSession(w http.ResponseWriter, r *http.Request,
 	clientMeta := clientMetaFromReq(r)
 	session, err := h.auth.AuthenticateWebUser(r.Context(), req, clientMeta)
 	if err != nil {
+		// Since checking for private key policy meant that they passed authn,
+		// return policy error as is to help direct user.
+		if keys.IsPrivateKeyPolicyError(err) {
+			return nil, trace.Wrap(err)
+		}
+		// Obscure all other errors.
 		return nil, trace.AccessDenied("invalid credentials")
 	}
 

lib/web/apiserver_login_test.go

@@ -16,6 +16,7 @@ package web
 
 import (
 	"context"
+	"crypto"
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
@@ -27,9 +28,11 @@ import (
 	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/constants"
 	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/api/utils/keys"
 	"github.com/gravitational/teleport/lib/auth"
 	"github.com/gravitational/teleport/lib/client"
 	"github.com/gravitational/teleport/lib/defaults"
+	"github.com/gravitational/teleport/lib/modules"
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ssh"
@@ -138,6 +141,65 @@ func TestWebauthnLogin_web(t *testing.T) {
 	require.NotEmpty(t, createSessionResp.SessionExpires.Unix())
 }
 
+func TestWebauthnLogin_webWithPrivateKeyEnabledError(t *testing.T) {
+	ctx := context.Background()
+	env := newWebPack(t, 1)
+	authPref := &types.AuthPreferenceSpecV2{
+		Type:         constants.Local,
+		SecondFactor: constants.SecondFactorOn,
+		Webauthn: &types.Webauthn{
+			RPID: env.server.TLS.ClusterName(),
+		},
+	}
+
+	// configureClusterForMFA will creates a user and a webauthn device,
+	// so we will enable the private key policy afterwards.
+	clusterMFA := configureClusterForMFA(t, env, authPref)
+	user := clusterMFA.User
+	password := clusterMFA.Password
+	device := clusterMFA.WebDev.Key
+
+	authPref.RequireMFAType = types.RequireMFAType_HARDWARE_KEY_TOUCH
+	cap, err := types.NewAuthPreference(*authPref)
+	require.NoError(t, err)
+	authServer := env.server.Auth()
+	err = authServer.SetAuthPreference(ctx, cap)
+	require.NoError(t, err)
+
+	modules.SetTestModules(t, &modules.TestModules{
+		MockAttestHardwareKey: func(_ context.Context, _ interface{}, policy keys.PrivateKeyPolicy, _ *keys.AttestationStatement, _ crypto.PublicKey, _ time.Duration) (keys.PrivateKeyPolicy, error) {
+			return "", keys.NewPrivateKeyPolicyError(policy)
+		},
+	})
+
+	clt, err := client.NewWebClient(env.proxies[0].webURL.String(), roundtrip.HTTPClient(client.NewInsecureWebClient()))
+	require.NoError(t, err)
+
+	// 1st login step: request challenge.
+	beginResp, err := clt.PostJSON(ctx, clt.Endpoint("webapi", "mfa", "login", "begin"), &client.MFAChallengeRequest{
+		User: user,
+		Pass: password,
+	})
+	require.NoError(t, err)
+	authChallenge := &client.MFAAuthenticateChallenge{}
+	require.NoError(t, json.Unmarshal(beginResp.Bytes(), authChallenge))
+	require.NotNil(t, authChallenge.WebauthnChallenge)
+
+	// Sign Webauthn challenge (requires user interaction in real-world
+	// scenarios).
+	assertionResp, err := device.SignAssertion("https://"+env.server.TLS.ClusterName(), authChallenge.WebauthnChallenge)
+	require.NoError(t, err)
+
+	// 2nd login step: reply with signed challenged.
+	sessionResp, err := clt.PostJSON(ctx, clt.Endpoint("webapi", "mfa", "login", "finishsession"), &client.AuthenticateWebUserRequest{
+		User:                      user,
+		WebauthnAssertionResponse: assertionResp,
+	})
+	var resErr httpErrorResponse
+	require.NoError(t, json.Unmarshal(sessionResp.Bytes(), &resErr))
+	require.Contains(t, resErr.Error.Message, keys.PrivateKeyPolicyHardwareKeyTouch)
+}
+
 func TestAuthenticate_passwordless(t *testing.T) {
 	env := newWebPack(t, 1)
 	clusterMFA := configureClusterForMFA(t, env, &types.AuthPreferenceSpecV2{