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

ssh/tailssh: use a local error instead of gossh.ErrDenied #10743

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

awly
Copy link
Contributor

@awly awly commented Jan 4, 2024

ErrDenied was added in our fork of
x/crypto/ssh
to short-circuit auth attempts once one fails.

In the case of our callbacks, this error is returned when SSH policy check determines that a connection should not be allowed. Both NoClientAuthCallback and PublicKeyHandler check the policy and will fail anyway. The fakePasswordHandler returns true only if NoClientAuthCallback succeeds the policy check, so it checks it indirectly too.

The difference here is that a client might attempt all 2-3 auth methods instead of just none but will fail to authenticate regardless.

Updates #8593

ErrDenied was added in [our fork of
x/crypto/ssh](golang/crypto@acc6f8f)
to short-circuit auth attempts once one fails.

In the case of our callbacks, this error is returned when SSH policy
check determines that a connection should not be allowed. Both
`NoClientAuthCallback` and `PublicKeyHandler` check the policy and will
fail anyway. The `fakePasswordHandler` returns true only if
`NoClientAuthCallback` succeeds the policy check, so it checks it
indirectly too.

The difference here is that a client might attempt all 2-3 auth methods
instead of just `none` but will fail to authenticate regardless.

Updates #8593

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
@awly awly merged commit 29e98e1 into main Jan 5, 2024
46 checks passed
@awly awly deleted the awly/ssh-errdenied branch January 5, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants