From bc5f6100bb41140d9fe3868b6cb1c520c2cbadcf Mon Sep 17 00:00:00 2001 From: joerger Date: Mon, 7 Oct 2024 17:27:51 -0700 Subject: [PATCH] Add SSO MFA ceremony support to WebUI per-session MFA. --- lib/auth/github.go | 5 +++ lib/client/sso/ceremony.go | 6 ++-- lib/client/sso/redirector.go | 5 +++ lib/client/weblogin.go | 4 +++ lib/defaults/defaults.go | 4 +-- lib/srv/desktop/tdp/proto.go | 8 ++--- lib/srv/desktop/tdp/proto_test.go | 4 +-- lib/web/apiserver.go | 8 +++++ lib/web/apiserver_test.go | 10 +++--- lib/web/desktop.go | 4 +-- lib/web/fuzz_test.go | 8 ++--- lib/web/mfa.go | 6 ++++ lib/web/mfa_codec.go | 4 +-- lib/web/mfajson/mfajson.go | 52 +++++++++++++++++++++---------- lib/web/terminal.go | 29 ++++++++++------- lib/web/terminal/terminal.go | 8 ++--- 16 files changed, 110 insertions(+), 55 deletions(-) diff --git a/lib/auth/github.go b/lib/auth/github.go index ed80928ce7a68..f07f53299f9ed 100644 --- a/lib/auth/github.go +++ b/lib/auth/github.go @@ -44,6 +44,7 @@ import ( "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/authz" + "github.com/gravitational/teleport/lib/client/sso" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/loginrule" @@ -978,6 +979,10 @@ func ValidateClientRedirect(clientRedirect string, ssoTestFlow bool, settings *t // they're used a lot in test code return nil } + if clientRedirect == sso.WebMFARedirect { + // If this is a SSO redirect in the WebUI, allow. + return nil + } u, err := url.Parse(clientRedirect) if err != nil { return trace.Wrap(err, "parsing client redirect URL") diff --git a/lib/client/sso/ceremony.go b/lib/client/sso/ceremony.go index 84dae6c14d6a8..815c77b323429 100644 --- a/lib/client/sso/ceremony.go +++ b/lib/client/sso/ceremony.go @@ -65,14 +65,14 @@ func NewCLICeremony(rd *Redirector, init CeremonyInit) *Ceremony { // Ceremony is a customizable SSO MFA ceremony. type MFACeremony struct { - clientCallbackURL string + ClientCallbackURL string HandleRedirect func(ctx context.Context, redirectURL string) error GetCallbackMFAToken func(ctx context.Context) (string, error) } // GetClientCallbackURL returns the client callback URL. func (m *MFACeremony) GetClientCallbackURL() string { - return m.clientCallbackURL + return m.ClientCallbackURL } // Run the SSO MFA ceremony. @@ -99,7 +99,7 @@ func (m *MFACeremony) Run(ctx context.Context, chal *proto.MFAAuthenticateChalle // NewCLIMFACeremony creates a new CLI SSO ceremony from the given redirector. func NewCLIMFACeremony(rd *Redirector) *MFACeremony { return &MFACeremony{ - clientCallbackURL: rd.ClientCallbackURL, + ClientCallbackURL: rd.ClientCallbackURL, HandleRedirect: rd.OpenRedirect, GetCallbackMFAToken: func(ctx context.Context) (string, error) { loginResp, err := rd.WaitForResponse(ctx) diff --git a/lib/client/sso/redirector.go b/lib/client/sso/redirector.go index 0becda83a9b8c..14d531e75e960 100644 --- a/lib/client/sso/redirector.go +++ b/lib/client/sso/redirector.go @@ -79,6 +79,11 @@ const ( // DefaultLoginURL is the default login page. DefaultLoginURL = "/web/login" + + // WebMFARedirect is the landing page for SSO MFA in the WebUI. The WebUI set up a listener + // on this page in order to capture the SSO MFA response regardless of what page the challenge + // was requested from. + WebMFARedirect = "/web/sso_confirm" ) // RedirectorConfig is configuration for an sso redirector. diff --git a/lib/client/weblogin.go b/lib/client/weblogin.go index 48e35696d8f5f..ebcb86ad92915 100644 --- a/lib/client/weblogin.go +++ b/lib/client/weblogin.go @@ -111,6 +111,8 @@ type MFAChallengeResponse struct { TOTPCode string `json:"totp_code,omitempty"` // WebauthnResponse is a response from a webauthn device. WebauthnResponse *wantypes.CredentialAssertionResponse `json:"webauthn_response,omitempty"` + // SSOResponse is a response from an SSO MFA flow. + SSOResponse *proto.SSOResponse `json:"sso_response,omitempty"` } // GetOptionalMFAResponseProtoReq converts response to a type proto.MFAAuthenticateResponse, @@ -457,6 +459,8 @@ type MFAAuthenticateChallenge struct { WebauthnChallenge *wantypes.CredentialAssertion `json:"webauthn_challenge"` // TOTPChallenge specifies whether TOTP is supported for this user. TOTPChallenge bool `json:"totp_challenge"` + // SSOChallenge is an SSO MFA challenge. + SSOChallenge *proto.SSOChallenge `json:"sso_challenge"` } // MFARegisterChallenge is an MFA register challenge sent on new MFA register. diff --git a/lib/defaults/defaults.go b/lib/defaults/defaults.go index df8061d6743bb..60d59896b3578 100644 --- a/lib/defaults/defaults.go +++ b/lib/defaults/defaults.go @@ -707,8 +707,8 @@ const ( // made for an existing file transfer request WebsocketFileTransferDecision = "t" - // WebsocketWebauthnChallenge is sending a webauthn challenge. - WebsocketWebauthnChallenge = "n" + // MFAChallenge is sending an MFA challenge. Only supports WebAuthn and SSO MFA. + MFAChallenge = "n" // WebsocketSessionMetadata is sending the data for a ssh session. WebsocketSessionMetadata = "s" diff --git a/lib/srv/desktop/tdp/proto.go b/lib/srv/desktop/tdp/proto.go index c7e063b1f8e7c..59c00dd19bde3 100644 --- a/lib/srv/desktop/tdp/proto.go +++ b/lib/srv/desktop/tdp/proto.go @@ -737,10 +737,10 @@ func DecodeMFA(in byteReader) (*MFA, error) { } s := string(mt) switch s { - case defaults.WebsocketWebauthnChallenge: + case defaults.MFAChallenge: default: return nil, trace.BadParameter( - "got mfa type %v, expected %v (WebAuthn)", mt, defaults.WebsocketWebauthnChallenge) + "got mfa type %v, expected %v (WebAuthn)", mt, defaults.MFAChallenge) } var length uint32 @@ -780,10 +780,10 @@ func DecodeMFAChallenge(in byteReader) (*MFA, error) { } s := string(mt) switch s { - case defaults.WebsocketWebauthnChallenge: + case defaults.MFAChallenge: default: return nil, trace.BadParameter( - "got mfa type %v, expected %v (WebAuthn)", mt, defaults.WebsocketWebauthnChallenge) + "got mfa type %v, expected %v (WebAuthn)", mt, defaults.MFAChallenge) } var length uint32 diff --git a/lib/srv/desktop/tdp/proto_test.go b/lib/srv/desktop/tdp/proto_test.go index 2967aadee8ec9..60c8e59fe203f 100644 --- a/lib/srv/desktop/tdp/proto_test.go +++ b/lib/srv/desktop/tdp/proto_test.go @@ -125,7 +125,7 @@ func TestMFA(t *testing.T) { c := NewConn(&fakeConn{Buffer: &buff}) mfaWant := &MFA{ - Type: defaults.WebsocketWebauthnChallenge[0], + Type: defaults.MFAChallenge[0], MFAAuthenticateChallenge: &client.MFAAuthenticateChallenge{ WebauthnChallenge: &wantypes.CredentialAssertion{ Response: wantypes.PublicKeyCredentialRequestOptions{ @@ -159,7 +159,7 @@ func TestMFA(t *testing.T) { require.Equal(t, mfaWant, mfaGot) respWant := &MFA{ - Type: defaults.WebsocketWebauthnChallenge[0], + Type: defaults.MFAChallenge[0], MFAAuthenticateResponse: &authproto.MFAAuthenticateResponse{ Response: &authproto.MFAAuthenticateResponse_Webauthn{ Webauthn: &wanpb.CredentialAssertionResponse{ diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 8e1e4ba43f691..7a841897dcd27 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -80,6 +80,7 @@ import ( "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/automaticupgrades" "github.com/gravitational/teleport/lib/client" + "github.com/gravitational/teleport/lib/client/sso" "github.com/gravitational/teleport/lib/defaults" dtconfig "github.com/gravitational/teleport/lib/devicetrust/config" "github.com/gravitational/teleport/lib/events" @@ -2207,6 +2208,13 @@ func ConstructSSHResponse(response AuthParams) (*url.URL, error) { return nil, trace.Wrap(err) } + // We don't use a secret key for WebUI SSO MFA redirects. The request ID itself is + // kept a secret on the front end to minimize the risk of a phishing attack. + if response.ClientRedirectURL == sso.WebMFARedirect && response.MFAToken != "" { + u.RawQuery = url.Values{"response": {string(out)}}.Encode() + return u, nil + } + // Extract secret out of the request. secretKey := u.Query().Get("secret_key") if secretKey == "" { diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 5e3fea922d245..7f8cc8a474f63 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -2181,7 +2181,7 @@ func TestTerminalRequireSessionMFA(t *testing.T) { envelope := &terminal.Envelope{ Version: defaults.WebsocketVersion, - Type: defaults.WebsocketWebauthnChallenge, + Type: defaults.MFAChallenge, Payload: string(webauthnResBytes), } protoBytes, err := proto.Marshal(envelope) @@ -2388,7 +2388,7 @@ func handleDesktopMFAWebauthnChallenge(t *testing.T, ws *websocket.Conn, dev *au }) require.NoError(t, err) err = tdpConn.WriteMessage(tdp.MFA{ - Type: defaults.WebsocketWebauthnChallenge[0], + Type: defaults.MFAChallenge[0], MFAAuthenticateResponse: &authproto.MFAAuthenticateResponse{ Response: &authproto.MFAAuthenticateResponse_Webauthn{ Webauthn: res.GetWebauthn(), @@ -10060,7 +10060,7 @@ func TestModeratedSessionWithMFA(t *testing.T) { envelope := &terminal.Envelope{ Version: defaults.WebsocketVersion, - Type: defaults.WebsocketWebauthnChallenge, + Type: defaults.MFAChallenge, Payload: string(webauthnResBytes), } envelopeBytes, err := proto.Marshal(envelope) @@ -10091,7 +10091,7 @@ func TestModeratedSessionWithMFA(t *testing.T) { envelope := &terminal.Envelope{ Version: defaults.WebsocketVersion, - Type: defaults.WebsocketWebauthnChallenge, + Type: defaults.MFAChallenge, Payload: string(webauthnResBytes), } envelopeBytes, err := proto.Marshal(envelope) @@ -10129,7 +10129,7 @@ func TestModeratedSessionWithMFA(t *testing.T) { envelope := &terminal.Envelope{ Version: defaults.WebsocketVersion, - Type: defaults.WebsocketWebauthnChallenge, + Type: defaults.MFAChallenge, Payload: string(webauthnResBytes), } envelopeBytes, err := proto.Marshal(envelope) diff --git a/lib/web/desktop.go b/lib/web/desktop.go index 9173a7bf57fc0..9bd869aceb2e6 100644 --- a/lib/web/desktop.go +++ b/lib/web/desktop.go @@ -379,7 +379,7 @@ func (h *Handler) performSessionMFACeremony( &client.MFAAuthenticateChallenge{ WebauthnChallenge: wantypes.CredentialAssertionFromProto(chal.WebauthnChallenge), }, - defaults.WebsocketWebauthnChallenge, + defaults.MFAChallenge, ) if err != nil { return nil, trace.Wrap(err) @@ -421,7 +421,7 @@ func (h *Handler) performSessionMFACeremony( break } - assertion, err := codec.DecodeResponse(buf, defaults.WebsocketWebauthnChallenge) + assertion, err := codec.DecodeResponse(buf, defaults.MFAChallenge) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/web/fuzz_test.go b/lib/web/fuzz_test.go index 6c1ec1363a847..1e3495d61b7cc 100644 --- a/lib/web/fuzz_test.go +++ b/lib/web/fuzz_test.go @@ -55,9 +55,9 @@ func FuzzTdpMFACodecDecodeChallenge(f *testing.F) { var normalBuf bytes.Buffer var maxSizeBuf bytes.Buffer // add initial bytes for protocol - _, err = normalBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.WebsocketWebauthnChallenge)[0]}) + _, err = normalBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.MFAChallenge)[0]}) require.NoError(f, err) - _, err = maxSizeBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.WebsocketWebauthnChallenge)[0]}) + _, err = maxSizeBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.MFAChallenge)[0]}) require.NoError(f, err) // Write the length using BigEndian encoding require.NoError(f, binary.Write(&normalBuf, binary.BigEndian, uint32(len(jsonData)))) @@ -84,9 +84,9 @@ func FuzzTdpMFACodecDecodeResponse(f *testing.F) { var normalBuf bytes.Buffer var maxSizeBuf bytes.Buffer // add initial bytes for protocol - _, err := normalBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.WebsocketWebauthnChallenge)[0]}) + _, err := normalBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.MFAChallenge)[0]}) require.NoError(f, err) - _, err = maxSizeBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.WebsocketWebauthnChallenge)[0]}) + _, err = maxSizeBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.MFAChallenge)[0]}) require.NoError(f, err) mfaData := []byte("fake-data") // Write the length using BigEndian encoding diff --git a/lib/web/mfa.go b/lib/web/mfa.go index ffa0dc641a1c2..6447fc0886285 100644 --- a/lib/web/mfa.go +++ b/lib/web/mfa.go @@ -30,6 +30,7 @@ import ( mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" "github.com/gravitational/teleport/lib/client" + "github.com/gravitational/teleport/lib/client/sso" "github.com/gravitational/teleport/lib/httplib" "github.com/gravitational/teleport/lib/reversetunnelclient" "github.com/gravitational/teleport/lib/web/ui" @@ -175,6 +176,7 @@ func (h *Handler) createAuthenticateChallengeHandle(w http.ResponseWriter, r *ht AllowReuse: allowReuse, UserVerificationRequirement: req.UserVerificationRequirement, }, + SSOClientRedirectURL: sso.WebMFARedirect, }) if err != nil { return nil, trace.Wrap(err) @@ -192,6 +194,7 @@ func (h *Handler) createAuthenticateChallengeWithTokenHandle(w http.ResponseWrit ChallengeExtensions: &mfav1.ChallengeExtensions{ Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ACCOUNT_RECOVERY, }, + SSOClientRedirectURL: sso.WebMFARedirect, }) if err != nil { return nil, trace.Wrap(err) @@ -484,5 +487,8 @@ func makeAuthenticateChallenge(protoChal *proto.MFAAuthenticateChallenge) *clien if protoChal.GetWebauthnChallenge() != nil { chal.WebauthnChallenge = wantypes.CredentialAssertionFromProto(protoChal.WebauthnChallenge) } + if protoChal.GetSSOChallenge() != nil { + chal.SSOChallenge = protoChal.GetSSOChallenge() + } return chal } diff --git a/lib/web/mfa_codec.go b/lib/web/mfa_codec.go index 5b916f1bbf654..1064efabba59c 100644 --- a/lib/web/mfa_codec.go +++ b/lib/web/mfa_codec.go @@ -76,10 +76,10 @@ type tdpMFACodec struct{} func (tdpMFACodec) Encode(chal *client.MFAAuthenticateChallenge, envelopeType string) ([]byte, error) { switch envelopeType { - case defaults.WebsocketWebauthnChallenge: + case defaults.MFAChallenge: default: return nil, trace.BadParameter( - "received envelope type %v, expected %v (WebAuthn)", envelopeType, defaults.WebsocketWebauthnChallenge) + "received envelope type %v, expected %v (WebAuthn)", envelopeType, defaults.MFAChallenge) } tdpMsg := tdp.MFA{ diff --git a/lib/web/mfajson/mfajson.go b/lib/web/mfajson/mfajson.go index d7a877224e4c2..799d9e51cabf7 100644 --- a/lib/web/mfajson/mfajson.go +++ b/lib/web/mfajson/mfajson.go @@ -25,30 +25,50 @@ import ( authproto "github.com/gravitational/teleport/api/client/proto" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" - "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/client" ) +// TODO(Joerger): DELETE IN v18.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. +type challengeResponse struct { + client.MFAChallengeResponse + *wantypes.CredentialAssertionResponse +} + // Decode parses a JSON-encoded MFA authentication response. // Only webauthn (type="n") is currently supported. func Decode(b []byte, typ string) (*authproto.MFAAuthenticateResponse, error) { - var resp *authproto.MFAAuthenticateResponse + var resp challengeResponse + if err := json.Unmarshal(b, &resp); err != nil { + return nil, trace.Wrap(err) + } - switch typ { - case defaults.WebsocketWebauthnChallenge: - var webauthnResponse wantypes.CredentialAssertionResponse - if err := json.Unmarshal(b, &webauthnResponse); err != nil { - return nil, trace.Wrap(err) - } - resp = &authproto.MFAAuthenticateResponse{ + switch { + case resp.CredentialAssertionResponse != nil: + return &authproto.MFAAuthenticateResponse{ Response: &authproto.MFAAuthenticateResponse_Webauthn{ - Webauthn: wantypes.CredentialAssertionResponseToProto(&webauthnResponse), + Webauthn: wantypes.CredentialAssertionResponseToProto(resp.WebauthnResponse), }, - } - + }, nil + case resp.WebauthnResponse != nil: + return &authproto.MFAAuthenticateResponse{ + Response: &authproto.MFAAuthenticateResponse_Webauthn{ + Webauthn: wantypes.CredentialAssertionResponseToProto(resp.WebauthnResponse), + }, + }, nil + case resp.SSOResponse != nil: + return &authproto.MFAAuthenticateResponse{ + Response: &authproto.MFAAuthenticateResponse_SSO{ + SSO: resp.SSOResponse, + }, + }, nil + case resp.TOTPCode != "": + // Note: we can support TOTP through the websocket if desired, we just need to add + // a TOTP prompt modal and flip the switch here. + return nil, trace.BadParameter("totp is not supported in the WebUI") default: - return nil, trace.BadParameter( - "received type %v, expected %v (WebAuthn)", typ, defaults.WebsocketWebauthnChallenge) + return nil, trace.BadParameter("invalid MFA response from web") } - - return resp, nil } diff --git a/lib/web/terminal.go b/lib/web/terminal.go index 4727499ce281c..315d080255d27 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -53,6 +53,7 @@ import ( "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/client/sso" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/multiplexer" @@ -615,25 +616,31 @@ func newMFACeremony(stream *terminal.WSStream, createAuthenticateChallenge mfa.C PromptConstructor: func(...mfa.PromptOpt) mfa.Prompt { return newMFAPrompt(stream) }, + SSOMFACeremonyConstructor: func(ctx context.Context) (mfa.SSOMFACeremony, error) { + return &sso.MFACeremony{ + ClientCallbackURL: sso.WebMFARedirect, + }, nil + }, } } func newMFAPrompt(stream *terminal.WSStream) mfa.Prompt { return mfa.PromptFunc(func(ctx context.Context, chal *authproto.MFAAuthenticateChallenge) (*authproto.MFAAuthenticateResponse, error) { - var challenge *client.MFAAuthenticateChallenge - var codec protobufMFACodec - // Convert from proto to JSON types. - switch { - case chal.GetWebauthnChallenge() != nil: - challenge = &client.MFAAuthenticateChallenge{ - WebauthnChallenge: wantypes.CredentialAssertionFromProto(chal.WebauthnChallenge), - } - default: - return nil, trace.AccessDenied("only hardware keys are supported on the web terminal, please register a hardware device to connect to this server") + var challenge client.MFAAuthenticateChallenge + if chal.WebauthnChallenge != nil { + challenge.WebauthnChallenge = wantypes.CredentialAssertionFromProto(chal.WebauthnChallenge) + } + if chal.SSOChallenge != nil { + challenge.SSOChallenge = chal.SSOChallenge + } + + if chal.WebauthnChallenge == nil && chal.SSOChallenge == nil { + return nil, trace.AccessDenied("only WebAuthn and SSO MFA methods are supported on the web terminal, please register a supported mfa method to connect to this server") } - if err := stream.WriteChallenge(challenge, codec); err != nil { + var codec protobufMFACodec + if err := stream.WriteChallenge(&challenge, codec); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/web/terminal/terminal.go b/lib/web/terminal/terminal.go index 58c7a732513e9..f4f1bc5f8ceaa 100644 --- a/lib/web/terminal/terminal.go +++ b/lib/web/terminal/terminal.go @@ -175,7 +175,7 @@ func (t *WSStream) processMessages(ctx context.Context) { switch envelope.Type { case defaults.WebsocketClose: return - case defaults.WebsocketWebauthnChallenge: + case defaults.MFAChallenge: select { case <-ctx.Done(): return @@ -223,7 +223,7 @@ type MFACodec interface { // websocket in the correct format. func (t *WSStream) WriteChallenge(challenge *client.MFAAuthenticateChallenge, codec MFACodec) error { // Send the challenge over the socket. - msg, err := codec.Encode(challenge, defaults.WebsocketWebauthnChallenge) + msg, err := codec.Encode(challenge, defaults.MFAChallenge) if err != nil { return trace.Wrap(err) } @@ -238,7 +238,7 @@ func (t *WSStream) ReadChallengeResponse(codec MFACodec) (*authproto.MFAAuthenti if !ok { return nil, io.EOF } - resp, err := codec.DecodeResponse([]byte(envelope.Payload), defaults.WebsocketWebauthnChallenge) + resp, err := codec.DecodeResponse([]byte(envelope.Payload), defaults.MFAChallenge) return resp, trace.Wrap(err) } @@ -249,7 +249,7 @@ func (t *WSStream) ReadChallenge(codec MFACodec) (*authproto.MFAAuthenticateChal if !ok { return nil, io.EOF } - challenge, err := codec.DecodeChallenge([]byte(envelope.Payload), defaults.WebsocketWebauthnChallenge) + challenge, err := codec.DecodeChallenge([]byte(envelope.Payload), defaults.MFAChallenge) return challenge, trace.Wrap(err) }