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

Disables IPv6 via cmdline option for Focal #5810

Merged
merged 6 commits into from
Feb 24, 2021
Merged

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Feb 23, 2021

Status

Ready for review

Description of Changes

Fixes #5807

Adds a Focal-only cmdline option for the boot to disable IPv6
functionality completely. Adds a config test to ensure no IPv6 addresses
are assigned.

Testing

  • CI must pass on both Xenial & Focal
  • On a Focal staging VM, run sudo ip addr and ensure you see no IPv6 addresses on any interface.

Deployment

Changes are Focal-specific, Xenial behavior remains unchanged.

@conorsch
Copy link
Contributor Author

Marking WIP because I encountered the following error during local testing on Qubes staging:

    TASK [grsecurity : Check if reboot is required due to inactive grsecurity lock.] ***
    changed: [app-staging]
    changed: [mon-staging]
[WARNING]: flush_handlers task does not support when conditional
    included: /home/user/securedrop/install_files/ansible-base/tasks/reboot.yml for app-staging, mon-staging

    RUNNING HANDLER [common : Gracefully halt Qubes staging VM] ********************
    changed: [mon-staging -> localhost]
    changed: [app-staging -> localhost]

    RUNNING HANDLER [common : Boot Qubes staging VM] *******************************
    changed: [app-staging -> localhost]
    changed: [mon-staging -> localhost]

    TASK [grsecurity : Set sysctl flags for grsecurity.] ***************************
    changed: [mon-staging] => (item={'name': 'kernel.grsecurity.rwxmap_logging', 'value': '0'})
    changed: [mon-staging] => (item={'name': 'kernel.grsecurity.grsec_lock', 'value': '1'})
    changed: [mon-staging] => (item={'name': 'vm.heap_stack_gap', 'value': '1048576'})
    failed: [app-staging] (item={'name': 'kernel.grsecurity.rwxmap_logging', 'value': '0'}) => {"ansible_loop_var": "item", "item": {"name": "kernel.grsecurity.rwxmap_logging", "value": "0"}, "msg": "Data could not be sent to remote host \"10.137.0.50\". Make sure this host can be reached over ssh: ssh: connect to host 10.137.0.50 port 22: Connection timed out\r\n", "unreachable": true}
    failed: [app-staging] (item={'name': 'kernel.grsecurity.grsec_lock', 'value': '1'}) => {"ansible_loop_var": "item", "item": {"name": "kernel.grsecurity.grsec_lock", "value": "1"}, "msg": "Data could not be sent to remote host \"10.137.0.50\". Make sure this host can be reached over ssh: ssh: connect to host 10.137.0.50 port 22: No route to host\r\n", "unreachable": true}
    failed: [app-staging] (item={'name': 'vm.heap_stack_gap', 'value': '1048576'}) => {"ansible_loop_var": "item", "item": {"name": "vm.heap_stack_gap", "value": "1048576"}, "msg": "Data could not be sent to remote host \"10.137.0.50\". Make sure this host can be reached over ssh: ssh: connect to host 10.137.0.50 port 22: No route to host\r\n", "unreachable": true}
    fatal: [app-staging]: UNREACHABLE! => {"changed": false, "msg": "All items completed", "results": [{"ansible_loop_var": "item", "item": {"name": "kernel.grsecurity.rwxmap_logging", "value": "0"}, "msg": "Data could not be sent to remote host \"10.137.0.50\". Make sure this host can be reached over ssh: ssh: connect to host 10.137.0.50 port 22: Connection timed out\r\n", "unreachable": true}, {"ansible_loop_var": "item", "item": {"name": "kernel.grsecurity.grsec_lock", "value": "1"}, "msg": "Data could not be sent to remote host \"10.137.0.50\". Make sure this host can be reached over ssh: ssh: connect to host 10.137.0.50 port 22: No route to host\r\n", "unreachable": true}, {"ansible_loop_var": "item", "item": {"name": "vm.heap_stack_gap", "value": "1048576"}, "msg": "Data could not be sent to remote host \"10.137.0.50\". Make sure this host can be reached over ssh: ssh: connect to host 10.137.0.50 port 22: No route to host\r\n", "unreachable": true}]}

A reboot of the VM resolved, which seems to suggest an iptables ratelimit on SSH. Can't see how that's related, so more testing required.

@conorsch conorsch force-pushed the 5807-disable-ipv6-for-focal branch from 1f2f575 to ecbfb47 Compare February 23, 2021 01:16
@conorsch conorsch force-pushed the 5807-disable-ipv6-for-focal branch from ecbfb47 to 6ca880e Compare February 23, 2021 15:17
@conorsch
Copy link
Contributor Author

Updated the sysctl syntax to skip ipv6 settings under Focal, since disabling boot time means the associated sysctl tasks will fail. Waiting for CI to pass, then will mark ready.

@rmol
Copy link
Contributor

rmol commented Feb 23, 2021

Ah, I saw that same connection error on Qubes. Good. 😄

@conorsch
Copy link
Contributor Author

Hmm, based on CI failure in https://app.circleci.com/pipelines/github/freedomofpress/securedrop/1982/workflows/4abc8b06-ffb1-4c22-a117-24a7d72ef407/jobs/51269, looks like the securedrop-grsec package in the apt-test repo is taking precedence over the locally built one for staging environments. That means the config added here isn't live in staging.

@rmol
Copy link
Contributor

rmol commented Feb 23, 2021

Ah, that's another error I saw but hadn't had time to dig into. reprepro is converting the last hyphen to an underscore, so apt-test's package sorts later, despite being the same version. Probably easiest to change the naming under build/.

@conorsch
Copy link
Contributor Author

It looks to me like the package simply wasn't being held, so I've tacked on a commit to hold it and see if that resolves. The "last hyphen" you mention shouldn't matter in this context, since apt will still find it and pull it in. Let's see what CI thinks of that idea.

@eloquence eloquence added this to the 1.8.0 milestone Feb 23, 2021
@conorsch conorsch force-pushed the 5807-disable-ipv6-for-focal branch from 413bae2 to c2a46a3 Compare February 23, 2021 21:03
@rmol
Copy link
Contributor

rmol commented Feb 23, 2021

I just got through a clean Focal staging run on Qubes. No IPv6 addresses on any interface, app or mon.

I kicked CI in the hopes that it might be similarly happy.

@conorsch conorsch force-pushed the 5807-disable-ipv6-for-focal branch from c2a46a3 to 5620ecc Compare February 23, 2021 22:43
@conorsch
Copy link
Contributor Author

Thanks, @rmol. I'm definitely seeing the tests passing on Focal locally. To get CI happy, I just pushed another small diff to the package-hold logic that should resolve for Xenial, too:

 --- a/install_files/ansible-base/group_vars/all/securedrop
+++ b/install_files/ansible-base/group_vars/all/securedrop
@@ -45,3 +45,6 @@ securedrop_pkg_grsec_xenial:
 securedrop_pkg_grsec_focal:
   ver: "5.4.97"
   depends: "linux-image-5.4.97-grsec-securedrop,intel-microcode"
+
+# Mostly useful for local package installation
+grsec_version: "{{ securedrop_pkg_grsec_xenial.ver if securedrop_target_distribution == 'xenial' else securedrop_pkg_grsec_focal.ver }}"

CI running now.

@conorsch conorsch marked this pull request as ready for review February 23, 2021 22:44
@@ -28,7 +28,7 @@ set_grub_default() {
# When using CONFIG_PAX_KERNEXEC, the grsecurity team recommends the kernel
# is booted with "noefi" on the kernel command line if "CONFIG_EFI" is
# enabled, as EFI runtime services are necessarily mapped as RWX.
sed -i '/^GRUB_CMDLINE_LINUX_DEFAULT=/s/=.*/=\"noefi\"/' /etc/default/grub
perl -pi -e 's|^GRUB_CMDLINE_LINUX_DEFAULT=|GRUB_CMDLINE_LINUX_DEFAULT="noefi ipv6.disable=1"|' /etc/default/grub
Copy link
Contributor

Choose a reason for hiding this comment

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

This perl expression is causing the trouble.

@kushaldas
Copy link
Contributor

This CI failure is valid:

5\.4\.97\-grsec\-securedrop$|^linux-.*-5\.4\.97\-grsec\-securedrop$|^kfreebsd-.*-5\.4\.97\-grsec\-securedrop$|^gnumach-.*-5\.4\.97\-grsec\-securedrop$|^.*-modules-5\.4\.97\-grsec\-securedrop$|^.*-kernel-5\.4\.97\-grsec\-securedrop$) regexp to find running kernel packages
Checking: securedrop-grsec ([<Origin component:'main' archive:'' origin:'SecureDrop' label:'' site:'apt-test.freedom.press' isTrusted:True>])
pkgs that look like they should be upgraded: securedrop-grsec
Get:1 https://apt-test.freedom.press focal/main amd64 linux-image-4.14.188-grsec-securedrop amd64 4.14.188-grsec-securedrop-1 [50.8 MB] 

not sure why.

@emkll
Copy link
Contributor

emkll commented Feb 24, 2021

not sure why.

The securedrop-grsec package is installed by both local debs and through apt-test, in the staging scenario. The test failure here might be related to holding the package in https://github.com/freedomofpress/securedrop/pull/5810/files#diff-8164feba234adc81cf7c91b9f402a0a90f3f7bc1983231168a61db05f56be279R10

The failing test (test_unattended_upgrades_functional) ensures all packages are up to date on the running system. The output of when a package is held (per the command above) might be different here and require the test to be updated, or perhaps the hold is not honored by unattended-upgrades ?

@conorsch
Copy link
Contributor Author

Magnificent, so at least marking the package as held gets the relevant tests passing, so the boot option is enabled and IPv6 is fully disabled on Focal. That's good! Looking at the unattended-upgrades test failure locally. We mark all of the locally built packages in staging as held, so I'm surprised that securedrop-grsec is a problem. @rmol had a hypothesis in #5810 (comment) that's worth a closer look

Conor Schaefer and others added 4 commits February 24, 2021 09:47
Adds a Focal-only cmdline option for the boot to disable IPv6
functionality completely. Adds a config test to ensure no IPv6 addresses
are assigned. Since the IPv6 stack is disabled at boot time, the
associated sysctl tasks won't exist. Therefore we'll add those only on
Xenial. This is the type of config that could be moved into a
metapackage.
Ensures that the "securedrop-grsec" package built locally for staging
takes precedence, so that the version served from the
apt-test.freedom.press repository doesn't win out.
Adding the old style sed command instead of the perl command.
This makes sure that we have only one value within double quotes
in the correct location in /etc/default/grub.
@rmol
Copy link
Contributor

rmol commented Feb 24, 2021

@rmol had a hypothesis

Bzzzt. 🙂 You were right; that wasn't it. It's just the default preferences. With the local filenames matching apt-test and the hold reverted, apt still prefers installing from apt-test:

# apt-cache policy securedrop-grsec
securedrop-grsec:
  Installed: 5.4.97+focal
  Candidate: 5.4.97+focal
  Version table:
 *** 5.4.97+focal 500
        500 https://apt-test.freedom.press focal/main amd64 Packages
        100 /var/lib/dpkg/status

@conorsch
Copy link
Contributor Author

Thanks for testing, @rmol! At least that's predictable: it seems the lack of a hold on that package would explain the behavior we're seeing nicely. I've got a potential fix (works locally, anyway) running over here: https://app.circleci.com/pipelines/github/freedomofpress/securedrop/2012/workflows/66426b84-4c72-451b-83f7-2e0a342653ab Didn't want to append here until I was sure that works, especially since you were looking into the problem, as well.

@conorsch conorsch force-pushed the 5807-disable-ipv6-for-focal branch from 1f68980 to 0f94db2 Compare February 24, 2021 15:47
@conorsch
Copy link
Contributor Author

Aye, we have fully green CI! https://app.circleci.com/pipelines/github/freedomofpress/securedrop/2012/workflows/66426b84-4c72-451b-83f7-2e0a342653ab I've pushed those same commits here, waiting on confirmation of green across the board.

@@ -53,22 +53,16 @@
tags:
- ntp

- name: Disable VirtualBox service vboxadd to avoid conflict with systemd-timesyncd.
- name: Disable VirtualBox services to avoid conflict with systemd-timesyncd.
systemd:
name: vboxadd
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be {{ item}} now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fixing...

Conor Schaefer added 2 commits February 24, 2021 11:37
The VirtualBox services are CI/dev-specific, and won't exist on
hardware. Don't fail if the services aren't found.
Adds the "dry-run" flag so that system state is not changed during the
test.
@conorsch conorsch force-pushed the 5807-disable-ipv6-for-focal branch from 0f94db2 to 600fcfb Compare February 24, 2021 16:37
@conorsch
Copy link
Contributor Author

@rmol Behold, CI is passing here. Thanks for your patience in passing this one back and forth. Mind giving it a final review prior to merge? This is the last PR we want in before we can cut the 1.8.0 branch (#5794).

@conorsch conorsch requested a review from rmol February 24, 2021 18:39
Copy link
Contributor

@rmol rmol left a comment

Choose a reason for hiding this comment

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

:shipit: Rebuilt staging, all good, no IPv6.

@rmol rmol merged commit 94fdaac into develop Feb 24, 2021
@rmol rmol deleted the 5807-disable-ipv6-for-focal branch February 24, 2021 18:57
@kushaldas kushaldas mentioned this pull request Feb 26, 2021
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ipv6 not completely disabled on Focal
5 participants