Bug 2162

Summary:

Log needs to contain the port on which connection is made

Product:

Portable OpenSSH

Reporter:

Zzu <yuri>

Component:

sshd

Assignee:

Damien Miller <djm>

Status:

CLOSED FIXED

Severity:

normal

CC:

djm, dtucker

Priority:

Version:

6.2p1

Hardware:

All

OS:

FreeBSD

Bug Depends on:

Bug Blocks:

2130

Attachments:

Description	Flags
include port in invalid banner message	none

Description Zzu 2013-10-18 06:06:56 AEDT

I have sshd listening on multiple ports.
Currently I see the log message:
Oct 17 11:59:32 myhost sshd[13784]: Bad protocol version identification '\200F\001\003\001' from NNN.NNN.NNN.NNN

Somebody connected to it and protocol was invalid. The message will become much more informative if it contained the port on which the connection wad made.

This should be fixed for all messages that sshd prints on per-connection basis.

OpenSSH 6.2p2 from FreeBSD tree.

Comment 1 Damien Miller 2013-10-18 09:08:07 AEDT

Created attachment 2355 [details]
include port in invalid banner message

Comment 2 Damien Miller 2013-10-18 09:08:47 AEDT

Patch has been committed; this will be in OpenSSH-6.4 due in 2-3 months time.

Comment 3 Darren Tucker 2013-10-18 13:22:04 AEDT

Comment on attachment 2355 [details]
include port in invalid banner message

>+		logit("Bad protocol version identification '%.100s' "
>+		    "from %s port %d", client_version_string,
>+		    get_remote_ipaddr(), get_remote_port());

that's the remote port, the request was for the local (ie listening) port.  Certainly I think it's worth having the remote port (eg for NAT pools).

Comment 4 Zzu 2013-10-18 22:55:19 AEDT

Also, there are other messages of the same kind (issued per-connection) that should be fixed as well. Ex. "Did not receive identification string from %s"

I suggest you create the variant of logit for this. I realize with the current code just using "va_list args" this isn't trivial to do without the code duplication.

Comment 5 Damien Miller 2013-10-20 16:00:40 AEDT

I don't mind adding full host/port information to a few important messages (authentication events, etc.), but I don't think that every message needs to include the same redundant information. We have a "Connection from ..." message that should include it all.

How about:

> - verbose("Connection from %.500s port %d", remote_ip, remote_port);
> + verbose("Connection from %s port %d on %s port %d",
> +     remote_ip, remote_port,
> +     get_local_ipaddr(sock_in), get_local_port());

?

Comment 6 Damien Miller 2013-10-24 10:37:26 AEDT

I added the local host address and port to the "Connection from ..." message that is shown at loglevel>=verbose.

Comment 7 Damien Miller 2021-04-23 14:57:37 AEST

closing resolved bugs as of 8.6p1 release