Bug 3126 - Mark the RDomain configuration option unsupported on non-openbsd builds
Summary: Mark the RDomain configuration option unsupported on non-openbsd builds
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 8.2p1
Hardware: Other Linux
: P5 enhancement
Assignee: Assigned to nobody
URL:
Keywords: patch
Depends on:
Blocks: V_8_3
  Show dependency treegraph
 
Reported: 2020-02-27 20:11 AEDT by Jakub Jelen
Modified: 2021-10-14 01:41 AEDT (History)
3 users (show)

See Also:


Attachments
Mark the RDomain configuration option unsupported on non-openbsd builds (1.28 KB, patch)
2020-02-27 20:11 AEDT, Jakub Jelen
no flags Details | Diff
Mark the RDomain configuration option unsupported on non-openbsd builds v2 (1.72 KB, patch)
2020-03-24 21:36 AEDT, Jakub Jelen
no flags Details | Diff
fatal out if config has unsupported rdomain keyword (1.17 KB, patch)
2020-04-24 14:34 AEST, Darren Tucker
djm: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelen 2020-02-27 20:11:27 AEDT
Created attachment 3358 [details]
Mark the RDomain configuration option unsupported on non-openbsd builds

Experimenting with RDomain configuration option on non-OpenBSD platform prevents sshd from accepting connections. The release notes properly described this option as supported only on OpenBSD, but this was not propagated to manual page and user is left without warning until the server does not accept any new connections with this option.

I believe the option should be marked as unsupported to avoid these surprises and documentation should be adjusted accordingly.
Comment 1 Darren Tucker 2020-02-27 23:05:56 AEDT
(In reply to Jakub Jelen from comment #0)
> Created attachment 3358 [details]
> Mark the RDomain configuration option unsupported on non-openbsd
> builds

I don't think that patch is correct.  sshd should fail with a warning on platforms that don't have an rdomain equivalent (in sshd.c:set_process_rdomain).  In the case of Linux, it's a little convoluted but it should end up using the code in openbsd-compat/port-net.c.

> Experimenting with RDomain configuration option on non-OpenBSD
> platform prevents sshd from accepting connections.

If the Linux rdomain code doesn't work as expected that's a separate issue.

> The release notes
> properly described this option as supported only on OpenBSD,

That was true when rdomain was added, but it's since added Linux support.
Comment 2 Jakub Jelen 2020-02-28 00:47:03 AEDT
(In reply to Darren Tucker from comment #1)
> (In reply to Jakub Jelen from comment #0)
> > Created attachment 3358 [details]
> > Mark the RDomain configuration option unsupported on non-openbsd
> > builds
> 
> I don't think that patch is correct.  sshd should fail with a
> warning on platforms that don't have an rdomain equivalent (in
> sshd.c:set_process_rdomain).  In the case of Linux, it's a little
> convoluted but it should end up using the code in
> openbsd-compat/port-net.c.

Yes, that code contains only

> fatal("%s: not supported", __func__);

> > Experimenting with RDomain configuration option on non-OpenBSD
> > platform prevents sshd from accepting connections.
> 
> If the Linux rdomain code doesn't work as expected that's a separate
> issue.

There is no linux code for this configuration option.

> > The release notes
> > properly described this option as supported only on OpenBSD,
> 
> That was true when rdomain was added, but it's since added Linux
> support.

According to the code, this is still the case. See above. I did not investigate whether there is already a way to change rdomain for process in Linux, but having an option which is without warning in manual page breaking all connections is dangerous.
Comment 3 Darren Tucker 2020-02-29 11:28:08 AEDT
(In reply to Jakub Jelen from comment #2)
> (In reply to Darren Tucker from comment #1)
[...]
> > If the Linux rdomain code doesn't work as expected that's a
> > separate issue.
> 
> There is no linux code for this configuration option.

https://github.com/openssh/openssh-portable/blob/master/openbsd-compat/port-net.c#L48

> I did not
> investigate whether there is already a way to change rdomain for
> process in Linux, but having an option which is without warning in
> manual page breaking all connections is dangerous.

The problem looks to be specific to the Linux code.  On any platform other than OpenBSD or Linux (this is FreeBSD 10) I get this error at startup when attempting to set an rdomain:

$ sudo `pwd`/sshd -ddd -o rdomain=1
Routing domains are not supported on this platform
command-line line 0: bad routing domain
Comment 4 Jakub Jelen 2020-03-02 20:19:35 AEDT
(In reply to Darren Tucker from comment #3)
> (In reply to Jakub Jelen from comment #2)
> > (In reply to Darren Tucker from comment #1)
> [...]
> > > If the Linux rdomain code doesn't work as expected that's a
> > > separate issue.
> > 
> > There is no linux code for this configuration option.
> 
> https://github.com/openssh/openssh-portable/blob/master/openbsd-
> compat/port-net.c#L48

But this is for set/get_rdomain() on socket. The above option is about setting the context for process, which is not supported in Linux:

https://github.com/openssh/openssh-portable/blob/master/openbsd-compat/port-net.c#L119

> > I did not
> > investigate whether there is already a way to change rdomain for
> > process in Linux, but having an option which is without warning in
> > manual page breaking all connections is dangerous.
> 
> The problem looks to be specific to the Linux code.  On any platform
> other than OpenBSD or Linux (this is FreeBSD 10) I get this error at
> startup when attempting to set an rdomain:
> 
> $ sudo `pwd`/sshd -ddd -o rdomain=1
> Routing domains are not supported on this platform
> command-line line 0: bad routing domain

Right. Thank you for checking. I missed this part of the code which is shared between listenaddress and rdomain options. But the patch as it is above, still address the original issue. If you think that the option should fail and not report as unsupported, I can rewrite the patch to do that.
Comment 5 Jakub Jelen 2020-03-23 18:26:18 AEDT
One more clarification. The reproducer is just setting this in the sshd_config:

RDomain %D

With this configuration, restart and configuration parsing passes fine, but fails to accept connections:

sshd[4831]: Server listening on :: port 22.
systemd[1]: Started OpenSSH server daemon.
sshd[4833]: fatal: Unable to set routing domain: not supported in this platform

Indeed, setting up anything else fails already during the configuration parsing, so marking the configuration option as unsupported on non-OpenBSD builds would make sense from here.
Comment 6 Darren Tucker 2020-03-23 18:54:24 AEDT
ok thanks.  putting on list for next release.
Comment 7 Jakub Jelen 2020-03-24 21:36:44 AEDT
Created attachment 3370 [details]
Mark the RDomain configuration option unsupported on non-openbsd builds v2

Second version -- we need to prevent dumping the configuration option too as it is would create invalid configuration files and testsuite would fail on Linux.
Comment 8 Darren Tucker 2020-04-24 14:34:26 AEST
Created attachment 3386 [details]
fatal out if config has unsupported rdomain keyword

Revised patch which causes it to fatal out at parse time if rdomain is unsupported instead of just logging a warning.  Rationale: the user might think they have applied a restriction but actually have not.

It uses HAVE_SYS_SET_PROCESS_RDOMAIN in addition to __OpenBSD__ since that would be set if a platform did support it (although no platform currently does, the infrastructure is there).

It also removes the entry in the man page.  Rationale: there are many configuration options that depend on platform and/or build time support  and including all of them in the man page would be a) unwieldy and b) a maintenance burden since it would make it harder to keep in sync with upstream.
Comment 9 Darren Tucker 2020-04-24 15:12:34 AEST
Patch applied, please let us know if you have any additional feedback or problems with it.

Thanks.
Comment 10 Damien Miller 2021-04-23 14:53:10 AEST
closing resolved bugs as of 8.6p1 release
Comment 11 Ahmed Sayeed 2021-10-14 01:41:12 AEDT
[spam removed]