diff --git a/.github/ISSUE_TEMPLATE/testplan.md b/.github/ISSUE_TEMPLATE/testplan.md index 7f7d4d1abe6..a1a44bf8072 100644 --- a/.github/ISSUE_TEMPLATE/testplan.md +++ b/.github/ISSUE_TEMPLATE/testplan.md @@ -733,38 +733,17 @@ This feature has additional build requirements, so it should be tested with a pr #### Server Access -These tests should be carried out sequentially. `tsh` tests should be carried out on Linux, MacOS, and Windows. - -1. [ ] `tsh login` as user with [Webauthn](https://goteleport.com/docs/access-controls/guides/webauthn/) login and no hardware key requirement. -2. [ ] Request a role with `role.role_options.require_session_mfa: hardware_key` - `tsh login --request-roles=hardware_key_required` - - [ ] Assuming the role should force automatic re-login with yubikey - - [ ] `tsh ssh` - - [ ] Requires yubikey to be connected for re-login - - [ ] Prompts for per-session MFA -3. [ ] Request a role with `role.role_options.require_session_mfa: hardware_key_touch` - `tsh login --request-roles=hardware_key_touch_required` - - [ ] Assuming the role should force automatic re-login with yubikey - - [ ] Prompts for touch if not cached (last touch within 15 seconds) - - [ ] `tsh ssh` - - [ ] Requires yubikey to be connected for re-login - - [ ] Prompts for touch if not cached -4. [ ] `tsh logout` and `tsh login` as the user with no hardware key requirement. -5. [ ] Upgrade auth settings to `auth_service.authentication.require_session_mfa: hardware_key` - - [ ] Using the existing login session (`tsh ls`) should force automatic re-login with yubikey - - [ ] `tsh ssh` - - [ ] Requires yubikey to be connected for re-login - - [ ] Prompts for per-session MFA -6. [ ] Upgrade auth settings to `auth_service.authentication.require_session_mfa: hardware_key_touch` - - [ ] Using the existing login session (`tsh ls`) should force automatic re-login with yubikey - - [ ] Prompts for touch if not cached - - [ ] `tsh ssh` - - [ ] Requires yubikey to be connected for re-login - - [ ] Prompts for touch if not cached - -#### Other - -Set `auth_service.authentication.require_session_mfa: hardware_key_touch` in your cluster auth settings. +This test should be carried out on Linux, MacOS, and Windows. +Set `auth_service.authentication.require_session_mfa: hardware_key_touch` in your cluster auth settings and login. +- [ ] `tsh login` + - [ ] Prompts for Yubikey touch with message "Tap your YubiKey" (separate from normal MFA prompt). +- [ ] Server Access `tsh ssh` + - [ ] Requires yubikey to be connected + - [ ] Prompts for touch (if not cached) - [ ] Database Access: `tsh proxy db --tunnel` + - [ ] Requires yubikey to be connected + - [ ] Prompts for touch (if not cached) ### HSM Support diff --git a/Makefile b/Makefile index fda57c7b1f8..68da8598cd4 100644 --- a/Makefile +++ b/Makefile @@ -162,12 +162,12 @@ TOUCHID_MESSAGE := with-Touch-ID TOUCHID_TAG := touchid endif -# Enable PIV for testing? -# Eagerly enable if we detect the dynamic libpcsclite library, we want to test as much as possible. -ifeq ("$(shell pkg-config libpcsclite 2>/dev/null; echo $$?)", "0") -# This test tag should not be used for builds/releases, only tests. -PIV_TEST_TAG := piv -endif +# Enable PIV test packages for testing. +# This test tag should never be used for builds/releases, only tests. +PIV_TEST_TAG := pivtest + +# enable PIV package for linting. +PIV_LINT_TAG := piv # Build teleport/api with PIV? This requires the libpcsclite library for linux. # @@ -998,7 +998,7 @@ endif .PHONY: lint-go lint-go: GO_LINT_FLAGS ?= lint-go: - golangci-lint run -c .golangci.yml --build-tags='$(LIBFIDO2_TEST_TAG) $(TOUCHID_TAG) $(PIV_TEST_TAG)' $(GO_LINT_FLAGS) + golangci-lint run -c .golangci.yml --build-tags='$(LIBFIDO2_TEST_TAG) $(TOUCHID_TAG) $(PIV_LINT_TAG)' $(GO_LINT_FLAGS) .PHONY: fix-imports fix-imports: diff --git a/api/utils/keys/privatekey.go b/api/utils/keys/privatekey.go index 67fb453c006..8abcad2b22a 100644 --- a/api/utils/keys/privatekey.go +++ b/api/utils/keys/privatekey.go @@ -224,7 +224,7 @@ func MarshalPrivateKey(key crypto.Signer) ([]byte, error) { Bytes: x509.MarshalPKCS1PrivateKey(privateKey), }) return privPEM, nil - case *ecdsa.PrivateKey, *ed25519.PrivateKey: + case *ecdsa.PrivateKey, ed25519.PrivateKey: der, err := x509.MarshalPKCS8PrivateKey(privateKey) if err != nil { return nil, trace.Wrap(err) diff --git a/api/utils/keys/privatekey_test.go b/api/utils/keys/privatekey_test.go index 17f48c52a17..ffba01c134c 100644 --- a/api/utils/keys/privatekey_test.go +++ b/api/utils/keys/privatekey_test.go @@ -18,76 +18,38 @@ package keys import ( "bytes" + "crypto" "crypto/ecdsa" "crypto/ed25519" + "crypto/elliptic" + "crypto/rand" "crypto/rsa" "crypto/tls" "encoding/pem" "testing" - "github.com/gravitational/trace" "github.com/stretchr/testify/require" ) -// TestParsePrivateKey tests that ParsePrivateKey successfully parses private key PEM. -func TestParsePrivateKey(t *testing.T) { - for _, tt := range []struct { - desc string - keyPEM []byte - assertError require.ErrorAssertionFunc - assertKey require.ValueAssertionFunc - }{ - { - desc: "invalid PEM", - keyPEM: []byte(`non-pem data`), - assertError: func(t require.TestingT, err error, i ...interface{}) { - require.True(t, trace.IsBadParameter(err), "expected trace.BadParameter, got %T", err) - }, - assertKey: require.Nil, - }, - { - desc: "invalid key", - keyPEM: invalidKeyPEM, - assertError: func(t require.TestingT, err error, i ...interface{}) { - require.True(t, trace.IsBadParameter(err), "expected trace.BadParameter, got %T", err) - }, - assertKey: require.Nil, - }, - { - desc: "rsa key", - keyPEM: rsaKeyPEM, - assertError: require.NoError, - assertKey: func(tt require.TestingT, key interface{}, i2 ...interface{}) { - privateKey, ok := key.(*PrivateKey) - require.True(t, ok) - require.IsType(t, &rsa.PrivateKey{}, privateKey.Signer) - }, - }, - { - desc: "ecdsa key", - keyPEM: ecdsaKeyPEM, - assertError: require.NoError, - assertKey: func(tt require.TestingT, key interface{}, i2 ...interface{}) { - privateKey, ok := key.(*PrivateKey) - require.True(t, ok) - require.IsType(t, &ecdsa.PrivateKey{}, privateKey.Signer) - }, - }, - { - desc: "ed25519 key", - keyPEM: ed25519KeyPEM, - assertError: require.NoError, - assertKey: func(tt require.TestingT, key interface{}, i2 ...interface{}) { - privateKey, ok := key.(*PrivateKey) - require.True(t, ok) - require.IsType(t, ed25519.PrivateKey{}, privateKey.Signer) - }, - }, +func TestMarshalAndParsePrivateKey(t *testing.T) { + rsaKey, err := rsa.GenerateKey(rand.Reader, 1024) + require.NoError(t, err) + ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + _, edKey, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + + for keyType, key := range map[string]crypto.Signer{ + "rsa": rsaKey, + "ecdsa": ecKey, + "ed25519": edKey, } { - t.Run(tt.desc, func(t *testing.T) { - priv, err := ParsePrivateKey(tt.keyPEM) - tt.assertError(t, err) - tt.assertKey(t, priv) + t.Run(keyType, func(t *testing.T) { + keyPEM, err := MarshalPrivateKey(key) + require.NoError(t, err) + gotKey, err := ParsePrivateKey(keyPEM) + require.NoError(t, err) + require.Equal(t, key, gotKey.Signer) }) } } @@ -129,9 +91,6 @@ func TestX509KeyPair(t *testing.T) { } var ( - invalidKeyPEM = []byte(`-----BEGIN INVALID KEY----- ------END INVALID KEY----- -`) // generated with `openssl req -x509 -out rsa.crt -keyout rsa.key -newkey rsa:2048 -nodes -sha256` rsaKeyPEM = []byte(`-----BEGIN PRIVATE KEY----- MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCudYRUc0u2xdQi @@ -182,16 +141,4 @@ mg0exCUFW40aXpfm0z0dNNwoN+FPSefKMYMQ1LV87I6zGnmVTYH9Nix3REiuliIQ 7XXnJc7A6tsc6yXdVG6IpGnKXuTvl/r4iIbH+JDv3MDSvZSCE5kzAPFjgB3zMAZ8 Z0+424ERgom0Zdy75Y8I -----END CERTIFICATE-----`) - // generated with `openssl ecparam -genkey -name prime256v1 -noout -out ecdsa.key` - ecdsaKeyPEM = []byte(`-----BEGIN EC PRIVATE KEY----- -MHcCAQEEIMDaz87Hngva0Wm+QkhCJ0Nz5o958+dsyH0DzsCe6Fl6oAoGCCqGSM49 -AwEHoUQDQgAEI06FHb4RKoYKcj+51w6WcN7kNI9OVSTp6H8BlljYYs2zxuIh6LQ3 -hXIC6UT+IOGQBnvq86SAbnPEWMLowtQc/Q== ------END EC PRIVATE KEY----- -`) - // generated with `openssl genpkey -algorithm ed25519 -out ed25519.key` - ed25519KeyPEM = []byte(`-----BEGIN PRIVATE KEY----- -MC4CAQAwBQYDK2VwBCIEIGf81V4UAiKXFehNALvwlSlB8ZYb/RbRUMSdTG3mSZLN ------END PRIVATE KEY----- -`) ) diff --git a/api/utils/keys/yubikey.go b/api/utils/keys/yubikey.go index f1d00b6b78b..3b38c43f4d0 100644 --- a/api/utils/keys/yubikey.go +++ b/api/utils/keys/yubikey.go @@ -1,4 +1,4 @@ -//go:build piv +//go:build piv && !pivtest /* Copyright 2022 Gravitational, Inc. diff --git a/api/utils/keys/yubikey_fake.go b/api/utils/keys/yubikey_fake.go new file mode 100644 index 00000000000..1924a0f8e1e --- /dev/null +++ b/api/utils/keys/yubikey_fake.go @@ -0,0 +1,75 @@ +//go:build pivtest + +/* +Copyright 2024 Gravitational, Inc. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package keys + +import ( + "context" + "crypto" + "crypto/ed25519" + "crypto/rand" + "errors" + + "github.com/gravitational/trace" +) + +var errPIVUnavailable = errors.New("PIV is unavailable in current build") + +// Return a fake YubiKey private key. +func getOrGenerateYubiKeyPrivateKey(_ context.Context, policy PrivateKeyPolicy, _ PIVSlot) (*PrivateKey, error) { + _, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return nil, trace.Wrap(err) + } + + keyPEM, err := MarshalPrivateKey(priv) + if err != nil { + return nil, trace.Wrap(err) + } + + signer := &fakeYubiKeyPrivateKey{ + Signer: priv, + privateKeyPolicy: policy, + } + + return NewPrivateKey(signer, keyPEM) +} + +func parseYubiKeyPrivateKeyData(_ []byte) (*PrivateKey, error) { + // TODO(Joerger): add custom marshal/unmarshal logic for fakeYubiKeyPrivateKey (if necessary). + return nil, trace.Wrap(errPIVUnavailable) +} + +func (s PIVSlot) validate() error { + return trace.Wrap(errPIVUnavailable) +} + +type fakeYubiKeyPrivateKey struct { + crypto.Signer + privateKeyPolicy PrivateKeyPolicy +} + +// GetAttestationStatement returns an AttestationStatement for this private key. +func (y *fakeYubiKeyPrivateKey) GetAttestationStatement() *AttestationStatement { + // Since this is only used in tests, we will ignore the attestation statement in the end. + // We just need it to be non-nil so that it goes through the test modules implementation + // of AttestHardwareKey. + return &AttestationStatement{} +} + +// GetPrivateKeyPolicy returns the PrivateKeyPolicy supported by this private key. +func (y *fakeYubiKeyPrivateKey) GetPrivateKeyPolicy() PrivateKeyPolicy { + return y.privateKeyPolicy +} diff --git a/api/utils/keys/yubikey_other.go b/api/utils/keys/yubikey_other.go index f6decf2443e..9fa747724bd 100644 --- a/api/utils/keys/yubikey_other.go +++ b/api/utils/keys/yubikey_other.go @@ -1,4 +1,4 @@ -//go:build !piv +//go:build !piv && !pivtest /* Copyright 2022 Gravitational, Inc. diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 477f751cb56..2c58972148e 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -2014,13 +2014,14 @@ func (a *Server) GenerateOpenSSHCert(ctx context.Context, req *proto.OpenSSHCert // GenerateUserTestCertsRequest is a request to generate test certificates. type GenerateUserTestCertsRequest struct { - Key []byte - Username string - TTL time.Duration - Compatibility string - RouteToCluster string - PinnedIP string - MFAVerified string + Key []byte + Username string + TTL time.Duration + Compatibility string + RouteToCluster string + PinnedIP string + MFAVerified string + AttestationStatement *keys.AttestationStatement } // GenerateUserTestCerts is used to generate user certificate, used internally for tests @@ -2040,16 +2041,17 @@ func (a *Server) GenerateUserTestCerts(req GenerateUserTestCertsRequest) ([]byte return nil, nil, trace.Wrap(err) } certs, err := a.generateUserCert(ctx, certRequest{ - user: userState, - ttl: req.TTL, - compatibility: req.Compatibility, - publicKey: req.Key, - routeToCluster: req.RouteToCluster, - checker: checker, - traits: userState.GetTraits(), - loginIP: req.PinnedIP, - pinIP: req.PinnedIP != "", - mfaVerified: req.MFAVerified, + user: userState, + ttl: req.TTL, + compatibility: req.Compatibility, + publicKey: req.Key, + routeToCluster: req.RouteToCluster, + checker: checker, + traits: userState.GetTraits(), + loginIP: req.PinnedIP, + pinIP: req.PinnedIP != "", + mfaVerified: req.MFAVerified, + attestationStatement: req.AttestationStatement, }) if err != nil { return nil, nil, trace.Wrap(err) diff --git a/lib/modules/test.go b/lib/modules/test.go index 412c88384c9..fac9d682a5d 100644 --- a/lib/modules/test.go +++ b/lib/modules/test.go @@ -45,7 +45,8 @@ type TestModules struct { defaultModules // MockAttestationData is fake attestation data to return - // during tests when hardware key support is enabled. + // during tests when hardware key support is enabled. This + // attestation data is shared by all logins when set. MockAttestationData *keys.AttestationData } diff --git a/tool/tsh/common/app_aws_test.go b/tool/tsh/common/app_aws_test.go index 8efcd302535..67bf0a572bb 100644 --- a/tool/tsh/common/app_aws_test.go +++ b/tool/tsh/common/app_aws_test.go @@ -59,11 +59,8 @@ func TestAWS(t *testing.T) { // Log into Teleport cluster. err = Run(context.Background(), []string{ - "login", "--insecure", "--debug", "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), CliOption(func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, user) - return nil - })) + "login", "--insecure", "--debug", "--proxy", proxyAddr.String(), + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, user, connector.GetName())) require.NoError(t, err) // Run "tsh aws". Use a custom "cmdRunner" instead of executing AWS CLI. We diff --git a/tool/tsh/common/app_azure_test.go b/tool/tsh/common/app_azure_test.go index 2cea912b550..5d1c622d1a5 100644 --- a/tool/tsh/common/app_azure_test.go +++ b/tool/tsh/common/app_azure_test.go @@ -61,10 +61,7 @@ func TestAzure(t *testing.T) { // helper function run := func(args []string, opts ...CliOption) { opts = append(opts, setHomePath(tmpHomePath)) - opts = append(opts, func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, user) - return nil - }) + opts = append(opts, setMockSSOLogin(authServer, user, connector.GetName())) err := Run(context.Background(), args, opts...) require.NoError(t, err) } @@ -73,7 +70,7 @@ func TestAzure(t *testing.T) { t.Setenv("MSI_ENDPOINT", "https://azure-msi.teleport.dev/very-secret") // Log into Teleport cluster. - run([]string{"login", "--insecure", "--debug", "--auth", connector.GetName(), "--proxy", proxyAddr.String()}) + run([]string{"login", "--insecure", "--debug", "--proxy", proxyAddr.String()}) // Log into the "azure-api" app. // Verify `tsh az login ...` gets called. @@ -133,7 +130,6 @@ func TestAzure(t *testing.T) { require.NotZero(t, req.ExpiresOn) require.NotZero(t, req.ExtExpiresIn) require.NotZero(t, req.NotBefore) - }, }, } diff --git a/tool/tsh/common/app_test.go b/tool/tsh/common/app_test.go index 0a71b27aac7..45199154077 100644 --- a/tool/tsh/common/app_test.go +++ b/tool/tsh/common/app_test.go @@ -142,7 +142,6 @@ func TestAppLoginLeaf(t *testing.T) { return false } return len(servers) == 1 && servers[0].GetName() == "leafapp" - }, 10*time.Second, 100*time.Millisecond, "leaf cluster did not come online") // helpers @@ -166,15 +165,12 @@ func TestAppLoginLeaf(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", rootProxyAddr.String(), - cluster} - - opt := func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, rootAuth.GetAuthServer(), alice) - return nil + cluster, } + opt := setMockSSOLogin(rootAuth.GetAuthServer(), alice, connector.GetName()) + return run(args, opt) } tsh := func(args ...string) string { return run(args) } diff --git a/tool/tsh/common/db_test.go b/tool/tsh/common/db_test.go index cbe13d26c23..d60040acb33 100644 --- a/tool/tsh/common/db_test.go +++ b/tool/tsh/common/db_test.go @@ -154,7 +154,8 @@ func testDatabaseLogin(t *testing.T) { ExternalID: "123123123", Region: "us-west-1", }, - }} + }, + } }), ) s.user = alice @@ -430,11 +431,8 @@ func TestLocalProxyRequirement(t *testing.T) { // Log into Teleport cluster. err = Run(context.Background(), []string{ - "login", "--insecure", "--debug", "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), CliOption(func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - return nil - })) + "login", "--insecure", "--debug", "--proxy", proxyAddr.String(), + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) defaultAuthPref, err := authServer.GetAuthPreference(ctx) diff --git a/tool/tsh/common/hardware_key_test.go b/tool/tsh/common/hardware_key_test.go new file mode 100644 index 00000000000..a87ab89c7eb --- /dev/null +++ b/tool/tsh/common/hardware_key_test.go @@ -0,0 +1,169 @@ +//go:build pivtest + +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package common + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" + "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/modules" + "github.com/gravitational/teleport/lib/service/servicecfg" + testserver "github.com/gravitational/teleport/tool/teleport/testenv" +) + +// TestHardwareKeyLogin tests Hardware Key login and relogin flows. +func TestHardwareKeyLogin(t *testing.T) { + ctx := context.Background() + + testModules := &modules.TestModules{TestBuildType: modules.BuildEnterprise} + modules.SetTestModules(t, testModules) + + connector := mockConnector(t) + + alice, err := types.NewUser("alice@example.com") + require.NoError(t, err) + aliceRole, err := types.NewRole("alice", types.RoleSpecV6{}) + require.NoError(t, err) + alice.SetRoles([]string{aliceRole.GetName()}) + + testServer := testserver.MakeTestServer(t, testserver.WithBootstrap(connector, alice, aliceRole), func(o *testserver.TestServersOpts) { + o.ConfigFuncs = append(o.ConfigFuncs, func(cfg *servicecfg.Config) { + // TODO (Joerger): This test fails to propagate hardware key policy errors from Proxy SSH connections + // for unknown reasons unless Multiplex mode is on. I could not reproduce these errors on a live + // cluster, so the issue likely lies with the test server setup. Perhaps the test certs generated + // are not 1-to-1 with live certs. + cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Multiplex) + }) + }) + authServer := testServer.GetAuthServer() + + proxyAddr, err := testServer.ProxyWebAddr() + require.NoError(t, err) + + // mock SSO login and count the number of login attempts. + var lastLoginCount int + mockSSOLogin := mockSSOLogin(authServer, alice) + mockSSOLoginWithCountAndAttestation := func(ctx context.Context, connectorID string, priv *keys.PrivateKey, protocol string) (*auth.SSHLoginResponse, error) { + lastLoginCount++ + + // Set MockAttestationData to attest the expected key policy and reset it after login. + testModules.MockAttestationData = &keys.AttestationData{ + PrivateKeyPolicy: priv.GetPrivateKeyPolicy(), + } + defer func() { + testModules.MockAttestationData = nil + }() + + return mockSSOLogin(ctx, connectorID, priv, protocol) + } + setMockSSOLogin := setMockSSOLoginCustom(mockSSOLoginWithCountAndAttestation, connector.GetName()) + + t.Run("cap", func(t *testing.T) { + setRequireMFAType := func(t *testing.T, requireMFAType types.RequireMFAType) { + // Set require MFA type in the cluster auth preference. + _, err := authServer.UpsertAuthPreference(ctx, &types.AuthPreferenceV2{ + Spec: types.AuthPreferenceSpecV2{ + RequireMFAType: requireMFAType, + }, + }) + require.NoError(t, err) + } + + t.Cleanup(func() { + setRequireMFAType(t, types.RequireMFAType_OFF) + }) + + // login should use the private key policy reported by the proxy without + // needing to retry hardware key login. + setRequireMFAType(t, types.RequireMFAType_HARDWARE_KEY_TOUCH) + tmpHomePath := t.TempDir() + err = Run(context.Background(), []string{ + "login", + "--debug", + "--insecure", + "--proxy", proxyAddr.String(), + }, setHomePath(tmpHomePath), setMockSSOLogin) + require.NoError(t, err) + assert.Equal(t, 1, lastLoginCount, "expected one login attempt but got %v", lastLoginCount) + lastLoginCount = 0 // reset login count + + // Upgrading the auth preference requireMFAType should trigger relogin + // on the next command run. + setRequireMFAType(t, types.RequireMFAType_HARDWARE_KEY_TOUCH_AND_PIN) + err = Run(context.Background(), []string{ + "ls", + "--debug", + "--insecure", + "--proxy", proxyAddr.String(), + }, setHomePath(tmpHomePath), setMockSSOLogin) + require.NoError(t, err) + assert.Equal(t, 1, lastLoginCount, "expected one login attempt but got %v", lastLoginCount) + lastLoginCount = 0 // reset login count + }) + + t.Run("role", func(t *testing.T) { + setRequireMFAType := func(t *testing.T, requireMFAType types.RequireMFAType) { + // Set require MFA type in the user's role. + aliceRole.SetOptions(types.RoleOptions{ + RequireMFAType: requireMFAType, + }) + _, err = authServer.UpsertRole(ctx, aliceRole) + require.NoError(t, err) + } + + t.Cleanup(func() { + setRequireMFAType(t, types.RequireMFAType_OFF) + }) + + // login should initially fail using the private key policy reported by the proxy (off), + // then trigger a retry with the hardware key policy parsed from the error. + setRequireMFAType(t, types.RequireMFAType_HARDWARE_KEY_TOUCH) + tmpHomePath := t.TempDir() + err = Run(context.Background(), []string{ + "login", + "--debug", + "--insecure", + "--proxy", proxyAddr.String(), + }, setHomePath(tmpHomePath), setMockSSOLogin) + require.NoError(t, err) + assert.Equal(t, 2, lastLoginCount, "expected two login attempts but got %v", lastLoginCount) + lastLoginCount = 0 // reset login count + + // Upgrading the auth preference requireMFAType should trigger relogin + // on the next command run. + setRequireMFAType(t, types.RequireMFAType_HARDWARE_KEY_TOUCH_AND_PIN) + err = Run(context.Background(), []string{ + "ls", + "--debug", + "--insecure", + "--proxy", proxyAddr.String(), + }, setHomePath(tmpHomePath), setMockSSOLogin) + require.NoError(t, err) + assert.Equal(t, 1, lastLoginCount, "expected one login attempt but got %v", lastLoginCount) + lastLoginCount = 0 // reset login count + }) +} diff --git a/tool/tsh/common/proxy_test.go b/tool/tsh/common/proxy_test.go index 2492f02f329..fd20fde98a2 100644 --- a/tool/tsh/common/proxy_test.go +++ b/tool/tsh/common/proxy_test.go @@ -166,7 +166,7 @@ func testJumpHostSSHAccess(t *testing.T, s *suite) { err := Run(context.Background(), []string{ "login", s.leaf.Config.Auth.ClusterName.GetClusterName(), - }, setMockSSOLogin(t, s), setHomePath(tshHome)) + }, s.setMockSSOLogin(t), setHomePath(tshHome)) require.NoError(t, err) // Connect to leaf node though jump host set to leaf proxy SSH port. @@ -175,7 +175,7 @@ func testJumpHostSSHAccess(t *testing.T, s *suite) { "-J", s.leaf.Config.Proxy.SSHAddr.Addr, s.leaf.Config.Hostname, "echo", "hello", - }, setMockSSOLogin(t, s), setHomePath(tshHome)) + }, s.setMockSSOLogin(t), setHomePath(tshHome)) require.NoError(t, err) t.Run("root cluster online", func(t *testing.T) { @@ -185,7 +185,7 @@ func testJumpHostSSHAccess(t *testing.T, s *suite) { "-J", s.leaf.Config.Proxy.WebAddr.Addr, s.leaf.Config.Hostname, "echo", "hello", - }, setMockSSOLogin(t, s), setHomePath(tshHome)) + }, s.setMockSSOLogin(t), setHomePath(tshHome)) require.NoError(t, err) }) @@ -200,7 +200,7 @@ func testJumpHostSSHAccess(t *testing.T, s *suite) { "-J", s.leaf.Config.Proxy.WebAddr.Addr, s.leaf.Config.Hostname, "echo", "hello", - }, setMockSSOLogin(t, s), setHomePath(tshHome)) + }, s.setMockSSOLogin(t), setHomePath(tshHome)) require.NoError(t, err) }) } @@ -534,7 +534,7 @@ func TestProxySSH(t *testing.T) { t.Run("re-login", func(t *testing.T) { t.Parallel() - err := runProxySSH(proxyRequest, setHomePath(t.TempDir()), setKubeConfigPath(filepath.Join(t.TempDir(), teleport.KubeConfigFile)), setMockSSOLogin(t, s)) + err := runProxySSH(proxyRequest, setHomePath(t.TempDir()), setKubeConfigPath(filepath.Join(t.TempDir(), teleport.KubeConfigFile)), s.setMockSSOLogin(t)) require.NoError(t, err) }) @@ -549,7 +549,7 @@ func TestProxySSH(t *testing.T) { // it's legal to specify any username before the request invalidLoginRequest := fmt.Sprintf("%s@%s", "invalidUser", proxyRequest) - err := runProxySSH(invalidLoginRequest, setHomePath(homePath), setKubeConfigPath(kubeConfigPath), setMockSSOLogin(t, s)) + err := runProxySSH(invalidLoginRequest, setHomePath(homePath), setKubeConfigPath(kubeConfigPath), s.setMockSSOLogin(t)) require.NoError(t, err) }) }) @@ -956,12 +956,8 @@ func disableAgent(t *testing.T) { t.Setenv(teleport.SSHAuthSock, "") } -func setMockSSOLogin(t *testing.T, s *suite) CliOption { - return func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, s.root.GetAuthServer(), s.user) - cf.AuthConnector = s.connector.GetName() - return nil - } +func (s *suite) setMockSSOLogin(t *testing.T) CliOption { + return setMockSSOLogin(s.root.GetAuthServer(), s.user, s.connector.GetName()) } func mustLogin(t *testing.T, s *suite, args ...string) (tshHome, kubeConfig string) { @@ -974,7 +970,7 @@ func mustLogin(t *testing.T, s *suite, args ...string) (tshHome, kubeConfig stri "--proxy", s.root.Config.Proxy.WebAddr.String(), }, args...) err := Run(context.Background(), args, - setMockSSOLogin(t, s), + s.setMockSSOLogin(t), setHomePath(tshHome), setKubeConfigPath(kubeConfig), ) diff --git a/tool/tsh/common/tctl_test.go b/tool/tsh/common/tctl_test.go index f35286c94d3..97d2dc534dc 100644 --- a/tool/tsh/common/tctl_test.go +++ b/tool/tsh/common/tctl_test.go @@ -53,12 +53,8 @@ func TestLoadConfigFromProfile(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), CliOption(func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - return nil - })) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) tests := []struct { @@ -115,12 +111,8 @@ func TestRemoteTctlWithProfile(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), CliOption(func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - return nil - })) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) t.Setenv(types.HomeEnvVar, tmpHomePath) @@ -181,12 +173,8 @@ func TestSetAuthServerFlagWhileLoggedIn(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), CliOption(func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - return nil - })) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) // we're now logged in with a profile in tmpHomePath. diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index f5c4a07274a..dc668b26cc7 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -386,12 +386,8 @@ func TestFailedLogin(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = ssoLogin - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLoginCustom(ssoLogin, connector.GetName())) require.ErrorIs(t, err, loginFailed) } @@ -480,13 +476,11 @@ func TestOIDCLogin(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "--user", "alice", // explicitly use wrong name - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - cf.SiteName = "localhost" - cf.overrideStderr = buf + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName()), func(c *CLIConf) error { + c.overrideStderr = buf + c.SiteName = "localhost" return nil }) @@ -595,13 +589,9 @@ func TestLoginIdentityOut(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "--out", identPath, - }, tt.extraArgs...), setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - return nil - }) + }, tt.extraArgs...), setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) tt.validationFunc(t, identPath) }) @@ -654,10 +644,8 @@ func TestRelogin(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName()), func(cf *CLIConf) error { cf.overrideStderr = buf return nil }) @@ -670,12 +658,10 @@ func TestRelogin(t *testing.T) { "--debug", "--proxy", proxyAddr.String(), "localhost", - }, setHomePath(tmpHomePath), - func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - cf.overrideStderr = buf - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName()), func(cf *CLIConf) error { + cf.overrideStderr = buf + return nil + }) require.NoError(t, err) findMOTD(t, sc, motd) @@ -690,11 +676,9 @@ func TestRelogin(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "localhost", - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName()), func(cf *CLIConf) error { cf.overrideStderr = buf return nil }) @@ -731,10 +715,8 @@ func TestIgnoreHTTPSPrefix(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddress, - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName()), func(cf *CLIConf) error { cf.overrideStderr = &buf return nil }) @@ -779,39 +761,26 @@ func TestSwitchingProxies(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr1.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer1, alice) - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer1, alice, connector.GetName())) require.NoError(t, err) err = Run(context.Background(), []string{ "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr2.String(), - }, setHomePath(tmpHomePath), - func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer2, alice) - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer2, alice, connector.GetName())) require.NoError(t, err) // login again while both proxies are still valid and ensure it is successful without an SSO login provided - err = Run(context.Background(), []string{ "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr1.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - return nil - }) + }, setHomePath(tmpHomePath)) require.NoError(t, err) @@ -819,20 +788,14 @@ func TestSwitchingProxies(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr2.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - return nil - }) + }, setHomePath(tmpHomePath)) require.NoError(t, err) // logout - err = Run(context.Background(), []string{"logout"}, setHomePath(tmpHomePath), - func(cf *CLIConf) error { - return nil - }) + err = Run(context.Background(), []string{"logout"}, setHomePath(tmpHomePath)) require.NoError(t, err) // after logging out, make sure that any attempt to log in without providing a valid login function fails @@ -842,24 +805,16 @@ func TestSwitchingProxies(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr1.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - return nil - }) - + }, setHomePath(tmpHomePath)) require.Error(t, err) err = Run(ctx, []string{ "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr2.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - return nil - }) - + }, setHomePath(tmpHomePath)) require.Error(t, err) cancel() @@ -1720,13 +1675,11 @@ func TestSSHOnMultipleNodes(t *testing.T) { "login", "-d", "--insecure", - "--auth", connector.GetName(), "--proxy", tt.proxyAddr, "--user", user.GetName(), tt.cluster, - }, setHomePath(tmpHomePath), + }, setHomePath(tmpHomePath), setMockSSOLogin(tt.auth, user, connector.GetName()), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, tt.auth, user) cf.WebauthnLogin = tt.webauthnLogin return nil }, @@ -1894,13 +1847,9 @@ func TestSSHAccessRequest(t *testing.T) { err = Run(ctx, []string{ "login", "--insecure", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "--user", "alice", - }, setHomePath(tmpHomePath), CliOption(func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, rootAuth.GetAuthServer(), alice) - return nil - })) + }, setHomePath(tmpHomePath), setMockSSOLogin(rootAuth.GetAuthServer(), alice, connector.GetName())) require.NoError(t, err) // won't request if can't list node @@ -1991,13 +1940,9 @@ func TestSSHAccessRequest(t *testing.T) { err = Run(ctx, []string{ "login", "--insecure", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "--user", "alice", - }, setHomePath(tmpHomePath), CliOption(func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, rootAuth.GetAuthServer(), alice) - return nil - })) + }, setHomePath(tmpHomePath), setMockSSOLogin(rootAuth.GetAuthServer(), alice, connector.GetName())) require.NoError(t, err) // ssh with request, by host ID @@ -2109,12 +2054,8 @@ func TestAccessRequestOnLeaf(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", rootProxyAddr.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, rootAuthServer, alice) - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLogin(rootAuthServer, alice, connector.GetName())) require.NoError(t, err) err = Run(ctx, []string{ @@ -2258,22 +2199,18 @@ func TestKubeCredentialsLock(t *testing.T) { require.NoError(t, err) var ssoCalls atomic.Int32 - mockSSO := mockSSOLogin(t, authServer, alice) - ssoFunc := func(ctx context.Context, connectorID string, priv *keys.PrivateKey, protocol string) (*auth.SSHLoginResponse, error) { + mockSSOLogin := mockSSOLogin(authServer, alice) + mockSSOLoginWithCountCalls := func(ctx context.Context, connectorID string, priv *keys.PrivateKey, protocol string) (*auth.SSHLoginResponse, error) { ssoCalls.Add(1) - return mockSSO(ctx, connectorID, priv, protocol) + return mockSSOLogin(ctx, connectorID, priv, protocol) } err = Run(context.Background(), []string{ "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = ssoFunc - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLoginCustom(mockSSOLoginWithCountCalls, connector.GetName())) require.NoError(t, err) _, err = profile.FromDir(tmpHomePath, "") require.NoError(t, err) @@ -2328,13 +2265,9 @@ iUK/veLmZ6XoouiWLCdU1VJz/1Fcwe/IEamg6ETfofvsqOCgcNYJ "credentials", "--insecure", "--proxy", proxyAddr.String(), - "--auth", connector.GetName(), "--teleport-cluster", teleportClusterName.GetClusterName(), "--kube-cluster", kubeClusterName, - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = ssoFunc - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLoginCustom(mockSSOLoginWithCountCalls, connector.GetName())) errChan <- credErr } @@ -3365,12 +3298,8 @@ func TestAuthClientFromTSHProfile(t *testing.T) { "login", "--insecure", "--debug", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) profile, err := profile.FromDir(tmpHomePath, "") @@ -3551,26 +3480,34 @@ func mockConnector(t *testing.T) types.OIDCConnector { return connector } -func mockSSOLogin(t *testing.T, authServer *auth.Server, user types.User) client.SSOLoginFunc { +func mockSSOLogin(authServer *auth.Server, user types.User) client.SSOLoginFunc { return func(ctx context.Context, connectorID string, priv *keys.PrivateKey, protocol string) (*auth.SSHLoginResponse, error) { // generate certificates for our user clusterName, err := authServer.GetClusterName() - require.NoError(t, err) + if err != nil { + return nil, trace.Wrap(err) + } + sshCert, tlsCert, err := authServer.GenerateUserTestCerts(auth.GenerateUserTestCertsRequest{ - Key: priv.MarshalSSHPublicKey(), - Username: user.GetName(), - TTL: time.Hour, - Compatibility: constants.CertificateFormatStandard, - RouteToCluster: clusterName.GetClusterName(), + Key: priv.MarshalSSHPublicKey(), + Username: user.GetName(), + TTL: time.Hour, + Compatibility: constants.CertificateFormatStandard, + RouteToCluster: clusterName.GetClusterName(), + AttestationStatement: priv.GetAttestationStatement(), }) - require.NoError(t, err) + if err != nil { + return nil, trace.Wrap(err) + } // load CA cert authority, err := authServer.GetCertAuthority(ctx, types.CertAuthID{ Type: types.HostCA, DomainName: clusterName.GetClusterName(), }, false) - require.NoError(t, err) + if err != nil { + return nil, trace.Wrap(err) + } // build login response return &auth.SSHLoginResponse{ @@ -3667,6 +3604,18 @@ func setCmdRunner(cmdRunner func(*exec.Cmd) error) CliOption { } } +func setMockSSOLogin(authServer *auth.Server, user types.User, connectorName string) CliOption { + return setMockSSOLoginCustom(mockSSOLogin(authServer, user), connectorName) +} + +func setMockSSOLoginCustom(mockSSOLogin client.SSOLoginFunc, connectorName string) CliOption { + return func(cf *CLIConf) error { + cf.MockSSOLogin = mockSSOLogin + cf.AuthConnector = connectorName + return nil + } +} + func testSerialization(t *testing.T, expected string, serializer func(string) (string, error)) { t.Helper() out, err := serializer(teleport.JSON) @@ -4812,13 +4761,9 @@ func TestForwardingTraces(t *testing.T) { err = Run(context.Background(), []string{ "login", "--insecure", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "--trace", - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) if traceCfg.Enabled && traceCfg.SamplingRate > 0 { @@ -4832,7 +4777,6 @@ func TestForwardingTraces(t *testing.T) { err = Run(context.Background(), []string{ "ls", "--insecure", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "--trace", }, setHomePath(tmpHomePath)) @@ -4930,14 +4874,10 @@ func TestExportingTraces(t *testing.T) { err = Run(context.Background(), []string{ "login", "--insecure", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "--trace", "--trace-exporter", tshCollector.GRPCAddr(), - }, setHomePath(tmpHomePath), func(cf *CLIConf) error { - cf.MockSSOLogin = mockSSOLogin(t, authServer, alice) - return nil - }) + }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) if traceCfg.Enabled { @@ -4951,7 +4891,6 @@ func TestExportingTraces(t *testing.T) { err = Run(context.Background(), []string{ "ls", "--insecure", - "--auth", connector.GetName(), "--proxy", proxyAddr.String(), "--trace", "--trace-exporter", tshCollector.GRPCAddr(),