Hi,
Here is a small patch that tries to fix the problem mentioned below. Looking a
bit into history, it seems that the custom logs part of `auth_log` (within
`ifdef`) was missed out when the `partial` parameter was added to `auth_log`
(15b05cfa17592da7470d7bd4b2de063188697471 in openssh-portable).
The patch is modifying a bit the existing `ifdef` logic to remove the
duplicated `if (authenticated == 0 && !authctxt->postponed`, which I
understand was flawed (missing the partial check). I'm not sure if it's
better, because it makes it a bit more complicated to follow all cases
(depending on the defined variables), but at the same time it simplifies the
if statements. Let me know what's best.
I'm not too sure why the `if (authenticated)` part for `WITH_AIXAUTHENTICATE`
is within `CUSTOM_FAILED_LOGIN`, as it's not logging a failed login, but I
didn't want to change this logic...
Please let me know if I'm missing something,
Vincent Brillault
On 04/05/2020 10:56, Vincent Brillault wrote:
> Hi,
>
> Trying to understand why some spurious `There was 1 failed login attempt since
> the last successful logins`, that seems to appear on every single login, I
> think there is a bug in auth.c's auth_log with the handling of partial logins:
> https://github.com/openssh/openssh-portable/blob/c697e46c314aa94574af0d393d80f23e0ebc9748/auth.c#L355-L372
>
> If I read this code correctly, when auth_log is called with authenticated=0
> and partial=1 without authctxt->postponed being set (which is normal on
> partial authentications) then:
> - if method is password, keyboard-interactive or challenge-response (not sure
> why the others are not considered?), record_failed_login is called
> - audit_event is called with an event from audit_classify_auth which always
> seems to return a failure events (or unknown).
>
> So it seems that partial authentications are considered as failures :/
>
> The simplest fix for me seems to be to return before L355 if partial or
> authctxt->postponed are set (maybe after checking that there isn't a logic
> flow and authenticated was set?).
>
> Am I missing something?
> Thanks in advance,
> Vincent Brillault
>
From 04fade9d7c1ac0acbecdea4ee3b5496523698cdf Mon Sep 17 00:00:00 2001
From: Vincent Brillault <vincent.brillault@xxxxxxx>
Date: Sun, 24 May 2020 09:15:06 +0200
Subject: [PATCH] auth_log: dont log partial successes as failures
By design, 'partial' logins are successful logins, so initially with
authenticated set to 1, for which another authentication is required. As
a result, authenticated is always reset to 0 when partial is set to 1.
However, even if authenticated is 0, those are not failed login
attempts, similarly to attempts with authctxt->postponed set to 1.
---
auth.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/auth.c b/auth.c
index 086b8ebb..ba3e03b0 100644
--- a/auth.c
+++ b/auth.c
@@ -352,23 +352,26 @@ auth_log(struct ssh *ssh, int authenticated, int partial,
free(extra);
-#ifdef CUSTOM_FAILED_LOGIN
- if (authenticated == 0 && !authctxt->postponed &&
- (strcmp(method, "password") == 0 ||
- strncmp(method, "keyboard-interactive", 20) == 0 ||
- strcmp(method, "challenge-response") == 0))
- record_failed_login(ssh, authctxt->user,
- auth_get_canonical_hostname(ssh, options.use_dns), "ssh");
-# ifdef WITH_AIXAUTHENTICATE
+#if defined(CUSTOM_FAILED_LOGIN) || defined(SSH_AUDIT_EVENTS)
+ if (authenticated == 0 && !(authctxt->postponed || partial)) {
+ /* Log failed login attempt */
+# ifdef CUSTOM_FAILED_LOGIN
+ if (strcmp(method, "password") == 0 ||
+ strncmp(method, "keyboard-interactive", 20) == 0 ||
+ strcmp(method, "challenge-response") == 0)
+ record_failed_login(ssh, authctxt->user,
+ auth_get_canonical_hostname(ssh, options.use_dns), "ssh");
+# endif
+# ifdef SSH_AUDIT_EVENTS
+ audit_event(ssh, audit_classify_auth(method));
+# endif
+ }
+#endif
+#if defined(CUSTOM_FAILED_LOGIN) && defined(WITH_AIXAUTHENTICATE)
if (authenticated)
sys_auth_record_login(authctxt->user,
auth_get_canonical_hostname(ssh, options.use_dns), "ssh",
loginmsg);
-# endif
-#endif
-#ifdef SSH_AUDIT_EVENTS
- if (authenticated == 0 && !authctxt->postponed)
- audit_event(ssh, audit_classify_auth(method));
#endif
}
--
2.26.2
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
