jtesta / ssh-audit

SSH server & client security auditing (banner, key exchange, encryption, mac, compression, compatibility, security, etc)
MIT License
3.44k stars 179 forks source link

Switch `connect_ex` result checks to use `errno` lookups #289

Closed drewmnoel closed 3 months ago

drewmnoel commented 4 months ago

Closes #288 by performing the errno lookups dynamically based on the host platform.

michael-o commented 3 months ago

Something lke:

$ git diff -U0
diff --git a/src/ssh_audit/dheat.py b/src/ssh_audit/dheat.py
index 8700040..7b93ef7 100644
--- a/src/ssh_audit/dheat.py
+++ b/src/ssh_audit/dheat.py
@@ -23,0 +24 @@
+import errno
@@ -445 +446 @@ class DHEat:
-                if ret in [0, 115]:  # Check if connection is successful or EINPROGRESS.
+                if ret in [0, errno.EINPROGRESS]:
@@ -448 +449 @@ class DHEat:
-                    out.d("connect_ex() returned: %d" % ret, write_now=True)
+                    out.d("connect_ex() returned: %s (%d)" % errno.errorcode[ret], ret, write_now=True)
michael-o commented 3 months ago

After using this patch, I'm getting the rate-throttling message again, even though I have PerSourceMaxStartups 1 enabled in sshd_config. The message didn't show before the patch.

I'm using FreeBSD 13.3-RELEASE-p5

38 connections were created in 0.180 seconds, or 210.7 conns/sec; server must respond with a rate less than 20.0 conns/sec per IPv4/IPv6 source address to be considered safe. For rate-throttling options, please see https://www.ssh-audit.com/hardening_guides.html. Be aware that using 'PerSourceMaxStartups 1' properly protects the server from this attack, but will cause this test to yield a false positive. Suppress this test and message with the --skip-rate-test option.

I have the same FreeBSD version, but how is the message related to this patch?

qcybb commented 3 months ago

All I know is I updated to the latest version this morning from ports, and this problem happened. The PR pointed to this thread, so that's why I'm posting it here. Feel free to delete my posts, I'll send to Piotr Kubaj instead.

jtesta commented 3 months ago

Merged. Thanks @drewmnoel for the draft PR, and thanks @michael-o for the PR refinement and testing!