Skip to content

Commit

Permalink
Handle non-webauthn mfa response for file transfer, admin actions, an…
Browse files Browse the repository at this point in the history
…d app session.
  • Loading branch information
Joerger committed Dec 9, 2024
1 parent fa4ad23 commit 4d032e6
Show file tree
Hide file tree
Showing 16 changed files with 112 additions and 116 deletions.
32 changes: 27 additions & 5 deletions lib/client/weblogin.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ type MFAChallengeResponse struct {
WebauthnResponse *wantypes.CredentialAssertionResponse `json:"webauthn_response,omitempty"`
// SSOResponse is a response from an SSO MFA flow.
SSOResponse *SSOResponse `json:"sso_response"`
// TODO(Joerger): DELETE IN v19.0.0, WebauthnResponse used instead.
WebauthnAssertionResponse *wantypes.CredentialAssertionResponse `json:"webauthnAssertionResponse"`
}

// SSOResponse is a json compatible [proto.SSOResponse].
Expand All @@ -128,15 +130,25 @@ func (r *MFAChallengeResponse) GetOptionalMFAResponseProtoReq() (*proto.MFAAuthe
return nil, trace.BadParameter("only one MFA response field can be set")
}

if r.TOTPCode != "" {
switch {
case r.WebauthnResponse != nil:
return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_Webauthn{
Webauthn: wantypes.CredentialAssertionResponseToProto(r.WebauthnResponse),
}}, nil
case r.SSOResponse != nil:
return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_SSO{
SSO: &proto.SSOResponse{
RequestId: r.SSOResponse.RequestID,
Token: r.SSOResponse.Token,
},
}}, nil
case r.TOTPCode != "":
return &proto.MFAAuthenticateResponse{Response: &proto.MFAAuthenticateResponse_TOTP{
TOTP: &proto.TOTPResponse{Code: r.TOTPCode},
}}, nil
}

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

Expand All @@ -152,6 +164,16 @@ func (r *MFAChallengeResponse) GetOptionalMFAResponseProtoReq() (*proto.MFAAuthe
return nil, nil
}

func ParseMFAChallengeResponse(mfaResponseJSON []byte) (*proto.MFAAuthenticateResponse, error) {
var resp MFAChallengeResponse
if err := json.Unmarshal(mfaResponseJSON, &resp); err != nil {
return nil, trace.Wrap(err)
}

protoResp, err := resp.GetOptionalMFAResponseProtoReq()
return protoResp, trace.Wrap(err)
}

// CreateSSHCertReq is passed by tsh to authenticate a local user without MFA
// and receive short-lived certificates.
type CreateSSHCertReq struct {
Expand Down
10 changes: 3 additions & 7 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -4842,16 +4842,12 @@ func parseMFAResponseFromRequest(r *http.Request) error {
// context and returned.
func contextWithMFAResponseFromRequestHeader(ctx context.Context, requestHeader http.Header) (context.Context, error) {
if mfaResponseJSON := requestHeader.Get("Teleport-MFA-Response"); mfaResponseJSON != "" {
var resp mfaResponse
if err := json.Unmarshal([]byte(mfaResponseJSON), &resp); err != nil {
mfaResp, err := client.ParseMFAChallengeResponse([]byte(mfaResponseJSON))
if err != nil {
return nil, trace.Wrap(err)
}

return mfa.ContextWithMFAResponse(ctx, &proto.MFAAuthenticateResponse{
Response: &proto.MFAAuthenticateResponse_Webauthn{
Webauthn: wantypes.CredentialAssertionResponseToProto(resp.WebauthnAssertionResponse),
},
}), nil
return mfa.ContextWithMFAResponse(ctx, mfaResp), nil
}

return ctx, nil
Expand Down
8 changes: 3 additions & 5 deletions lib/web/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5589,10 +5589,6 @@ func TestCreateAppSession_RequireSessionMFA(t *testing.T) {
require.NoError(t, err)
mfaResp, err := webauthnDev.SolveAuthn(chal)
require.NoError(t, err)
mfaRespJSON, err := json.Marshal(mfaResponse{
WebauthnAssertionResponse: wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn()),
})
require.NoError(t, err)

// Extract the session ID and bearer token for the current session.
rawCookie := *pack.cookies[0]
Expand Down Expand Up @@ -5626,7 +5622,9 @@ func TestCreateAppSession_RequireSessionMFA(t *testing.T) {
PublicAddr: "panel.example.com",
ClusterName: "localhost",
},
MFAResponse: string(mfaRespJSON),
MFAResponse: client.MFAChallengeResponse{
WebauthnAssertionResponse: wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn()),
},
},
expectMFAVerified: true,
},
Expand Down
29 changes: 15 additions & 14 deletions lib/web/apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package web

import (
"context"
"encoding/json"
"net/http"
"sort"

Expand All @@ -33,7 +32,7 @@ import (
"github.com/gravitational/teleport/api/client/proto"
apidefaults "github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/httplib"
"github.com/gravitational/teleport/lib/reversetunnelclient"
"github.com/gravitational/teleport/lib/utils"
Expand Down Expand Up @@ -191,7 +190,10 @@ type CreateAppSessionRequest struct {
// AWSRole is the AWS role ARN when accessing AWS management console.
AWSRole string `json:"arn,omitempty"`
// MFAResponse is an optional MFA response used to create an MFA verified app session.
MFAResponse string `json:"mfa_response"`
MFAResponse client.MFAChallengeResponse `json:"mfaResponse"`
// TODO(Joerger): DELETE IN v19.0.0
// Backwards compatible version of MFAResponse
MFAResponseJSON string `json:"mfa_response"`
}

// CreateAppSessionResponse is a response to POST /v1/webapi/sessions/app
Expand Down Expand Up @@ -230,17 +232,16 @@ func (h *Handler) createAppSession(w http.ResponseWriter, r *http.Request, p htt
}
}

var mfaProtoResponse *proto.MFAAuthenticateResponse
if req.MFAResponse != "" {
var resp mfaResponse
if err := json.Unmarshal([]byte(req.MFAResponse), &resp); err != nil {
return nil, trace.Wrap(err)
}
mfaResponse, err := req.MFAResponse.GetOptionalMFAResponseProtoReq()
if err != nil {
return nil, trace.Wrap(err)
}

mfaProtoResponse = &proto.MFAAuthenticateResponse{
Response: &proto.MFAAuthenticateResponse_Webauthn{
Webauthn: wantypes.CredentialAssertionResponseToProto(resp.WebauthnAssertionResponse),
},
// Fallback to backwards compatible mfa response.
if mfaResponse == nil && req.MFAResponseJSON != "" {
mfaResponse, err = client.ParseMFAChallengeResponse([]byte(req.MFAResponseJSON))
if err != nil {
return nil, trace.Wrap(err)
}
}

Expand All @@ -263,7 +264,7 @@ func (h *Handler) createAppSession(w http.ResponseWriter, r *http.Request, p htt
PublicAddr: result.App.GetPublicAddr(),
ClusterName: result.ClusterName,
AWSRoleARN: req.AWSRole,
MFAResponse: mfaProtoResponse,
MFAResponse: mfaResponse,
AppName: result.App.GetName(),
URI: result.App.GetURI(),
ClientAddr: r.RemoteAddr,
Expand Down
44 changes: 19 additions & 25 deletions lib/web/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package web

import (
"context"
"encoding/json"
"errors"
"net/http"
"time"
Expand All @@ -35,7 +34,6 @@ import (
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/api/utils/sshutils"
"github.com/gravitational/teleport/lib/auth/authclient"
wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/multiplexer"
"github.com/gravitational/teleport/lib/reversetunnelclient"
Expand All @@ -56,8 +54,8 @@ type fileTransferRequest struct {
remoteLocation string
// filename is a file name
filename string
// webauthn is an optional parameter that contains a webauthn response string used to issue single use certs
webauthn string
// mfaResponse is an optional parameter that contains an mfa response string used to issue single use certs
mfaResponse string
// fileTransferRequestID is used to find a FileTransferRequest on a session
fileTransferRequestID string
// moderatedSessonID is an ID of a moderated session that has completed a
Expand All @@ -74,11 +72,22 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou
remoteLocation: query.Get("location"),
filename: query.Get("filename"),
namespace: defaults.Namespace,
webauthn: query.Get("webauthn"),
mfaResponse: query.Get("mfaResponse"),
fileTransferRequestID: query.Get("fileTransferRequestId"),
moderatedSessionID: query.Get("moderatedSessionId"),
}

// Check for old query parameter, uses the same data structure.
// TODO(Joerger): DELETE IN v19.0.0
if req.mfaResponse == "" {
req.mfaResponse = query.Get("webauthn")
}

mfaResponse, err := client.ParseMFAChallengeResponse([]byte(req.mfaResponse))
if err != nil {
return nil, trace.Wrap(err)
}

// Send an error if only one of these params has been sent. Both should exist or not exist together
if (req.fileTransferRequestID != "") != (req.moderatedSessionID != "") {
return nil, trace.BadParameter("fileTransferRequestId and moderatedSessionId must both be included in the same request.")
Expand Down Expand Up @@ -107,7 +116,7 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou
return nil, trace.Wrap(err)
}

if mfaReq.Required && query.Get("webauthn") == "" {
if mfaReq.Required && mfaResponse == nil {
return nil, trace.AccessDenied("MFA required for file transfer")
}

Expand Down Expand Up @@ -135,8 +144,8 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou
return nil, trace.Wrap(err)
}

if req.webauthn != "" {
err = ft.issueSingleUseCert(req.webauthn, r, tc)
if req.mfaResponse != "" {
err = ft.issueSingleUseCert(mfaResponse, r, tc)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -216,21 +225,10 @@ func (f *fileTransfer) createClient(req fileTransferRequest, httpReq *http.Reque
return tc, nil
}

type mfaResponse struct {
// WebauthnResponse is the response from authenticators.
WebauthnAssertionResponse *wantypes.CredentialAssertionResponse `json:"webauthnAssertionResponse"`
}

// issueSingleUseCert will take an assertion response sent from a solved challenge in the web UI
// and use that to generate a cert. This cert is added to the Teleport Client as an authmethod that
// can be used to connect to a node.
func (f *fileTransfer) issueSingleUseCert(webauthn string, httpReq *http.Request, tc *client.TeleportClient) error {
var mfaResp mfaResponse
err := json.Unmarshal([]byte(webauthn), &mfaResp)
if err != nil {
return trace.Wrap(err)
}

func (f *fileTransfer) issueSingleUseCert(mfaResponse *proto.MFAAuthenticateResponse, httpReq *http.Request, tc *client.TeleportClient) error {
pk, err := keys.ParsePrivateKey(f.sctx.cfg.Session.GetSSHPriv())
if err != nil {
return trace.Wrap(err)
Expand All @@ -241,11 +239,7 @@ func (f *fileTransfer) issueSingleUseCert(webauthn string, httpReq *http.Request
SSHPublicKey: pk.MarshalSSHPublicKey(),
Username: f.sctx.GetUser(),
Expires: time.Now().Add(time.Minute).UTC(),
MFAResponse: &proto.MFAAuthenticateResponse{
Response: &proto.MFAAuthenticateResponse_Webauthn{
Webauthn: wantypes.CredentialAssertionResponseToProto(mfaResp.WebauthnAssertionResponse),
},
},
MFAResponse: mfaResponse,
})
if err != nil {
return trace.Wrap(err)
Expand Down
21 changes: 12 additions & 9 deletions lib/web/mfajson/mfajson.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"github.com/gravitational/teleport/lib/client"
)

// TODO(Joerger): DELETE IN v18.0.0 and use client.MFAChallengeResponse instead.
// TODO(Joerger): DELETE IN v19.0.0 and use client.MFAChallengeResponse instead.
// Before v17, the WebUI sends a flattened webauthn response instead of a full
// MFA challenge response. Newer WebUI versions v17+ will send both for
// backwards compatibility.
Expand All @@ -45,14 +45,17 @@ func Decode(b []byte, typ string) (*authproto.MFAAuthenticateResponse, error) {
return nil, trace.Wrap(err)
}

// TODO(Joerger): DELETE in v18.0.0, client.MFAChallengeResponse is be used instead.
if resp.CredentialAssertionResponse != nil {
return &authproto.MFAAuthenticateResponse{
Response: &authproto.MFAAuthenticateResponse_Webauthn{
Webauthn: wantypes.CredentialAssertionResponseToProto(resp.CredentialAssertionResponse),
},
}, nil
// Move flattened webauthn response into resp.
resp.MFAChallengeResponse.WebauthnAssertionResponse = resp.CredentialAssertionResponse

protoResp, err := resp.GetOptionalMFAResponseProtoReq()
if err != nil {
return nil, trace.Wrap(err)
}

if protoResp == nil {
return nil, trace.BadParameter("invalid MFA response from web")
}

return resp.GetOptionalMFAResponseProtoReq()
return protoResp, trace.Wrap(err)
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import auth from 'teleport/services/auth';

import { DeleteAuthDeviceWizardStepProps } from './DeleteAuthDeviceWizard';

import { dummyPasskey, dummyHardwareDevice, deviceCases } from './deviceCases';
import { dummyPasskey, dummyHardwareDevice } from './deviceCases';

import { DeleteAuthDeviceWizard } from '.';

Expand Down
7 changes: 2 additions & 5 deletions web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,14 @@ export default function useGetScpUrl(addMfaToScpUrls: boolean) {
scope: MfaChallengeScope.USER_SESSION,
});

const response = await auth.getMfaChallengeResponse(
challenge,
'webauthn'
);
const mfaResponse = await auth.getMfaChallengeResponse(challenge);

setAttempt({
status: 'success',
statusText: '',
});
return cfg.getScpUrl({
webauthn: response.webauthn_response,
mfaResponse,
...params,
});
} catch (error) {
Expand Down
20 changes: 14 additions & 6 deletions web/packages/teleport/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ import type {

import type { SortType } from 'teleport/services/agents';
import type { RecordingType } from 'teleport/services/recordings';
import type { WebauthnAssertionResponse } from './services/mfa';
import type {
MfaChallengeResponse,
WebauthnAssertionResponse,
} from './services/mfa';
import type {
PluginKind,
Regions,
Expand Down Expand Up @@ -871,20 +874,25 @@ const cfg = {
});
},

getScpUrl({ webauthn, ...params }: UrlScpParams) {
getScpUrl({ mfaResponse, ...params }: UrlScpParams) {
let path = generatePath(cfg.api.scp, {
...params,
});

if (!webauthn) {
if (!mfaResponse) {
return path;
}
// non-required MFA will mean this param is undefined and generatePath doesn't like undefined
// or optional params. So we append it ourselves here. Its ok to be undefined when sent to the server
// as the existence of this param is what will issue certs
return `${path}&webauthn=${JSON.stringify({
webauthnAssertionResponse: webauthn,

// TODO(Joerger): DELETE IN v19.0.0
// We include webauthn for backwards compatibility.
path = `${path}&webauthn=${JSON.stringify({
webauthnAssertionResponse: mfaResponse.webauthn_response,
})}`;

return `${path}&mfaResponse=${JSON.stringify(mfaResponse)}`;
},

getRenewTokenUrl() {
Expand Down Expand Up @@ -1232,7 +1240,7 @@ export interface UrlScpParams {
filename: string;
moderatedSessionId?: string;
fileTransferRequestId?: string;
webauthn?: WebauthnAssertionResponse;
mfaResponse?: MfaChallengeResponse;
}

export interface UrlSshParams {
Expand Down
Loading

0 comments on commit 4d032e6

Please sign in to comment.