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

Drop setting vm.heap_stack_gap and net.ipv4 sysctl flags #7324

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Nov 1, 2024

Status

Ready for review

Description of Changes

These are now set via the securedrop-grsec metapackage (see freedomofpress/kernel-builder#55).

Refs #7323.

Testing

How should the reviewer test this PR?

Deployment

Any special considerations for deployment?

This can only be deployed after a kernel with freedomofpress/kernel-builder#55 is released stable. It's safe to merge ahead of time though once the kernel-builder one is approved.

Checklist

@legoktm legoktm added the noble Ubuntu Noble related work label Nov 1, 2024
@legoktm legoktm requested a review from a team as a code owner November 1, 2024 21:38
@legoktm legoktm added this to the SecureDrop 2.11.0 milestone Nov 4, 2024
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

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

kernel-builder change looks good, but unless there's a compelling reason I'd keep the testinfra check.

@@ -7,34 +7,6 @@
testinfra_hosts = [sdvars.app_hostname, sdvars.monitor_hostname]


@pytest.mark.parametrize(
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO keeping the testinfra checks makes sense, in case the metapackage-set values get borked somehow.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, happy to leave it in for now but long-term I think we don't want to run into needing to constantly keep this repo updated if we change things in kernel-builder (but I doubt we'll be changing that many sysctl flags).

@@ -81,7 +81,6 @@ def test_grsecurity_kernel_is_running(host):
[
("kernel.grsecurity.grsec_lock", 1),
("kernel.grsecurity.rwxmap_logging", 0),
("vm.heap_stack_gap", 1048576),
Copy link
Contributor

Choose a reason for hiding this comment

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

(repeat) IMO keeping the testinfra checks makes sense, in case the metapackage-set values get borked somehow.

These are now set via the securedrop-grsec metapackage (see
<freedomofpress/kernel-builder#55>).

Tests are left in to verify the migration works properly.

Refs #7323.
@legoktm
Copy link
Member Author

legoktm commented Nov 6, 2024

This should fail CI now because we haven't built new kernel packages yet, I will see about doing some today or tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
noble Ubuntu Noble related work
Projects
Status: Ready For Review
Development

Successfully merging this pull request may close these issues.

2 participants