From cf9fc44ebb5e0712c632977b8edb0baeb857daaa Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sun, 19 Nov 2023 10:59:12 -0300 Subject: [PATCH 01/22] d/jinja2_render: new tool to render the apparmor profile template --- debian/jinja2_render | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100755 debian/jinja2_render diff --git a/debian/jinja2_render b/debian/jinja2_render new file mode 100755 index 0000000000..e5e9f467d2 --- /dev/null +++ b/debian/jinja2_render @@ -0,0 +1,20 @@ +#!/usr/bin/python3 + +import sys + +from jinja2 import Template + +input_file = sys.argv[1] +output_file = sys.argv[2] +kwargs = {} + +# key=value pairs in the command-line +if len(sys.argv) > 3: + kwargs = {arg.split("=")[0]:arg.split("=")[1] for arg in sys.argv[3:]} + +t = Template(open(input_file, "r").read()) +output = t.render(**kwargs) + +with open(output_file, "w") as f: + f.write(output) + From f9d61d6be242161d3613945c388b02ab9ff4d9aa Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Tue, 10 Oct 2023 10:30:19 -0300 Subject: [PATCH 02/22] d/rules: render and install the apparmor profile for apt_news --- debian/rules | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/debian/rules b/debian/rules index 7b04ac101d..70030fdb86 100755 --- a/debian/rules +++ b/debian/rules @@ -63,6 +63,10 @@ override_dh_systemd_start: override_dh_auto_install: dh_auto_install --destdir=debian/ubuntu-advantage-tools + debian/jinja2_render debian/apparmor/ubuntu_advantage_apt_news.jinja2 debian/apparmor/ubuntu_advantage_apt_news ubuntu_codename=${UBUNTU_CODENAME} + install -D -m 644 $(CURDIR)/debian/apparmor/ubuntu_advantage_apt_news $(CURDIR)/debian/ubuntu-advantage-tools/etc/apparmor.d/ubuntu_advantage_apt_news + dh_apparmor -pubuntu-advantage-tools --profile-name=ubuntu_advantage_apt_news + flist=$$(find $(CURDIR)/debian/ -type f -name version.py) && sed -i 's,@@PACKAGED_VERSION@@,$(DEB_VERSION),' $${flist:-did-not-find-version-py-for-replacement} # We install the conf file even on non-LTS version to avoid issues on upgrade scenarios @@ -86,3 +90,4 @@ endif override_dh_auto_clean: dh_auto_clean make clean + rm -f debian/apparmor/ubuntu_advantage_apt_news From 69fe8d003fe93e4f0c953a0b25c8e256e7cf87cc Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Tue, 10 Oct 2023 10:34:39 -0300 Subject: [PATCH 03/22] d/apparmor: add apparmor profile for apt_news --- .../apparmor/ubuntu_advantage_apt_news.jinja2 | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 debian/apparmor/ubuntu_advantage_apt_news.jinja2 diff --git a/debian/apparmor/ubuntu_advantage_apt_news.jinja2 b/debian/apparmor/ubuntu_advantage_apt_news.jinja2 new file mode 100644 index 0000000000..5bc72b2b72 --- /dev/null +++ b/debian/apparmor/ubuntu_advantage_apt_news.jinja2 @@ -0,0 +1,51 @@ +{% if ubuntu_codename not in ["xenial", "bionic", "focal"] %} +abi , +{% endif %} +include + +profile ubuntu_advantage_apt_news flags=(attach_disconnected) { + include + include + include + include + + # Needed because apt-news calls apt_pkg.init() which tries to + # switch to the _apt system user/group. + capability setgid, + capability setuid, + capability dac_read_search, + + /etc/apt/** r, + /etc/default/apport r, + /etc/ubuntu-advantage/* r, + /usr/bin/python3.{1,}[0-9] mrix, +{% if ubuntu_codename in ["focal"] %} + # "import uuid" in focal triggers an uname call + /usr/bin/uname mrix, +{% endif %} + /usr/lib/apt/methods/http mrix, + /usr/lib/apt/methods/https mrix, + /usr/lib/ubuntu-advantage/apt_news.py r, + /usr/share/dpkg/* r, + /var/log/ubuntu-advantage.log rw, + /var/lib/ubuntu-advantage/** r, + /var/lib/ubuntu-advantage/messages/ rw, + /var/lib/ubuntu-advantage/messages/* rw, + /run/ubuntu-advantage/ rw, + /run/ubuntu-advantage/* rw, + + /tmp/** r, + + owner @{PROC}/@{pid}/fd/ r, + @{PROC}/@{pid}/cgroup r, +{% if ubuntu_codename in ["bionic", "xenial"] %} + # see https://bugs.python.org/issue40501 + /sbin/ldconfig rix, + /sbin/ldconfig.real rix, + @{PROC}/@{pid}/mounts r, + /usr/bin/@{multiarch}-gcc-* rix, + /usr/bin/@{multiarch}-ld.bfd rix, + /usr/lib/gcc/@{multiarch}/*/collect2 rix, + /usr/bin/@{multiarch}-objdump rix, +{% endif %} +} From 9343f4ed0e7d5a0a9de9a778bcdde8c423cb145e Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Tue, 10 Oct 2023 10:55:00 -0300 Subject: [PATCH 04/22] d/control: build-depends for dh-apparmor and jinja2 --- debian/control | 2 ++ 1 file changed, 2 insertions(+) diff --git a/debian/control b/debian/control index e7473910d5..105a145e96 100644 --- a/debian/control +++ b/debian/control @@ -6,6 +6,7 @@ Build-Depends: bash-completion, debhelper (>=9), debianutils (>= 4.7), dh-python, + dh-apparmor, # After debhelper 13.3 we no longer need dh-systemd. # On hirsute and later, dh-systemd doesn't even exist. # On recent releases, the first alternative will be used. @@ -21,6 +22,7 @@ Build-Depends: bash-completion, po-debconf, python3 (>= 3.4), python3-flake8, + python3-jinja2, python3-mock, python3-pytest, python3-setuptools, From bbb967a55c40ced1d37f8e70fcced98c9f335ac9 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Tue, 10 Oct 2023 11:32:19 -0300 Subject: [PATCH 05/22] systemd/apt-news.service: isolation --- systemd/apt-news.service | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/systemd/apt-news.service b/systemd/apt-news.service index 2f591d51b1..3dcf1a3650 100644 --- a/systemd/apt-news.service +++ b/systemd/apt-news.service @@ -14,3 +14,29 @@ Description=Update APT News [Service] Type=oneshot ExecStart=/usr/bin/python3 /usr/lib/ubuntu-advantage/apt_news.py +AppArmorProfile=ubuntu_advantage_apt_news +CapabilityBoundingSet=~CAP_SYS_ADMIN +CapabilityBoundingSet=~CAP_NET_ADMIN +CapabilityBoundingSet=~CAP_NET_BIND_SERVICE +CapabilityBoundingSet=~CAP_SYS_PTRACE +CapabilityBoundingSet=~CAP_NET_RAW +PrivateTmp=true +RestrictAddressFamilies=~AF_NETLINK +RestrictAddressFamilies=~AF_PACKET +# These may break some tests, and should be enabled carefully +#NoNewPrivileges=true +#PrivateDevices=true +#ProtectControlGroups=true +# ProtectHome=true seems to reliably break the GH integration test with a lunar lxd on jammy host +#ProtectHome=true +#ProtectKernelModules=true +#ProtectKernelTunables=true +#ProtectSystem=full +#RestrictSUIDSGID=true +# Unsupported in bionic +# Suggestion from systemd.exec(5) manpage on SystemCallFilter +#SystemCallFilter=@system-service +#SystemCallFilter=~@mount +#SystemCallErrorNumber=EPERM +#ProtectClock=true +#ProtectKernelLogs=true From 47d8a0c114019bc70e16611fba724618c1d4cdeb Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Fri, 13 Oct 2023 20:32:15 +0000 Subject: [PATCH 06/22] Move temporary test files to /var/lib/ubuntu-advantage Due to the PrivateTmp=yes restriction set on the apt-news.service unit, tests that use the /tmp directory will fail. This happens because the unit will get a brand new and empty /tmp when it starts up, and anything it does in that /tmp will be gone when it terminates. --- features/apt_messages.feature | 2 +- features/attached_commands.feature | 16 ++++++++-------- features/attached_enable.feature | 2 +- features/attached_status.feature | 4 ++-- features/i18n.feature | 4 ++-- features/motd_messages.feature | 2 +- features/steps/contract.py | 7 +++++-- 7 files changed, 20 insertions(+), 17 deletions(-) diff --git a/features/apt_messages.feature b/features/apt_messages.feature index b3a47dd098..c04c039184 100644 --- a/features/apt_messages.feature +++ b/features/apt_messages.feature @@ -638,7 +638,7 @@ Feature: APT Messages """ "*Your Ubuntu Pro subscription has EXPIRED*\nRenew your subscription at https://ubuntu.com/pro/dashboard" """ - When I create the file `/tmp/machine-token-overlay.json` with the following: + When I create the file `/var/lib/ubuntu-advantage/machine-token-overlay.json` with the following: """ { "machineTokenInfo": { diff --git a/features/attached_commands.feature b/features/attached_commands.feature index 2421f45fc5..ae44a9ae11 100644 --- a/features/attached_commands.feature +++ b/features/attached_commands.feature @@ -298,7 +298,7 @@ Feature: Command behaviour when attached to an Ubuntu Pro subscription Scenario Outline: Attached status in a ubuntu machine with feature overrides Given a `` `` machine with ubuntu-advantage-tools installed - When I create the file `/tmp/machine-token-overlay.json` with the following: + When I create the file `/var/lib/ubuntu-advantage/machine-token-overlay.json` with the following: """ { "machineTokenInfo": { @@ -316,7 +316,7 @@ Feature: Command behaviour when attached to an Ubuntu Pro subscription And I append the following on uaclient config: """ features: - machine_token_overlay: "/tmp/machine-token-overlay.json" + machine_token_overlay: "/var/lib/ubuntu-advantage/machine-token-overlay.json" disable_auto_attach: true other: false """ @@ -332,7 +332,7 @@ Feature: Command behaviour when attached to an Ubuntu Pro subscription """ FEATURES disable_auto_attach: True - machine_token_overlay: /tmp/machine-token-overlay.json + machine_token_overlay: /var/lib/ubuntu-advantage/machine-token-overlay.json other: False """ When I run `pro status --all` as non-root @@ -346,7 +346,7 @@ Feature: Command behaviour when attached to an Ubuntu Pro subscription """ FEATURES disable_auto_attach: True - machine_token_overlay: /tmp/machine-token-overlay.json + machine_token_overlay: /var/lib/ubuntu-advantage/machine-token-overlay.json other: False """ When I run `pro detach --assume-yes` with sudo @@ -745,7 +745,7 @@ Feature: Command behaviour when attached to an Ubuntu Pro subscription Then I verify that `activityInfo.activityToken` value has been updated on the contract And I verify that `activityInfo.activityID` value has been updated on the contract # We are keeping this test to guarantee that the activityPingInterval is also updated - When I create the file `/tmp/machine-token-overlay.json` with the following: + When I create the file `/var/lib/ubuntu-advantage/machine-token-overlay.json` with the following: """ { "machineTokenInfo": { @@ -756,7 +756,7 @@ Feature: Command behaviour when attached to an Ubuntu Pro subscription } } """ - And I create the file `/tmp/response-overlay.json` with the following: + And I create the file `/var/lib/ubuntu-advantage/response-overlay.json` with the following: """ { "https://contracts.canonical.com/v1/contracts/testCID/machine-activity/testMID": [ @@ -773,8 +773,8 @@ Feature: Command behaviour when attached to an Ubuntu Pro subscription And I append the following on uaclient config: """ features: - machine_token_overlay: "/tmp/machine-token-overlay.json" - serviceclient_url_responses: "/tmp/response-overlay.json" + machine_token_overlay: "/var/lib/ubuntu-advantage/machine-token-overlay.json" + serviceclient_url_responses: "/var/lib/ubuntu-advantage/response-overlay.json" """ When I delete the file `/var/lib/ubuntu-advantage/jobs-status.json` And I run `python3 /usr/lib/ubuntu-advantage/timer.py` with sudo diff --git a/features/attached_enable.feature b/features/attached_enable.feature index 6e62b67ee8..ae48290461 100644 --- a/features/attached_enable.feature +++ b/features/attached_enable.feature @@ -81,7 +81,7 @@ Feature: Enable command behaviour when attached to an Ubuntu Pro subscription One moment, checking your subscription first Ubuntu Pro: ESM Infra is not available for Ubuntu .* """ - When I create the file `/tmp/machine-token-overlay.json` with the following: + When I create the file `/var/lib/ubuntu-advantage/machine-token-overlay.json` with the following: """ { "machineTokenInfo": { diff --git a/features/attached_status.feature b/features/attached_status.feature index 541d639e23..adb7345386 100644 --- a/features/attached_status.feature +++ b/features/attached_status.feature @@ -9,7 +9,7 @@ Feature: Attached status When I run `pro status --format yaml` as non-root Then stdout is a yaml matching the `ua_status` schema - When I create the file `/tmp/machine-token-overlay.json` with the following: + When I create the file `/var/lib/ubuntu-advantage/machine-token-overlay.json` with the following: """ { "machineTokenInfo": { @@ -22,7 +22,7 @@ Feature: Attached status And I append the following on uaclient config: """ features: - machine_token_overlay: "/tmp/machine-token-overlay.json" + machine_token_overlay: "/var/lib/ubuntu-advantage/machine-token-overlay.json" """ And I run `pro status` with sudo Then stdout contains substring: diff --git a/features/i18n.feature b/features/i18n.feature index 39969f7875..ef4cdfbc7e 100644 --- a/features/i18n.feature +++ b/features/i18n.feature @@ -160,7 +160,7 @@ Feature: Pro supports multiple languages Then stdout is a json matching the `ua_status` schema When I run `pro status --format yaml` as non-root Then stdout is a yaml matching the `ua_status` schema - When I create the file `/tmp/machine-token-overlay.json` with the following: + When I create the file `/var/lib/ubuntu-advantage/machine-token-overlay.json` with the following: """ { "machineTokenInfo": { @@ -173,7 +173,7 @@ Feature: Pro supports multiple languages And I append the following on uaclient config: """ features: - machine_token_overlay: "/tmp/machine-token-overlay.json" + machine_token_overlay: "/var/lib/ubuntu-advantage/machine-token-overlay.json" """ And I run `pro status` with sudo Then stdout contains substring: diff --git a/features/motd_messages.feature b/features/motd_messages.feature index ba40e3cfb2..c04901b43d 100644 --- a/features/motd_messages.feature +++ b/features/motd_messages.feature @@ -119,7 +119,7 @@ Feature: MOTD Messages Renew your subscription at https:\/\/ubuntu.com\/pro\/dashboard """ - When I create the file `/tmp/machine-token-overlay.json` with the following: + When I create the file `/var/lib/ubuntu-advantage/machine-token-overlay.json` with the following: """ { "machineTokenInfo": { diff --git a/features/steps/contract.py b/features/steps/contract.py index 0df23d65d2..3238331711 100644 --- a/features/steps/contract.py +++ b/features/steps/contract.py @@ -124,10 +124,13 @@ def when_i_set_the_machine_token_overlay(context): yaml.safe_load(context.text), cls=util.DatetimeAwareJSONEncoder ) when_i_create_file_with_content( - context, "/tmp/machine-token-overlay.json", text=json_text + context, + "/var/lib/ubuntu-advantage/machine-token-overlay.json", + text=json_text, ) change_config_key_to_use_value( context, "features", - "{ machine_token_overlay: /tmp/machine-token-overlay.json}", + "{ machine_token_overlay: " + "/var/lib/ubuntu-advantage/machine-token-overlay.json}", ) From 9fa0d98028a59491e1fd324f56ab443e90f9c6ec Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Mon, 27 Nov 2023 16:30:14 -0300 Subject: [PATCH 07/22] test: apt-news starts in the background, give it some time --- features/apt_messages.feature | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/features/apt_messages.feature b/features/apt_messages.feature index c04c039184..dad4c328dc 100644 --- a/features/apt_messages.feature +++ b/features/apt_messages.feature @@ -335,6 +335,8 @@ Feature: APT Messages When I run `rm -rf /var/lib/ubuntu-advantage/messages` with sudo When I run `rm /var/lib/apt/periodic/update-success-stamp` with sudo When I run `apt-get update` with sudo + # the apt-news.service unit runs in the background, give it some time to fetch the json file + When I wait `5` seconds When I run `apt upgrade` with sudo Then I will see the following on stdout """ @@ -567,6 +569,8 @@ Feature: APT Messages # test that apt update will trigger hook to update apt_news for local override When I run `rm -f /var/lib/apt/periodic/update-success-stamp` with sudo When I run `apt-get update` with sudo + # the apt-news.service unit runs in the background, give it some time to fetch the json file + When I wait `5` seconds When I run `apt upgrade` with sudo Then I will see the following on stdout """ From 7a07c40116b76e8592e8ec899a0f7c2c652b2be8 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Fri, 15 Dec 2023 11:05:39 -0300 Subject: [PATCH 08/22] Check the syntax of the rendered apparmor profile --- debian/control | 1 + debian/rules | 2 ++ 2 files changed, 3 insertions(+) diff --git a/debian/control b/debian/control index 105a145e96..78e3a08033 100644 --- a/debian/control +++ b/debian/control @@ -7,6 +7,7 @@ Build-Depends: bash-completion, debianutils (>= 4.7), dh-python, dh-apparmor, + apparmor, # After debhelper 13.3 we no longer need dh-systemd. # On hirsute and later, dh-systemd doesn't even exist. # On recent releases, the first alternative will be used. diff --git a/debian/rules b/debian/rules index 70030fdb86..4462fd0e51 100755 --- a/debian/rules +++ b/debian/rules @@ -64,6 +64,8 @@ override_dh_systemd_start: override_dh_auto_install: dh_auto_install --destdir=debian/ubuntu-advantage-tools debian/jinja2_render debian/apparmor/ubuntu_advantage_apt_news.jinja2 debian/apparmor/ubuntu_advantage_apt_news ubuntu_codename=${UBUNTU_CODENAME} + # quick syntax check on the generated profile + apparmor_parser -K -T -Q debian/apparmor/ubuntu_advantage_apt_news install -D -m 644 $(CURDIR)/debian/apparmor/ubuntu_advantage_apt_news $(CURDIR)/debian/ubuntu-advantage-tools/etc/apparmor.d/ubuntu_advantage_apt_news dh_apparmor -pubuntu-advantage-tools --profile-name=ubuntu_advantage_apt_news From 2888c630d5baefbf0c6b190997cd960aa7956508 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sun, 24 Dec 2023 17:19:16 -0300 Subject: [PATCH 09/22] d/apparmor/ubuntu_pro_apt_news.jinja2: renamed from ubuntu_advantage_* --- ...ntu_advantage_apt_news.jinja2 => ubuntu_pro_apt_news.jinja2} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename debian/apparmor/{ubuntu_advantage_apt_news.jinja2 => ubuntu_pro_apt_news.jinja2} (95%) diff --git a/debian/apparmor/ubuntu_advantage_apt_news.jinja2 b/debian/apparmor/ubuntu_pro_apt_news.jinja2 similarity index 95% rename from debian/apparmor/ubuntu_advantage_apt_news.jinja2 rename to debian/apparmor/ubuntu_pro_apt_news.jinja2 index 5bc72b2b72..348d1176f8 100644 --- a/debian/apparmor/ubuntu_advantage_apt_news.jinja2 +++ b/debian/apparmor/ubuntu_pro_apt_news.jinja2 @@ -3,7 +3,7 @@ abi , {% endif %} include -profile ubuntu_advantage_apt_news flags=(attach_disconnected) { +profile ubuntu_pro_apt_news flags=(attach_disconnected) { include include include From cab7b498f2e486c5d32371952fae5acba290787a Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sun, 24 Dec 2023 17:19:44 -0300 Subject: [PATCH 10/22] d/rules: rename apparmor profile: ubuntu_advantage_apt_news to ubuntu_pro_apt_news --- debian/rules | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/debian/rules b/debian/rules index 4462fd0e51..5fcf5f88fa 100755 --- a/debian/rules +++ b/debian/rules @@ -63,11 +63,11 @@ override_dh_systemd_start: override_dh_auto_install: dh_auto_install --destdir=debian/ubuntu-advantage-tools - debian/jinja2_render debian/apparmor/ubuntu_advantage_apt_news.jinja2 debian/apparmor/ubuntu_advantage_apt_news ubuntu_codename=${UBUNTU_CODENAME} + debian/jinja2_render debian/apparmor/ubuntu_pro_apt_news.jinja2 debian/apparmor/ubuntu_pro_apt_news ubuntu_codename=${UBUNTU_CODENAME} # quick syntax check on the generated profile - apparmor_parser -K -T -Q debian/apparmor/ubuntu_advantage_apt_news - install -D -m 644 $(CURDIR)/debian/apparmor/ubuntu_advantage_apt_news $(CURDIR)/debian/ubuntu-advantage-tools/etc/apparmor.d/ubuntu_advantage_apt_news - dh_apparmor -pubuntu-advantage-tools --profile-name=ubuntu_advantage_apt_news + apparmor_parser -K -T -Q debian/apparmor/ubuntu_pro_apt_news + install -D -m 644 $(CURDIR)/debian/apparmor/ubuntu_pro_apt_news $(CURDIR)/debian/ubuntu-advantage-tools/etc/apparmor.d/ubuntu_pro_apt_news + dh_apparmor -pubuntu-advantage-tools --profile-name=ubuntu_pro_apt_news flist=$$(find $(CURDIR)/debian/ -type f -name version.py) && sed -i 's,@@PACKAGED_VERSION@@,$(DEB_VERSION),' $${flist:-did-not-find-version-py-for-replacement} @@ -92,4 +92,4 @@ endif override_dh_auto_clean: dh_auto_clean make clean - rm -f debian/apparmor/ubuntu_advantage_apt_news + rm -f debian/apparmor/ubuntu_pro_apt_news From 00a1610c23c16b41b1906cdeb0ba8a61fc04d1e4 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sun, 24 Dec 2023 17:19:56 -0300 Subject: [PATCH 11/22] systemd/apt-news.service: rename apparmor profile: ubuntu_advantage_apt_news to ubuntu_pro_apt_news --- systemd/apt-news.service | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/systemd/apt-news.service b/systemd/apt-news.service index 3dcf1a3650..2bab966bdd 100644 --- a/systemd/apt-news.service +++ b/systemd/apt-news.service @@ -14,7 +14,7 @@ Description=Update APT News [Service] Type=oneshot ExecStart=/usr/bin/python3 /usr/lib/ubuntu-advantage/apt_news.py -AppArmorProfile=ubuntu_advantage_apt_news +AppArmorProfile=ubuntu_pro_apt_news CapabilityBoundingSet=~CAP_SYS_ADMIN CapabilityBoundingSet=~CAP_NET_ADMIN CapabilityBoundingSet=~CAP_NET_BIND_SERVICE From 64698190d5a18c6379cbd947e73c04fe630945bb Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Mon, 25 Dec 2023 11:23:17 -0300 Subject: [PATCH 12/22] apparmor apt_news: allow /proc/pid/status for older ubuntu releases --- debian/apparmor/ubuntu_pro_apt_news.jinja2 | 1 + 1 file changed, 1 insertion(+) diff --git a/debian/apparmor/ubuntu_pro_apt_news.jinja2 b/debian/apparmor/ubuntu_pro_apt_news.jinja2 index 348d1176f8..7bb80902e8 100644 --- a/debian/apparmor/ubuntu_pro_apt_news.jinja2 +++ b/debian/apparmor/ubuntu_pro_apt_news.jinja2 @@ -43,6 +43,7 @@ profile ubuntu_pro_apt_news flags=(attach_disconnected) { /sbin/ldconfig rix, /sbin/ldconfig.real rix, @{PROC}/@{pid}/mounts r, + @{PROC}/@{pid}/status r, /usr/bin/@{multiarch}-gcc-* rix, /usr/bin/@{multiarch}-ld.bfd rix, /usr/lib/gcc/@{multiarch}/*/collect2 rix, From dd24fd8cf2cab9670365e1815d58e30e070b504d Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sat, 30 Dec 2023 22:21:24 -0300 Subject: [PATCH 13/22] apparmor apt-news: updates for xenial --- debian/apparmor/ubuntu_pro_apt_news.jinja2 | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/debian/apparmor/ubuntu_pro_apt_news.jinja2 b/debian/apparmor/ubuntu_pro_apt_news.jinja2 index 7bb80902e8..4a2165c11f 100644 --- a/debian/apparmor/ubuntu_pro_apt_news.jinja2 +++ b/debian/apparmor/ubuntu_pro_apt_news.jinja2 @@ -48,5 +48,16 @@ profile ubuntu_pro_apt_news flags=(attach_disconnected) { /usr/bin/@{multiarch}-ld.bfd rix, /usr/lib/gcc/@{multiarch}/*/collect2 rix, /usr/bin/@{multiarch}-objdump rix, + + # for some reason, these are not needed in later ubuntu releases + capability chown, + capability fowner, + capability dac_override, + + /etc/apt/auth.conf.d/90ubuntu-advantage rw, + /var/lib/apt/lists/partial/ rw, + /var/lib/apt/lists/partial/* rw, + /var/cache/apt/archives/partial/ rw, + /var/cache/apt/archives/partial/* rw, {% endif %} } From a6b0ce3ceafd0a80fdb7a4e348ea8bbd4b355e32 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sun, 31 Dec 2023 18:05:07 -0300 Subject: [PATCH 14/22] apparmor apt-news: distinguish xenial and bionic where applicable --- debian/apparmor/ubuntu_pro_apt_news.jinja2 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/debian/apparmor/ubuntu_pro_apt_news.jinja2 b/debian/apparmor/ubuntu_pro_apt_news.jinja2 index 4a2165c11f..9c0c059b74 100644 --- a/debian/apparmor/ubuntu_pro_apt_news.jinja2 +++ b/debian/apparmor/ubuntu_pro_apt_news.jinja2 @@ -48,8 +48,9 @@ profile ubuntu_pro_apt_news flags=(attach_disconnected) { /usr/bin/@{multiarch}-ld.bfd rix, /usr/lib/gcc/@{multiarch}/*/collect2 rix, /usr/bin/@{multiarch}-objdump rix, - - # for some reason, these are not needed in later ubuntu releases +{% endif %} +{% if ubuntu_codename in ["xenial"] %} + # for some reason, these were just needed in xenial capability chown, capability fowner, capability dac_override, From 3e800c0703b7a4374a88992a426542dd9a20c457 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sun, 24 Dec 2023 17:51:54 -0300 Subject: [PATCH 15/22] actions.py: write apparmor logs to apparmor_logs.txt --- uaclient/actions.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/uaclient/actions.py b/uaclient/actions.py index 7f4deaff90..f8b14ed9cf 100644 --- a/uaclient/actions.py +++ b/uaclient/actions.py @@ -2,6 +2,8 @@ import glob import logging import os +import re +import shutil from typing import List, Optional # noqa: F401 from uaclient import ( @@ -44,6 +46,11 @@ USER_LOG_COLLECTED_LIMIT = 10 +# XXX handle /etc/apparmor.d/local/, which has the local admin overrides +APPARMOR_PROFILES = [ + "/etc/apparmor.d/ubuntu_pro_apt_news", +] + def attach_with_token( cfg: config.UAConfig, token: str, allow_enable: bool @@ -285,3 +292,30 @@ def collect_logs(cfg: config.UAConfig, output_dir: str): system.write_file( os.path.join(output_dir, os.path.basename(f)), content ) + + # get apparmor logs + # can't use journalctl's --grep, because xenial doesn't support it :/ + kernel_logs, _ = system.subp( + ["journalctl", "-b", "-k", "--since=1 day ago"] + ) + apparmor_logs = [] + if kernel_logs: + # filter out only what interests us + for kernel_line in kernel_logs.split("\n"): + if re.search( + r"apparmor=\".*(profile=\"ubuntu_pro_|name=\"ubuntu_pro_)", + kernel_line, + ): + apparmor_logs.append(kernel_line) + system.write_file( + "{}/apparmor_logs.txt".format(output_dir), "\n".join(apparmor_logs) + ) + + # include apparmor profiles + for f in APPARMOR_PROFILES: + if os.path.isfile(f): + try: + shutil.copy(f, output_dir) + except Exception as e: + LOG.warning("Failed to copy file: %s\n%s", f, str(e)) + continue From e9a3343ad7459a85475c3fe34f1cabac7c9f70ac Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sun, 31 Dec 2023 18:18:16 -0300 Subject: [PATCH 16/22] test_actions: adjust test for new apparmor log --- uaclient/tests/test_actions.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/uaclient/tests/test_actions.py b/uaclient/tests/test_actions.py index cce0266672..1a514a8adc 100644 --- a/uaclient/tests/test_actions.py +++ b/uaclient/tests/test_actions.py @@ -8,6 +8,12 @@ from uaclient.testing import fakes, helpers M_PATH = "uaclient.actions." +APPARMOR_DENIED = ( + 'audit: type=1400 audit(1703513431.601:36): apparmor="DENIED" ' + 'operation="open" profile="ubuntu_pro_apt_news" ' + 'name="/proc/1422/status" pid=1422 comm="python3" ' + 'requested_mask="r" denied_mask="r" fsuid=0 ouid=0' +) def fake_instance_factory(): @@ -335,8 +341,10 @@ class TestCollectLogs: @mock.patch("uaclient.actions._get_state_files") @mock.patch("glob.glob") @mock.patch("uaclient.log.get_user_log_file") + @mock.patch("uaclient.system.subp", return_value=(APPARMOR_DENIED, "")) def test_collect_logs_invalid_file( self, + m_system_subp, m_get_user, m_glob, m_get_state_files, @@ -362,11 +370,19 @@ def test_collect_logs_invalid_file( mock.call("a"), mock.call("b"), ] == m_load_file.call_args_list - assert 2 == m_write_file.call_count + assert 3 == m_write_file.call_count + + # apparmor checks + assert 1 == m_system_subp.call_count + assert [ + mock.call(["journalctl", "-b", "-k", "--since=1 day ago"]), + ] == m_system_subp.call_args_list + print(m_write_file.call_args_list) assert [ mock.call("test/user0.log", "test"), mock.call("test/b", "test"), + mock.call("test/apparmor_logs.txt", APPARMOR_DENIED), ] == m_write_file.call_args_list assert [ mock.call("Failed to load file: %s\n%s", "a", "test") From 347c484a9e2127bb7ef7ee69f8a88782344b21c1 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Sun, 31 Dec 2023 19:25:53 -0300 Subject: [PATCH 17/22] test_cli_collect_logs.py: adjust test for new apparmor files --- uaclient/cli/tests/test_cli_collect_logs.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/uaclient/cli/tests/test_cli_collect_logs.py b/uaclient/cli/tests/test_cli_collect_logs.py index e4ced3331e..60c523ddef 100644 --- a/uaclient/cli/tests/test_cli_collect_logs.py +++ b/uaclient/cli/tests/test_cli_collect_logs.py @@ -4,6 +4,7 @@ import mock import pytest +from uaclient.actions import APPARMOR_PROFILES from uaclient.cli import ( action_collect_logs, collect_logs_parser, @@ -62,6 +63,7 @@ def test_collect_logs_help( @mock.patch("pathlib.Path.stat") @mock.patch("os.chown") @mock.patch("os.path.isfile", return_value=True) + @mock.patch("shutil.copy") @mock.patch("uaclient.system.write_file") @mock.patch("uaclient.system.load_file") @mock.patch("uaclient.system.subp", return_value=(None, None)) @@ -74,6 +76,7 @@ def test_collect_logs( m_subp, _load_file, _write_file, + m_shutilcopy, m_isfile, _chown, _stat, @@ -92,7 +95,7 @@ def test_collect_logs( tmpdir.join("user1-log").strpath, tmpdir.join("user2-log").strpath, ] - is_file_calls = 17 + is_file_calls = 17 + len(APPARMOR_PROFILES) user_log_files = [mock.call(m_get_user())] if util_we_are_currently_root(): user_log_files = [ @@ -161,6 +164,7 @@ def test_collect_logs( mock.call( ["systemctl", "status", "ubuntu-advantage.service"], rcs=[0, 3] ), + mock.call(["journalctl", "-b", "-k", "--since=1 day ago"]), ] assert m_isfile.call_count == is_file_calls @@ -182,8 +186,13 @@ def test_collect_logs( mock.call("/etc/apt/sources.list.d/ubuntu-ros-updates.list"), mock.call("/var/log/ubuntu-advantage.log"), mock.call("/var/log/ubuntu-advantage.log.1"), + *[mock.call(f) for f in APPARMOR_PROFILES], ] - assert redact.call_count == is_file_calls + len(user_log_files) + # APPARMOR_PROFILES are not redacted + assert redact.call_count == is_file_calls + len(user_log_files) - len( + APPARMOR_PROFILES + ) + assert m_shutilcopy.call_count == len(APPARMOR_PROFILES) class TestParser: From 48696956db349b3af2af11e0a6292be95ce5f14e Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Mon, 1 Jan 2024 14:29:44 -0300 Subject: [PATCH 18/22] apport: include apparmor logs and profiles --- apport/source_ubuntu-advantage-tools.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apport/source_ubuntu-advantage-tools.py b/apport/source_ubuntu-advantage-tools.py index 6489d1db33..d66b610e18 100644 --- a/apport/source_ubuntu-advantage-tools.py +++ b/apport/source_ubuntu-advantage-tools.py @@ -3,7 +3,7 @@ from apport.hookutils import attach_file_if_exists from uaclient import defaults -from uaclient.actions import collect_logs +from uaclient.actions import collect_logs, APPARMOR_PROFILES from uaclient.config import UAConfig @@ -11,6 +11,7 @@ def add_info(report, ui=None): report["LaunchpadPrivate"] = "1" report["LaunchpadSubscribe"] = "ua-client" cfg = UAConfig() + apparmor_files = [os.path.basename(f) for f in APPARMOR_PROFILES] with tempfile.TemporaryDirectory() as output_dir: collect_logs(cfg, output_dir) auto_include_log_files = { @@ -21,6 +22,8 @@ def add_info(report, ui=None): "livepatch-status.txt", "livepatch-status.txt-error", "pro-journal.txt", + "apparmor_logs.txt", + *apparmor_files, os.path.basename(cfg.cfg_path), os.path.basename(cfg.log_file), os.path.basename(cfg.data_path("jobs-status")), From aef745c8918230b145fc2fafc8094d5a5031adbb Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Wed, 3 Jan 2024 17:36:50 -0300 Subject: [PATCH 19/22] actions.py: handle exceptions when gathering kernel logs --- uaclient/actions.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/uaclient/actions.py b/uaclient/actions.py index f8b14ed9cf..feb399b1c6 100644 --- a/uaclient/actions.py +++ b/uaclient/actions.py @@ -295,9 +295,13 @@ def collect_logs(cfg: config.UAConfig, output_dir: str): # get apparmor logs # can't use journalctl's --grep, because xenial doesn't support it :/ - kernel_logs, _ = system.subp( - ["journalctl", "-b", "-k", "--since=1 day ago"] - ) + try: + kernel_logs, _ = system.subp( + ["journalctl", "-b", "-k", "--since=1 day ago"] + ) + except exceptions.ProcessExecutionError as e: + LOG.warning("Failed to collect kernel logs:\n%s", str(e)) + kernel_logs = None apparmor_logs = [] if kernel_logs: # filter out only what interests us From ab101e9fa43e29970241677f730e635346394f63 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Thu, 4 Jan 2024 10:08:38 -0300 Subject: [PATCH 20/22] Move APPARMOR_PROFILES to defaults Most or all of the settings are defined there, and since that module doesn't import anything, this also avoids potential future dependencies. --- uaclient/actions.py | 6 +----- uaclient/cli/tests/test_cli_collect_logs.py | 2 +- uaclient/defaults.py | 5 +++++ 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/uaclient/actions.py b/uaclient/actions.py index feb399b1c6..c511a015f5 100644 --- a/uaclient/actions.py +++ b/uaclient/actions.py @@ -23,6 +23,7 @@ CLOUD_BUILD_INFO, DEFAULT_CONFIG_FILE, DEFAULT_LOG_PREFIX, + APPARMOR_PROFILES, ) from uaclient.files.state_files import ( AttachmentData, @@ -46,11 +47,6 @@ USER_LOG_COLLECTED_LIMIT = 10 -# XXX handle /etc/apparmor.d/local/, which has the local admin overrides -APPARMOR_PROFILES = [ - "/etc/apparmor.d/ubuntu_pro_apt_news", -] - def attach_with_token( cfg: config.UAConfig, token: str, allow_enable: bool diff --git a/uaclient/cli/tests/test_cli_collect_logs.py b/uaclient/cli/tests/test_cli_collect_logs.py index 60c523ddef..df7592fda0 100644 --- a/uaclient/cli/tests/test_cli_collect_logs.py +++ b/uaclient/cli/tests/test_cli_collect_logs.py @@ -4,13 +4,13 @@ import mock import pytest -from uaclient.actions import APPARMOR_PROFILES from uaclient.cli import ( action_collect_logs, collect_logs_parser, get_parser, main, ) +from uaclient.defaults import APPARMOR_PROFILES M_PATH = "uaclient.cli." diff --git a/uaclient/defaults.py b/uaclient/defaults.py index 62e91e2aa2..0b825bc553 100644 --- a/uaclient/defaults.py +++ b/uaclient/defaults.py @@ -60,3 +60,8 @@ USER_CACHE_SUBDIR = "ubuntu-pro" SSL_CERTS_PATH = "/etc/ssl/certs/ca-certificates.crt" + +# used by apport, collect-logs, and tests +APPARMOR_PROFILES = [ + "/etc/apparmor.d/ubuntu_pro_apt_news", +] From b67f8872cfdd9a05f13c6883327fba1688f459e8 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Thu, 4 Jan 2024 10:16:34 -0300 Subject: [PATCH 21/22] Local black run is happy, but not github's --- uaclient/actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uaclient/actions.py b/uaclient/actions.py index c511a015f5..8e1c2dda2a 100644 --- a/uaclient/actions.py +++ b/uaclient/actions.py @@ -20,10 +20,10 @@ from uaclient.clouds import AutoAttachCloudInstance # noqa: F401 from uaclient.clouds import identity from uaclient.defaults import ( + APPARMOR_PROFILES, CLOUD_BUILD_INFO, DEFAULT_CONFIG_FILE, DEFAULT_LOG_PREFIX, - APPARMOR_PROFILES, ) from uaclient.files.state_files import ( AttachmentData, From 77b4d75b9c472a6111b572dd2a7cd7a69b31a0b0 Mon Sep 17 00:00:00 2001 From: Andreas Hasenack Date: Thu, 4 Jan 2024 11:25:39 -0300 Subject: [PATCH 22/22] actions.py: function to get apparmor logs --- uaclient/actions.py | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/uaclient/actions.py b/uaclient/actions.py index 8e1c2dda2a..bf7b457230 100644 --- a/uaclient/actions.py +++ b/uaclient/actions.py @@ -181,6 +181,30 @@ def status( return status, ret +def _write_apparmor_logs_to_file(filename: str) -> None: + """ + Helper which gets ubuntu_pro apparmor logs from the kernel from the last + day and writes them to the specified filename. + """ + # can't use journalctl's --grep, because xenial doesn't support it :/ + cmd = ["journalctl", "-b", "-k", "--since=1 day ago"] + apparmor_re = r"apparmor=\".*(profile=\"ubuntu_pro_|name=\"ubuntu_pro_)" + kernel_logs = None + try: + kernel_logs, _ = system.subp(cmd) + except exceptions.ProcessExecutionError as e: + LOG.warning("Failed to collect kernel logs:\n%s", str(e)) + system.write_file("{}-error".format(filename), str(e)) + else: + if kernel_logs: # some unit tests mock subp to return (None,None) + apparmor_logs = [] + # filter out only what interests us + for kernel_line in kernel_logs.split("\n"): + if re.search(apparmor_re, kernel_line): + apparmor_logs.append(kernel_line) + system.write_file(filename, "\n".join(apparmor_logs)) + + def _write_command_output_to_file( cmd, filename: str, return_codes: Optional[List[int]] = None ) -> None: @@ -290,26 +314,7 @@ def collect_logs(cfg: config.UAConfig, output_dir: str): ) # get apparmor logs - # can't use journalctl's --grep, because xenial doesn't support it :/ - try: - kernel_logs, _ = system.subp( - ["journalctl", "-b", "-k", "--since=1 day ago"] - ) - except exceptions.ProcessExecutionError as e: - LOG.warning("Failed to collect kernel logs:\n%s", str(e)) - kernel_logs = None - apparmor_logs = [] - if kernel_logs: - # filter out only what interests us - for kernel_line in kernel_logs.split("\n"): - if re.search( - r"apparmor=\".*(profile=\"ubuntu_pro_|name=\"ubuntu_pro_)", - kernel_line, - ): - apparmor_logs.append(kernel_line) - system.write_file( - "{}/apparmor_logs.txt".format(output_dir), "\n".join(apparmor_logs) - ) + _write_apparmor_logs_to_file("{}/apparmor_logs.txt".format(output_dir)) # include apparmor profiles for f in APPARMOR_PROFILES: