openssh bugs - Dec 2016 - [Bug 2653] New: Including files without read access in ssh configuration fails without error

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

            Bug ID: 2653
           Summary: Including files without read access in ssh
                    configuration fails without error
           Product: Portable OpenSSH
           Version: 7.3p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jjelen at redhat.com

Created attachment 2920
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2920&action=edit
proposed patch

When one is using Include directive in ssh_config and the file is not
readable for a user running ssh, it fails without reasonable error
message:

    /etc/ssh/ssh_config: terminating, 1 bad configuration options

It is pretty hard to get, especially when the include works on the
whole drop-in directory, such as:

    Include /etc/ssh/ssh_config.d/*.conf

Only log level DEBUG3 shows some pointer where does it come from.

    debug3: /etc/ssh/ssh_config line 56: Including file
/etc/ssh/ssh_config.d/10-kex.conf depth 0
    /etc/ssh/ssh_config: terminating, 1 bad configuration options

We already ignore failures if the included file does not exist. If it
exists and we don't have permissions to read it, we should either get
better error or ignore it too (as we already do in case of standard
configuration files).

This is not a problem when reading the configuration files directly,
because the return value of the call to read_config_file() is ignored
in ssh.c (honored only in case of -F switch).

Possible solution to resolve this issue, to report read error, but
ignore it from the include files is to introduce new flag (see attached
patch, which fixes the problem for me).

This was originally reported as in Red Hat bugzilla [1].

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1408558

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

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2920|0                           |1
        is obsolete|                            |
             Status|NEW                         |ASSIGNED
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org
                 CC|                            |djm at mindrot.org,
                   |                            |dtucker at zip.com.au
   Attachment #2928|                            |ok?(dtucker at zip.com.au)
              Flags|                            |

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
Created attachment 2928
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2928&action=edit
fatal() on Include errors other than ENOENT

read_config_file_depth() only ever returns failure on fopen() errors,
everything else goes via fatal(), so we can simplify this a bit.

-- 
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=2653

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2647


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2647
[Bug 2647] Tracking bug for OpenSSH 7.5 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=2653

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2928|ok?(dtucker at zip.com.au)     |ok+
              Flags|                            |

-- 
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=2653

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
appled - thanks

-- 
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=2653

Jakub Jelen <jjelen at redhat.com> changed:

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

--- Comment #3 from Jakub Jelen <jjelen at redhat.com> ---
This does not look right. Trying with the attached patch I am
experiencing weird behavior:

[root at f24 openssh]# ssh -vvv localhost
OpenSSH_7.4p1, OpenSSL 1.1.0c-fips  10 Nov 2016
debug1: Reading configuration data /etc/ssh/ssh_config
debug3: /etc/ssh/ssh_config line 56: Including file
/etc/ssh/ssh_config.d/01-test.conf depth 0
debug1: Reading configuration data /etc/ssh/ssh_config.d/01-test.conf
Can't open user config file /etc/ssh/ssh_config.d/01-test.conf: Success

This is happening only when the first included file does not include
any further files that would set errno to ENOENT. If all the parsing
went without any problem, the errno stays zero and the failure as above
prevents the execution.

-- 
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=2653

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
This has been fixed for a while. The test now looks like: 
> if (r != 1 && errno != ENOENT) {
-- 
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=2653

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #5 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.