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

Disable fwupd-refresh.timer, triggers OSSEC warnings #6401

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Apr 13, 2022

Status

Ready for review

Description of Changes

For various reasons, the timer to run fwupdmgr refresh ocassionally
triggers OSSEC alerts, which admins can't do anything about.

We currently don't use fwupd for firmware updates, so the daily refresh
of metadata is useless and should be safe to disable. If in the future
we do want admins to install updates with fwupd, they can run refresh
manually as part of the process.

Fixes #6204.

Testing

  • Run make build-debs to get a new securedrop-config deb package
  • Verify on a staging instance that systemctl is-enabled fwupd-refresh.timer prints "enabled", and systemctl list-timers shows it too
  • Install the newly built package, sudo apt install ./securedrop-config[...].deb
  • Verify package installation was successful, and the is-enabled command from earlier now prints "disabled"
  • Re-install the package again with sudo apt install --reinstall ./securedrop-config[...].deb, to run the postinst again, against an already-disabled state.
  • Verify package re-installation was successful, and the is-enabled command still prints "disabled".
  • Reboot, then verify fwupd-refresh is no longer printed by systemctl list-timers

Deployment

Any special considerations for deployment?

Yes, this affects the postinst of the package, which means it cannot be allowed to fail under any circumstances.

Checklist

  • Linting (make lint) and tests (make test) pass in the development container
  • I have written a test plan and validated it for this PR
  • These changes do not require documentation

@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2022

Codecov Report

Merging #6401 (22dca80) into develop (f0dd9a8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #6401   +/-   ##
========================================
  Coverage    84.02%   84.02%           
========================================
  Files           61       61           
  Lines         4312     4312           
  Branches       524      524           
========================================
  Hits          3623     3623           
  Misses         565      565           
  Partials       124      124           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@legoktm
Copy link
Member Author

legoktm commented Apr 13, 2022

Of course, the securedrop-app-code package isn't installed on the monitor server, so probably the securedrop-config package is a better option, like Conor had originally suggested.

For various reasons, the timer to run `fwupdmgr refresh` ocassionally
triggers OSSEC alerts, which admins can't do anything about.

We currently don't use fwupd for firmware updates, so the daily refresh
of metadata is useless and should be safe to disable. If in the future
we do want admins to install updates with fwupd, they can run refresh
manually as part of the process.

Fixes #6204.
@legoktm legoktm marked this pull request as ready for review April 14, 2022 04:54
@legoktm legoktm requested a review from a team as a code owner April 14, 2022 04:54
@zenmonkeykstop zenmonkeykstop self-assigned this Apr 26, 2022
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.

  • Followed test plan in staging against both app-staging and mon-staging servers, everything checks out.
  • Also did a fresh staging install with the debs built from this branch, confirmed the fwupd-refresh timer is disabled there too.

LGTM! Looking forward to a world of fewer OSSEC false positives.

@zenmonkeykstop zenmonkeykstop merged commit b917f9c into develop Apr 27, 2022
@zenmonkeykstop zenmonkeykstop mentioned this pull request May 10, 2022
35 tasks
@legoktm legoktm deleted the disable-fwupd branch April 13, 2023 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fwupd produces multiple alerts
3 participants