Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v15] Fix relogin on hardware key policy errors #38819

Merged
merged 2 commits into from Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 18 additions & 5 deletions lib/client/api.go
Expand Up @@ -674,6 +674,20 @@ func WithBeforeLoginHook(fn func() error) RetryWithReloginOption {

// IsErrorResolvableWithRelogin returns true if relogin is attempted on `err`.
func IsErrorResolvableWithRelogin(err error) bool {
// Private key policy errors indicate that the user must login with an
// unexpected private key policy requirement satisfied. This can occur
// in the following cases:
// - User is logging in for the first time, and their strictest private
// key policy requirement is specified in a role.
// - User is assuming a role with a stricter private key policy requirement
// than the user's given roles.
// - The private key policy in the user's roles or the cluster auth
// preference have been upgraded since the user last logged in, making
// their current login session invalid.
if keys.IsPrivateKeyPolicyError(err) {
return true
}

// Ignore any failures resulting from RPCs.
// These were all materialized as status.Error here before
// https://github.com/gravitational/teleport/pull/30578.
Expand All @@ -682,11 +696,10 @@ func IsErrorResolvableWithRelogin(err error) bool {
return false
}

return keys.IsPrivateKeyPolicyError(err) ||
// TODO(codingllama): Retrying BadParameter is a terrible idea.
// We should fix this and remove the RemoteError condition above as well.
// Any retriable error should be explicitly marked as such.
trace.IsBadParameter(err) ||
// TODO(codingllama): Retrying BadParameter is a terrible idea.
// We should fix this and remove the RemoteError condition above as well.
// Any retriable error should be explicitly marked as such.
return trace.IsBadParameter(err) ||
trace.IsTrustError(err) ||
utils.IsCertExpiredError(err) ||
// Assume that failed handshake is a result of expired credentials.
Expand Down
31 changes: 31 additions & 0 deletions lib/client/api_test.go
Expand Up @@ -37,6 +37,8 @@ import (

"github.com/gravitational/teleport/api/client/webclient"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/utils/grpc/interceptors"
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/observability/tracing"
Expand Down Expand Up @@ -1215,3 +1217,32 @@ func TestConnectToProxyCancelledContext(t *testing.T) {
require.Nil(t, proxy)
require.Error(t, err)
}

func TestIsErrorResolvableWithRelogin(t *testing.T) {
for _, tt := range []struct {
name string
err error
expectResolvable bool
}{
{
name: "private key policy error should be resolvable",
err: keys.NewPrivateKeyPolicyError(keys.PrivateKeyPolicyHardwareKey),
expectResolvable: true,
}, {
name: "wrapped private key policy error should be resolvable",
err: &interceptors.RemoteError{
Err: keys.NewPrivateKeyPolicyError(keys.PrivateKeyPolicyHardwareKey),
},
expectResolvable: true,
},
} {
t.Run(tt.name, func(t *testing.T) {
resolvable := IsErrorResolvableWithRelogin(tt.err)
if tt.expectResolvable {
require.True(t, resolvable, "Expected error to be resolvable with relogin")
} else {
require.False(t, resolvable, "Expected error to be unresolvable with relogin")
}
})
}
}