Bug 3129 - Add IP address to error kex_exchange_identification error message
Summary: Add IP address to error kex_exchange_identification error message
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 8.1p1
Hardware: Other Linux
: P5 enhancement
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_8_3
  Show dependency treegraph
 
Reported: 2020-03-03 22:48 AEDT by Ulrich Windl
Modified: 2021-10-14 01:40 AEDT (History)
3 users (show)

See Also:


Attachments
Add peer info to kex_exchange_identification error messages (2.99 KB, patch)
2020-03-04 00:38 AEDT, Darren Tucker
no flags Details | Diff
use sshpkt_fatal() for kex_exchange_identification() errors (3.60 KB, patch)
2020-03-13 14:07 AEDT, Damien Miller
dtucker: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ulrich Windl 2020-03-03 22:48:25 AEDT
On one server I periodically see this error message in syslog:
sshd[...]: error: kex_exchange_identification: Connection closed by remote host

Nothing more. That doesn't make it easy to find out who is causing this.
Therefore I suggest to add the peer's IP address to this or to a related syslog message.
Comment 1 Darren Tucker 2020-03-04 00:38:27 AEDT
Created attachment 3359 [details]
Add peer info to kex_exchange_identification error messages

Please try this patch.
Comment 2 Ulrich Windl 2020-03-04 18:34:11 AEDT
(In reply to Darren Tucker from comment #1)

The patch looks OK for me, but I cannot really test it as the machine where I see it is some appliance where I can't replace code, and I'm not able to trigger this error on any machines where I could replace sshd.
The only thing in the patch that makes me (as a no-"ssh developer") wonder is the size reserved for the identification: 512
It should be large enough, but maybe even it's too large. Isn't there a symbolic constant for that?

Other things I'm thinking of is the consistency of messages containing the peer:
"from server %s" (at end)
"with peer %s" (at end)
"remote host %s" (at end)
"%s: peer %s" (at beginning)
"from peer %s" (in middle)
"with peer %s" (in middle)
Comment 3 Damien Miller 2020-03-05 20:09:51 AEDT
Maybe we should instead downgrade some of the error messages in kex_exchange_identification() to debug severity and have the caller call sshpkt_fatal() as that logs the connection details in a semi-standard format
Comment 4 Damien Miller 2020-03-13 14:07:54 AEDT
Created attachment 3365 [details]
use sshpkt_fatal() for kex_exchange_identification() errors

This uses sshpkt_fatal() to record errors from kex_exchange_identification(). This should make it consistent with most other exit messages. 

Examples:

[djm@djm openssh]$ ./ssh -p 2222 127.0.0.1
Protocol major versions differ: 2 vs. 1
banner exchange: Connection to 127.0.0.1 port 2222: could not read protocol version
[djm@djm openssh]$ ./ssh -p 2222 127.0.0.1
kex_exchange_identification: Connection closed by remote host
Connection closed by 127.0.0.1 port 2222

There's arguably a little duplication between the error messages in some cases, but I think that's acceptable.

Note that this required some renovation of kex_exchange_identification() to preserve errno for SSH_ERR_SYSTEM_ERROR cases. That's the bulk of the diff.
Comment 5 Damien Miller 2020-03-13 15:02:07 AEDT
Patch applied and will be in openssh-8.2
Comment 6 Damien Miller 2021-04-23 15:10:42 AEST
closing resolved bugs as of 8.6p1 release
Comment 7 Damien Miller 2021-04-23 15:10:43 AEST
closing resolved bugs as of 8.6p1 release
Comment 8 Ahmed Sayeed 2021-10-14 01:40:55 AEDT
[spam removed]