Skip to content

Commit

Permalink
Don't reuse the "ssh" group for our own access control
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
legoktm committed Nov 4, 2024
1 parent eb39e65 commit 3af5a1f
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 -%}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion molecule/testinfra/app/test_app_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand Down
2 changes: 1 addition & 1 deletion molecule/testinfra/common/test_system_hardening.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
2 changes: 1 addition & 1 deletion molecule/testinfra/mon/test_mon_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
59 changes: 59 additions & 0 deletions securedrop/debian/config/usr/bin/securedrop-migrate-ssh-group
Original file line number Diff line number Diff line change
@@ -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()
1 change: 1 addition & 0 deletions securedrop/debian/securedrop-config.install
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
debian/config/etc /
debian/config/opt /
debian/config/usr /
2 changes: 2 additions & 0 deletions securedrop/debian/securedrop-config.postinst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 3af5a1f

Please sign in to comment.