apparmor="DENIED" operation="open" class="file" profile="virt-aa-helper" name="/etc/gnutls/config"

[Martin Pitt]

Christian summarizes this after the great reports by Martin:

gnutls started to ship forceful disables in pkg/import/3.8.1-4ubuntu3 and added more later.

Due to that anything linked against gnutls while being apparmor isolated now hits similar denials, preventing the desired effect of the config change BTW.

I think for safety we WANT to always allow this access, otherwise people will subtly not have crypto control about the more important (those isolated) software. Because after the denial I'd expect this to not really disable it in the program linked to gnutls (details might vary depending what they really use gnutls for).

I do not nkow of a gnutls abstraction to use, but TBH I'm afraid now fixing a few but leaving this open in some others not spotted.

I'd therefore suggest, but we need to discuss, to therefore change it in /etc/apparmor.d/abstractions/base.

Therefore I'm adding gnutls (and Adrien) as well as apparmor to the bug tasks.

--- --- --- --- --- --- --- --- --- --- --- ---
--- --- --- --- --- --- --- --- --- --- --- ---

Merely booting current noble cloud image with "chrony" installed causes this:

audit: type=1400 audit(1710152842.540:107): apparmor="DENIED" operation="open" class="file" profile="/usr/sbin/chronyd" name="/etc/gnutls/config" pid=878 comm="chronyd" requested_mask="r" denied_mask="r" fsuid=0 ouid=0

--- --- --- --- --- --- --- --- --- --- --- ---
--- --- --- --- --- --- --- --- --- --- --- ---

Running any VM in libvirt causes a new AppArmor violation in current noble. This is a regression, this didn't happen in any previous release.

Reproducer:

  virt-install --memory 50 --pxe --virt-type qemu --os-variant alpinelinux3.8 --disk none --wait 0 --name test1

(This is the simplest way to create a test VM. But it's form or shape doesn't matter at all).

Results in lots of

audit: type=1400 audit(1710146677.570:108): apparmor="DENIED" operation="open" class="file" profile="virt-aa-helper" name="/etc/gnutls/config" pid=1480 comm="virt-aa-helper" requested_mask="r" denied_mask="r" fsuid=0 ouid=0

libvirt-daemon 10.0.0-2ubuntu1
apparmor 4.0.0~alpha4-0ubuntu1
libgnutls30:amd64 3.8.3-1ubuntu1

[Christian Ehrhardt  (paelzer)]

Hi Martin,
as always thanks for your post FF testing and reports.

Thank you for also filing bug 2056747 - it starts to show that this is a generic thing which probably anything linked against gnutls and being confined will hit.

reverse-depends --release=noble --build-depends libgnutls28-dev | wc -l
182

Unless later decided otherwise I'd think we should not look for many many individual rules but adding it to an abstraction or so, so for now I'd mark these as dups to each other and file it against gnutls as well.

[Christian Ehrhardt  (paelzer)]

There is precedence in /etc/apparmor.d/abstractions/base holding various rules like these
$ grep etc_ro /etc/apparmor.d/abstractions/base
  @{etc_ro}/locale/** r,
  @{etc_ro}/locale.alias r,
  @{etc_ro}/localtime r,
  @{etc_ro}/bindresvport.blacklist r,
  @{etc_ro}/ld.so.cache mr,
  @{etc_ro}/ld.so.conf r,
  @{etc_ro}/ld.so.conf.d/{,*.conf} r,
  @{etc_ro}/ld.so.preload r,
  @{etc_ro}/ld-musl-*.path r,

I'd think the better fix is to allow it there.

Actually, base isn't the best.
I think it should go into /etc/apparmor.d/abstractions/crypto (which is included by base)

If Adrien knows about similar, "whoever uses it should have read access to that config to restrict it accordingly" config files we might want to add them all in one block there.

[Christian Ehrhardt  (paelzer)]

Suggestion would be something like:

--- /etc/apparmor.d/abstractions/crypto.orig 2024-03-11 11:05:24.027597234 +0000
+++ /etc/apparmor.d/abstractions/crypto 2024-03-11 11:06:12.035895701 +0000
@@ -24,4 +24,7 @@
   /etc/crypto-policies/*/*.txt r,
   /usr/share/crypto-policies/*/*.txt r,

+ # Global gnutls config
+ @{etc_ro}/gnutls/config
+
   include if exists <abstractions/crypto.d>

[Adrien Nader (adrien)]

Hey,

I think everything in the gnutls/ directory should be allowed: there can be profiles with arbitrary names (or at least alnum I guess) which define priority/configuration strings that can be used by gnutls applications. I'm not aware of anything else that typically goes there but I haven't checked. I'll have another look today.

More generally, there can be the same issue for openssl which has its own abstraction file but isn't included by default AFAIU.

A similar issue could apply to ssl_certs since some apps/libraries ship their own cert bundle and could function despite not having access to the system store (I'm looking at you python). I don't know what would be a typical behavior here but I'm pretty sure that the whole range of possible behavior exists in the wild.

I'm wondering if I understood the current rules fine because based on my understanding, I would have expected warnings for these too.

A noteworthy change is https://bugs.launchpad.net/ubuntu/+source/nss/+bug/2016303 : it would access to /etc/nss . I don't know if NSS silently ignores inaccessible system-wide configuration or not. You might want to include it already.

I think all these libraries should probably fail on EPERM. Probably 0 change upstreams accept such a change if it's needed however. :P

[Martin Pitt (pitti)]

There is also a similar violation [1] in a specific machine instead of virt-aa-helper:

   apparmor="DENIED" operation="open" class="file" profile="libvirt-edef5ae7-03e3-48b1-ac28-a9d8a116492b" name="/etc/gnutls/config" pid=1576 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0

Christian's proposed solution should take care of that as well, but I wanted to mention it just for completeness.

[1] https://cockpit-logs.us-east-1.linodeobjects.com/pull-6048-20240311-121547-77e2f887-ubuntu-stable-cockpit-project-cockpit-machines/log.html#53

[Christian Ehrhardt  (paelzer)]

FYI - submitted as https://gitlab.com/apparmor/apparmor/-/merge_requests/1178
@John if merged, would you mind adding a bug-ref to the Ubuntu upload changelog so this bug 2056739 closes?

Given that there seems to be some agreement to fix this in apparmor, I'll set the other tasks to "Won't Fix"

[John Johansen (jjohansen)]

Yes, will do I added both reference you provided to the upstream merge commit and all fixes/closes references will be going into the changelog.

[Christian Ehrhardt  (paelzer)]

FYI the fix and a related cleanup are merged into upstream apparmor and I'd expect the next upload to Ubuntu to then fix this issue.

@Martin
Thanks for the extra info for completeness, I assume we might find even more if we spend more time (but tat would provide no extra gain).

@John
Up to you then, I'll assign the apparmor task to you to represent that I'm not driving that part

[Launchpad Janitor (janitor)]

This bug was fixed in the package apparmor - 4.0.0-beta3-0ubuntu2

---------------
apparmor (4.0.0-beta3-0ubuntu2) noble; urgency=medium

  * d/apparmor.install
    - install new profiles
      - geary
      - goldendict
      - kchmviewer
      - loupe
      - notepadqq
      - pageedit
      - privacybrowser
      - qmapshack
      - qutebrowser
      - rssguard
      - scide
      - tuxedo-control-center
      - unix-chkpwd

apparmor (4.0.0-beta3-0ubuntu1) noble; urgency=medium

  * New upstream release.
    (LP: #2058329, LP: #2056747, LP: #2056739, LP: #2046844)
  * Refresh patches
    - d/p/u/samba-systemd-interaction.patch
    - d/p/u/parser-add-support-for-prompting.patch
  * Drop patches which have now been applied upstream
    - ubuntu/parser-support-uin128_t-key-as-a-pair-of-uint64_t-nu.patch
    - ubuntu/Minor-improvements-for-MountRule.patch
  * Add patches from upstream that are post Beta3 and will be in Beta4
    - d/p/u/parser-fix-issues-appointed-by-coverity.patch
    - d/p/u/profiles-add-unconfined-profile-for-tuxedo-control-c.patch
      (LP: #2046844)

 -- John Johansen <email address hidden> Mon, 18 Mar 2024 18:34:14 -0700