Address U2F sunset TODOs (#17030)

Address TODOs, add deprecation warnings and remove as many U2F code references as possible. Existing behavior is kept unaltered: it's still possible to inform Teleport of old U2F AppIDs and U2F configurations are still silently converted to WebAuthn. There's no reason to break that, so we don't. Most server-side references to SecondFactorU2F are removed, but client-side references remain: this makes it possible to interop newer clients with old clusters (something else may break, but hopefully not this part). Closes #10375.
gravitational · Oct 5, 2022 · 57508a9 · 57508a9
1 parent 9515fe8
commit 57508a9
Show file tree

Hide file tree

Showing 10 changed files with 26 additions and 60 deletions.
diff --git a/api/constants/constants.go b/api/constants/constants.go
@@ -164,9 +164,10 @@ const (
 	// SecondFactorOTP means that only OTP is supported for 2FA and 2FA is
 	// required for all users.
 	SecondFactorOTP = SecondFactorType("otp")
-	// SecondFactorU2F means that only U2F is supported for 2FA and 2FA is
-	// required for all users.
-	// U2F is marked for removal. It currently works as an alias for "webauthn".
+	// SecondFactorU2F means that only Webauthn is supported for 2FA and 2FA
+	// is required for all users.
+	// Deprecated: "u2f" is aliased to "webauthn". Prefer using
+	// SecondFactorWebauthn instead.
 	SecondFactorU2F = SecondFactorType("u2f")
 	// SecondFactorWebauthn means that only Webauthn is supported for 2FA and 2FA
 	// is required for all users.

diff --git a/api/proto/teleport/legacy/types/types.proto b/api/proto/teleport/legacy/types/types.proto
@@ -1389,6 +1389,8 @@ message AuthPreferenceSpecV2 {
 }
 
 // U2F defines settings for U2F device.
+// Deprecated: U2F is transparently converted to WebAuthn by Teleport. Prefer
+// using WebAuthn instead.
 message U2F {
   // AppID returns the application ID for universal second factor.
   string AppID = 1 [(gogoproto.jsontag) = "app_id,omitempty"];
@@ -1400,7 +1402,6 @@ message U2F {
 
   // DeviceAttestationCAs contains the trusted attestation CAs for U2F
   // devices.
-  // DELETE IN 11.0, time to sunset U2F (codingllama).
   repeated string DeviceAttestationCAs = 3 [(gogoproto.jsontag) = "device_attestation_cas,omitempty"];
 }
 

diff --git a/api/types/authentication.go b/api/types/authentication.go
@@ -60,8 +60,6 @@ type AuthPreference interface {
 	IsSecondFactorEnforced() bool
 	// IsSecondFactorTOTPAllowed checks if users are allowed to register TOTP devices.
 	IsSecondFactorTOTPAllowed() bool
-	// IsSecondFactorU2FAllowed checks if users are allowed to register U2F devices.
-	IsSecondFactorU2FAllowed() bool
 	// IsSecondFactorWebauthnAllowed checks if users are allowed to register
 	// Webauthn devices.
 	IsSecondFactorWebauthnAllowed() bool
@@ -243,10 +241,8 @@ func (c *AuthPreferenceV2) GetPreferredLocalMFA() constants.SecondFactorType {
 	switch sf := c.GetSecondFactor(); sf {
 	case constants.SecondFactorOff:
 		return "" // Nothing to suggest.
-	case constants.SecondFactorOTP:
+	case constants.SecondFactorOTP, constants.SecondFactorWebauthn:
 		return sf // Single method.
-	case constants.SecondFactorU2F, constants.SecondFactorWebauthn:
-		return constants.SecondFactorWebauthn // Always WebAuthn.
 	case constants.SecondFactorOn, constants.SecondFactorOptional:
 		// In order of preference:
 		// 1. WebAuthn (public-key based)
@@ -273,11 +269,6 @@ func (c *AuthPreferenceV2) IsSecondFactorTOTPAllowed() bool {
 		c.Spec.SecondFactor == constants.SecondFactorOn
 }
 
-// IsSecondFactorU2FAllowed checks if users are allowed to register U2F devices.
-func (c *AuthPreferenceV2) IsSecondFactorU2FAllowed() bool {
-	return false // Never allowed, marked for removal.
-}
-
 // IsSecondFactorWebauthnAllowed checks if users are allowed to register
 // Webauthn devices.
 func (c *AuthPreferenceV2) IsSecondFactorWebauthnAllowed() bool {
@@ -291,8 +282,7 @@ func (c *AuthPreferenceV2) IsSecondFactorWebauthnAllowed() bool {
 	}
 
 	// Are second factor settings in accordance?
-	return c.Spec.SecondFactor == constants.SecondFactorU2F ||
-		c.Spec.SecondFactor == constants.SecondFactorWebauthn ||
+	return c.Spec.SecondFactor == constants.SecondFactorWebauthn ||
 		c.Spec.SecondFactor == constants.SecondFactorOptional ||
 		c.Spec.SecondFactor == constants.SecondFactorOn
 }
@@ -445,7 +435,6 @@ func (c *AuthPreferenceV2) CheckAndSetDefaults() error {
 		return trace.BadParameter("authentication type %q not supported", c.Spec.Type)
 	}
 
-	// DELETE IN 11.0, time to sunset U2F (codingllama).
 	if c.Spec.SecondFactor == constants.SecondFactorU2F {
 		log.Warnf(`` +
 			`Second Factor "u2f" is deprecated and marked for removal, using "webauthn" instead. ` +

diff --git a/api/types/authentication_authpreference_test.go b/api/types/authentication_authpreference_test.go
@@ -70,13 +70,11 @@ func TestAuthPreferenceV2_CheckAndSetDefaults_secondFactor(t *testing.T) {
 	secondFactorAll := []constants.SecondFactorType{
 		constants.SecondFactorOff,
 		constants.SecondFactorOTP,
-		constants.SecondFactorU2F,
 		constants.SecondFactorWebauthn,
 		constants.SecondFactorOn,
 		constants.SecondFactorOptional,
 	}
 	secondFactorWebActive := []constants.SecondFactorType{
-		constants.SecondFactorU2F,
 		constants.SecondFactorWebauthn,
 		constants.SecondFactorOn,
 		constants.SecondFactorOptional,
@@ -235,12 +233,10 @@ func TestAuthPreferenceV2_CheckAndSetDefaults_secondFactor(t *testing.T) {
 			name: "OK second factor enforced",
 			secondFactors: []constants.SecondFactorType{
 				constants.SecondFactorOTP,
-				constants.SecondFactorU2F,
 				constants.SecondFactorWebauthn,
 				constants.SecondFactorOn,
 			},
 			spec: types.AuthPreferenceSpecV2{
-				U2F:      minimalU2F,
 				Webauthn: minimalWeb,
 			},
 			assertFn: func(t *testing.T, got *types.AuthPreferenceV2) {
@@ -279,7 +275,6 @@ func TestAuthPreferenceV2_CheckAndSetDefaults_secondFactor(t *testing.T) {
 			name: "OK OTP second factor not allowed",
 			secondFactors: []constants.SecondFactorType{
 				constants.SecondFactorOff,
-				constants.SecondFactorU2F,
 				constants.SecondFactorWebauthn,
 			},
 			spec: types.AuthPreferenceSpecV2{
@@ -290,27 +285,9 @@ func TestAuthPreferenceV2_CheckAndSetDefaults_secondFactor(t *testing.T) {
 				require.False(t, got.IsSecondFactorTOTPAllowed(), "OTP allowed")
 			},
 		},
-		{
-			name: "OK U2F second factor never allowed",
-			secondFactors: []constants.SecondFactorType{
-				constants.SecondFactorOff,
-				constants.SecondFactorOTP,
-				constants.SecondFactorU2F,
-				constants.SecondFactorWebauthn,
-				constants.SecondFactorOn,
-				constants.SecondFactorOptional,
-			},
-			spec: types.AuthPreferenceSpecV2{
-				U2F: minimalU2F,
-			},
-			assertFn: func(t *testing.T, got *types.AuthPreferenceV2) {
-				require.False(t, got.IsSecondFactorU2FAllowed(), "U2F allowed")
-			},
-		},
 		{
 			name: "OK Webauthn second factor allowed",
 			secondFactors: []constants.SecondFactorType{
-				constants.SecondFactorU2F,
 				constants.SecondFactorWebauthn,
 				constants.SecondFactorOn,
 				constants.SecondFactorOptional,
@@ -358,13 +335,11 @@ func TestAuthPreferenceV2_CheckAndSetDefaults_secondFactor(t *testing.T) {
 		{
 			name: "OK preferred local MFA = Webauthn",
 			secondFactors: []constants.SecondFactorType{
-				constants.SecondFactorU2F,
 				constants.SecondFactorWebauthn,
 				constants.SecondFactorOn,
 				constants.SecondFactorOptional,
 			},
 			spec: types.AuthPreferenceSpecV2{
-				U2F:      minimalU2F,
 				Webauthn: minimalWeb,
 			},
 			assertFn: func(t *testing.T, got *types.AuthPreferenceV2) {

diff --git a/lib/auth/init_test.go b/lib/auth/init_test.go
@@ -292,7 +292,7 @@ func TestAuthPreference(t *testing.T) {
 			return conf.AuthPreference
 		},
 		withAnotherConfigFile: func(t *testing.T, conf *InitConfig) types.ResourceWithOrigin {
-			conf.AuthPreference = newU2FAuthPreferenceFromConfigFile(t)
+			conf.AuthPreference = newWebauthnAuthPreferenceConfigFromFile(t)
 			return conf.AuthPreference
 		},
 		setDynamic: func(t *testing.T, authServer *Server) {
@@ -612,12 +612,12 @@ func setupConfig(t *testing.T) InitConfig {
 	}
 }
 
-func newU2FAuthPreferenceFromConfigFile(t *testing.T) types.AuthPreference {
+func newWebauthnAuthPreferenceConfigFromFile(t *testing.T) types.AuthPreference {
 	ap, err := types.NewAuthPreferenceFromConfigFile(types.AuthPreferenceSpecV2{
 		Type:         constants.Local,
-		SecondFactor: constants.SecondFactorU2F,
-		U2F: &types.U2F{
-			AppID: "foo",
+		SecondFactor: constants.SecondFactorWebauthn,
+		Webauthn: &types.Webauthn{
+			RPID: "localhost",
 		},
 	})
 	require.NoError(t, err)

diff --git a/lib/config/configuration_test.go b/lib/config/configuration_test.go
@@ -50,6 +50,7 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/testing/protocmp"
 )
 
 type testConfigFiles struct {
@@ -786,10 +787,10 @@ func TestApplyConfig(t *testing.T) {
 		},
 		Spec: types.AuthPreferenceSpecV2{
 			Type:         constants.Local,
-			SecondFactor: constants.SecondFactorOTP,
-			U2F: &types.U2F{
-				AppID: "app-id",
-				DeviceAttestationCAs: []string{
+			SecondFactor: constants.SecondFactorOptional,
+			Webauthn: &types.Webauthn{
+				RPID: "goteleport.com",
+				AttestationAllowedCAs: []string{
 					string(u2fCAFromFile),
 					`-----BEGIN CERTIFICATE-----
 MIIDFzCCAf+gAwIBAgIDBAZHMA0GCSqGSIb3DQEBCwUAMCsxKTAnBgNVBAMMIFl1
@@ -816,9 +817,9 @@ SREzU8onbBsjMg9QDiSf5oJLKvd/Ren+zGY7
 			AllowLocalAuth:        types.NewBoolOption(true),
 			DisconnectExpiredCert: types.NewBoolOption(false),
 			LockingMode:           constants.LockingModeBestEffort,
-			AllowPasswordless:     types.NewBoolOption(false),
+			AllowPasswordless:     types.NewBoolOption(true),
 		},
-	}))
+	}, protocmp.Transform()))
 
 	require.Equal(t, pkcs11LibPath, cfg.Auth.KeyStore.Path)
 	require.Equal(t, "example_token", cfg.Auth.KeyStore.TokenLabel)

diff --git a/lib/config/fileconf.go b/lib/config/fileconf.go
@@ -967,9 +967,8 @@ type Webauthn struct {
 	RPID                  string   `yaml:"rp_id,omitempty"`
 	AttestationAllowedCAs []string `yaml:"attestation_allowed_cas,omitempty"`
 	AttestationDeniedCAs  []string `yaml:"attestation_denied_cas,omitempty"`
-	// Disabled has no effect, it is kept solely to not break existing
+	// Deprecated: Disabled has no effect, it is kept solely to not break existing
 	// configurations.
-	// DELETE IN 11.0, time to sunset U2F (codingllama).
 	Disabled bool `yaml:"disabled,omitempty"`
 }
 

diff --git a/lib/config/testdata_test.go b/lib/config/testdata_test.go
@@ -122,9 +122,10 @@ auth_service:
       slot_number: 1
       pin: "example_pin"
   authentication:
-    u2f:
-      app_id: "app-id"
-      device_attestation_cas:
+    second_factor: "optional"
+    webauthn:
+      rp_id: "goteleport.com"
+      attestation_allowed_cas:
       - "testdata/u2f_attestation_ca.pem"
       - |
         -----BEGIN CERTIFICATE-----

diff --git a/lib/services/local/resource.go b/lib/services/local/resource.go
@@ -414,7 +414,7 @@ func itemToMFADevice(item backend.Item) (*types.MFADevice, error) {
 
 // userFromUserItems is an extended variant of itemToUser which can be used
 // with the `userItems` collector to include additional backend.Item values
-// such as password hash or u2f registration.
+// such as password hash or MFA devices.
 func userFromUserItems(name string, items userItems) (types.User, error) {
 	if items.params == nil {
 		return nil, trace.BadParameter("cannot itemTo user %q without primary item %q", name, paramsPrefix)

diff --git a/operator/crdgen/testdata/protofiles/types.proto b/operator/crdgen/testdata/protofiles/types.proto
@@ -1144,7 +1144,6 @@ message U2F {
 
     // DeviceAttestationCAs contains the trusted attestation CAs for U2F
     // devices.
-    // DELETE IN 11.0, time to sunset U2F (codingllama).
     repeated string DeviceAttestationCAs = 3
         [ (gogoproto.jsontag) = "device_attestation_cas,omitempty" ];
 }