Skip to content
This repository has been archived by the owner on Jan 6, 2025. It is now read-only.

Move handshake success processing outside the callbacks #671

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

tsipinakis
Copy link
Member

Processing the end of the authentication step and the start of the container inside the auth callbacks was always finicky, with the recent issues of go crypto/ssh[1] with regards to the public key callback it is clear that this is not the intended way for them to be used.

After investigation of the aforementioned security issue in our dependency, no security compromise was found, the only side-effect was that a container is created before the end of the authentication step during the public key callback, but that is promptly cleaned up when the authentication failed.

No access is given if the proper key is not verified.

I'll merge this PR in 1 week unless someone stops me for a review.

[1] golang/go#70779


By contributing to this repository, I agree to the contribution guidelines.

@tsipinakis tsipinakis force-pushed the handshakeok_in_main branch 2 times, most recently from 83756a4 to e16dfd8 Compare December 17, 2024 18:16
Processing the end of the authentication step and the start of the
container inside the auth callbacks was always finicky, with the recent
issues of go crypto/ssh[1] with regards to the public key callback it is
clear that this is not the intended way for them to be used.

After investigation of the aforementioned security issue in our
dependency, no security compromise was found, the only side-effect was
that a container is created before the end of the authentication step
during the public key callback, but that is promptly cleaned up when the
authentication failed.

No access is given if the proper key is not verified.

[1] golang/go#70779

Signed-off-by: Nikos Tsipinakis <[email protected]>
@tsipinakis tsipinakis merged commit 26481e4 into main Jan 6, 2025
6 checks passed
@tsipinakis tsipinakis deleted the handshakeok_in_main branch January 6, 2025 18:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant