aa-complain traceback when marking multiple profiles

[Steve Beattie]

[SRU justification]

[Impact]

$ sudo aa-complain /etc/apparmor.d/usr.lib.postfix.*
Setting /etc/apparmor.d/usr.lib.postfix.anvil to complain mode.
Traceback (most recent call last):
  File "/usr/sbin/aa-complain", line 30, in <module>
    tool.cmd_complain()
  File "/usr/lib/python3/dist-packages/apparmor/tools.py", line 171, in cmd_complain
    apparmor.read_profiles()
  File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 2573, in read_profiles
    read_profile(profile_dir + '/' + file, True)
  File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 2599, in read_profile
    profile_data = parse_profile_data(data, file, 0)
  File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 2853, in parse_profile_data
    store_list_var(filelist[file]['lvar'], list_var, value, var_operation, file)
  File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 3277, in store_list_var
    raise AppArmorException(_('Redefining existing variable %s: %s in %s') % (list_var, value, filename))
apparmor.common.AppArmorException: 'Redefining existing variable @{TFTP_DIR}: /var/tftp /srv/tftpboot in /etc/apparmor.d/usr.sbin.dnsmasq'

$ sudo grep -R TFTP_DIR /etc/apparmor.d/
/etc/apparmor.d/usr.sbin.dnsmasq:@{TFTP_DIR}=/var/tftp /srv/tftpboot
/etc/apparmor.d/usr.sbin.dnsmasq: @{TFTP_DIR}/ r,
/etc/apparmor.d/usr.sbin.dnsmasq: @{TFTP_DIR}/** r,

Looks like the tools are re-parsing everything, but not resetting whatever is storing the variable declarations.

[Test Case]

sudo aa-enforce /etc/apparmor.d/*

got error

[Regression Potential]

[Other Info]

[Seyeong Kim (seyeongkim)]

I need code review for this.

[Christian Boltz (cboltz)]

Thanks for the patch!

Just curious - did you test if for cases where the profile filename differs from the binary filename? For example, I slightly ;-) doubt a profile for /bin/foo in /etc/apparmor.d/my_fancy_foo_profile will be found with your patch.

To make things more interesting, I have a completely different patch for this bug pending on the AppArmor mailinglist:
https://lists.ubuntu.com/archives/apparmor/2015-February/007219.html

[Seyeong Kim (seyeongkim)]

@cboltz

ah, your patch is already proposed to upstream! i think your patch is better :)

hmm i don't get excatly your example. please correct me.

profile filename is already different to binary in ubuntu..

eg. sbin.syslogd file is in apparmor.d directory

print(profile)
/etc/apparmor.d/sbin.syslogd

in that file, binary filename is /sbin/syslogd

[Christian Boltz (cboltz)]

Well, I probably just need to explain what "profile filename differs from the binary filename" means (for me) ;-)

AppArmor has a default naming scheme for profile files - drop the leading "/", replace all remaining "/" with "." and store that file in /etc/apparmor.d/

This means /sbin/syslogd -> /etc/apparmor.d/sbin.syslogd follows the default naming scheme, and therefore does _not_ differ (at least from my POV). [1]

Now rename /etc/apparmor.d/sbin.syslogd to /etc/apparmor.d/fancy_syslogd_profile - that's unusual, but valid and supported (and still delivers a working profile for /sbin/syslogd). That's what I meant with "different filename" ;-) [2]

[1] I'm quite sure the tools would find the profile in /etc/apparmor.d/sbin.syslogd even if they don't read all profiles before because it's the default filename.

[2] In this case, the tools need to read all profiles to find the profile for /sbin/syslogd in the non-expected, non-default file /etc/apparmor.d/fancy_syslogd_profile

[Seyeong Kim (seyeongkim)]

Thanks a lot for your comment.

Could you please SRU this patch for trusty, utopic?

if you are busy and agreed, I can do this with your patch.

[Christian Boltz (cboltz)]

Wrong distribution - I typically do a SR for openSUSE, not a SRU for ubuntu ;-) so please do it yourself.

Note that there are some more bugfixes in the 2.9 branch or pending on the ML. I'd recommend to wait until all off them are in bzr and then package a snapshot from the 2.9 branch. (As an alternative, ask Steve to release 2.9.2 _now_ ;-)

[Christian Boltz (cboltz)]

Fix commited to trunk r2872 and 2.9 branch r2846.

A rare crash in aa-autodep remains - I filed bug 1426372 for it.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Chris J Arges (arges)]

@xtrusia,
Can you use DEP-3 formatting for the SRU patches please:
http://dep.debian.net/deps/dep3/

It would be very useful to know which revision this upstream patch comes from, etc.
Thanks,

[Tyler Hicks (tyhicks)]

Hi Seyeong and Chris - The Ubuntu Security Team is planning on updating the AppArmor Python utilities by doing a full update of the code from one of our stable upstream releases. There are a number of bugs, outside of this one, in the Python utilities and it'll be best if we just fix them all at once.

Would you mind holding off on this SRU?

Thanks!

[Tyler Hicks (tyhicks)]

I forgot to mention the reasoning. :)

We're already planning on doing the packaging work to incorporate all of the new fixes from upstream so it would be a bit of a duplication of efforts to carry out this single SRU. This would allow you to focus on other SRUs that do not have anyone that is not planned by a team.

[Seyeong Kim (seyeongkim)]

@tyhicks

got it Thanks :)

[Seyeong Kim (seyeongkim)]

@tyhicks

quiestion :)

you are planning to adopt you mentioned for multiple release? trusty, utopic etc..

Thanks

[Sebastien Bacher (seb128)]

the fix has been uploaded to wily it seems
https://launchpad.net/ubuntu/+source/apparmor/2.9.2-0ubuntu1

unsubscribing sponsors as well since previous comment suggested that the security team want to deal with the update and not have this particular fix sponsored on its own

[Steve Beattie (sbeattie)]

Seyong Kim, the plan is to address this in trusty via the SRU for the apparmor utilities (and other things) described in bug 1449769.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apparmor - 2.9.2-0ubuntu1

---------------
apparmor (2.9.2-0ubuntu1) wily; urgency=medium

  * Update to apparmor 2.9.2
    - Fix minitools to work with multiple profiles at once (LP: #1378095)
    - Parse mounts that have non-ascii UTF-8 chars (LP: #1310598)
    - Update dovecot profiles (LP: #1296667)
    - Allow ubuntu-helpers to build texlive fonts (LP: #1010909)
  * dropped patches incorporated upstream:
    add-mir-abstraction-lp1422521.patch, systemd-dev-log-lp1413232.patch
    parser-fix_modifier_compilation_+_tests.patch,
    tests-fix_systemd_breakage_in_pivot_root-lp1436109.patch,
    GDM_X_authority-lp1432126.patch, and
    debian/patches/easyprof-framework-policy.patch
  * Partial merge with debian apparmor package:
    - debian/rules: enable the bindnow hardening flag during build.
    - debian/upstream/signing-key.asc: add new upstream public
      signing key
    - debian/watch: fix watch file, add gpg signature checking
    - install libapparmor.so dev symlink under /usr not /lib
    - debian/patches/reproducible-pdf.patch: make techdoc.pdf
      reproducible even in face of timezone variations.
    - debian/control: sync fields
    - debian/debhelper/postrm-apparmor: remove
      /etc/apparmor.d/{disable,} on package purge
    - debian/libapache2-mod-apparmor.postrm: on package purge, delete
      /etc/apparmor.d/{,disable} if empty
    - debian/libapparmor1.symbols: Use Build-Depends-Package in the
      symbols file.
    - debian/copyright: sync

 -- Steve Beattie <email address hidden> Mon, 11 May 2015 22:03:04 -0700

[Steve Beattie (sbeattie)]

My apologies, I neglected to include this bug report in the changelog for my apparmor trusty SRU. It has ben accepted into trusty-proposed and is available at https://launchpad.net/ubuntu/+source/apparmor/2.8.95~2430-0ubuntu5.2 as well as from the trusty-proposed repository. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed.

The relevant part of the SRU for this bug report is the update to the upstream apparmor 2.9.2 version of the python utils. This is being tracked in bug 1449769; please leave feedback on this update in that bug report. Thanks for your patience!

[Steve Beattie (sbeattie)]

I have reproduced this issue with apparmor 2.8.95~2430-0ubuntu5.1 from trusty-updates, and have verified that apparmor 2.8.95~2430-0ubuntu5.2 fixes the issue. Marking verification-done.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apparmor - 2.8.95~2430-0ubuntu5.2

---------------
apparmor (2.8.95~2430-0ubuntu5.2) trusty-proposed; urgency=medium

  * debian/patches/php5-Zend_semaphore-lp1401084.patch: allow php5
    abstraction access to Zend opcache files (LP: #1401084)
  * debian/patches/dnsmasq-lxc_networking-lp1403468.patch: update
    profile for lxc support (LP: #1403468)
  * debian/patches/profiles-texlive_font_generation-lp1010909.patch:
    allow generation of texlive fonts by sanitized-helpers
    (LP: #1010909)
  * debian/apport/source_apparmor.py: fix the apparmor apport hook
    so it does not raise an exception if a non-unicode character is
    found in /var/log/kern.log or in /var/log/syslog. This should
    work under python3 or python2.7 (LP: #1304447)
  * debian/patches/profiles-dovecot-updates-lp1296667.patch: update
    dovecot profiles to address several missing permissions.
    (LP: #1296667)
  * debian/patches/profiles-adjust_X_for_lightdm-lp1339727.patch:
    adjust X abstraction for LightDM xauthority location (LP: #1339727)
  * debian/patches/libapparmor-fix_memory_leaks-lp1340927.patch; fix
    memory leaks in log parsing component of libapparmor (LP: #1340927)
  * debian/patches/libapparmor-another_audit_format-lp1399027.patch:
    add support for another log format style (LP: #1399027)
  * debian/patches/tests-workaround_for_unix_socket_change-lp1425398.patch:
    work around apparmor kernel behavioral change in regression tests
    (LP: #1425398)
  * debian/control: add breaks on python3-apparmor against older
    apparmor-utils that used to be where python bits lived
    (LP: #1373259)
  * debian/patches/utils-update_to_2.9.2.patch: update the python
    utilities to the upstream 2.9.2 (LP: #1449769, incorporating a
    large number of fixes and improvements, including:
    - fix aa-genprof traceback with apparmor 2.8.95 (LP: #1294797)
    - fix aa-genprof crashing when selecting scan on Ubuntu 14.04 server
      (LP: #1319829)
    - make aa-logprof read profile instead of program binary
      (LP: #1317176, LP: #1324154)
    - aa-complain: don't traceback when marking multiple profiles
      (LP: #1378095)
    - make python tools able to parse mounts with UTF-8 non-ascii
      characters (LP: #1310598)

 -- Steve Beattie <email address hidden> Thu, 30 Apr 2015 12:18:08 -0700

[Adam Conrad (adconrad)]

The verification of the Stable Release Update for apparmor has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.