openssh bugs - Jan 2005 - [Bug 974] Record Badlogins for all supported Authentication methods

http://bugzilla.mindrot.org/show_bug.cgi?id=974


dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
           Platform|HPPA                        |All
            Summary|Enhancement : Record        |Record Badlogins for all
                   |Badlogins for all supported |supported Authentication
                   |Authentication methods      |methods




------- Additional Comments From dtucker at zip.com.au  2005-01-20 16:58 -------
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.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From dtucker at zip.com.au  2005-01-20 17:17 -------
(From update of attachment 774)
>+  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.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-01-20 18:07
-------
> 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"





------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From dtucker at zip.com.au  2005-01-20 18:15 -------
(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.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From djm at mindrot.org  2005-01-20 19:48 -------
Created an attachment (id=776)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=776&action=view)
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.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-01-20 21:09
-------
Created an attachment (id=777)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=777&action=view)
BTMP PATCH 

Optimized BTMP patch attached .



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-01-20 21:12
-------
> 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. 




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974


ashok_kovai at hotmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #774 is|1                           |0
              patch|                            |
 Attachment #774 is|0                           |1
           obsolete|                            |






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-01-21 22:18
-------
Created an attachment (id=780)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=780&action=view)
Optimized BTMP PATCH 




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-01-21 22:20
-------
> 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 . 
 



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974


ashok_kovai at hotmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #777 is|0                           |1
           obsolete|                            |






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974


ashok_kovai at hotmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #780 is|1                           |0
              patch|                            |
 Attachment #780 is|0                           |1
           obsolete|                            |






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-01-21 23:13
-------
Created an attachment (id=781)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=781&action=view)
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 . 



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From djm at mindrot.org  2005-01-21 23:26 -------
(From update of attachment 781)
>--- 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




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From dtucker at zip.com.au  2005-01-21 23:34 -------
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).




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-01-24 17:46
-------
Created an attachment (id=785)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=785&action=view)
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 .




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974


ashok_kovai at hotmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #781 is|0                           |1
           obsolete|                            |






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-01-25 22:50
-------
Created an attachment (id=787)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=787&action=view)
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 .



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974


ashok_kovai at hotmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #785 is|1                           |0
              patch|                            |
 Attachment #785 is|0                           |1
           obsolete|                            |






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From dtucker at zip.com.au  2005-02-02 13:06 -------
(From update of attachment 787)
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.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From dtucker at zip.com.au  2005-02-02 17:29 -------
Created an attachment (id=798)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=798&action=view)
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).



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974


dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #798 is|0                           |1
           obsolete|                            |




------- Additional Comments From dtucker at zip.com.au  2005-02-02 17:48 -------
Created an attachment (id=799)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=799&action=view)
btmp logging: normalise mapped 4in6 addresses too.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974


djm at mindrot.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
    Attachment #799|                            |ok+
               Flag|                            |




------- Additional Comments From djm at mindrot.org  2005-02-02 21:01 -------
(From update of attachment 799)
looks sane to me




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974


dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED




------- Additional Comments From dtucker at zip.com.au  2005-02-02 23:45 -------
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.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-02-03 16:49
-------
> + 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 .



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From dtucker at zip.com.au  2005-02-03 17:14 -------
(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).



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From ashok_kovai at hotmail.com  2005-02-03 20:07
-------
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" ) && ....



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=974





------- Additional Comments From dtucker at zip.com.au  2005-02-04 15:22 -------
(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.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.