Bug 2653

Summary:

Including files without read access in ssh configuration fails without error

Product:

Portable OpenSSH

Reporter:

Jakub Jelen <jjelen>

Component:

ssh

Assignee:

Damien Miller <djm>

Status:

CLOSED FIXED

Severity:

enhancement

CC:

djm, dtucker

Priority:

Version:

7.3p1

Hardware:

Other

OS:

Linux

Bug Depends on:

Bug Blocks:

2647

Attachments:

Description	Flags
proposed patch	none
fatal() on Include errors other than ENOENT	dtucker: ok+

Description Jakub Jelen 2016-12-27 09:41:30 AEDT

Created attachment 2920 [details]
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

Comment 1 Damien Miller 2017-01-06 14:08:07 AEDT

Created attachment 2928 [details]
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.

Comment 2 Damien Miller 2017-01-06 14:54:09 AEDT

appled - thanks

Comment 3 Jakub Jelen 2017-01-06 23:06:11 AEDT

This does not look right. Trying with the attached patch I am experiencing weird behavior:

[root@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.

Comment 4 Damien Miller 2017-03-12 10:57:37 AEDT

This has been fixed for a while. The test now looks like: 

> if (r != 1 && errno != ENOENT) {

Comment 5 Damien Miller 2021-04-23 15:02:24 AEST

closing resolved bugs as of 8.6p1 release