Merge ~locnnil/ubuntu/+source/linuxptp:oracular-linuxptp-add-apparmor into ubuntu/+source/linuxptp:ubuntu/devel

[Lincoln Wallace]

Add AppArmor rules for linuxptp services (ptp4l, timemaster and phc2sys).

Related to LP: #2083458

[Andreas Hasenack (ahasenack)]

Comments inline, and here.

Commit db64e3ff9c7a60b8164b2dab035add4587d6f177 is also changing d/usr.sbin.ptp4l, which is not mentioned in the commit message. This of course does not change the final outcome, but since you have split commits, maybe you would prefer to move that ptp4l change to commit f1788a09e84df88d2c142a34ab6b5ad110ddb050 which is the one adding that apparmor profile.

review: Needs Fixing

[Lincoln Wallace (locnnil)]

Addressed review! Awaiting the next review.

[Lincoln Wallace (locnnil)]

Addressed review! I've put back the comment about failure on armhf. Awaiting the next review.

[Andreas Hasenack (ahasenack)]

review: Needs Fixing

[Lincoln Wallace (locnnil)]

Added 'needs-sudo' capability! Ready for review.

[Farshid Tavakolizadeh (farshidtz)]

This change isn't needed for Oracular. I suggest we hold until we perform more testing. LinuxPTP is used in a wide range of applications (incl. TSN), some of which aren't covered by the autopkgtests and the simulated environment.

review: Abstain

[Andreas Hasenack (ahasenack)]

For plucky, we could put this profile in complain mode, but how would we be getting the apparmor logs showing what would have been denied...

I agree a lot of testing is required. We can upload this to plucky, and keep a watchful eye out for bugs while it's in the devel release. That sounds like a good approach to me, if we can react quickly to breakages while in the devel release.

I'll run the dep8 tests of this branch in the meantime in the real infrastructure. If you want to proceed with the plucky upload, and assuming all tests pass as is, then the only change needed is the ubuntu release name in d/changelog.

[Alex Murray (alexmurray)]

From a security team perspective, these profiles look quite good - just a few minor nitpicks and stylistic things that need addressing. Thanks :)

review: Needs Fixing

a7bdc8c... by Lincoln Wallace on 2024-12-09

changelog

Signed-off-by: Lincoln Wallace <email address hidden>

[Alex Murray (alexmurray)]

Thanks Lincoln - apologies I just noticed a couple other minor issues.

review: Needs Fixing

[Lincoln Wallace (locnnil)]

> Thanks Lincoln - apologies I just noticed a couple other minor issues.

No problem at all! Fixed!

Let me know if you find some other improvement that I could do, and I will be happy to help! Thank you very much, and have a great day!

[Alex Murray (alexmurray)]

Thanks again - LGTM!

review: Approve

Unmerged commits

a7bdc8c... by Lincoln Wallace on 2024-12-09

changelog

Signed-off-by: Lincoln Wallace <email address hidden>

9853a0c... by Lincoln Wallace on 2024-09-09

* Add AppArmor rules for phc2sys (ref LP #2083458)

  - d/linuxptp.install: Add aa rules destdir for phc2sys
  - d/rules: Add phc2sys aa profile entry
  - d/tests/simulation-test-suite: Insert rule on aa profile for autopkgtest
  - d/usr.sbin.phc2sys: aa profile for phc2sys service

Signed-off-by: Lincoln Wallace <email address hidden>

af3bf4c... by Lincoln Wallace on 2024-12-09

* Add AppArmor rules for timemaster (ref LP #2083458)

  - d/linuxptp.install: Add aa rules destdir for timemaster
  - d/rules: Add timemaster aa profile entry
  - d/usr.sbin.timemaster: aa profile for timemaster service

Signed-off-by: Lincoln Wallace <email address hidden>

89b1525... by Lincoln Wallace on 2024-12-09

* Add AppArmor rules for ptp4l (LP: #2083458)

  - d/control: Add dh-apparmor dependency
  - d/linuxptp.install: Add aa rules destdir
  - d/rules: Add section for aa profile adition
  - d/tests/simulation-test-suite: Add script function to inject additional local
      aa rules needed for autopkgtest
  - d/tests/control: Add needs-sudo capability
  - d/usr.sbin.ptp4l: aa profile for ptp4l application

Signed-off-by: Lincoln Wallace <email address hidden>

diff --git a/debian/changelog b/debian/changelog
index 6de9fd8..920f51a 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,27 @@
+linuxptp (4.2-1ubuntu2) plucky; urgency=medium
+
+  * Add AppArmor rules for ptp4l (LP: #2083458)
+    - d/control: Add dh-apparmor dependency
+    - d/linuxptp.install: Add aa rules destdir
+    - d/rules: Add section for aa profile adition
+    - d/tests/simulation-test-suite: Add script function to inject additional local
+        aa rules needed for autopkgtest
+    - d/tests/control: Add needs-sudo capability
+    - d/usr.sbin.ptp4l: aa profile for ptp4l application
+
+  * Add AppArmor rules for timemaster (ref LP #2083458)
+    - d/linuxptp.install: Add aa rules destdir for timemaster
+    - d/rules: Add timemaster aa profile entry
+    - d/usr.sbin.timemaster: aa profile for timemaster service
+
+  * Add AppArmor rules for phc2sys (ref LP #2083458)
+    - d/linuxptp.install: Add aa rules destdir for phc2sys
+    - d/rules: Add phc2sys aa profile entry
+    - d/tests/simulation-test-suite: Insert rule on aa profile for autopkgtest
+    - d/usr.sbin.phc2sys: aa profile for phc2sys service
+
+ -- Lincoln Wallace <[e-mail]>  Wed, 02 Oct 2024 15:50:17 -0300
+
 linuxptp (4.2-1ubuntu1) oracular; urgency=medium
 
   * Merge with Debian unstable (LP: #2073737). Remaining changes:
diff --git a/debian/control b/debian/control
index efefa1f..74e587c 100644
--- a/debian/control
+++ b/debian/control
@@ -9,6 +9,7 @@ Uploaders:
 Build-Depends:
  debhelper-compat (= 13),
  dh-exec,
+ dh-apparmor,
 Standards-Version: 4.6.2
 Rules-Requires-Root: no
 Homepage: http://linuxptp.sourceforge.net/
diff --git a/debian/linuxptp.install b/debian/linuxptp.install
index ad008d9..f0706a0 100755
--- a/debian/linuxptp.install
+++ b/debian/linuxptp.install
@@ -1,3 +1,6 @@
 #!/usr/bin/dh-exec
 configs/default.cfg => /etc/linuxptp/ptp4l.conf
 debian/timemaster.conf /etc/linuxptp/
+debian/usr.sbin.ptp4l etc/apparmor.d
+debian/usr.sbin.timemaster etc/apparmor.d
+debian/usr.sbin.phc2sys etc/apparmor.d
diff --git a/debian/rules b/debian/rules
index 38cfc04..01e297f 100755
--- a/debian/rules
+++ b/debian/rules
@@ -12,6 +12,11 @@ export DH_VERBOSE = 1
 override_dh_auto_install:
 	dh_auto_install -- prefix=/usr mandir=/usr/share/man
 
+execute_after_dh_install:
+	dh_apparmor --profile-name=usr.sbin.ptp4l -plinuxptp
+	dh_apparmor --profile-name=usr.sbin.timemaster -plinuxptp
+	dh_apparmor --profile-name=usr.sbin.phc2sys -plinuxptp
+
 override_dh_installsystemd:
 	dh_installsystemd --no-enable --no-start --name=ptp4l@
 	dh_installsystemd --no-enable --no-start --name=phc2sys@
diff --git a/debian/tests/control b/debian/tests/control
index c367202..147f518 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -1,3 +1,3 @@
 Tests: simulation-test-suite
 Depends: wget, g++, make, linuxptp
-Restrictions: allow-stderr, isolation-container, build-needed, skippable, needs-internet
+Restrictions: allow-stderr, isolation-container, build-needed, skippable, needs-internet, needs-sudo
diff --git a/debian/tests/simulation-test-suite b/debian/tests/simulation-test-suite
index 1abb4bd..0649aa0 100755
--- a/debian/tests/simulation-test-suite
+++ b/debian/tests/simulation-test-suite
@@ -5,6 +5,28 @@ clknetsim_ver=0a11a35
 clknetsim_src=https://github.com/mlichvar/clknetsim/archive/"$clknetsim_ver"/clknetsim-"$clknetsim_ver".tar.gz
 clknetsim_archive=$(basename "$clknetsim_src")
 
+inject_local_aa() {
+  app="$1"
+  apparmor_profile=/etc/apparmor.d/usr.sbin.${app}
+  echo "-- Injecting complementary AppArmor rules for $app"
+
+  sudo -E bash -c "
+    if [ -f '$apparmor_profile' ]; then
+      if aa-status --enabled 2>/dev/null; then
+        cat <<EOF >>/etc/apparmor.d/local/usr.sbin.'${app}'
+/tmp/autopkgtest*/** rwm,
+/proc/[0-9]*/comm r,
+EOF
+        apparmor_parser -r -W -T '${apparmor_profile}' || {
+          echo 'Failed to reload the ${apparmor_profile} AppArmor profile, continuing anyway.'
+        }
+      fi
+    else
+      echo 'AppArmor profile for $app does not exist, skipping.'
+    fi
+  "
+}
+
 # Always use the same seed to get deterministic results
 export CLKNETSIM_RANDOM_SEED=24508
 
@@ -25,6 +47,9 @@ echo "-- Building clknetsim"
 make
 echo ""
 
+inject_local_aa "ptp4l"
+inject_local_aa "phc2sys"
+
 echo "-- Running test-suite"
 cd "$testdir"
 ./run
diff --git a/debian/usr.sbin.phc2sys b/debian/usr.sbin.phc2sys
new file mode 100644
index 0000000..05d30e3
--- /dev/null
+++ b/debian/usr.sbin.phc2sys
@@ -0,0 +1,44 @@
+
+# vim:syntax=apparmor
+# Last Modified: Sun Sep 05 16:48:05 2021
+
+abi <abi/4.0>,
+include <tunables/global>
+
+profile phc2sys /usr/sbin/phc2sys {
+  include <abstractions/base>
+
+  # Needed phc2sys capabilities
+  capability dac_read_search,
+  capability dac_override,
+  capability sys_time,
+  capability sys_module,
+  capability net_admin,
+
+  # Allow dgram type for all net domains
+  network dgram,
+
+  # The phc2sys application it's own
+  /{usr/,}sbin/phc2sys mr,
+
+  # phc2sys runtime data
+  @{run}/phc2sys.[0-9]* rw,
+
+  # To be able to comunicate with ptp4l
+  @{run}/ptp4lro rw,
+
+  # Needed signals in case of being executed by other application
+  signal (receive) set=(term),
+
+  # Needed when being executed by timemaster
+  @{run}/timemaster/ptp4l.[0-9]*.socket rw,
+
+  # PTP devices
+  /dev/ptp[0-9]* rw,
+
+  # PPS devices
+  /dev/pps[0-9]* r,
+
+  # Site-specific additions and overrides. See local/README for details.
+  include if exists <local/usr.sbin.ptp4l>
+}
diff --git a/debian/usr.sbin.ptp4l b/debian/usr.sbin.ptp4l
new file mode 100644
index 0000000..fd74494
--- /dev/null
+++ b/debian/usr.sbin.ptp4l
@@ -0,0 +1,63 @@
+# vim:syntax=apparmor
+# Last Modified: Sun Sep 05 16:48:05 2021
+
+abi <abi/4.0>,
+include <tunables/global>
+
+profile ptp4l /usr/sbin/ptp4l {
+  include <abstractions/base>
+
+  # The ptp4l application it's own
+  /{usr/,}sbin/ptp4l mr,
+
+  # Needed signals in case of being executed by other application
+  signal (receive) set=(term),
+
+  # Needed capabilities
+  capability net_admin,
+  capability sys_module,
+  capability net_bind_service,
+
+  # Needed Network domains
+  network netlink,
+  network packet,
+  network inet,
+  network inet6,
+  network unix,
+  network unspec,
+
+  # Needed to define mac address for
+  # a network interface
+  /sys/class/net/*/address r,
+
+  # ptp devices
+  /dev/ptp[0-9]* rw,
+
+  # serial devices
+  /dev/ttyS[0-9]* rw,
+  /dev/ttyACM[0-9]* rw,
+
+  # Needed for ipc and runtime status
+  @{run}/cmlds_client rw,
+  @{run}/cmlds_server rw,
+  @{run}/refclock.ptp.sock rw,
+  @{run}/run/ptp4l rw,
+  @{run}/ptp4lro rw,
+  @{run}/ptp4l rw,
+  @{run}/ptp4lro rw,
+  @{run}/phc2sys.[0-9]+ r,
+
+  # When being runned by timemaster
+  @{run}/timemaster/chrony.SOCK* rw,
+  @{run}/timemaster/ptp4l.*.conf r,
+  @{run}/timemaster/ptp4l.*.socket rw,
+  @{run}/timemaster/ptp4lro.*.socket rw,
+  @{run}/phc2sys.* rw,
+
+  # ptp4l config files
+  @{etc_ro}/linuxptp/** r,
+
+  # Site-specific additions and overrides. See local/README for details.
+  include if exists <local/usr.sbin.ptp4l>
+}
+
diff --git a/debian/usr.sbin.timemaster b/debian/usr.sbin.timemaster
new file mode 100644
index 0000000..aaba051
--- /dev/null
+++ b/debian/usr.sbin.timemaster
@@ -0,0 +1,47 @@
+
+# vim:syntax=apparmor
+# Last Modified: Mon Sep 15 14:48:05 2024
+
+abi <abi/4.0>,
+include <tunables/global>
+
+profile timemaster /usr/sbin/timemaster {
+  include <abstractions/base>
+  include <abstractions/nameservice>
+
+  # To be able to kill child processes
+  capability kill,
+
+  # To be able to send the term signal
+  signal (send) set=(term),
+
+  # Runtime config files for timemaster
+  @{run}/timemaster/*.conf rw,
+  @{run}/timemaster/ rwk,
+
+  ## system files access
+
+  # RPi4 and RPi5 specific
+  @{sys}/devices/platform/axi/[0-9a-f]*.pcie/[0-9a-f]*.ethernet/net/*/ptp[0-9]*/ r,
+  @{sys}/devices/platform/axi/[0-9a-f]*.pcie/[0-9a-f]*.ethernet/net/*/ptp[0-9]*/n_vclocks rw,
+
+  # Generic arch's
+  @{sys}/ptp[0-9]+/ptp[0-9]* rw,
+  @{sys}/ptp[0-9]+/n_vclocks rw,
+  @{sys}/net/*/ptp[0-9]*/n_vclocks rw,
+
+  # Default location for config files
+  @{etc_ro}/linuxptp/** r,
+  @{etc_ro}/chrony/** r,
+  @{etc_ro}/ntpsec/** r,
+
+  # Backend applications
+  # Px = exec to apparmor profile that matches executable name, with environment variable scrubbing
+  /usr/sbin/chronyd Px,
+  /usr/sbin/ntpd Px,
+  /usr/sbin/ptp4l Px,
+  /usr/sbin/phc2sys Px,
+
+  # Site-specific additions and overrides. See local/README for details.
+  include if exists <local/usr.sbin.timemaster>
+}