[Bug 2468] New: Option to include external files to sshd_config

[b***@bugzilla.mindrot.org]

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

Bug ID: 2468
Summary: Option to include external files to sshd_config
Product: Portable OpenSSH
Version: 7.1p1
Hardware: Other
OS: Linux
Status: NEW
Severity: enhancement
Priority: P5
Component: sshd
Assignee: unassigned-***@mindrot.org
Reporter: ***@redhat.com
Depends on: 2463

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

This topic was discussed in few bugs (namely 1613, 2146, 1585) for
client side and the last one also contains patch that can be applied to
the current openssh. Having the same feature for server side makes the
same sense for me so I created patch also for server side.

To understand the main reason behind this feature, it is our
"system-wide crypto policy", which should allow us to enforce specific
security policies in uniform way over the most crypto tools in whole
system.

This feature will also allow us to have default drop-in directory,
which will also improve the packaging possibilities for third party
tools and will make possible to update main config without conflict
with changes made by users.

All tests still passing. If you would like from me to implement also
test cases to cover this feature, let me know. I see there are tests
for most of the recent features.

Please note, that the prerequisite for this feature is solving the bug
#2463 (openbsd compat glob), which makes sshd segfault with this patch
in kerberos library.


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2463
[Bug 2463] Conflict with openbsd compat glob() function in shared
libraries

[b***@bugzilla.mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=2468
Bug 2468 depends on bug 2463, which changed state.

Bug 2463 Summary: Conflict with openbsd compat glob() function in shared libraries
https://bugzilla.mindrot.org/show_bug.cgi?id=2463

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

[b***@bugzilla.mindrot.org]

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

Damien Miller <***@mindrot.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@mindrot.org

--- Comment #1 from Damien Miller <***@mindrot.org> ---
IMO the desirable semantics of Include in sshd_config are even more
subtle and fraught than those of ssh_config. For example, how should
the following behave:

Port 22
Include /etc/ssh/config_a
PasswordAuthentication no
Include /etc/ssh/config_b

Match user djm
Banner /etc/banner
Include /etc/ssh/config_c

---

If inclusion operates just by pasting text in, then config_a could
radically alter the following configuration if it includes a Match
directive.

Similarly, config_c's application conditional on the previous Match
succeeding?

I wish we had a brace-ful configuration language - it would make
resolving these way simpler :/

[b***@bugzilla.mindrot.org]

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

Zdenek Sedlak <***@apgrco.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@apgrco.com

--- Comment #2 from Zdenek Sedlak <***@apgrco.com> ---
I would propose to simply pasting the lines from the config file at the
position where the Include option is placed in the sshd_config, and
furthermore, it would be nice to have an Includedir option like the
sudo has.

[b***@bugzilla.mindrot.org]

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

Jakub Jelen <***@redhat.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #2706|0 |1
is obsolete| |

--- Comment #3 from Jakub Jelen <***@redhat.com> ---
Created attachment 2869
--> https://bugzilla.mindrot.org/attachment.cgi?id=2869&action=edit
Include server side (with same semantics as client)

(In reply to Zdenek Sedlak from comment #2)

Post by b***@bugzilla.mindrot.org
I would propose to simply pasting the lines from the config file at
the position where the Include option is placed in the sshd_config

That was the initial idea and, if I am right. It is the way how it was
initially implemented. But the client side config was implemented in
more complex manner, that even the Include depends on the Match context
so implementing server side in different way does not seem like a good
idea.

Post by b***@bugzilla.mindrot.org
it would be nice to have an Includedir option like
the sudo has.

Include with GLOB expansion does the same without additional complexity
of another option.

Reading the implementation of the client side config include, there
should be certainly some limit to the recursion and some regression
sanity test to make sure it does what it is supposed to do. Introducing
some more complexity would make it much harder to understand what is
going on there (though the debug log level is very helpful here).

(In reply to Damien Miller from comment #1)

Post by b***@bugzilla.mindrot.org
If inclusion operates just by pasting text in, then config_a could
radically alter the following configuration if it includes a Match
directive.
Similarly, config_c's application conditional on the previous Match
succeeding?

The other possibility would be to reset the context for each include
file, but that looks even more confusing to me.

Post by b***@bugzilla.mindrot.org
I wish we had a brace-ful configuration language - it would make
resolving these way simpler :/

That would be nice-to-have, but probably impossible to change now.

Attaching a new patch with a regression tests, providing the same
behavior as the client side config include.

Also added a check to make sure that the Include list is not empty
(missing in the client).

[b***@bugzilla.mindrot.org]

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

Paul Wise <***@bonedaddy.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@bonedaddy.net

--- Comment #4 from Paul Wise <***@bonedaddy.net> ---
If this is implemented, a default/implicit include of
/etc/sshd_config.d/* would be much appreciated.

[b***@bugzilla.mindrot.org]

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

--- Comment #5 from Jakub Jelen <***@redhat.com> ---
(In reply to Paul Wise from comment #4)

Post by b***@bugzilla.mindrot.org
If this is implemented, a default/implicit include of
/etc/sshd_config.d/* would be much appreciated.

Yes, we have this already in fedora in client configuration files. The
server should go with the same approach. Incorporating this uniformly
into the default configuration file would be nice.

[b***@bugzilla.mindrot.org]

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

SpiderX <***@spiderx.dp.ua> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@spiderx.dp.ua

[b***@bugzilla.mindrot.org]

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

***@bendem.be changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@gmail.com

--- Comment #6 from ***@bendem.be ---
*** Bug 2351 has been marked as a duplicate of this bug. ***

[b***@bugzilla.mindrot.org]

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

***@bendem.be changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@bendem.be