Mailing List Archive

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





------- Additional Comments From dtucker@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@zip.com.au changed:

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




------- Additional Comments From dtucker@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@mindrot.org changed:

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




------- Additional Comments From djm@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@zip.com.au changed:

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




------- Additional Comments From dtucker@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@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.