Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add root password SSH login override checkbox (#1716282) #2042

Merged
merged 1 commit into from
Jul 23, 2019

Conversation

poncovka
Copy link
Contributor

@poncovka poncovka commented Jul 8, 2019

Open SSH, which provides the SSH server Fedora uses already for some
time disallowed password based root logins. Fedora used to patch this
out so far, but this patch has been recently dropped, restoring the
upstream behavior.

This change does not affect key based SSH logins and it will continue
to be possible to login as root with key based authentication.

To make the transition easier for user who might have valid use cases
for logging in as root with password over SSH we will add a checkbox
to the root password spoke. This checkbox makes it possible to manually
enable password based SSH logins for the root account.

Resolves: rhbz#1716282

@poncovka poncovka added the master Please, use the `f39` label instead. label Jul 8, 2019
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't part of the discussions but I have a bad feeling about making the change by the environment variable for systemd unit. For me personally it would be a pain to find in the installed system how to disable the root logins afterward.

@poncovka
Copy link
Contributor Author

poncovka commented Jul 9, 2019

@jkonecny12, you can easily disable it by removing the config file generated by anaconda. The environment variable is just used to propagate additional options to the service.

@jkonecny12
Copy link
Member

jkonecny12 commented Jul 10, 2019

It's not problem to disable the option but to find what is causing that I'm not able able to log as root.

You would probably look into /etc/ssh/sshd_config or /etc/ssh/ssh_config.d/ but I wouldn't guess that I should look on the drop dir of the service file. I don't think this is an expected way to modify ssh configuration.

The other problem is that you will even miss the environment variable because that will be used only for the unit and not by the system so another way how to find what is happening is hidden.

EDIT: Fix the incorrect statements.

@Jakuje
Copy link
Contributor

Jakuje commented Jul 10, 2019

It's not problem to disable the option but to find what is causing that I'm not able to log as root.

Not able to login as a root using password is default in OpenSSH for quite some time and will be a default in Fedora 31. Manual page for sshd_config describes the semantics of PermitRootLogin configuration option as well as the defaults. For Fedora change, the release notes based on the Fedora Change should address that. If you miss something there, please let me know.

You would probably look into /etc/ssh/ssh_config or /etc/ssh/ssh_config.d/ but I wouldn't guess that I should look on the drop dir of the service file. I don't think this is an expected way to modify ssh configuration.

The files you mention are client configuration files and they do not have anything to do with this change.

systemctl status sshd will show you what command-line options of running sshd daemon. systemct cat sshd will show you what drop-in files are used by sshd.service. I think this should be the first thing to check when working with systemd services.

Reiterating again, we could consider creating normal environment file (lets say /etc/anaconda/permitrootlogin or similar if you already use some other directory under /etc/) to be loaded rather than a systemd drop-in file, which will have the advantage of working also for the socket activation sshd@.service (which was missed so far). This would be very similar how crypto policies are handled now.

In the long term, we would indeed like to use sshd_config drop-in directory, but it is not possible at this moment, because there is no Include functionality (only the client config supports that).

The other problem is that you will even miss the environment variable because that will be used only for the unit and not by the system so another way how to find what is happening is hidden.

What do you mean here by "not used by the system"?

@jkonecny12
Copy link
Member

@Jakuje thanks for your input. My main question was if this change was consulted by someone from SSH maintainers. As I know now it was.

It's not problem to disable the option but to find what is causing that I'm not able to log as root.

Not able to login as a root using password is default in OpenSSH for quite some time and will be a default in Fedora 31. Manual page for sshd_config describes the semantics of PermitRootLogin configuration option as well as the defaults. For Fedora change, the release notes based on the Fedora Change should address that. If you miss something there, please let me know.

My bad, I meant it the other way. When users later in the installed system wants to disable root login again.

I know that the ssh root login is disabled some time, that is not a problem and if there is a release notes then I guess it should be fine for users to make it disabled again.

You would probably look into /etc/ssh/ssh_config or /etc/ssh/ssh_config.d/ but I wouldn't guess that I should look on the drop dir of the service file. I don't think this is an expected way to modify ssh configuration.

The files you mention are client configuration files and they do not have anything to do with this change.

Oh my... of course I mean server configuration sshd_config. So there is no drop dir, I see.

systemctl status sshd will show you what command-line options of running sshd daemon. systemct cat sshd will show you what drop-in files are used by sshd.service. I think this should be the first thing to check when working with systemd services.

For me personally, I would not expect configuration of that kind in the systemd unit. I think this should be part of the system configuration file not the unit. I think of a systemd unit that it should only start the service and set properties which are required to run the service in the given environment (distribution) but not the options how it should behave. However, maybe that is only my PoV.

Reiterating again, we could consider creating normal environment file (lets say /etc/anaconda/permitrootlogin or similar if you already use some other directory under /etc/) to be loaded rather than a systemd drop-in file, which will have the advantage of working also for the socket activation sshd@.service (which was missed so far). This would be very similar how crypto policies are handled now.

This does not solve the problem I'm thinking of. It really makes it even worse because then we have another configuration file taken into account. Also, I'm not that convinced that we want to have anaconda specific configuration files in the system. The configuration should stay specific for the service.

In the other hand if it will solve socket activation we may start talking about that. What do you recommend?

In the long term, we would indeed like to use sshd_config drop-in directory, but it is not possible at this moment, because there is no Include functionality (only the client config supports that).

That would be an ideal solution. I'm looking forward into that feature :)

The other problem is that you will even miss the environment variable because that will be used only for the unit and not by the system so another way how to find what is happening is hidden.

What do you mean here by "not used by the system"?

I meant that you won't see the environment variable when you type env into the terminal but that is really not important.


Sorry for the mistakes in the previous comment. :(

@Jakuje
Copy link
Contributor

Jakuje commented Jul 11, 2019

@Jakuje thanks for your input. My main question was if this change was consulted by someone from SSH maintainers. As I know now it was.

Yes, I am OpenSSH maintainer in Fedora.

It's not problem to disable the option but to find what is causing that I'm not able to log as root.

Not able to login as a root using password is default in OpenSSH for quite some time and will be a default in Fedora 31. Manual page for sshd_config describes the semantics of PermitRootLogin configuration option as well as the defaults. For Fedora change, the release notes based on the Fedora Change should address that. If you miss something there, please let me know.

My bad, I meant it the other way. When users later in the installed system wants to disable root login again.

Yes, that is a trivial configuration change as any other configuration change that the user would do, preferably directly in sshd_config.

I know that the ssh root login is disabled some time, that is not a problem and if there is a release notes then I guess it should be fine for users to make it disabled again.

You would probably look into /etc/ssh/ssh_config or /etc/ssh/ssh_config.d/ but I wouldn't guess that I should look on the drop dir of the service file. I don't think this is an expected way to modify ssh configuration.

The files you mention are client configuration files and they do not have anything to do with this change.

Oh my... of course I mean server configuration sshd_config. So there is no drop dir, I see.

systemctl status sshd will show you what command-line options of running sshd daemon. systemct cat sshd will show you what drop-in files are used by sshd.service. I think this should be the first thing to check when working with systemd services.

For me personally, I would not expect configuration of that kind in the systemd unit. I think this should be part of the system configuration file not the unit. I think of a systemd unit that it should only start the service and set properties which are required to run the service in the given environment (distribution) but not the options how it should behave. However, maybe that is only my PoV.

You are right. Modification of configuration files by scripts/applications is something that I am not a big fan of (even though some do that ... see ipa), because it causes issues on its own mostly much later during updates, because users tend to ignore .rpmnew files. And using environment files is probably the second best thing.

Reiterating again, we could consider creating normal environment file (lets say /etc/anaconda/permitrootlogin or similar if you already use some other directory under /etc/) to be loaded rather than a systemd drop-in file, which will have the advantage of working also for the socket activation sshd@.service (which was missed so far). This would be very similar how crypto policies are handled now.

This does not solve the problem I'm thinking of. It really makes it even worse because then we have another configuration file taken into account. Also, I'm not that convinced that we want to have anaconda specific configuration files in the system. The configuration should stay specific for the service.

This would be instead of the service drop-in configuration and it would be really on one place. If the path is the issue, we could try something like /etc/sysconfig/sshd-permitrootlogin?

In the other hand if it will solve socket activation we may start talking about that. What do you recommend?

If we would go the systemd service drop-in file-way, we would need two to address also the socket activation, adding second file, which would mean more confusion and harder and more error-prone for user to remove later.

The single environment file is probably best we can do.

In the long term, we would indeed like to use sshd_config drop-in directory, but it is not possible at this moment, because there is no Include functionality (only the client config supports that).

That would be an ideal solution. I'm looking forward into that feature :)

#metoo since 2015

The other problem is that you will even miss the environment variable because that will be used only for the unit and not by the system so another way how to find what is happening is hidden.

What do you mean here by "not used by the system"?

I meant that you won't see the environment variable when you type env into the terminal but that is really not important.

Sorry for the mistakes in the previous comment. :(

No problem.

So to summarize, I think we should modify the PR to create a file /etc/sysconfig/sshd-permitrootlogin containing the string PERMITROOTLOGIN="-oPermitRootLogin=yes", maybe also some additional comment describing where does it come from and how to opt-out after installation (remove this file).

Then, OpenSSH service files will be modified to contain the following new line

EnvironmentFile=-/etc/sysconfig/sshd-permitrootlogin
ExecStart=/usr/sbin/sshd -D $OPTIONS $CRYPTO_POLICY $PERMITROOTLOGIN

which will load the file, if it is in place and apply the configuration on commandline. Does it make sense?

@jkonecny12
Copy link
Member

jkonecny12 commented Jul 12, 2019

Sounds good to me and thanks for all the explanation. And I agree that users are ignoring the .rpmnew files, we should avoid that path.

The next steps will be:

  • We should change this PR to create /etc/sysconfig/sshd-permitrootlogin containing PERMITROOTLOGIN="-oPermitRootLogin=yes"
  • Who will change the systemd unit? Anaconda or the service file will be changed in the rpm file? I guess we shouldn't change the systemd unit because we don't own that file.

@Jakuje
Copy link
Contributor

Jakuje commented Jul 12, 2019

I will change the systemd unit later today.

@Jakuje
Copy link
Contributor

Jakuje commented Jul 12, 2019

@jkonecny12 fixed in https://src.fedoraproject.org/rpms/openssh/c/358f62be and rawhide build is on the way.

@jkonecny12
Copy link
Member

@Jakuje thanks a lot for your help!
Unfortunately creator of this PR @poncovka is on a vacation now. She will be available from the next week. I hope that is OK. If we need to get it there ASAP then I can make the changes and the build but I would rather wait.

@Jakuje
Copy link
Contributor

Jakuje commented Jul 21, 2019

Hi. I was also on vacation last week so no problem for me so far. But since we are getting closer to the change completion deadline, I would like to see a progress next week.

@poncovka poncovka force-pushed the master-permit_ssh_root_login branch from 175e45d to a2c2b0b Compare July 22, 2019 12:50
Open SSH, which provides the SSH server Fedora uses already for some
time disallowed password based root logins. Fedora used to patch this
out so far, but this patch has been recently dropped, restoring the
upstream behavior.

This change does not affect key based SSH logins and it will continue
to be possible to login as root with key based authentication.

To make the transition easier for user who might have valid use cases
for logging in as root with password over SSH we will add a checkbox
to the root password spoke. This checkbox makes it possible to manually
enable password based SSH logins for the root account.

Resolves: rhbz#1716282
@poncovka poncovka force-pushed the master-permit_ssh_root_login branch from a2c2b0b to 1302809 Compare July 22, 2019 13:42
@poncovka
Copy link
Contributor Author

Updated.

@Jakuje
Copy link
Contributor

Jakuje commented Jul 23, 2019

Thank you. Looks good to me as I went through the relevant changes.

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@poncovka poncovka merged commit ff78566 into rhinstaller:master Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master Please, use the `f39` label instead.
Development

Successfully merging this pull request may close these issues.

4 participants