[reopen] Handle private key policy errors for the web UI (#17928)

- When user registers or resets, when running into
policy errors, we don't send back an error, but instead
a 200 (to indicate user has successfully registered/resetted)
and a flag to determine if policy was enabled
- Send back policy configurations for cluster config,
this will allow the web UI login page to just display
redirection messages without login attempts
- For role configured policy, login attempts will be
required and a specific error will be returned so that
the UI can then better redirect the user
This commit is contained in:
Lisa Kim
2022-10-31 23:18:54 +00:00
committed by GitHub
parent cf189090d9
commit ef92182ef4
12 changed files with 1016 additions and 732 deletions
File diff suppressed because it is too large Load Diff
+6 -1
View File
@@ -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"`
}
@@ -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
+2 -2
View File
@@ -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))
}
+6 -6
View File
@@ -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,
},
+12
View File
@@ -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)
}
+6 -1
View File
@@ -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
}
+35 -4
View File
@@ -54,6 +54,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"
@@ -1107,6 +1108,7 @@ func (h *Handler) getWebConfig(w http.ResponseWriter, r *http.Request, p httprou
AuthType: authType,
PreferredLocalMFA: cap.GetPreferredLocalMFA(),
LocalConnectorName: localConnectorName,
PrivateKeyPolicy: cap.GetPrivateKeyPolicy(),
}
}
@@ -1570,6 +1572,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")
}
@@ -1721,6 +1729,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 {
@@ -1737,12 +1760,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
}
@@ -1896,6 +1921,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")
}
+63
View File
@@ -16,6 +16,7 @@ package web
import (
"context"
"crypto"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
@@ -31,10 +32,12 @@ 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"
wanlib "github.com/gravitational/teleport/lib/auth/webauthn"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/modules"
)
func TestWebauthnLogin_ssh(t *testing.T) {
@@ -138,6 +141,66 @@ 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,
})
require.Error(t, err)
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{
+132
View File
@@ -22,6 +22,7 @@ import (
"bytes"
"compress/gzip"
"context"
"crypto"
"crypto/tls"
"crypto/x509"
"encoding/base32"
@@ -77,6 +78,7 @@ import (
apidefaults "github.com/gravitational/teleport/api/defaults"
"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/auth"
"github.com/gravitational/teleport/lib/auth/mocku2f"
"github.com/gravitational/teleport/lib/auth/native"
@@ -1548,6 +1550,58 @@ func TestPlayback(t *testing.T) {
t.Cleanup(func() { require.NoError(t, ws.Close()) })
}
type httpErrorMessage struct {
Message string `json:"message"`
}
type httpErrorResponse struct {
Error httpErrorMessage `json:"error"`
}
func TestLogin_PrivateKeyEnabledError(t *testing.T) {
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)
},
})
s := newWebSuite(t)
ap, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{
Type: constants.Local,
SecondFactor: constants.SecondFactorOff,
RequireMFAType: types.RequireMFAType_HARDWARE_KEY_TOUCH,
})
require.NoError(t, err)
err = s.server.Auth().SetAuthPreference(s.ctx, ap)
require.NoError(t, err)
// create user
s.createUser(t, "user1", "root", "password", "")
loginReq, err := json.Marshal(CreateSessionReq{
User: "user1",
Pass: "password",
})
require.NoError(t, err)
clt := s.client()
req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions"), bytes.NewBuffer(loginReq))
require.NoError(t, err)
ua := "test-ua"
req.Header.Set("User-Agent", ua)
csrfToken := "2ebcb768d0090ea4368e42880c970b61865c326172a4a2343b645cf5d7f20992"
addCSRFCookieToReq(req, csrfToken)
req.Header.Set("Content-Type", "application/json")
req.Header.Set(csrf.HeaderName, csrfToken)
re, err := clt.Client.RoundTrip(func() (*http.Response, error) {
return clt.Client.HTTPClient().Do(req)
})
require.NoError(t, err)
var resErr httpErrorResponse
require.NoError(t, json.Unmarshal(re.Bytes(), &resErr))
require.Contains(t, resErr.Error.Message, keys.PrivateKeyPolicyHardwareKeyTouch)
}
func TestLogin(t *testing.T) {
t.Parallel()
s := newWebSuite(t)
@@ -2942,6 +2996,7 @@ func TestGetWebConfig(t *testing.T) {
AuthType: constants.Local,
PreferredLocalMFA: constants.SecondFactorWebauthn,
LocalConnectorName: constants.PasswordlessConnector,
PrivateKeyPolicy: keys.PrivateKeyPolicyNone,
},
CanJoinSessions: true,
ProxyClusterName: env.server.ClusterName(),
@@ -3690,6 +3745,7 @@ func TestChangeUserAuthentication_recoveryCodesReturnedForCloud(t *testing.T) {
})
require.NoError(t, err)
require.Nil(t, re.Recovery)
require.False(t, re.PrivateKeyPolicyEnabled)
// Create a user that is valid for recovery.
teleUser, err = types.NewUser("valid-username@example.com")
@@ -3720,6 +3776,82 @@ func TestChangeUserAuthentication_recoveryCodesReturnedForCloud(t *testing.T) {
require.NoError(t, err)
require.Len(t, re.Recovery.Codes, 3)
require.NotEmpty(t, re.Recovery.Created)
require.False(t, re.PrivateKeyPolicyEnabled)
}
// TestChangeUserAuthentication_WithPrivacyPolicyEnabledError tests
// that when there is a privacy policy enabled error, we still get
// a non error response with recovery codes and a privacy policy
// flag set to true.
func TestChangeUserAuthentication_WithPrivacyPolicyEnabledError(t *testing.T) {
env := newWebPack(t, 1)
ctx := context.Background()
// Enable second factor required by cloud and a privacy policy.
ap, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{
Type: constants.Local,
SecondFactor: constants.SecondFactorOTP,
RequireMFAType: types.RequireMFAType_HARDWARE_KEY_TOUCH,
})
require.NoError(t, err)
err = env.server.Auth().SetAuthPreference(ctx, ap)
require.NoError(t, err)
// Enable cloud feature.
modules.SetTestModules(t, &modules.TestModules{
TestFeatures: modules.Features{
Cloud: true,
},
MockAttestHardwareKey: func(_ context.Context, _ interface{}, policy keys.PrivateKeyPolicy, _ *keys.AttestationStatement, _ crypto.PublicKey, _ time.Duration) (keys.PrivateKeyPolicy, error) {
return "", keys.NewPrivateKeyPolicyError(policy)
},
})
// Create a user that is valid for recovery.
teleUser, err := types.NewUser("valid-username@example.com")
require.NoError(t, err)
require.NoError(t, env.server.Auth().CreateUser(ctx, teleUser))
// Create a reset password token and secrets.
resetToken, err := env.server.Auth().CreateResetPasswordToken(ctx, auth.CreateUserTokenRequest{
Name: "valid-username@example.com",
})
require.NoError(t, err)
res, err := env.server.Auth().CreateRegisterChallenge(ctx, &authproto.CreateRegisterChallengeRequest{
TokenID: resetToken.GetName(),
DeviceType: authproto.DeviceType_DEVICE_TYPE_TOTP,
})
require.NoError(t, err)
totpCode, err := totp.GenerateCode(res.GetTOTP().GetSecret(), env.clock.Now())
require.NoError(t, err)
// Craft http request data.
clt := env.proxies[0].newClient(t)
req := changeUserAuthenticationRequest{
SecondFactorToken: totpCode,
Password: []byte("abc123"),
TokenID: resetToken.GetName(),
}
httpReqData, err := json.Marshal(req)
require.NoError(t, err)
// CSRF protected endpoint.
csrfToken := "2ebcb768d0090ea4368e42880c970b61865c326172a4a2343b645cf5d7f20992"
httpReq, err := http.NewRequest("PUT", clt.Endpoint("webapi", "users", "password", "token"), bytes.NewBuffer(httpReqData))
require.NoError(t, err)
addCSRFCookieToReq(httpReq, csrfToken)
httpReq.Header.Set("Content-Type", "application/json")
httpReq.Header.Set(csrf.HeaderName, csrfToken)
httpRes, err := httplib.ConvertResponse(clt.RoundTrip(func() (*http.Response, error) {
return clt.HTTPClient().Do(httpReq)
}))
require.NoError(t, err)
var apiRes ui.ChangedUserAuthn
require.NoError(t, json.Unmarshal(httpRes.Bytes(), &apiRes))
require.Len(t, apiRes.Recovery.Codes, 3)
require.NotEmpty(t, apiRes.Recovery.Created)
require.True(t, apiRes.PrivateKeyPolicyEnabled)
}
func TestParseSSORequestParams(t *testing.T) {
-27
View File
@@ -1,27 +0,0 @@
/*
Copyright 2020 Gravitational, Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package ui
import "time"
// RecoveryCodes describes RecoveryCodes UI object.
type RecoveryCodes struct {
// Codes are user's new recovery codes.
Codes []string `json:"codes,omitempty"`
// Created is when the codes were created.
Created *time.Time `json:"created,omitempty"`
}
+14
View File
@@ -29,3 +29,17 @@ type ResetPasswordToken struct {
// Expiry is token expiration time
Expiry time.Time `json:"expiry,omitempty"`
}
// RecoveryCodes describes RecoveryCodes UI object.
type RecoveryCodes struct {
// Codes are user's new recovery codes.
Codes []string `json:"codes,omitempty"`
// Created is when the codes were created.
Created *time.Time `json:"created,omitempty"`
}
// ChangedUserAuthn describes response after successfully changing authn.
type ChangedUserAuthn struct {
Recovery RecoveryCodes `json:"recovery"`
PrivateKeyPolicyEnabled bool `json:"privateKeyPolicyEnabled,omitempty"`
}