[xenial] Updated release-upgrades Prompt for Trusty, Xenial as appropriate #4116

heartsucker · 2019-02-11T11:57:31Z

Status

Ready for review

Description of Changes

Update the Prompt in the release manage file depending on system codename. Additionally, attempt to rewrite the OSSEC alerts to prevent admins from running do-release-upgrade and breaking everything.

Testing

Trusty
- Build trusty debs
- Install on trusty box
- Verify that /etc/update-manager/release-upgrades contains Prompt=lts
- Verify that /usr/lib/ubuntu-release-upgrader/check-new-release contains "Visit https://securedrop.org/xenial-upgrade for more information" (and so does /var/lib/ubuntu-release-upgrader/release-upgrade-available if it exits)
Xenial
- Build xenial debs
- Install on xenial box
- Verify that /etc/update-manager/release-upgrades contains Prompt=never
- Verify that /usr/lib/ubuntu-release-upgrader/check-new-release does not contain "Visit https://securedrop.org/xenial-upgrade for more information" (and neither does /var/lib/ubuntu-release-upgrader/release-upgrade-available if it exits)

Deployment

All installs on Trusty will have files modified to the state SD requires. All installs on Xenial will have their files reverted to the default state.

Checklist

If you made changes to the system configuration:

Configuration tests pass

If you made non-trivial code changes:

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

…upgrades

conorsch · 2019-02-11T17:53:02Z

These changes appear well structured, based on visual review. Will step through the test plan to validate functionality. Flagging that we should add config tests to verify the LTS upgrade strings are set correctly on both Trusty & Xenial.

redshiftzero · 2019-02-11T17:55:29Z

We should also verify as part of review that the OSSEC notification that is triggered includes the expected text: "* Visit https://securedrop.org/xenial-upgrade for more information"

conorsch · 2019-02-12T01:12:38Z

Stepped through Trusty test plan today:

Build trusty debs
Install on trusty box
Verify that /etc/update-manager/release-upgrades contains Prompt=lts
Verify that /usr/lib/ubuntu-release-upgrader/check-new-release contains "Visit https://securedrop.org/xenial-upgrade for more information"
🔴 and so does /var/lib/ubuntu-release-upgrader/release-upgrade-available if it exits)
OSSEC notification that is triggered includes the expected text: "* Visit https://securedrop.org/xenial-upgrade for more information"

Will leave these VMs running overnight to see if OSSEC emails contain the correct text (although I used staging machines, I made sure to configure OSSEC via email). Given that the /var/lib/ubuntu-release-upgrader/release-upgrade-available file didn't satisfy the test plan (file was zero-bytes on both app-staging and mon-staging), perhaps not.

Validates that Trusty should honor an upgrade request to Xenial, but Xenial should not honor upgrade requests to Bionic.

emkll · 2019-02-12T15:06:54Z

Went through the Xenial test plan, LGTM:

Build xenial
Install on xenial box
Verify that /etc/update-manager/release-upgrades contains Prompt=never
Verify that /usr/lib/ubuntu-release-upgrader/check-new-release does not contain "Visit https://securedrop.org/xenial-upgrade for more information" (and neither does /var/lib/ubuntu-release-upgrader/release-upgrade-available if it exits) NOTE: release-upgrade-available does not exist

Also did an upgrade test under trusty, since these will be shipped to Trusty installs via the securedrop-config package, LGTM:

local upgrade successful
Verify that /etc/update-manager/release-upgrades contains Prompt=never
Verify that /usr/lib/ubuntu-release-upgrader/check-new-release does not contain "Visit https://securedrop.org/xenial-upgrade for more information" (and neither does /var/lib/ubuntu-release-upgrader/release-upgrade-available if it exits) NOTE: release-upgrade-available does not exist

I am (seemingly unrelated) error while upgrading, likely introduced by #4080, for which I have opened #4125

emkll · 2019-02-12T17:00:08Z

Happy to do so, @heartsucker .

I am also seeing the same behavior @conorsch is experiencing when upgrading in trusty:

After running the cron.weekly updater-notifier-common, /var/lib/ubuntu-release-upgrader/release-upgrade-available is 0 bytes. No OSSEC emails are sent as a result.

heartsucker · 2019-02-12T17:51:59Z

I'm in the middle of another PR review right now so I can't start up a VM, but I think what's happening is that the permissions are wrong on that file and it's breaking things because cron cant' access it.

heartsucker · 2019-02-12T21:38:15Z

I think what's happening is in some cases the /usr/lib/ubuntu-release-upgrader/release-upgrade-motd part of all of the above runs with a & (in the background), so the file /var/lib/ubuntu-release-upgrader/release-upgrade-available doesn't exist yet when we sed it. On a staging machine, if I remove that file then run dpkg-reconfigure securedrop-config, everything seems to work. Should we add a force rm to that in the postinst just to ensure we blow it up to trigger it's recreation? I'm still not totally sure how this ends up in the OSSEC alerts (plugin/rule?) but I think this should do the trick.

heartsucker · 2019-02-12T21:45:34Z

I just added a line that should trigger all the above logic. If that's not actually the fix, feel free to remove it.

codecov-io · 2019-02-12T22:29:46Z

Codecov Report

Merging #4116 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #4116   +/-   ##
========================================
  Coverage    84.77%   84.77%           
========================================
  Files           43       43           
  Lines         2779     2779           
  Branches       303      303           
========================================
  Hits          2356     2356           
  Misses         355      355           
  Partials        68       68

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8339e32...cf14d40. Read the comment docs.

conorsch · 2019-02-13T01:55:55Z

Confirming that the /var/lib/ubuntu-release-upgrader/release-upgrade-available file is now correctly populated for me. Built twice on Trusty and it was true both times.

Why the & in the postinst, though? If it's to avoid aborting the script due to a non-zero exit code, then we should use || true instead. If it's to avoid long run times of the postinst, it appears to save only 1-1.5s. At first I was concerned we had a race condition here, and we'd need a corresponding wait for that process to exit, but as long as we don't care about success, it doesn't seem cause problems.

Have not yet verified the Xenial story functionality, nor confirmed the OSSEC emails, so more testing still required.

emkll · 2019-02-13T16:19:34Z

Thanks for the fix @heartsucker ! I can also confim that /var/lib/ubuntu-release-upgrader/release-upgrade-available is now properly populated in the trusty scenario.
Also tested again in Xenial and the test plan is successful.

The final piece is the ossec alert and unfortunately I still have not received an ossec alert requesting me to upgrade. I do get a syscheck alert informing me that the file was changed.

I can't seem to find a relevant ossec alert/rule for this in the upstream repo ( https://github.com/ossec/ossec-hids/tree/master/etc/rules)

The easiest path would be to add a custom ossec notification (that would require xenial-specific logic to remove):

diff --git a/install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf b/install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf
index 9b14f4a81..651fff66d 100644
--- a/install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf
+++ b/install_files/securedrop-ossec-agent/var/ossec/etc/ossec.conf
@@ -115,6 +115,12 @@
   </localfile>
 
   <localfile>
+    <log_format>full_command</log_format>
+    <command>/usr/lib/ubuntu-release-upgrader/check-new-release -q</command>
+    <frequency>864900</frequency> <!-- 24 hours -->
+  </localfile>
+
+  <localfile>
     <log_format>syslog</log_format>
     <location>/var/log/kern.log</location>
   </localfile>
diff --git a/install_files/securedrop-ossec-server/var/ossec/rules/local_rules.xml b/install_files/securedrop-ossec-server/var/ossec/rules/local_rules.xml
index 97f0cfe62..9994f6c37 100644
--- a/install_files/securedrop-ossec-server/var/ossec/rules/local_rules.xml
+++ b/install_files/securedrop-ossec-server/var/ossec/rules/local_rules.xml
@@ -200,4 +200,11 @@
   </rule>
 </group>
 
-
+<group name="Daily dist upgrade notifications">
+  <rule id="400700" level="7" >
+    <if_sid>530</if_sid>
+    <options>alert_by_email</options> <!-- force email to be sent -->
+    <match>ossec: output: '/usr/lib/ubuntu-release-upgrader/check-new-release -q</match>
+    <description>This alert is to inform admins to upgrade to Xenial</description>
+  </rule>

heartsucker · 2019-02-13T17:35:03Z

Why the & in the postinst, though?

That Python script will download the do-release-upgrade and other required packages in the background it seems. I think it can be a very heavy process, and it's ok if it doesn't exit in postinst. There were some posts complaining about it being very slow so it seems like to avoid that we don't want to run in the foreground.

heartsucker · 2019-02-13T17:37:46Z

About that OSSEC rule, I think we could ship either two different deb packges or two different conf files and then move the right one into place on postinst. I guess we could also apply/unapply the diff with diff too. Thoughts?

kushaldas

Worked as expected, approved.

Build Trusty
Install on Trusty box
Verify that /etc/update-manager/release-upgrades contains Prompt=lts
Verify that /usr/lib/ubuntu-release-upgrader/check-new-release contains "Visit https://securedrop.org/xenial-upgrade for more information"
Build xenial
Install on xenial box
Verify that /etc/update-manager/release-upgrades contains Prompt=never
Verify that /usr/lib/ubuntu-release-upgrader/check-new-release does not contain "Visit https://securedrop.org/xenial-upgrade for more information" (and neither does /var/lib/ubuntu-release-upgrader/release-upgrade-available if it exits)

heartsucker · 2019-02-14T09:42:29Z

So as this stands, we don't have the OSSEC rule for this. Since @conorsch seems to do the most Debian packaging stuff here, it would be good to get your feedback on whether or not we should update those rules via some postinst logic.

emkll · 2019-02-14T14:04:57Z

Given the short timeline for 0.12.0 and the complexities around the management of separate ossec rules, we could also write a very simple cron job on the mon server that simply calls send_encrypted_alarm.sh to send an email to the admin, only if the base os is Trusty.

heartsucker · 2019-02-14T16:25:30Z

@emkll, take a peeksy at the change I pushed. This should do the thing.

emkll · 2019-02-14T21:11:39Z

@heartsucker thanks for the change. We've discussed this in the engineering meeting and concluded that perhaps more research was required on the ossec front. Any objections to removing ce49036 and opening a follow up to monitor/update the ossec rule? We can keep the scope of this PR to only the release-upgrades for the purposes of admins running do-release-upgrade
The release-upgrades is a very important/significant change, and might be best to merge just that part for QA, and following up on the ossec portion later.
What do you think?

heartsucker · 2019-02-14T21:15:53Z

I think that's reasonable. I'll for push with this change yanked.

heartsucker · 2019-02-14T21:17:53Z

@kushaldas Can you stamp this one again? I added a commit since your review, then removed it, so it's unchanged since your last approval.

kushaldas

Restampping.

heartsucker requested review from conorsch, emkll, kushaldas and msheiny as code owners February 11, 2019 11:57

update upgrade prompt via postinst script

bc94759

heartsucker force-pushed the updated-release-upgrades branch from ff73169 to a9ca2b8 Compare February 11, 2019 15:47

heartsucker added 2 commits February 11, 2019 17:30

update the script and log file for new releases to prevent incorrect …

b2efbea

…upgrades

revert update script changes once we move to xenial

047a15b

heartsucker force-pushed the updated-release-upgrades branch from a9ca2b8 to 047a15b Compare February 11, 2019 16:31

eloquence changed the title ~~Updated release upgrades~~ Updated release-upgrades Prompt for Trusty, Xenial as appropriate Feb 11, 2019

eloquence changed the title ~~Updated release-upgrades Prompt for Trusty, Xenial as appropriate~~ [xenial] Updated release-upgrades Prompt for Trusty, Xenial as appropriate Feb 11, 2019

Adds config tests for release-manager channels

4da235c

Validates that Trusty should honor an upgrade request to Xenial, but Xenial should not honor upgrade requests to Bionic.

force re-run the script to trigger an ossec alert

cf14d40

kushaldas previously approved these changes Feb 14, 2019

View reviewed changes

heartsucker dismissed kushaldas’s stale review via ce49036 February 14, 2019 16:25

heartsucker force-pushed the updated-release-upgrades branch from ce49036 to cf14d40 Compare February 14, 2019 21:16

kushaldas approved these changes Feb 15, 2019

View reviewed changes

emkll merged commit 10b16ce into develop Feb 15, 2019

emkll deleted the updated-release-upgrades branch February 15, 2019 13:51

emkll mentioned this pull request Feb 15, 2019

[0.12.0] Updated release-upgrades Prompt for Trusty, Xenial as appropriate #4142

Merged

redshiftzero mentioned this pull request Feb 15, 2019

[xenial] ensure OSSEC alert is sent pointing to xenial upgrade docs #4147

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[xenial] Updated release-upgrades Prompt for Trusty, Xenial as appropriate #4116

[xenial] Updated release-upgrades Prompt for Trusty, Xenial as appropriate #4116

heartsucker commented Feb 11, 2019

conorsch commented Feb 11, 2019

redshiftzero commented Feb 11, 2019

conorsch commented Feb 12, 2019

emkll commented Feb 12, 2019 •

edited

Loading

emkll commented Feb 12, 2019

heartsucker commented Feb 12, 2019

heartsucker commented Feb 12, 2019

heartsucker commented Feb 12, 2019

codecov-io commented Feb 12, 2019

conorsch commented Feb 13, 2019

emkll commented Feb 13, 2019 •

edited

Loading

heartsucker commented Feb 13, 2019

heartsucker commented Feb 13, 2019

kushaldas left a comment

heartsucker commented Feb 14, 2019

emkll commented Feb 14, 2019

heartsucker commented Feb 14, 2019

emkll commented Feb 14, 2019

heartsucker commented Feb 14, 2019

heartsucker commented Feb 14, 2019

kushaldas left a comment

[xenial] Updated release-upgrades Prompt for Trusty, Xenial as appropriate #4116

[xenial] Updated release-upgrades Prompt for Trusty, Xenial as appropriate #4116

Conversation

heartsucker commented Feb 11, 2019

Status

Description of Changes

Testing

Deployment

Checklist

If you made changes to the system configuration:

If you made non-trivial code changes:

conorsch commented Feb 11, 2019

redshiftzero commented Feb 11, 2019

conorsch commented Feb 12, 2019

emkll commented Feb 12, 2019 • edited Loading

emkll commented Feb 12, 2019

heartsucker commented Feb 12, 2019

heartsucker commented Feb 12, 2019

heartsucker commented Feb 12, 2019

codecov-io commented Feb 12, 2019

Codecov Report

conorsch commented Feb 13, 2019

emkll commented Feb 13, 2019 • edited Loading

heartsucker commented Feb 13, 2019

heartsucker commented Feb 13, 2019

kushaldas left a comment

Choose a reason for hiding this comment

heartsucker commented Feb 14, 2019

emkll commented Feb 14, 2019

heartsucker commented Feb 14, 2019

emkll commented Feb 14, 2019

heartsucker commented Feb 14, 2019

heartsucker commented Feb 14, 2019

kushaldas left a comment

Choose a reason for hiding this comment

emkll commented Feb 12, 2019 •

edited

Loading

emkll commented Feb 13, 2019 •

edited

Loading