Bug 1970566 - sshd_config.d/50-redhat.conf causes a binding between system policies and other options
Summary: sshd_config.d/50-redhat.conf causes a binding between system policies and ot...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: openssh
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Dmitry Belyavskiy
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-06-10 17:37 UTC by Paul Wouters
Modified: 2023-08-03 11:05 UTC (History)
8 users (show)

Fixed In Version: openssh-9.3p1-8.fc39
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-08-03 11:05:59 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Paul Wouters 2021-06-10 17:37:36 UTC
If you want to use non-default options in sshd you can drop a file into /etc/ssh/sshd_config/

sshd_config parser works in a way that the first time a keyword is found that value is used. If the keyword is specified again, it is ignored.

I want to disable X11Forwarding yes but I want to keep using the system wide crypto policies.

That means I both have to include my new file before 50.redhat.conf (to be first with X11Forwarding) and last (to ensure I don't override system wide crypto policies included in 50.redhat.conf.

Suggested fix:

split 50.redhat.conf in two. One is called 00-redhat-systemwide-crypto.conf and it only contains:

# This system is following system-wide crypto policy. The changes to
# crypto properties (Ciphers, MACs, ...) will not have any effect in
# this or following included files. To override some configuration option,
# write it before this block or include it before this file.
# Please, see manual pages for update-crypto-policies(8) and sshd_config(5).
Include /etc/crypto-policies/back-ends/opensshserver.config

and one 50-redhat.conf that contains the rest, such as X11Forwarding

The AcceptEnv is allowed to be specified multiple times, but I'm not sure if those multiple entries can be split by other keywords.


The end result is that sshd_config.conf does not have to be rewritten completely to resolve the above problem, and that updated dedaults are automatically used by not creating .rpmnew files.

Comment 1 Paul Wouters 2021-06-10 18:19:07 UTC
note it is also unclear to me what effect /etc/sysconfig/sshd still has.

It is pulled into the environment for the keygen service and sshd service, but the keygen service does not specify any environment variables on the command lines it has, so I think it does nothing there. The sshd service uses $OPTIONS which could come in via /etc/sysconfig/sshd, but should really be done using /etc/ssh/sshd_config.d/ and perhaps should contain a legacy notice/warning ?

The man page of sshd does not tell me that it takes anything from environment variables, so I think $OPTIONS is the only variable that is read from /etc/sysconfig/sshd.

Maybe this should be a separate rhbz ?

Comment 2 Jakub Jelen 2021-06-10 18:55:48 UTC
Hi Paul.

the crypto policies do not affect X11 forwarding so it really does not matter if you put it after or before that. I think we initially wanted to make it simple, which the explosion of configuration files certainly is not. We consider also the crypto policies our configuration changes and overriding it is a valid use case (therefore the 50-name), but in future should be handled by crypto policies too.

The acceptEnv can be placed anywhere to take effect.

The /etc/sysconfig/sshd now has the only purpose of possibility to set OPTIONS for the sshd.service (which is not very useful, I agree). All other needs (overriding crypto policy in RHEL8, possibility to set strong RNG seeding through environment some time ago, enabling root login in some older Fedora, affecting what hostkeys are generated in RHEL6) are gone now. So for next RHEL and Fedoras, it might make sense to dropping it altogether (but I would leave it up to Dima). Now it has just couple of comments pointing to the some right places.

Comment 3 Paul Wouters 2021-06-10 22:13:14 UTC
The crypto policies don't affect X11 forwarding, but by being in the same file causes an artificial binding of the two.

So if others drop in a file with their values, and they want to ensure never to accidentally override the crypto wide policies, it should go _after_ 50-redhat.conf.
But that does not work for disabling X11Forwarding because 50-redhat.conf will have already set it and sshd will ignore the the keywords it has already read before.

So now people customizing it will need to install two files, one before and one after 50-redhat.conf

That is why I'm suggesting to keep crypto-policies by itself in its own file without mixing it with any other options such as X11Forwarding


As for acceptEnv, I don't know what its relation is with respect to multiple entries. The sshd_config man page says:

      For each keyword, the first obtained value will be used.

Clearly, that is not true for acceptEnv which can be spread over multiple lines. But what happens when you have:

      acceptEnv foo bar
      port 23
      acceptEnv xxx

Will it still "add" these? Because this is what would happen if these are specified in multiple includes, and 50-redhat.conf contains one. In my case I want to add another env variable to pass, but I don't know if 50-redhat.conf would prevent that if i use 100-local.conf. Also, I wouldn't know if using 10-local.conf would cause the 50-redhat.conf ones to no longer happen.

Comment 4 Jakub Jelen 2021-06-11 07:34:45 UTC
From manual page for sshd_config:

> AcceptEnv
>         [...] Multiple environment variables may be separated by whitespace or spread across multiple AcceptEnv directives.

Includes are parsed the same way as single file. The only difference is that they are affected by the preceding Match blocks:

> Include
>         [...] An Include directive may appear inside a Match block to perform conditional inclusion.

Comment 5 Alexander Sosedkin 2021-06-11 11:53:03 UTC
> That means I both have to include my new file before 50.redhat.conf (to be first with X11Forwarding) and last (to ensure I don't override system wide crypto policies included in 50.redhat.conf.

Sorry, but I find it hard to perceive that as a problem.
If you don't want to override crypto-policies, don't specify the options it uses.
If you want an extra layer of protection from that, or future-proofing from option addition,
split your modifications and make the conflicting ones lower-priority.
In fact, shipping a 00-redhat-systemwide-crypto.conf looks like a loss of flexibility to me.
Is there some cross-distro portability concern at play that makes a single 50-redhat.conf cumbersome/problematic that I fail to see?

Comment 6 Paul Wouters 2021-06-11 19:02:31 UTC
Sorry, I think I was not entirely clear.

The file 50-redhat.conf multiple things:

1) the system-crypto-policies include
2) The option controlling PAM authentication
3) The option controlling X11 forwarding
4) The option controlling GSSAPI
5) the option controlling ChallengeResponseAuthentication
6) the option controlling PrintMotd
7) The option controlling AcceptEnv

I am just asking to split 1) into its own file. So that people adding custom files, can do so that it goes AFTER the system wide crypto policies. That means anyone editing their own file cannot accidentally override the system wide crypto policies. Let's call this 10-system-crypto-policies.conf

Options 2) to 6) can stay in 50-redhat.conf.

Now when I add 20-mystuff.conf, I am ensured not to accidentally mess up the system wide crypto policies for ssh. And I can still override everything I don't like from 50-redhat.conf, such as X11Forwarding.

Now when 6 months later a colleague modifies our 20-mystuff.conf, not understanding anything about system wide crypto policies, they will also not break it by accident.

Currently, my only option is to specify 20-mystuff.conf because I must be sources before 50-redhat.conf otherwuse my setting of "X11Forwarding no" will be ignored because 50-redhat.conf already set it. And now 6 months
later my collegue might add to "our" file "Ciphers chacha20-poly1305" and suddenly the system wide crypto-policies are fully ignored, and if the machine is or will be placed in FIPS mode, sshd completely breaks policy (and probably locks out everyone as chacha will be blocked in openssl and there is no other cipher enabled)

As for whether system-crypto-policies.conf should be 00 or 10, I think from a design point of view, it makes more sense as 00, because otherwise you might break system wide policies without realising this. If you believe some people know how to override it properly with full understanding and they should not need to have to create 0000000-override.conf, then whatever, call it 20- instead of 00-. I think that would be wrong, but at least I wouldn't be affected by it either way.

As for 7), as I said I am little confused about AcceptEnv. Other keywords can only appear once, and subsequent mentions are ignored. But AcceptEnv is cumulative. But that means if I want to override the 50-redhat.conf values, my only option would seem to be to edit that file. Because I can only "add" to it with my own files. But, it _might_ be that case that sshd would (undocumented) treat this somewhat similar as real keyword processing. eg:

# image no other file mentions AcceptEnv
AcceptEnv foo
AcceptEnv bar
Other option
AcceptEnv xxx


It could be that sshd sees the first two, separated by a new keyword as a set. And from now on it won't look at any further AcceptEnv values. Or it could have no such concept, and just add "xxx". I just don't know what it does or what to expect. In my case I have an "xxx" I need to have, but I don't know what happens if I specify this in 20-mystuff.conf of in 100-mystuff.conf with respect to 50-redhat.conf using it.

Hopefully this makes my issues more clear.

Comment 7 Tomáš Mráz 2021-06-14 07:26:21 UTC
(In reply to Paul Wouters from comment #6)
> As for whether system-crypto-policies.conf should be 00 or 10, I think from
> a design point of view, it makes more sense as 00, because otherwise you
> might break system wide policies without realising this. If you believe some
> people know how to override it properly with full understanding and they
> should not need to have to create 0000000-override.conf, then whatever, call
> it 20- instead of 00-. I think that would be wrong, but at least I wouldn't
> be affected by it either way.

In general I would agree with Paul that the current setup is little bit too prone to have the system-wide crypto policy accidentally ignored. But I do not think we should try to outsmart the admins and make that crypto-policy config to be 00-. Whether it is 10- or 20- is really a bikeshedding kind of decision and I do not think it matters much.

There is one thing to consider though. If I have been already intentionally overriding the policy with something like 10-my-policy.conf or 20-my-policy.conf this would make the override to be ineffective. For that reason this change should be done only in a new release.

Comment 8 Paul Wouters 2021-06-14 17:03:37 UTC
Sure, do it only in rawhide.

Alternatively, split it from 50-redhat.conf into 50-system-crypto-policies.conf and 50-redhat-misc.conf as it would be unlikely people would re-use the 50- prefix if literally any other numbered prefix is still available.

Comment 9 Dmitry Belyavskiy 2021-06-15 12:49:22 UTC
The approach from https://bugzilla.redhat.com/show_bug.cgi?id=1970566#c8 looks reasonable to me

Comment 10 Tomáš Mráz 2021-06-15 14:33:54 UTC
I am confused. How would split it from 50-redhat.conf into 50-system-crypto-policies.conf and 50-redhat-misc.conf resolve the problem of not wanting to override crypto policies but wanting to override the redhat-misc stuff. I suppose I would want to preferably use a local file with something in between those config files but there is no space between 50 and 50.

So what does split to two files with 50- as prefix solves?

Comment 11 Paul Wouters 2021-06-17 22:29:35 UTC
You can use 49 and 51, but your argument was that it might break people who already do an include file that is on purpose before/after 50-redhat.conf.

Regardless, if we only fix this in rawhide, that's good enough for me.

Comment 12 Dmitry Belyavskiy 2021-06-18 13:52:40 UTC
Yes, having both on level 50 doesn't solve any problems, but I understand why these defaults should be in different files and processed independently.

Comment 13 Ben Cotton 2021-08-10 13:07:28 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 35 development cycle.
Changing version to 35.

Comment 14 Dmitry Belyavskiy 2022-02-01 13:44:59 UTC
Still worth doing, moving to rawhide

Comment 15 Ben Cotton 2022-02-08 20:22:33 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 36 development cycle.
Changing version to 36.

Comment 16 Dmitry Belyavskiy 2022-10-21 16:04:44 UTC
I kindly ask everybody who is interested in this bug to take a look at the PR

https://src.fedoraproject.org/rpms/openssh/pull-request/36

Comment 17 Alexander Sosedkin 2022-10-24 09:00:22 UTC
I have a hard time figuring out how does this address the original request and, if it doesn't, what the benefits of this change are.

Comment 18 Dmitry Belyavskiy 2022-10-24 09:05:23 UTC
See https://bugzilla.redhat.com/show_bug.cgi?id=1970566#c7 and #c8

I'd say that our crypto-policies are not intended to be overwritten (at leas by default). The other our settings are unrelated to algorithms choice and are more about system-specific configuration.

Comment 19 Alexander Sosedkin 2022-10-24 09:19:38 UTC
So it's for

1. the ease of overwriting all other settings in entirety, save for crypto-policies
2. being able to insert something between crypto-policies and other configuration,
   overwriting other configuration by preceeding but not anything set by crypto-policies?

Why is the split only done for the server, but not for the client?

Comment 20 Dmitry Belyavskiy 2022-10-24 09:27:17 UTC
For client we have much more per-host configuration capabilities.

Comment 21 Ben Cotton 2023-04-25 16:42:50 UTC
This message is a reminder that Fedora Linux 36 is nearing its end of life.
Fedora will stop maintaining and issuing updates for Fedora Linux 36 on 2023-05-16.
It is Fedora's policy to close all bug reports from releases that are no longer
maintained. At that time this bug will be closed as EOL if it remains open with a
'version' of '36'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, change the 'version' 
to a later Fedora Linux version. Note that the version field may be hidden.
Click the "Show advanced fields" button if you do not see it.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora Linux 36 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora Linux, you are encouraged to change the 'version' to a later version
prior to this bug being closed.

Comment 22 Ludek Smid 2023-05-25 15:51:33 UTC
Fedora Linux 36 entered end-of-life (EOL) status on 2023-05-16.

Fedora Linux 36 is no longer maintained, which means that it
will not receive any further security or bug fix updates. As a result we
are closing this bug.

If you can reproduce this bug against a currently maintained version of Fedora Linux
please feel free to reopen this bug against that version. Note that the version
field may be hidden. Click the "Show advanced fields" button if you do not see
the version field.

If you are unable to reopen this bug, please file a new report against an
active release.

Thank you for reporting this bug and we are sorry it could not be fixed.

Comment 23 Dmitry Belyavskiy 2023-08-03 08:16:31 UTC
Still worth fixing

Comment 24 Fedora Update System 2023-08-03 09:24:21 UTC
FEDORA-2023-64f8335634 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-64f8335634

Comment 25 Fedora Update System 2023-08-03 11:05:59 UTC
FEDORA-2023-64f8335634 has been pushed to the Fedora 39 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.