lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240113121556.12948-1-pchelkin@ispras.ru>
Date: Sat, 13 Jan 2024 15:15:50 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: John Johansen <john.johansen@...onical.com>
Cc: Fedor Pchelkin <pchelkin@...ras.ru>,
	Paul Moore <paul@...l-moore.com>,
	James Morris <jmorris@...ei.org>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	apparmor@...ts.ubuntu.com,
	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Alexey Khoroshilov <khoroshilov@...ras.ru>,
	lvc-project@...uxtesting.org
Subject: [PATCH 0/2] apparmor: fix namespace check in serialized stream headers from the same policy load

This series is intended to fix the supposedly invalid behaviour of
verify_header() function when checking unpacked profiles namespace
declarations.

Consider having a profiles file called 'testfile' with contents as
 profile :ns1:profile1 ... {}
 profile :ns2:profile2 ... {}

then packing it in a binary format and trying a load-replace operation
 $ apparmor_parser -Q -W -T testfile
 $ apparmor_parser -r -B /var/cache/apparmor/ac27e0ee.0/testfile

Per dmesg output this leads the profiles to be loaded into the same
namespace (trimmed for more convenient display):
 [  414.616098] audit: apparmor="STATUS" operation="profile_load" name="profile1" comm="apparmor_parser" ns="ns2"
 [  414.619304] audit: apparmor="STATUS" operation="profile_load" name="profile2" comm="apparmor_parser" ns="ns2"

while the string "ns1" unpacked inside verify_header() is leaked:

unreferenced object 0xffff888012ff9b18 (size 8):
  comm "apparmor_parser", pid 1198, jiffies 4295081077 (age 164.892s)
  hex dump (first 8 bytes):
    6e 73 31 00 80 88 ff ff                          ns1.....
  backtrace:
    [<ffffffff81ddb4b2>] __kmem_cache_alloc_node+0x1e2/0x2d0
    [<ffffffff81c47034>] __kmalloc_node_track_caller+0x54/0x170
    [<ffffffff81c224dc>] kstrdup+0x3c/0x70
    [<ffffffff83e2e4ef>] aa_unpack+0xb5f/0x1540
    [<ffffffff83e1e9c6>] aa_replace_profiles+0x1f6/0x4040
    [<ffffffff83def008>] policy_update+0x238/0x350
    [<ffffffff83def33e>] profile_replace+0x20e/0x2a0
    [<ffffffff81ead03f>] vfs_write+0x2af/0xe00
    [<ffffffff81eae556>] ksys_write+0x126/0x250
    [<ffffffff88f54426>] do_syscall_64+0x46/0xf0
    [<ffffffff890000ea>] entry_SYSCALL_64_after_hwframe+0x6e/0x76

With the following patches this situation is detected and the whole
profiles load set is denied because of the mixed namespaces.

Note that the following multiple profiles load will also fail after these
patches - quite similar to the namespace check inside aa_replace_profiles().
  profile profile1 ... {}
  profile :ns:profile2 ... {}

This behaviour may directly affect userspace part of AppArmor, though I've
not been able to find any valid use case when e.g. the user writes
  profile profile1 ... {}
  profile :ns:profile2 ... {}
and expects `profile1` to be associated with `ns`. I think it's actually
an invalid expectation so such profiles set should be also denied with a
'mixed namespaces' explaining message.

There is no difference in AppArmor regression tests with and without the
changes.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ