From 3af5a1f00bed97875e468368be98ef3b4a5fa45b Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Thu, 31 Oct 2024 13:10:54 -0400 Subject: [PATCH] Don't reuse the "ssh" group for our own access control The "ssh" group was supposed to be an internal group that was used by openssh itself for privilege separation. It's since been renamed to "_ssh"[1] to clarify that it's internal-only and flagged that we shouldn't be using it either. Instead, create a new "sdssh" group, add our users to it, and use it in the sshd_config. For upgrades, we migrate users to the new group and then update sshd_config and iptables rules. [1] https://salsa.debian.org/ssh-team/openssh/-/blob/eac38305fc5d8eb8301a106294cf6c79447bdeb3/debian/openssh-client.postinst Fixes #7316. --- .../roles/common/tasks/create_users.yml | 9 ++- .../restrict-direct-access/templates/rules_v4 | 4 +- .../templates/sshd_config | 4 +- molecule/testinfra/app/test_app_network.py | 2 +- .../testinfra/common/test_system_hardening.py | 2 +- molecule/testinfra/mon/test_mon_network.py | 2 +- .../usr/bin/securedrop-migrate-ssh-group | 59 +++++++++++++++++++ securedrop/debian/securedrop-config.install | 1 + securedrop/debian/securedrop-config.postinst | 2 + 9 files changed, 77 insertions(+), 8 deletions(-) create mode 100755 securedrop/debian/config/usr/bin/securedrop-migrate-ssh-group diff --git a/install_files/ansible-base/roles/common/tasks/create_users.yml b/install_files/ansible-base/roles/common/tasks/create_users.yml index 56b5bd414f..17e8cf126b 100644 --- a/install_files/ansible-base/roles/common/tasks/create_users.yml +++ b/install_files/ansible-base/roles/common/tasks/create_users.yml @@ -13,11 +13,18 @@ - users - sudoers +- name: Create "sdssh" group (for limiting SSH access) + group: + name: sdssh + state: present + tags: + - users + - name: Create shell accounts for SecureDrop admins. user: name: "{{ item }}" shell: /bin/bash - groups: sudo,ssh + groups: sudo,sdssh with_items: "{{ ssh_users }}" tags: - users diff --git a/install_files/ansible-base/roles/restrict-direct-access/templates/rules_v4 b/install_files/ansible-base/roles/restrict-direct-access/templates/rules_v4 index cc6155c535..e453df48b0 100644 --- a/install_files/ansible-base/roles/restrict-direct-access/templates/rules_v4 +++ b/install_files/ansible-base/roles/restrict-direct-access/templates/rules_v4 @@ -47,9 +47,9 @@ {% endif %} -# Block all other outbound access for users in the ssh group +# Block all other outbound access for users in the sdssh group # Load before generic loopback rules --A OUTPUT -m owner --gid-owner ssh -j LOGNDROP -m comment --comment "Drop all other outbound traffic for ssh user" +-A OUTPUT -m owner --gid-owner sdssh -j LOGNDROP -m comment --comment "Drop all other outbound traffic for ssh user" # DNS rules {% for address in dns_server -%} diff --git a/install_files/ansible-base/roles/restrict-direct-access/templates/sshd_config b/install_files/ansible-base/roles/restrict-direct-access/templates/sshd_config index dcd0d5aec1..166c2ab263 100644 --- a/install_files/ansible-base/roles/restrict-direct-access/templates/sshd_config +++ b/install_files/ansible-base/roles/restrict-direct-access/templates/sshd_config @@ -21,8 +21,8 @@ StrictModes yes RSAAuthentication yes PubkeyAuthentication yes PasswordAuthentication no -# Only users in the ssh group to authenticate -AllowGroups ssh +# Only users in the sdssh group to authenticate +AllowGroups sdssh # Don't use host-based authentication IgnoreRhosts yes RhostsRSAAuthentication no diff --git a/molecule/testinfra/app/test_app_network.py b/molecule/testinfra/app/test_app_network.py index fb5474ce40..b24ddd6912 100644 --- a/molecule/testinfra/app/test_app_network.py +++ b/molecule/testinfra/app/test_app_network.py @@ -20,7 +20,7 @@ def test_app_iptables_rules(host): tor_user_id=host.check_output("id -u debian-tor"), time_service_user=host.check_output("id -u systemd-timesync"), securedrop_user_id=host.check_output("id -u www-data"), - ssh_group_gid=host.check_output("getent group ssh | cut -d: -f3"), + ssh_group_gid=host.check_output("getent group sdssh | cut -d: -f3"), dns_server=securedrop_test_vars.dns_server, ) diff --git a/molecule/testinfra/common/test_system_hardening.py b/molecule/testinfra/common/test_system_hardening.py index 9f01dffed7..a3ec42c9a8 100644 --- a/molecule/testinfra/common/test_system_hardening.py +++ b/molecule/testinfra/common/test_system_hardening.py @@ -110,7 +110,7 @@ def test_twofactor_disabled_on_tty(host): ("PasswordAuthentication", "no"), ("PubkeyAuthentication", "yes"), ("RSAAuthentication", "yes"), - ("AllowGroups", "ssh"), + ("AllowGroups", "sdssh"), ("AllowTcpForwarding", "no"), ("AllowAgentForwarding", "no"), ("PermitTunnel", "no"), diff --git a/molecule/testinfra/mon/test_mon_network.py b/molecule/testinfra/mon/test_mon_network.py index 92efb7e296..28f11a8add 100644 --- a/molecule/testinfra/mon/test_mon_network.py +++ b/molecule/testinfra/mon/test_mon_network.py @@ -19,7 +19,7 @@ def test_mon_iptables_rules(host): default_interface=host.check_output("ip r | head -n 1 | awk '{ print $5 }'"), tor_user_id=host.check_output("id -u debian-tor"), time_service_user=host.check_output("id -u systemd-timesync"), - ssh_group_gid=host.check_output("getent group ssh | cut -d: -f3"), + ssh_group_gid=host.check_output("getent group sdssh | cut -d: -f3"), postfix_user_id=host.check_output("id -u postfix"), dns_server=securedrop_test_vars.dns_server, ) diff --git a/securedrop/debian/config/usr/bin/securedrop-migrate-ssh-group b/securedrop/debian/config/usr/bin/securedrop-migrate-ssh-group new file mode 100755 index 0000000000..78370fecf4 --- /dev/null +++ b/securedrop/debian/config/usr/bin/securedrop-migrate-ssh-group @@ -0,0 +1,59 @@ +#!/usr/bin/python3 +""" +Migrate users from the "ssh" group to "sdssh" +""" +import grp +import subprocess +from pathlib import Path + +SOURCE_GROUP = "ssh" +DEST_GROUP = "sdssh" + +def main(): + try: + grp.getgrnam(DEST_GROUP) + print(f"Group {DEST_GROUP} already exists") + except KeyError: + print(f"Creating group {DEST_GROUP}") + subprocess.run(['groupadd', DEST_GROUP], check=True) + + source_group_info = grp.getgrnam(SOURCE_GROUP) + source_users = source_group_info.gr_mem + print(f"Need to migrate: {source_users}") + + for username in source_users: + # Add user to new group while preserving other group memberships + subprocess.run(['usermod', '-a', '-G', DEST_GROUP, username], check=True) + print(f"Added {username} to {DEST_GROUP}") + subprocess.run(['usermod', '-r', '-G', username, SOURCE_GROUP], check=True) + print(f"Removed {username} from {SOURCE_GROUP}") + print("User migration complete") + + # Now update sshd_config + sshd_config = Path("/etc/ssh/sshd_config") + text = sshd_config.read_text() + if f"AllowGroups {SOURCE_GROUP}\n" in text: + # Update the AllowGroups stanza + text = text.replace(f"AllowGroups {SOURCE_GROUP}\n", f"AllowGroups {DEST_GROUP}\n") + # And the comment that precedes it + text = text.replace(f"in the {SOURCE_GROUP} group", f"in the {DEST_GROUP} group") + sshd_config.write_text(text) + print("Updated /etc/ssh/sshd_config") + # n.b. we don't restart sshd here, we'll let it take effect on boot + + # Now update iptables rules + iptables = Path("/etc/iptables/rules.v4") + text = iptables.read_text() + if f"--gid-owner {SOURCE_GROUP} -j LOGNDROP" in text: + # Update the --gid-owner stanza + text = text.replace(f"--gid-owner {SOURCE_GROUP} -j LOGNDROP", f"--gid-owner {DEST_GROUP} -j LOGNDROP") + # And the comment that precedes it + text = text.replace(f"for users in the {SOURCE_GROUP} group", f"for users in the {DEST_GROUP} group") + iptables.write_text(text) + print("Updated /etc/iptables/rules.v4") + + print("Done!") + + +if __name__ == '__main__': + main() diff --git a/securedrop/debian/securedrop-config.install b/securedrop/debian/securedrop-config.install index ab397f789e..1ee72d3b9f 100644 --- a/securedrop/debian/securedrop-config.install +++ b/securedrop/debian/securedrop-config.install @@ -1,2 +1,3 @@ debian/config/etc / debian/config/opt / +debian/config/usr / diff --git a/securedrop/debian/securedrop-config.postinst b/securedrop/debian/securedrop-config.postinst index 4e3086d5f8..3f36370d08 100755 --- a/securedrop/debian/securedrop-config.postinst +++ b/securedrop/debian/securedrop-config.postinst @@ -25,6 +25,8 @@ case "$1" in # And disable Ubuntu Pro's ua-timer and esm-cache (#6773) systemctl is-enabled ua-timer.timer && systemctl disable ua-timer.timer systemctl mask esm-cache + # Migrate the ssh group to sdssh + securedrop-migrate-ssh-group ;; abort-upgrade|abort-remove|abort-deconfigure)