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

Remove ufw from new and existing installs #7315

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Oct 31, 2024

Status

Ready for review

Description of Changes

We don't use ufw and in noble, it conflicts with iptables-persistent, which we do want to use.

Remove it during provisioning and install a systemd timer to remove it. (We can't do it during a postinst because we're already in an apt session at that time.)

Fixes #7313.

Testing

How should the reviewer test this PR?

  • Visual review
  • staging CI passes (tests fresh install case)
  • build debs and install on a focal system, wait 1 day and see that ufw was removed

Deployment

Any special considerations for deployment? Both new installs and upgrades are handled

Checklist

@legoktm legoktm added the noble Ubuntu Noble related work label Oct 31, 2024
@legoktm legoktm requested a review from a team as a code owner October 31, 2024 16:05
@legoktm
Copy link
Member Author

legoktm commented Oct 31, 2024

Note that I haven't tested the upgrade path yet, I was planning to set it on Friday afternoon and check on it on Monday.

@legoktm
Copy link
Member Author

legoktm commented Nov 5, 2024

I think I should rework this to be more flexible given #7326.

We don't use ufw and in noble, it conflicts with iptables-persistent,
which we do want to use.

Remove it during provisioning and install a systemd timer to remove it.
(We can't do it during a postinst because we're already in an apt
session at that time.) A testinfra check verifies that the unit does
remove the package.

This can be extended in the future by adding more ConditionPathExists
and more packages to the list.

Fixes #7313.
@legoktm
Copy link
Member Author

legoktm commented Nov 6, 2024

I renamed the unit to securedrop-remove-packages but kept everything else the same. To remove haveged we can do:

[Unit]
Description=Remove ufw if installed
ConditionPathExists|=/usr/sbin/ufw
ConditionPathExists|=/usr/sbin/haveged

[Service]
Type=oneshot
Environment="DEBIAN_FRONTEND=noninteractive"
ExecStart=/usr/bin/apt-get purge --yes ufw haveged
User=root

But, I don't want to do that in this PR because I haven't had the chance to look into haveged yet and let's keep the scope down.

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.

Approving based on visual review.

@zenmonkeykstop zenmonkeykstop added this pull request to the merge queue Nov 7, 2024
Merged via the queue into develop with commit 4a0053b Nov 7, 2024
45 checks passed
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: Done
Development

Successfully merging this pull request may close these issues.

Remove ufw from focal instances
2 participants