xaitax / CVE-2024-6387_Check

CVE-2024-6387_Check is a lightweight, efficient tool designed to identify servers running vulnerable versions of OpenSSH
GNU General Public License v3.0
380 stars 77 forks source link

Feature request: Option to test login timeout of more than X seconds #23

Closed agibson2 closed 1 day ago

agibson2 commented 1 week ago

A lot of workarounds are to remove the timeout for the login so that the race condition doesn't happen (LoginGraceTime 0). Even though that can open you up for DoS on the ssh logins, It does at least keep the original vulnerability being exploited. Just going by the banner version won't rule those out of course.

It would be great to have an option to test that the login timeout doesn't disconnect connection quicker than X seconds.... where X is a number that is longer than any default LoginGraceTime timeouts you have set in your environment. You could then flag any that have a longer timeout than X as possibly remediated by the LoginGraceTime 0 setting even though the banner identifies it as vulnerable. So after identifying the version, leave the session open for X amount of seconds (32 seconds or 302 seconds for example) to see if the server disconnects the session and flag them as possibly remediated if the server doesn't close the session at X seconds.

While this would take longer to scan all systems, I think this could be very helpful to help identify the servers that are more likely to be vulnerable so that we can spend more of our time patching systems that are more likely not remediated yet. With it being an option, there wouldn't be a down side to it as those that don't want to use this feature don't have to.

xaitax commented 4 days ago

Hi @agibson2 - added this feature. Thank you.

agibson2 commented 2 days ago

Thanks for looking at implementing the feature. Testing and looking at the code though, The sleep is happening before the socket is opened so the sleep isn't testing the connection to see if the socket times out.

I ended up doing something like this to get something workable... I basically set a socket timeout 4 second longer than what user specifies on --grace-time-check parameter. I set a start time (time.time()) to keep track of when the socket was opened so that when I set the timeout later on on a recv(), I can adjust to to be exactly the right time from the initial start of the socket connection instead of starting from the recv() call. I do a call to get/flush the banner receive buffer which takes some time. I then do a recv to wait on the socket so that any server close will immediately return or timeout occurs. It is wrapped in a try except to check to see if the socket timeout occurred on the return of the recv(). If the timeout did not occur then the server likely closed the connection which means LoginGraceTime is not set to 0 on the server. I also added a likely not vulnerable section for it. I don't do a try catch on the banner flush on the second connection to test the grace period so it could still use some work. Likely other errors on the socket might have unexpected results too as I am only checking for timeout error. SInce you are doing more error checking for the first banner socket though, The second should be less likely to have problems I would think.

🛡️ Servers not vulnerable: 2 [+] Server at 192.168.1.20:22 (running SSH-2.0-OpenSSH_for_Windows_8.6) [+] Server at 192.168.1.31:22 (running SSH-2.0-OpenSSH_7.9p1 Raspbian-10)

🛡️ Servers likely not vulnerable (possible LoginGraceTime remediation): 1 [+] Server at 192.168.1.127:22 (running SSH-2.0-OpenSSH_9.6 False negative possible depending on LoginGraceTime)

🚨 Servers likely vulnerable: 1 [+] Server at 192.168.1.1:22 (running SSH-2.0-OpenSSH_9.3) Version vulnerable and LoginGraceTime remediation not done (Session was closed by server at 30.009 seconds)

I put code here... https://github.com/agibson2/CVE-2024-6387_Check/tree/main

xaitax commented 2 days ago

Lovely, thanks so much and makes sense. Do you prefer you making a merge request or shall I adapt your code and implement it with "Co-Author"?

agibson2 commented 2 days ago

I added the pull request. Not sure if you like all the changes that I made though. Totally change it to something else or just accept parts of the changes. Doesn't matter to me. My changes likely could use some more socket error checking to be more robust for unexpected errors (especially for the call grace-time-check where the banner message is recv()'d and thrown away). Socket programming is not something I mess with much though and it works as is for me and the testing that I did with it.

Ideally the code to do the grace check would maybe best wrapped in the initial get_ssh_banner() function so that the code doesn't have to connect twice to get the banner and then test the timeout. I am sure you decided that it just isn't worth the effort to do that though.

xaitax commented 1 day ago

Thank you @agibson2