Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v10] Add per-session mfa support to connection testers (#22528) #22922

Merged
merged 4 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1,569 changes: 818 additions & 751 deletions api/client/proto/authservice.pb.go

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions api/proto/teleport/legacy/client/proto/authservice.proto
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,11 @@ message UserCertsRequest {
// ConnectionDiagnosticID is the ID of the ConnectionDiagnostic resource we should use to add
// traces as we pass certain checkpoints.
string ConnectionDiagnosticID = 16 [(gogoproto.jsontag) = "connection_diagnostic_id,omitempty"];

// MFAResponse is a response to a challenge from a user's MFA device.
// An optional field, that when provided, the response will be validated
// and the ID of the validated MFA device will be stored in the certificate.
MFAAuthenticateResponse MFAResponse = 17 [(gogoproto.jsontag) = "mfa_response,omitempty"];
}

// RouteToDatabase combines parameters for database service routing information.
Expand Down
11 changes: 11 additions & 0 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -2422,6 +2422,16 @@ func (a *ServerWithRoles) GenerateUserCerts(ctx context.Context, req proto.UserC
func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserCertsRequest, opts ...certRequestOption) (*proto.Certs, error) {
var err error

var verifiedMFADeviceID string
if req.MFAResponse != nil {
dev, _, err := a.authServer.validateMFAAuthResponse(
ctx, req.GetMFAResponse(), req.Username, false /* passwordless */)
if err != nil {
return nil, trace.Wrap(err)
}
verifiedMFADeviceID = dev.Id
}

// this prevents clients who have no chance at getting a cert and impersonating anyone
// from enumerating local users and hitting database
if !a.hasBuiltinRole(types.RoleAdmin) && !a.context.Checker.CanImpersonateSomeone() && req.Username != a.context.User.GetName() {
Expand Down Expand Up @@ -2594,6 +2604,7 @@ func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserC
// Generate certificate, note that the roles TTL will be ignored because
// the request is coming from "tctl auth sign" itself.
certReq := certRequest{
mfaVerified: verifiedMFADeviceID,
user: user,
ttl: req.Expires.Sub(a.authServer.GetClock().Now()),
compatibility: req.Format,
Expand Down
83 changes: 83 additions & 0 deletions lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/pquerna/otp/totp"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport"
Expand All @@ -50,6 +51,88 @@ import (
"github.com/gravitational/teleport/lib/tlsca"
)

func TestGenerateUserCerts_MFAVerifiedFieldSet(t *testing.T) {
t.Parallel()
srv := newTestTLSServer(t)

u, err := createUserWithSecondFactors(srv)
require.NoError(t, err)
client, err := srv.NewClient(TestUser(u.username))
require.NoError(t, err)

_, pub, err := testauthority.New().GenerateKeyPair()
require.NoError(t, err)

for _, test := range []struct {
desc string
getMFAResponse func() *proto.MFAAuthenticateResponse
wantErr string
}{
{
desc: "valid mfa response",
getMFAResponse: func() *proto.MFAAuthenticateResponse {
// Get a totp code to re-auth.
totpCode, err := totp.GenerateCode(u.totpDev.TOTPSecret, srv.AuthServer.Clock().Now().Add(30*time.Second))
require.NoError(t, err)

return &proto.MFAAuthenticateResponse{
Response: &proto.MFAAuthenticateResponse_TOTP{
TOTP: &proto.TOTPResponse{Code: totpCode},
},
}
},
},
{
desc: "valid empty mfa response",
getMFAResponse: func() *proto.MFAAuthenticateResponse {
return nil
},
},
{
desc: "invalid mfa response",
wantErr: "invalid totp token",
getMFAResponse: func() *proto.MFAAuthenticateResponse {
return &proto.MFAAuthenticateResponse{
Response: &proto.MFAAuthenticateResponse_TOTP{
TOTP: &proto.TOTPResponse{Code: "invalid-totp-code"},
},
}
},
},
} {
test := test
t.Run(test.desc, func(t *testing.T) {
mfaResponse := test.getMFAResponse()
certs, err := client.GenerateUserCerts(context.Background(), proto.UserCertsRequest{
PublicKey: pub,
Username: u.username,
Expires: time.Now().Add(time.Hour),
MFAResponse: mfaResponse,
})

switch {
case test.wantErr != "":
require.True(t, trace.IsAccessDenied(err), "GenerateUserCerts returned err = %v (%T), wanted trace.AccessDenied", err, err)
require.ErrorContains(t, err, test.wantErr)
return
default:
require.NoError(t, err)
}

sshCert, err := sshutils.ParseCertificate(certs.SSH)
require.NoError(t, err)
mfaVerified := sshCert.Permissions.Extensions[teleport.CertExtensionMFAVerified]

switch {
case mfaResponse == nil:
require.Empty(t, mfaVerified, "GenerateUserCerts returned certificate with non-empty CertExtensionMFAVerified")
default:
require.Equal(t, mfaVerified, u.totpDev.MFA.Id, "GenerateUserCerts returned certificate with unexpected CertExtensionMFAVerified")
}
})
}
}

// TestLocalUserCanReissueCerts tests that local users can reissue
// certificates for themselves with varying TTLs.
func TestLocalUserCanReissueCerts(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions lib/client/conntest/connection_tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@ import (
"github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/client"
)

// TestConnectionRequest contains
// - the identification of the resource kind and resource name to test
// - additional paramenters which depend on the actual kind of resource to test
// As an example, for SSH Node it also includes the User/Principal that will be used to login.
type TestConnectionRequest struct {
// MFAResponse is an optional field that holds a response to a MFA device challenge.
MFAResponse client.MFAChallengeResponse `json:"mfa_response,omitempty"`
// ResourceKind describes the type of resource to test.
ResourceKind string `json:"resource_kind"`
// ResourceName is the identification of the resource's instance to test.
Expand Down
10 changes: 8 additions & 2 deletions lib/client/conntest/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,12 @@ func (s *KubeConnectionTester) TestConnection(ctx context.Context, req TestConne
return nil, trace.Wrap(err)
}

tlsCfg, err := s.genKubeRestTLSClientConfig(ctx, connectionDiagnosticID, req.ResourceName, currentUser.GetName())
mfaResponse, err := req.MFAResponse.GetOptionalMFAResponseProtoReq()
if err != nil {
return nil, trace.Wrap(err)
}

tlsCfg, err := s.genKubeRestTLSClientConfig(ctx, mfaResponse, connectionDiagnosticID, req.ResourceName, currentUser.GetName())
diag, diagErr := s.handleUserGenCertsErr(ctx, req.ResourceName, connectionDiagnosticID, err)
if err != nil || diagErr != nil {
return diag, diagErr
Expand Down Expand Up @@ -147,7 +152,7 @@ func (s *KubeConnectionTester) TestConnection(ctx context.Context, req TestConne

// genKubeRestTLSClientConfig creates the Teleport user credentials to access
// the given Kubernetes cluster name.
func (s KubeConnectionTester) genKubeRestTLSClientConfig(ctx context.Context, connectionDiagnosticID string, clusterName, userName string) (rest.TLSClientConfig, error) {
func (s KubeConnectionTester) genKubeRestTLSClientConfig(ctx context.Context, mfaResponse *proto.MFAAuthenticateResponse, connectionDiagnosticID, clusterName, userName string) (rest.TLSClientConfig, error) {
key, err := client.GenerateRSAKey()
if err != nil {
return rest.TLSClientConfig{}, trace.Wrap(err)
Expand All @@ -159,6 +164,7 @@ func (s KubeConnectionTester) genKubeRestTLSClientConfig(ctx context.Context, co
Expires: time.Now().Add(time.Minute).UTC(),
ConnectionDiagnosticID: connectionDiagnosticID,
KubernetesCluster: clusterName,
MFAResponse: mfaResponse,
})
if err != nil {
return rest.TLSClientConfig{}, trace.Wrap(err)
Expand Down
6 changes: 6 additions & 0 deletions lib/client/conntest/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,17 @@ func (s *SSHConnectionTester) TestConnection(ctx context.Context, req TestConnec
return nil, trace.Wrap(err)
}

mfaResponse, err := req.MFAResponse.GetOptionalMFAResponseProtoReq()
if err != nil {
return nil, trace.Wrap(err)
}

certs, err := s.cfg.UserClient.GenerateUserCerts(ctx, proto.UserCertsRequest{
PublicKey: key.MarshalSSHPublicKey(),
Username: currentUser.GetName(),
Expires: time.Now().Add(time.Minute).UTC(),
ConnectionDiagnosticID: connectionDiagnosticID,
MFAResponse: mfaResponse,
})
if err != nil {
return nil, trace.Wrap(err)
Expand Down
30 changes: 30 additions & 0 deletions lib/client/weblogin.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,36 @@ type MFAChallengeRequest struct {
Passwordless bool `json:"passwordless"`
}

// MFAChallengeResponse holds the response to a MFA challenge.
type MFAChallengeResponse struct {
// TOTPCode is a code for a otp device.
TOTPCode string `json:"totp_code,omitempty"`
// WebauthnResponse is a response from a webauthn device.
WebauthnResponse *wanlib.CredentialAssertionResponse `json:"webauthn_response,omitempty"`
}

// GetOptionalMFAResponseProtoReq converts response to a type proto.MFAAuthenticateResponse,
// if there were any responses set. Otherwise returns nil.
func (r *MFAChallengeResponse) GetOptionalMFAResponseProtoReq() (*proto.MFAAuthenticateResponse, error) {
if r.TOTPCode != "" && r.WebauthnResponse != nil {
return nil, trace.BadParameter("only one MFA response field can be set")
}

if r.TOTPCode != "" {
return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_TOTP{
TOTP: &proto.TOTPResponse{Code: r.TOTPCode},
}}, nil
}

if r.WebauthnResponse != nil {
return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_Webauthn{
Webauthn: wanlib.CredentialAssertionResponseToProto(r.WebauthnResponse),
}}, nil
}

return nil, nil
}

// CreateSSHCertReq are passed by web client
// to authenticate against teleport server and receive
// a temporary cert signed by auth server authority
Expand Down
1 change: 1 addition & 0 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ func (h *Handler) bindDefaultEndpoints() {
h.POST("/webapi/github/login/console", h.WithLimiter(h.githubLoginConsole))

// MFA public endpoints.
h.POST("/webapi/sites/:site/mfa/required", h.WithClusterAuth(h.isMFARequired))
h.POST("/webapi/mfa/login/begin", h.WithLimiter(h.mfaLoginBegin))
h.POST("/webapi/mfa/login/finish", httplib.MakeHandler(h.mfaLoginFinish))
h.POST("/webapi/mfa/login/finishsession", httplib.MakeHandler(h.mfaLoginFinishSession))
Expand Down