-
Notifications
You must be signed in to change notification settings - Fork 44
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
Don't always Enable SSHD and Open SSH Port [SLE-15-SP5] #1091
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't always Enable SSHD and Open SSH Port (V2.0) [SLE-15-SP4]
teclator
approved these changes
Jun 19, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
✔️ Internal Jenkins job #756 successfully finished |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Target Branch
This is the merge of #1090 to SLE-15-SP5.
Bugzilla
https://bugzilla.suse.com/show_bug.cgi?id=1211764
Trello
https://trello.com/c/ofwfoQfm/
Problem
In the security proposal just before committing the installation, SSHD was always enabled, and the SSH port 22 was always opened by default, no matter if a root password was set or not.
If there is no root password, that makes sense since then it might be a case where only public key authentication was possible after the installation. But if a root password was set, it should not be the default to enable the SSHD service and open the SSH port.
Cause
This uses a class
Installation::SecuritySettings
which is meant to be used as a singleton. And as the singleton instance was used for the first time, all of its values were initialized, and those values were FINAL.That included the values for opening the ssh port and the firewall. And as a fallback, if there was no root user yet, or the root password was empty, it assumed that there was only public key authentication, and in that case, it opened the SSH port and enabled SSHD.
The trouble was that all this happened BEFORE the user was even prompted for the root password, so at that point, of course the root password was still empty, so it always fell back to public key authentication.
Fix
This factors out the check if only public key authentication is configured to a new separate method
SecuritySettings.propose
and calls that method when the security proposal is made in the general proposal dialog ("Installation Settings") during the installation.Test
Manual test in an inst-sys with the changed files bind-mounted on top. Selected different roles for the initial security settings; for any of the desktop roles (KDE, Gnome, Xfce), the SSH port should remain closed by default and no SSHD should be started, for the server roles they should.
Hacked up
security_settings.rb
a bit to pretend it's a public key only auth scenario when entering a trivial root password such as "root" and observed that it should also open the SSH port and enable SSHD in that case, but overriding it manually by clicking on those settings in the proposal needs to work (which should then give a warning):Related PRs