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
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.
appled - thanks
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.
This has been fixed for a while. The test now looks like: > if (r != 1 && errno != ENOENT) {
closing resolved bugs as of 8.6p1 release