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

Bump securedrop-grsec-focal metapackage to 5.4.88 #5772

Merged
merged 5 commits into from Feb 9, 2021
Merged

Conversation

emkll
Copy link
Contributor

@emkll emkll commented Feb 4, 2021

Status

Ready for review

Description of Changes

Fixes #5479 and towards #4768

Updates the kernel metapackage version to 5.4.88 for Focal installs.

See freedomofpress/securedrop-apt-test#89

Testing

  • Focal metapackage pulls in 5.4.88 kernel image
  • Xenial metapackage pulls in 4.14.188 kernel image

Deployment

For new and existing installs, these changes will be distributed via deb packages through apt.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

If you added or removed a file deployed with the application:

  • I have updated AppArmor rules to include the change

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

@@ -4,7 +4,10 @@
import testutils

sdvars = testutils.securedrop_test_vars
KERNEL_VERSION = sdvars.grsec_version
if host.system_info.codename == "xenial":
Copy link
Contributor

Choose a reason for hiding this comment

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

host isn't available outside function scope, will need a refactor.

emkll and others added 3 commits February 5, 2021 18:13
This will pull in and install 5.4 series kernels for Focal installs,
thanks to the split metapackage logic introduced in #5691
KERNEL_VERSION is defined for each function as required.
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

I have updated the test code to make sure it goes through CI.

Also verified that:

  • Focal metapackage pulls in 5.4.88 kernel image
  • Xenial metapackage pulls in 4.14.188 kernel image

But, the IRDA tests are failing for the kernel: https://app.circleci.com/pipelines/github/freedomofpress/securedrop/1812/workflows/411763ec-cf39-43d3-a77b-ab89388a6b36/jobs/49865 @emkll what do you think?

CONFIG_IRDA is simply not present in the kernel configuration for 5.4 series kernels, in previous kernel configurations it was commented out.
@emkll emkll moved this from Ready for Review to In Development in SecureDrop Team Board Feb 8, 2021
@emkll
Copy link
Contributor Author

emkll commented Feb 8, 2021

There's currently an issue with the split logic for the config and grsec metapackages (introduced in #5684 and #5691 respectively do not produce the expected package contents due to filepath not being the same as the package name, resulting in some unwanted subfolders within the package:

total 36K
drwxr-xr-x  5 user    user    4.0K Feb  5 14:57 .
drwxrwxrwt 29 root root  20K Feb  8 09:56 ..
drwxr-xr-x  3 user    user    4.0K Feb  5 14:57 etc
drwxr-xr-x  3 user    user    4.0K Feb  5 14:57 -focal
drwxr-xr-x  3 user    user    4.0K Feb  5 14:57 opt

@conorsch
Copy link
Contributor

conorsch commented Feb 8, 2021

Looks like that extraneous dir only appears in the Focal metapackage, not in the Xenial one. It's empty, but should certainly be removed. Digging a bit now.

Discovered by @emkll in [0]. The forked package build logic was
including an empty dir based on the distro string. Update the path
munging logic to use the proper var to avoid.

[0] #5772 (comment)
@conorsch
Copy link
Contributor

conorsch commented Feb 8, 2021

@emkll I pushed a fix that appears to resolve for both Focal & Xenial.

@emkll emkll moved this from In Development to Ready for Review in SecureDrop Team Board Feb 8, 2021
@conorsch
Copy link
Contributor

conorsch commented Feb 9, 2021

@kushaldas This looks good to my eye, any concerns with merging?

@rmol
Copy link
Contributor

rmol commented Feb 9, 2021

I have an alternative that simplifies the distribution branching that I've been hoping to share for review. It's just been hard to test because of all the other Focal problems on develop right now. Fingers crossed it'll be ready for a look by midday, and obviously I think it's worth trying to dig out from under this a bit now, but if you'd rather just cross this one off and move on, please go ahead and merge, and maybe we can clean it up later, in any event when we remove the Xenial logic.

@conorsch
Copy link
Contributor

conorsch commented Feb 9, 2021

Sounds great, @rmol, I can review later today. Happy to wait until then. Regarding keeping the distribution branching clean, I had similar thoughts on #5780, so perhaps we can reuse the approach you propose in a few places.

@kushaldas
Copy link
Contributor

@kushaldas This looks good to my eye, any concerns with merging?

Nope, I think it is good to merge when you want to.

@emkll emkll moved this from Ready for Review to In Development in SecureDrop Team Board Feb 9, 2021
@conorsch
Copy link
Contributor

conorsch commented Feb 9, 2021

if you'd rather just cross this one off and move on, please go ahead and merge, and maybe we can clean it up later

Indeed, some folks are blocked on Focal hardware installs, so proceeding with merge. Happy to revisit any additional logic you've got queued up, @rmol, just point me to a branch!

@conorsch conorsch dismissed kushaldas’s stale review February 9, 2021 21:20

@kushaldas explicitly stated no concerns with merge, given recent changes

@conorsch conorsch merged commit 5cd42af into develop Feb 9, 2021
SecureDrop Team Board automation moved this from In Development to Done Feb 9, 2021
@rmol rmol deleted the 5479-focal-54 branch March 3, 2021 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Provide a 5.4-series grsecurity-patched kernel with NUC10 support
4 participants