openssh bugs - Mar 2020 - [Bug 3129] New: Add IP address to error kex_exchange_identification

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

            Bug ID: 3129
           Summary: Add IP address to error kex_exchange_identification
           Product: Portable OpenSSH
           Version: 8.1p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: Ulrich.Windl at rz.uni-regensburg.de

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.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Ulrich Windl <Ulrich.Windl at rz.uni-regensburg.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Add IP address to error     |Add IP address to error
                   |kex_exchange_identification |kex_exchange_identification
                   |                            |error message

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net

--- Comment #1 from Darren Tucker <dtucker at dtucker.net> ---
Created attachment 3359
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3359&action=edit
Add peer info to kex_exchange_identification error messages

Please try this patch.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |3117


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3117
[Bug 3117] Tracking bug for 8.3 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

--- Comment #2 from Ulrich Windl <Ulrich.Windl at rz.uni-regensburg.de>
---
(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)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
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

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3365|                            |ok?(dtucker at dtucker.net)
              Flags|                            |

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
Created attachment 3365
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3365&action=edit
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 at 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 at 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.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3365|ok?(dtucker at dtucker.net)    |ok+
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
Patch applied and will be in openssh-8.2

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
closing resolved bugs as of 8.6p1 release

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

--- Comment #7 from Damien Miller <djm at mindrot.org> ---
closing resolved bugs as of 8.6p1 release

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3129

Ahmed Sayeed <ahmedsayeed1982 at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ahmedsayeed1982 at yahoo.com

--- Comment #8 from Ahmed Sayeed <ahmedsayeed1982 at yahoo.com> ---
$ ./gdb -nx -q --data-directory=data-directory ~/a.out -ex "set confirm
off" -ex "file -readnow ~/a.out"
http://www-look-4.com/category/technology/
        Reading symbols from /home/simark/a.out...
        Reading symbols from ~/a.out...
        /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8098:
https://komiya-dental.com/shopping/safety-for-online-shopping/
internal-error: void create_all_comp_units(dwarf2_per_objfile*):
Assertion `per_objfile->per_bfd->all_comp_units.empty ()' failed.
    http://www.iu-bloomington.com/health/health-and-cannabis/
    This is a recurring problem that exposes a design issue in the
DWARF
    per-BFD sharing feature.  Things work well when loading a binary
with https://waytowhatsnext.com/crypto/bought-house-with-bitcoin/ 
    the same method (with/without index, with/without readnow) twice in
a
    row.  But they don't work so well when loading a binary with
different
    methods.  See this previous fix, for example:
    http://www.wearelondonmade.com/category/health/
        efb763a5ea35 ("gdb: check for partial symtab presence in
dwarf2_initialize_objfile")

    That one handled the case where the first load is normal (uses
partial
    symbols) and the second load uses an index.
http://www.jopspeech.com/category/services/

    The problem is that when loading an objfile with a method A, we
create a
    dwarf2_per_bfd and some dwarf2_per_cu_data and initialize them with
the
    data belonging to that method. http://joerg.li/category/health/ 
When loading another obfile sharing the
    same BFD but with a different method B, it's not clear how to
re-use the
    dwarf2_per_bfd/dwarf2_per_cu_data previously created, because they
    contain the data specific to method A
http://connstr.net/category/health/
$ ./gdb -nx -q --data-directory=data-directory ~/a.out -ex "set confirm
off" -ex "file -readnow ~/a.out"
http://embermanchester.uk/category/health/
        Reading symbols from /home/simark/a.out...
        Reading symbols from ~/a.out...
        /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8098:
internal-error: void create_all_comp_units(dwarf2_per_objfile*):
Assertion http://www.slipstone.co.uk/category/health/ 
`per_objfile->per_bfd->all_comp_units.empty ()' failed.

    This is a recurring problem that exposes a design issue in the
DWARF http://www.logoarts.co.uk/category/services/ 
    per-BFD sharing feature.  Things work well when loading a binary
with
    the same method (with/without index, with/without readnow) twice in
a
    row.  But they don't work so well when
http://www.acpirateradio.co.uk/category/tech/ loading a binary with
different
    methods.  See this previous fix, for example:

        efb763a5ea35 ("gdb: check for partial symtab presence in
dwarf2_initialize_objfile")
     http://www.compilatori.com/category/health/
    That one handled the case where the first load is normal (uses
partial
    symbols) and the second load uses an index.

    The problem is that when loading an objfile with a method A, we
create a
    dwarf2_per_bfd and some dwarf2_per_cu_data and initialize them with
the
    data belonging to that method.  When loading another obfile sharing
the
    same BFD but with a different method B, it's not clear how to
re-use the
    dwarf2_per_bfd/dwarf2_per_cu_data previously created, because they
    contain the data specific to method A
https://www.webb-dev.co.uk/health/health-and-cannabis/
$ ./gdb -nx -q --data-directory=data-directory ~/a.out -ex "set confirm
off" -ex "file -readnow ~/a.out"
        Reading symbols from /home/simark/a.out...
        Reading symbols from ~/a.out...
        /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8098:
internal-error: void create_all_comp_units(dwarf2_per_objfile*):
Assertion `per_objfile->per_bfd->all_comp_units.empty ()' failed.

    This is a recurring problem that exposes a design issue in the
DWARF
    per-BFD sharing feature.  Things work well when loading a binary
with
    the same method (with/without index, with/without readnow) twice in
a
    row.  But they don't work so well when loading a binary with
different
    methods.  See this previous fix, for example:

        efb763a5ea35 ("gdb: check for partial symtab presence in
dwarf2_initialize_objfile"

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.