3 ms·

That's not the RCE fix, this is the RCE fix https://news.ycombinator.com/item?id=40843865 https://news.ycombinator.com/item?id=40843865 That's a previously-ann

by fanf2 2y ago

That's not the RCE fix, this is the RCE fix https://news.ycombinator.com/item?id=40843865 https://news.ycombinator.com/item?id=40843865 That's a previously-announced feature for dealing with junk connections that also happens to mitigate this vulnerability because it makes it harder to win the race. Discussed previously https://news.ycombinator.com/item?id=40610621 https://news.ycombinator.com/item?id=40610621

djmdjm 2y ago
No, it's a fix. It completely removes the signal race as well as introducing a mitigation for similar future bugs
FiloSottile 2y ago
The ones you link are the "minimal patches for those can't/don't want to upgrade". The commit I am linking to is taken straight from the advisory. On June 6, 2024, this signal handler race condition was fixed by commit 81c1099 ("Add a facility to sshd(8) to penalise particular problematic client behaviours"), which moved the async-signal-unsafe code from sshd's SIGALRM handler to sshd's listener process, where it can be handled synchronously: https://github.com/openssh/openssh-portable/commit/81c1099d22b81ebfd20a334ce986c4f753b0db29 Because this fix is part of a large commit (81c1099), on top of an even larger defense-in-depth commit (03e3de4, "Start the process of splitting sshd into separate binaries"), it might prove difficult to backport. In that case, the signal handler race condition itself can be fixed by removing or commenting out the async-signal-unsafe code from the sshsigdie() function The cleverness here is that this commit is both "a previously-announced feature for dealing with junk connections", and a mitigation for the exploit class against similar but unknown vulnerabilities, and a patch for the specific vulnerability because it "moved the async-signal-unsafe code from sshd's SIGALRM handler to sshd's listener process, where it can be handled synchronously". The cleverness is that it fixes the vulnerability as part of doing something that makes sense on its own, so you wouldn't know it's the patch even looking at it.
unixpickle 2y ago
These lines from the diff linked above are the fix: - /\* Log error and exit. \*/ - sigdie("Timeout before authentication for %s port %d", - ssh_remote_ipaddr(the_active_state), - ssh_remote_port(the_active_state)); + _exit(EXIT_LOGIN_GRACE);

by fanf2 2y ago

djmdjm 2y ago

No, it's a fix. It completely removes the signal race as well as introducing a mitigation for similar future bugs

FiloSottile 2y ago

The ones you link are the "minimal patches for those can't/don't want to upgrade". The commit I am linking to is taken straight from the advisory. On June 6, 2024, this signal handler race condition was fixed by commit 81c1099 ("Add a facility to sshd(8) to penalise particular problematic client behaviours"), which moved the async-signal-unsafe code from sshd's SIGALRM handler to sshd's listener process, where it can be handled synchronously: https://github.com/openssh/openssh-portable/commit/81c1099d22b81ebfd20a334ce986c4f753b0db29 Because this fix is part of a large commit (81c1099), on top of an even larger defense-in-depth commit (03e3de4, "Start the process of splitting sshd into separate binaries"), it might prove difficult to backport. In that case, the signal handler race condition itself can be fixed by removing or commenting out the async-signal-unsafe code from the sshsigdie() function The cleverness here is that this commit is both "a previously-announced feature for dealing with junk connections", and a mitigation for the exploit class against similar but unknown vulnerabilities, and a patch for the specific vulnerability because it "moved the async-signal-unsafe code from sshd's SIGALRM handler to sshd's listener process, where it can be handled synchronously". The cleverness is that it fixes the vulnerability as part of doing something that makes sense on its own, so you wouldn't know it's the patch even looking at it.

unixpickle 2y ago

These lines from the diff linked above are the fix: - /\* Log error and exit. \*/ - sigdie("Timeout before authentication for %s port %d", - ssh_remote_ipaddr(the_active_state), - ssh_remote_port(the_active_state)); + _exit(EXIT_LOGIN_GRACE);