OpenSSH doesn't record badlogin attempts in btmp file for all possible authentication methods supported ( given below ). This patch is an enhancement to support this feature. Authentication Methods Supported --------------------------------- "gssapi-with-mic" "publickey" "hostbased" "password" "keyboard-interactive" "challenge-response" "keyboard-interactive/pam"
Created attachment 774 [details] BTMP PATCH BTMP Enhancement Patch attached .
Redhat (and maybe Solaris?) also have btmp, so setting platform to "All". Just so it's clear: the btmp code in loginrec is derived from login from util-linux correct? If so that's BSD licensed (w/advertising clause) with UCB as the copyright holder. Since UCB rescinded the advertising clause I think we're OK to use the code.
Comment on attachment 774 [details] BTMP PATCH >+ if ( getuid() != 0 ){ >+ debug("=== calling log_btmp uid %d ===\n", getuid()); >+ mm_log_btmp(authctxt->user,get_canonical_hostname(options.use_dns)); >+ } else { >+ debug("=== calling log_btmp uid %d ===\n", getuid()); >+ og_btmp(authctxt->user,get_canonical_hostname(options.use_dns)); >+ } This bit is unnecessary, the PRIVSEP macro is for this purpose, ie: debug("=== calling log_btmp uid %d ===\n", getuid()); PRIVSEP(log_btmp(authctxt->user,get_canonical_hostname(options.use_dns))); >+int mm_answer_log_btmp(int socket, Buffer *m){ [...] >+ buffer_get(m, user ,1024); >+ buffer_get(m, hostname ,1024); Sending the username is unecessary as the monitor already knows it (ie authctxt->user). Hostname can be determined too (see mm_record_login). As a general rule, as little as possible should be sent from slave to monitor. It's out of the scope of this bug but I'd prefer to see a generalized audit_event like in bug #125 in both OpenBSD and Portable, which could be easily extended to handle cases like this.
> the btmp code in loginrec is derived from login from util-linux correct? Yes referred from util-linux . > This bit is unnecessary, the PRIVSEP macro is for this purpose, ie: > debug("=== calling log_btmp uid %d ===\n", getuid()); > PRIVSEP(log_btmp(authctxt->user,get_canonical_hostname(options.use_dns))); I tried this, PRIVSEP macro may require a redefinition in auth.c. since it reports a linker error "ld: Unsatisfied symbol "PRIVSEP" in file auth.o"
(In reply to comment #4) > I tried this, PRIVSEP macro may require a redefinition in auth.c. since it > reports a linker error "ld: Unsatisfied symbol "PRIVSEP" in file auth.o" You just need to add '#include "monitor_wrap.h"' to the headers of auth.c.
Created attachment 776 [details] License from util-linux-2.12p:login-utils/login.c You must be careful about licensing when importing code. Fortunately this part of util-linux has a compatible license, but you still must include it when adding derived code into loginrec.c.
Created attachment 777 [details] BTMP PATCH Optimized BTMP patch attached .
> You just need to add '#include "monitor_wrap.h"' to the headers of auth.c. Yes, including monitor_wrap.h header file in auth.c solved the problem and also Optimized the patch further. Thanks > you still must include it when adding derived code into loginrec.c. Yes I have included util-linux compatible license in loginrec.c Note: btmp Path ( /var/adm/btmp ) has been hardcoded in this patch.
Created attachment 780 [details] Optimized BTMP PATCH
> Sending the username is unecessary as the monitor already knows it (ie > authctxt->user). Hostname can be determined too (see mm_record_login). As a > general rule, as little as possible should be sent from slave to monitor. Yes, I have extracted user and hostname from authctxt->user and get_canonical_hostname () in monitor.c function .
Created attachment 781 [details] Optimized Patch Attached > Sending the username is unecessary as the monitor already knows it (ie > authctxt->user). Hostname can be determined too (see mm_record_login). As a > general rule, as little as possible should be sent from slave to monitor. Yes, I have extracted user and hostname from authctxt->user and get_canonical_hostname () in monitor.c function .
Comment on attachment 781 [details] Optimized Patch Attached >--- Orginal/openssh/auth.c 2004-08-12 18:10:25.000000000 +0530 >+++ Patched/openssh/auth.c 2005-01-20 15:13:08.281151112 +0530 >@@ -50,6 +50,7 @@ > #include "misc.h" > #include "bufaux.h" > #include "packet.h" >+#include "monitor_wrap.h" > > /* import */ > extern ServerOptions options; >@@ -230,6 +231,18 @@ > else > authmsg = authenticated ? "Accepted" : "Failed"; > >+ if(!authenticated && !authctxt->postponed && (!strcmp(method, "gssapi-with-mic") || !strcmp(method, "publickey") || !strcmp(method, "hostbased"))){ >+ debug("=== calling log_btmp uid %d ===\n", getuid()); >+ PRIVSEP(log_btmp(authctxt->user,get_canonical_hostname(options.use_dns))); >+ } >+ >+ >+ if(!authenticated && !authctxt->postponed && (!strcmp(method, "password") || !strcmp(method, " keyboard-interactive") || !strcmp(method,"challenge-response") || !strcmp(method,"keyboard-interactive/pam"))){ >+ if ( getuid() == 0) { >+ debug("=== calling log_btmp uid %d ===\n", getuid()); These two blocks can be merged. The logging is inconsistent with other debug calls. The long list of strcmp looks fragile, if we add more auth methods. >+#define _PATH_BTMP "/var/adm/btmp" Most of the paths are defined in header files. >+void >+log_btmp(const char *username, const char *hostname) { here (and elsewhere), you aren't following http://www.openbsd.org/cgi-bin/man.cgi?query=style
In reply to comment #12) > >+#define _PATH_BTMP "/var/adm/btmp" > > Most of the paths are defined in header files. This one isn't (in the system headers, that is), at least as far as I can tell. I figured we'd stick it in the appropriate part of configure when the time comes (but that's an unnecessary complication right now).
Created attachment 785 [details] Modified BTMP PATCH attached > if(!authenticated && !authctxt->postponed && (!strcmp(method, "gssapi-with-mic") || !strcmp(method, "publickey") || !strcmp(method, "hostbased"))){ > debug("=== calling log_btmp uid %d ===\n", getuid()); > PRIVSEP(log_btmp(authctxt->user,get_canonical_hostname(options.use_dns))); > } > > > if(!authenticated && !authctxt->postponed && (!strcmp(method, "password") || !strcmp(method, " keyboard-interactive") || !strcmp(method,"challenge-response") || !strcmp(method,"keyboard-interactive/pam"))){ > if ( getuid() == 0) { > debug("=== calling log_btmp uid %d ===\n", getuid()); > These two blocks can be merged. The logging is inconsistent with other debug calls. The long list of strcmp looks fragile, > if we add more auth methods. THese two bloacks were merged as given below. if (!authenticated && !authctxt->postponed && strcmp(method, "none")) { debug("Entering log_btmp uid %d ", getuid()); if((!strcmp(method, "gssapi-with-mic") || !strcmp(method, "publickey") || !strcmp(method, "hostbased"))) PRIVSEP(log_btmp(authctxt->user,get_canonical_hostname(options.use_dns))); else if ( getuid()==0) log_btmp(authctxt->user,get_canonical_hostname(options.use_dns)); } >void >log_btmp(const char *username, const char *hostname) { These coding convention were corrected based on the source file style guide .
Created attachment 787 [details] BTMP PATCH Attached This patch has some correction with that of the pervious one: 1. buffer_append ( ) in mm_log_btmp ( ) is been replaced with buffer_get_string ( ) to avoid some memory errors due to memcpy copy done from random heap data. 2 UnWanted entries were removed from two tables 1. struct mon_table mon_dispatch_postauth20[] 2. struct mon_table mon_dispatch_postauth15[] in monitor.c .
Comment on attachment 787 [details] BTMP PATCH Attached I think we could commit the btmp logging part hooked up to CUSTOM_FAILED_LOGIN (subject to some changes below) but I think the monitor parts should be deferred until bug #125 is sorted. >+#define _PATH_BTMP "/var/adm/btmp" We'll put this in configure.ac. No big deal. >+void >+log_btmp(const char *username, const char *hostname) >+{ [...] >+ strcpy(ut.ut_line,"ssh:notty"); We don't use strcpy. [lots of processing snipped] >+ if (stat(_PATH_BTMP,&fst) == -1){ [...] >+ fd = open(_PATH_BTMP, O_WRONLY | O_APPEND); Not that this is a big deal but stat()ing the file then opening it is racy, and building the record is a waste of time if you're not going to be able to write it. I think the sequence should be: open, fstat, construct record, write. I'll do a patch with the above changes.
Created attachment 798 [details] log failed password and kbdint to btmp on Linux and HP-UX Adapted to use existing record_failed_login hook for password and keyboard-interactive. Tested OK on Linux (RH9, IPv4 and IPv6) and HP-UX (11.11, IPv4 only). It won't do other auth types (pubkey, gssapi, hostbased) at the moment, however this can be added later (after bug #125 is sorted). It also extracts the remote socket address via getpeername rather than doing a lookup on the textual hostname (which is wasteful and may not resolve to the same address as the connection originated from).
Created attachment 799 [details] btmp logging: normalise mapped 4in6 addresses too.
Comment on attachment 799 [details] btmp logging: normalise mapped 4in6 addresses too. looks sane to me
Patch #799 committed, thanks. It will be in the tomorrow's snapshot and the next major release. As to logging failures for the other auth types (pubkey, gssapi, hostbased), I don't think that should be enabled by default since most clients will try those as a matter of course and some environments might do lockouts based on the failures.
> + AC_DEFINE(_PATH_BTMP, "/var/log/btmp", [log for bad login attempts]) In HP-UX it is "/var/adm/btmp" > As to logging failures for the other auth types (pubkey, gssapi, hostbased), I > don't think that should be enabled by default How about getting them under a sshd_config directive .
(In reply to comment #21) > > + AC_DEFINE(_PATH_BTMP, "/var/log/btmp", [log for bad login attempts]) > > In HP-UX it is "/var/adm/btmp" The line you quoted is from the Linux block. On HP-UX it picks up the BTMP_FILE definition from the system headers (see defines.h). BTMP_FILE is defined on my 11.11 box, if it needs to be added for other versions just let me know which. > > As to logging failures for the other auth types (pubkey, gssapi, hostbased), > > Idon't think that should be enabled by default > > How about getting them under a sshd_config directive . I'm not sure. As a rule we try to keep the -Portable only config uptions to a minimum to preserve our sanity. Maybe a compile-time option to begin with ("-DPARANOID_AUTH_RECORDING" or something).
Can we log bad login attempts of an invalid user ? This patch log for "none" method when login attempt is made by a In-valid User. But works correctly for existing valid user ( doesn't log for "none" method ) 1. Disabling "none" doesn't solve if (authenticated == 0 && !authctxt->postponed && strcmp(method, "none" ) && ... 2. Avoiding Invalid user doesn't solve if (authenticated == 0 && authctxt->valid && !authctxt->postponed && strcmp(method, "none" ) && ....
(In reply to comment #23) > This patch log for "none" method when login attempt is made by a invalid user. > But works correctly for existing valid user (doesn't log for "none" method ) That happens earlier than the auth loop (in getpwnamallow). I'm not sure I want to change that right now as it's the only place guaranteed to be called for an invalid user if they try only, eg pubkey authentication then disconnect (because that occurs purely in the unprivileged child). We may be able to address that with the AUDIT_EVENTS hooks but those aren't enabled by default.
Created attachment 862 [details] Patch for logging Bad key based Authentications The attached patch is an enhancement over OpenSSH4.0p1 to support logging of BAD login attempts for Keybased Authentications.