[v15] Scoped WebAuthn: Refactor SessionData

api/proto/teleport/legacy/types/webauthn/webauthn.proto

@@ -41,23 +41,6 @@ option (gogoproto.unmarshaler_all) = true;
 // WebAuthn messages used by server storage.
 // -----------------------------------------------------------------------------
 
-// SessionData stored by the Relying Party during authentication ceremonies.
-// Mirrors https://pkg.go.dev/github.com/go-webauthn/webauthn/webauthn#SessionData.
-message SessionData {
-  // Raw challenge used for the ceremony.
-  bytes challenge = 1 [(gogoproto.jsontag) = "challenge,omitempty"];
-  // Raw User ID.
-  bytes user_id = 2 [(gogoproto.jsontag) = "userId,omitempty"];
-  // Raw Credential IDs of the credentials allowed for the ceremony.
-  repeated bytes allow_credentials = 3 [(gogoproto.jsontag) = "allowCredentials,omitempty"];
-  // True if resident keys were required by the server / Relying Party.
-  bool resident_key = 4 [(gogoproto.jsontag) = "residentKey,omitempty"];
-  // Requested user verification requirement, either "discouraged" or
-  // "required".
-  // An empty value is treated equivalently to "discouraged".
-  string user_verification = 5 [(gogoproto.jsontag) = "userVerification,omitempty"];
-}
-
 // User represents a WebAuthn user.
 // Used mainly to correlated a WebAuthn user handle with a Teleport user.
 message User {

lib/auth/webauthn/login.go

@@ -33,7 +33,6 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport/api/types"
-	wanpb "github.com/gravitational/teleport/api/types/webauthn"
 	wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
 )
 
@@ -53,8 +52,8 @@ type loginIdentity interface {
 // * Passwordless uses global variants
 // (services.Identity.Update/Get/DeleteGlobalWebauthnSessionData methods).
 type sessionIdentity interface {
-	Upsert(ctx context.Context, user string, sd *wanpb.SessionData) error
-	Get(ctx context.Context, user string, challenge string) (*wanpb.SessionData, error)
+	Upsert(ctx context.Context, user string, sd *wantypes.SessionData) error
+	Get(ctx context.Context, user string, challenge string) (*wantypes.SessionData, error)
 	Delete(ctx context.Context, user string, challenge string) error
 }
 
@@ -163,11 +162,13 @@ func (f *loginFlow) begin(ctx context.Context, user string, passwordless bool) (
 	}
 
 	// Store SessionData - it's checked against the user response by Finish.
-	sessionDataPB, err := sessionToPB(sessionData)
+	sd, err := wantypes.SessionDataFromProtocol(sessionData)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	if err := f.sessionData.Upsert(ctx, user, sessionDataPB); err != nil {
+	// TODO(Joerger): set challenge extensions from caller
+
+	if err := f.sessionData.Upsert(ctx, user, sd); err != nil {
 		return nil, trace.Wrap(err)
 	}
 
@@ -254,11 +255,11 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred
 	// Fetch the previously-stored SessionData, so it's checked against the user
 	// response.
 	challenge := parsedResp.Response.CollectedClientData.Challenge
-	sessionDataPB, err := f.sessionData.Get(ctx, user, challenge)
+	sd, err := f.sessionData.Get(ctx, user, challenge)
 	if err != nil {
 		return nil, "", trace.Wrap(err)
 	}
-	sessionData := sessionFromPB(sessionDataPB)
+	sessionData := wantypes.SessionDataToProtocol(sd)
 
 	// Make sure _all_ credentials in the session are accounted for by the user.
 	// webauthn.ValidateLogin requires it.

lib/auth/webauthn/login_mfa.go

@@ -25,7 +25,6 @@ import (
 	"github.com/gravitational/trace"
 
 	"github.com/gravitational/teleport/api/types"
-	wanpb "github.com/gravitational/teleport/api/types/webauthn"
 	wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
 )
 
@@ -44,8 +43,8 @@ type LoginIdentity interface {
 
 	GetMFADevices(ctx context.Context, user string, withSecrets bool) ([]*types.MFADevice, error)
 	UpsertMFADevice(ctx context.Context, user string, d *types.MFADevice) error
-	UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wanpb.SessionData) error
-	GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wanpb.SessionData, error)
+	UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wantypes.SessionData) error
+	GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wantypes.SessionData, error)
 	DeleteWebauthnSessionData(ctx context.Context, user, sessionID string) error
 }
 
@@ -130,11 +129,11 @@ func (m mfaIdentity) GetTeleportUserByWebauthnID(_ context.Context, _ []byte) (s
 // userSessionStorage implements sessionIdentity using LoginFlow.
 type userSessionStorage LoginFlow
 
-func (s *userSessionStorage) Upsert(ctx context.Context, user string, sd *wanpb.SessionData) error {
+func (s *userSessionStorage) Upsert(ctx context.Context, user string, sd *wantypes.SessionData) error {
 	return s.Identity.UpsertWebauthnSessionData(ctx, user, scopeLogin, sd)
 }
 
-func (s *userSessionStorage) Get(ctx context.Context, user string, _ string) (*wanpb.SessionData, error) {
+func (s *userSessionStorage) Get(ctx context.Context, user string, _ string) (*wantypes.SessionData, error) {
 	return s.Identity.GetWebauthnSessionData(ctx, user, scopeLogin)
 }
 

lib/auth/webauthn/login_passwordless.go

@@ -24,7 +24,6 @@ import (
 	"errors"
 
 	"github.com/gravitational/teleport/api/types"
-	wanpb "github.com/gravitational/teleport/api/types/webauthn"
 	wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
 )
 
@@ -34,8 +33,8 @@ type PasswordlessIdentity interface {
 	GetMFADevices(ctx context.Context, user string, withSecrets bool) ([]*types.MFADevice, error)
 	UpsertMFADevice(ctx context.Context, user string, d *types.MFADevice) error
 
-	UpsertGlobalWebauthnSessionData(ctx context.Context, scope, id string, sd *wanpb.SessionData) error
-	GetGlobalWebauthnSessionData(ctx context.Context, scope, id string) (*wanpb.SessionData, error)
+	UpsertGlobalWebauthnSessionData(ctx context.Context, scope, id string, sd *wantypes.SessionData) error
+	GetGlobalWebauthnSessionData(ctx context.Context, scope, id string) (*wantypes.SessionData, error)
 	DeleteGlobalWebauthnSessionData(ctx context.Context, scope, id string) error
 	GetTeleportUserByWebauthnID(ctx context.Context, webID []byte) (string, error)
 }
@@ -84,12 +83,12 @@ func (p passwordlessIdentity) GetWebauthnLocalAuth(ctx context.Context, user str
 
 type globalSessionStorage PasswordlessFlow
 
-func (g *globalSessionStorage) Upsert(ctx context.Context, user string, sd *wanpb.SessionData) error {
+func (g *globalSessionStorage) Upsert(ctx context.Context, user string, sd *wantypes.SessionData) error {
 	id := base64.RawURLEncoding.EncodeToString(sd.Challenge)
 	return g.Identity.UpsertGlobalWebauthnSessionData(ctx, scopeLogin, id, sd)
 }
 
-func (g *globalSessionStorage) Get(ctx context.Context, user string, challenge string) (*wanpb.SessionData, error) {
+func (g *globalSessionStorage) Get(ctx context.Context, user string, challenge string) (*wantypes.SessionData, error) {
 	return g.Identity.GetGlobalWebauthnSessionData(ctx, scopeLogin, challenge)
 }
 

lib/auth/webauthn/login_test.go

@@ -26,14 +26,12 @@ import (
 	"time"
 
 	"github.com/go-webauthn/webauthn/protocol"
-	"github.com/gogo/protobuf/proto"
 	"github.com/google/go-cmp/cmp"
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport/api/types"
-	wanpb "github.com/gravitational/teleport/api/types/webauthn"
 	"github.com/gravitational/teleport/lib/auth/mocku2f"
 	wanlib "github.com/gravitational/teleport/lib/auth/webauthn"
 	wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
@@ -130,7 +128,7 @@ func TestLoginFlow_BeginFinish(t *testing.T) {
 			// Did we record the SessionData in storage?
 			require.Len(t, identity.SessionData, 1)
 			// Did we record the web ID in the SessionData?
-			var sd *wanpb.SessionData
+			var sd *wantypes.SessionData
 			for _, v := range identity.SessionData {
 				sd = v // Retrieve without guessing the key
 				break
@@ -412,20 +410,19 @@ func TestPasswordlessFlow_BeginAndFinish(t *testing.T) {
 
 			// Verify that we recorded user verification requirements in storage.
 			require.Len(t, identity.SessionData, 1)
-			var sd *wanpb.SessionData
+			var sd *wantypes.SessionData
 			for _, v := range identity.SessionData {
 				sd = v // Get SessionData without guessing the key.
 				break
 			}
-			wantSD := &wanpb.SessionData{
+			wantSD := &wantypes.SessionData{
 				Challenge:        sd.Challenge,
-				UserId:           nil,   // aka unset
-				AllowCredentials: nil,   // aka unset
-				ResidentKey:      false, // irrelevant for login
+				UserId:           nil,         // aka unset
+				AllowCredentials: [][]uint8{}, // aka unset
+				ResidentKey:      false,       // irrelevant for login
 				UserVerification: string(protocol.VerificationRequired),
 			}
-			if !proto.Equal(sd, wantSD) {
-				diff := cmp.Diff(wantSD, sd)
+			if diff := cmp.Diff(wantSD, sd); diff != "" {
 				t.Fatalf("SessionData mismatch (-want +got):\n%s", diff)
 			}
 
@@ -630,7 +627,7 @@ type fakeIdentity struct {
 	// It's automatically assigned when UpsertWebauthnLocalAuth is called.
 	MappedUser     string
 	UpdatedDevices []*types.MFADevice
-	SessionData    map[string]*wanpb.SessionData
+	SessionData    map[string]*wantypes.SessionData
 }
 
 func newFakeIdentity(user string, devices ...*types.MFADevice) *fakeIdentity {
@@ -645,7 +642,7 @@ func newFakeIdentity(user string, devices ...*types.MFADevice) *fakeIdentity {
 				},
 			},
 		},
-		SessionData: make(map[string]*wanpb.SessionData),
+		SessionData: make(map[string]*wantypes.SessionData),
 	}
 }
 
@@ -690,12 +687,12 @@ func (f *fakeIdentity) GetTeleportUserByWebauthnID(ctx context.Context, webID []
 	return f.MappedUser, nil
 }
 
-func (f *fakeIdentity) UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wanpb.SessionData) error {
+func (f *fakeIdentity) UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wantypes.SessionData) error {
 	f.SessionData[sessionDataKey(user, sessionID)] = sd
 	return nil
 }
 
-func (f *fakeIdentity) GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wanpb.SessionData, error) {
+func (f *fakeIdentity) GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wantypes.SessionData, error) {
 	sd, ok := f.SessionData[sessionDataKey(user, sessionID)]
 	if !ok {
 		return nil, trace.NotFound("not found")
@@ -712,12 +709,12 @@ func sessionDataKey(user string, sessionID string) string {
 	return fmt.Sprintf("user/%v/%v", user, sessionID)
 }
 
-func (f *fakeIdentity) UpsertGlobalWebauthnSessionData(ctx context.Context, scope, id string, sd *wanpb.SessionData) error {
+func (f *fakeIdentity) UpsertGlobalWebauthnSessionData(ctx context.Context, scope, id string, sd *wantypes.SessionData) error {
 	f.SessionData[globalSessionDataKey(scope, id)] = sd
 	return nil
 }
 
-func (f *fakeIdentity) GetGlobalWebauthnSessionData(ctx context.Context, scope, id string) (*wanpb.SessionData, error) {
+func (f *fakeIdentity) GetGlobalWebauthnSessionData(ctx context.Context, scope, id string) (*wantypes.SessionData, error) {
 	sd, ok := f.SessionData[globalSessionDataKey(scope, id)]
 	if !ok {
 		return nil, trace.NotFound("not found")

lib/auth/webauthn/register.go

@@ -34,7 +34,6 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport/api/types"
-	wanpb "github.com/gravitational/teleport/api/types/webauthn"
 	wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
 )
 
@@ -47,8 +46,8 @@ type RegistrationIdentity interface {
 
 	GetMFADevices(ctx context.Context, user string, withSecrets bool) ([]*types.MFADevice, error)
 	UpsertMFADevice(ctx context.Context, user string, d *types.MFADevice) error
-	UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wanpb.SessionData) error
-	GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wanpb.SessionData, error)
+	UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wantypes.SessionData) error
+	GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wantypes.SessionData, error)
 	DeleteWebauthnSessionData(ctx context.Context, user, sessionID string) error
 }
 
@@ -57,7 +56,7 @@ type RegistrationIdentity interface {
 func WithInMemorySessionData(identity RegistrationIdentity) RegistrationIdentity {
 	return &inMemoryIdentity{
 		RegistrationIdentity: identity,
-		sessionData:          make(map[string]*wanpb.SessionData),
+		sessionData:          make(map[string]*wantypes.SessionData),
 	}
 }
 
@@ -68,17 +67,17 @@ type inMemoryIdentity struct {
 	// We don't foresee concurrent use for inMemoryIdentity, but it's easy enough
 	// to play it safe.
 	mu          sync.RWMutex
-	sessionData map[string]*wanpb.SessionData
+	sessionData map[string]*wantypes.SessionData
 }
 
-func (identity *inMemoryIdentity) UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wanpb.SessionData) error {
+func (identity *inMemoryIdentity) UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wantypes.SessionData) error {
 	identity.mu.Lock()
 	defer identity.mu.Unlock()
 	identity.sessionData[sessionDataKey(user, sessionID)] = sd
 	return nil
 }
 
-func (identity *inMemoryIdentity) GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wanpb.SessionData, error) {
+func (identity *inMemoryIdentity) GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wantypes.SessionData, error) {
 	identity.mu.RLock()
 	defer identity.mu.RUnlock()
 	sd, ok := identity.sessionData[sessionDataKey(user, sessionID)]
@@ -189,11 +188,11 @@ func (f *RegistrationFlow) Begin(ctx context.Context, user string, passwordless
 	// TODO(codingllama): Send U2F App ID back in creation requests too. Useful to
 	//  detect duplicate devices.
 
-	sessionDataPB, err := sessionToPB(sessionData)
+	sd, err := wantypes.SessionDataFromProtocol(sessionData)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	if err := f.Identity.UpsertWebauthnSessionData(ctx, user, scopeSession, sessionDataPB); err != nil {
+	if err := f.Identity.UpsertWebauthnSessionData(ctx, user, scopeSession, sd); err != nil {
 		return nil, trace.Wrap(err)
 	}
 
@@ -280,11 +279,11 @@ func (f *RegistrationFlow) Finish(ctx context.Context, req RegisterResponse) (*t
 	}
 	u := newWebUser(req.User, wla.UserID, true /* credentialIDOnly */, nil /* devices */)
 
-	sessionDataPB, err := f.Identity.GetWebauthnSessionData(ctx, req.User, scopeSession)
+	sd, err := f.Identity.GetWebauthnSessionData(ctx, req.User, scopeSession)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	sessionData := sessionFromPB(sessionDataPB)
+	sessionData := wantypes.SessionDataToProtocol(sd)
 
 	// Activate passwordless switches (resident key, user verification) if we
 	// required verification in the begin step.

lib/auth/webauthn/session.go

@@ -18,16 +18,6 @@
 
 package webauthn
 
-import (
-	"encoding/base64"
-
-	"github.com/go-webauthn/webauthn/protocol"
-	wan "github.com/go-webauthn/webauthn/webauthn"
-	"github.com/gravitational/trace"
-
-	wanpb "github.com/gravitational/teleport/api/types/webauthn"
-)
-
 // scopeLogin identifies session data stored for login.
 // It is used as the scope for global session data and as the sessionID for
 // per-user session data.
@@ -38,27 +28,3 @@ const scopeLogin = "login"
 // Only one in-flight registration is supported per-user, baring registrations
 // that use in-memory storage.
 const scopeSession = "registration"
-
-func sessionToPB(sd *wan.SessionData) (*wanpb.SessionData, error) {
-	rawChallenge, err := base64.RawURLEncoding.DecodeString(sd.Challenge)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-	// TODO(codingllama): Record extensions in stored session data.
-	return &wanpb.SessionData{
-		Challenge:        rawChallenge,
-		UserId:           sd.UserID,
-		AllowCredentials: sd.AllowedCredentialIDs,
-		UserVerification: string(sd.UserVerification),
-	}, nil
-}
-
-func sessionFromPB(sd *wanpb.SessionData) *wan.SessionData {
-	// TODO(codingllama): Record extensions in stored session data.
-	return &wan.SessionData{
-		Challenge:            base64.RawURLEncoding.EncodeToString(sd.Challenge),
-		UserID:               sd.UserId,
-		AllowedCredentialIDs: sd.AllowCredentials,
-		UserVerification:     protocol.UserVerificationRequirement(sd.UserVerification),
-	}
-}

lib/auth/webauthncli/u2f_login_test.go

@@ -37,7 +37,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport/api/types"
-	wanpb "github.com/gravitational/teleport/api/types/webauthn"
 	"github.com/gravitational/teleport/lib/auth/mocku2f"
 	wanlib "github.com/gravitational/teleport/lib/auth/webauthn"
 	wancli "github.com/gravitational/teleport/lib/auth/webauthncli"
@@ -426,7 +425,7 @@ type fakeIdentity struct {
 	User        string
 	Devices     []*types.MFADevice
 	LocalAuth   *types.WebauthnLocalAuth
-	SessionData *wanpb.SessionData
+	SessionData *wantypes.SessionData
 }
 
 func (f *fakeIdentity) UpsertWebauthnLocalAuth(ctx context.Context, user string, wla *types.WebauthnLocalAuth) error {
@@ -457,12 +456,12 @@ func (f *fakeIdentity) UpsertMFADevice(ctx context.Context, user string, d *type
 	return nil
 }
 
-func (f *fakeIdentity) UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wanpb.SessionData) error {
+func (f *fakeIdentity) UpsertWebauthnSessionData(ctx context.Context, user, sessionID string, sd *wantypes.SessionData) error {
 	f.SessionData = sd
 	return nil
 }
 
-func (f *fakeIdentity) GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wanpb.SessionData, error) {
+func (f *fakeIdentity) GetWebauthnSessionData(ctx context.Context, user, sessionID string) (*wantypes.SessionData, error) {
 	return f.SessionData, nil
 }