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 a global setting to disable root login on local network #1551

Closed
wants to merge 4 commits into from

Conversation

npalix
Copy link
Contributor

@npalix npalix commented Dec 19, 2022

The problem

  • Fix and improve handling of PasswordAuthentication option
  • Add a setting for PermitRootLogin usage on local network

Solution

  • Directly use the value of ssh_password_authentication
  • Add ssh_allow_root_on_localnet to manage the last part of the SSH template

PR Status

First release and review

How to test

Play with the ssh settings.

The current template use if/else/endif which introduce
spurious empty lines. As the setting value is "yes" or "no", as expected
by the configuration file, the value is directly use.

All uses of passwordauthentication are addressed. This adds the one
used for the sftp group.

Finally, the global configuration sets the yes and no values
to "yes" and "no" respectively. Currently, the value is set to "0"
which breaks the configuration generation when "0" is compared to "False".

Signed-off-by: Nicolas Palix <nicolas.palix@imag.fr>
A global setting name ssh_allow_root_on_localnet is added
The old sshd_config template enable PermitRootLogin from
client on the localnet. According to the server where Yunohost
is deployed it doesn't make sense and could be a security issue
when hosted on a 3rd party.

Signed-off-by: Nicolas Palix <nicolas.palix@imag.fr>
Signed-off-by: Nicolas Palix <nicolas.palix@imag.fr>
@alexAubin alexAubin changed the title SSH config Add a global setting to disable root login on local network Dec 20, 2022
@alexAubin
Copy link
Member

Zblerg could you elaborate on the motivation behind having a setting to disable this ... I guess this is like to "harden security" but honestly meh, this exists as a good tradeoff between "not exposing root access" and "being able to access your server if LDAP is down for some reason", and I'm not looking forward to having posts on the forum about being locked out or their server because "yeah I disabled root login entirely because I thought this wasnt necessary" ...

@npalix
Copy link
Contributor Author

npalix commented Dec 20, 2022

About harden security and SSH access, less is more. I prefer to have a clean and small SSH config file. It's simpler to review.
My YunoHost instance is deployed on a 3rd party where I don't want to trust the network. In case of LDAP crash, I can connect with the SSH key of a local user account who has sudo right. I can improve the help message, but it's really cumbersome to manage a manually edited configuration file.

What about something like "Caution: you can be lock out in case of LDAP crash. In that case, you will need a backup access, either with a local account or physically to the server."

@npalix
Copy link
Contributor Author

npalix commented Dec 20, 2022

BTW commit 4a3a9f8 is about SSH but for the PasswordAuthentication issue, hence the original name of the PR.
Should I rebase on top of the " | int_to_bool" change from 47b9b8b ?

@alexAubin
Copy link
Member

Closing because honestly meh:

  • People are way too much obsessed about stuff like "disabling root login" when this is in fact motivated not by security itself, but for auditability (cf the comment from the Mozilla folks in their recommended SSHD conf)
  • On SSH, security is enforced by : 1) having strong-enough passwords or using asymetric keys and 2) allowing only a list of specific users to log in (ie not the default Debian conf where everybody can login on ssh). I don't know anybody that got pwned via SSH other than because some password was stupidly simple. No "using the latest top-notch ciphers" or "disabling root login" or "changing the default port" is gonna fix that.
  • People concerned about security should be way more obsessed with everything else going on on the system than the obvious SSH config. In particular if you don't trust your local network, then imho you should really have bigger concerns in mind than "maybe somebody is gonna try to bruteforce root" (which, if it ever happen, the diagnosis is still gonna report as "suspiciously number of auth failures recently")
  • We already have too many super-technical settings and it would be nice if YunoHost could avoid becoming the new webmin or whatever-boring-interface-with-a-gazillion-settings-all-over-the-place, YunoHost is supposed to stay minimalistic in its configuration with good one-size-fits-all settings such that things just work while being secure out of the box.
  • The risk of having users enabling this and ending up stuck with their system because LDAP is down for some reason is too high compared to whatever gain there is in enabling this.
  • If you absolutely want to do this, you can still edit the file, or write a custom regenconf hook

Sorry for the rant but I already spent way too much time in the past on similar PRs with stuff like "I've heard somewhere that cipher X is more secure than Y"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants