[MIR] audit (pulls in libprelude)

Bug #1026852 reported by Tyler Hicks
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
audit (Ubuntu)
Fix Released
Undecided
Tyler Hicks
libprelude (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

This is a MIR to bring a portion of binary packages built from the audit source
package into main. The binary packages of interest (some of which are created by the attached debdiff for the audit package) are:
 - auditd
 - libaudit-common
 - libaudit-dev
 - libaudit1
 - libauparse-dev
 - libauparse0
 - python-audit

The binary pacakges that may remain in universe are:
 - audispd-plugins

Availability:
 - Available in universe for all arches

Rationale:
 - Discussed as part of the P and Q security catch all blueprints
   + https://blueprints.launchpad.net/ubuntu/+spec/security-p-catch-all
   + https://blueprints.launchpad.net/ubuntu/+spec/security-q-catch-all
 - libaudit0 is a build dependency of the Debian cron package
   + https://launchpad.net/bugs/878155
 - The audit log can already be used by AppArmor
   + http://wiki.apparmor.net/index.php/AppArmor_Failures#Messages_in_the_Log_files
 - linux-tools perf depends on libaudit-dev

Security:
 - One CVE (CVE-2008-1628) in the project's history
 - Note that CVEs have been assigned for the kernel audit subsystem, but those
   are unrelated to the audit userspace code
 - Security risk involved since auditd is a daemon that runs as root
   + Implementing privilege dropping would not be trivial:
     http://www.redhat.com/archives/linux-audit/2009-October/msg00011.html
 - auditd can open up a port and listen for audit messages from remote machines
   + NOTE: This is no longer true as of auditd 1:2.2.2-1ubuntu2, which disabled
     the network listener support
   + The default auditd.conf is *not* configured to open a port
   + auditd doesn't create a socket unless tcp_listen_port is set in
     auditd.conf (see auditd_tcp_listen_init() in src/auditd-listen.c)
   + The upstream build system does not allow disabling of the networking code
 - The audispd-plugins binary package contains functionality to send audit
   messages to remote machines but a main inclusion is not being requested for
   audispd-plugins

Quality Assurance:
 - Basic audit logging works immediately after auditd package installation
 - The upstream maintainer is active on the mailing list
   + https://www.redhat.com/mailman/listinfo/linux-audit
 - The lastest upstream release was on March 23, 2012
 - 1 "low" bug opened against Ubuntu audit source package
   + https://bugs.launchpad.net/ubuntu/+source/audit
 - 4 bugs opened against the Debian audit source package
   + 1 important, 2 minor, 1 wishlist
   + http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=audit
 - 'make check' tests are enabled in the build
 - debian/watch exists

UI Standards:
 - The only end-user application is in the system-config-audit binary package,
   which is not included in this MIR

Dependencies:
 - One build dependency is not in main
   + libprelude-dev binary and source package is in universe
 - All external binary dependencies are in in main

Standards Compliance:
 - No lintian errors
 - 9 overridden lintian warnings due to non-standard file/dir permissions
   because config and log files are intentionally installed with restrictive
   file permissions due to the security-related nature of the package (see
   debian/auditd.lintian-overrides)

Maintenance:
 - This is a relatively simple package that seems to be well maintained
   upstream and in Debian
 - Should not require a dedicated maintainer in Ubuntu

Tags: patch
Tyler Hicks (tyhicks)
description: updated
Tyler Hicks (tyhicks)
summary: - [MIR] audit
+ [MIR] audit (pulls in libprelude and maybe libev)
description: updated
Revision history for this message
Tyler Hicks (tyhicks) wrote : Re: [MIR] audit (pulls in libprelude and maybe libev)

As Steve Langasek has previously pointed out[1], the audit source tree includes an embedded copy[2] of libev that is statically links against. If we leave it as-is, we can drop the libev build dependency but that doesn't seem like a clean solution.

1: https://bugs.launchpad.net/ubuntu/+source/cron/+bug/878155/comments/2
2: https://fedorahosted.org/audit/browser/trunk/src/libev?rev=698

Revision history for this message
Tyler Hicks (tyhicks) wrote :

After speaking with jdstrand, I'm going to explore the option of disabling the auditd code that listens on the network. Then we may be able to simply support the non-networked auditd and leave all networking capabilities in universe.

Unsubscribing ubuntu-mir and marking the libprelude and libev tasks as invalid while I look into this.

Changed in libev (Ubuntu):
status: New → Invalid
Changed in libprelude (Ubuntu):
status: New → Invalid
Revision history for this message
Tyler Hicks (tyhicks) wrote :

My patches to disable the auditd listener code at build time have been sent to the linux-audit list and the thread is archived here:

https://www.redhat.com/archives/linux-audit/2012-August/msg00007.html

They are pending review.

Revision history for this message
Adam Conrad (adconrad) wrote :

Worth noting that nscd (from the glibc build) can also take advantage of libaudit if it can link to it at runtime. nscd on its own isn't much of a reason to promote libaudit, but it would be lovely if someone pokes me to re-enable the build-dep if/when libaudit is deemed uncrackful enough to promote.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Here's a debdiff which backports the upstream patches that disable the auditd network listener and splits up the auditd package into auditd-common, auditd, and auditd-light. I've tested the resulting packages and everything looks good except for these dpkg warnings when doing a dist-upgrade to the new auditd package:

...
Preparing to replace auditd 1.7.18-1ubuntu1 (using .../auditd_1.7.18-1ubuntu2_amd64.deb) ...
Unpacking replacement auditd ...
dpkg: warning: unable to delete old directory '/etc/audit': Directory not empty
dpkg: warning: unable to delete old directory '/etc/audisp/plugins.d': Directory not empty
dpkg: warning: unable to delete old directory '/etc/audisp': Directory not empty
dpkg: warning: unable to delete old directory '/var/log/audit': Directory not empty
...

Those directories were moved from the auditd package to the auditd-common package. This is the first time that I've done a package split, so I'm not sure how serious those warnings are or how to fix them.

summary: - [MIR] audit (pulls in libprelude and maybe libev)
+ [MIR] audit (pulls in libprelude)
Tyler Hicks (tyhicks)
description: updated
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "audit_1.7.18-1ubuntu2.debdiff" of this bug report has been identified as being a patch in the form of a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-sponsors team please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

tags: added: patch
Revision history for this message
Laurent Bigonville (bigon) wrote :

I would suggest you to consider updating to 2.2 branch before doing the MIR

2.2 (from debian) has added some new packages and libaudit got a soname bump

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Unsubscribing ubuntu-sponsors for now, since debdiff should get rebased on 2.2 branch.

Revision history for this message
Laurent Bigonville (bigon) wrote :

FYI I've just uploaded 1:2.2.2-1 in debian experimental

Revision history for this message
Tim Gardner (timg-tpi) wrote :

linux-tools perf depends on libaudit-dev. I've disabled building perf until such time as libaudit-dev is promoted to main.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

After speaking with infinity, mdeslaur, and jdstrand, we've decided to *not* split the audit package into an audit daemon with networking support and another without. Instead, we've decided to disable network listener support in the existing auditd binary package.

If we have a large number of users who depend on the auditd network listener support, then we may try to get the split package layout upstream in Debian and then merge that back into Ubuntu. However, I do not believe that the centralized logging functionality in auditd is widely used.

This approach provides a nice balance of security and maintainability, while not confusing users with multiple auditd binary packages.

Here's the debdiff to disable the network listener and remove libwrap and libev as build dependencies. Please see the changelog for more details. I've successfully tested auditd and some of the auditd tools with this debdiff applied.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

ACK on the debdiff. Uploaded, thanks!

Tyler Hicks (tyhicks)
description: updated
Tyler Hicks (tyhicks)
description: updated
Changed in libprelude (Ubuntu):
status: Invalid → New
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Ok, the MIR description has been updated to reflect changes that have occurred since I originally opened this MIR request and I've re-subscribed ubuntu-mir.

Tyler Hicks (tyhicks)
no longer affects: libev (Ubuntu)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package audit - 1:2.2.2-1ubuntu2

---------------
audit (1:2.2.2-1ubuntu2) raring; urgency=low

  * Disable auditd network listener with --disable-listener (LP: #1026852)
    - debian/rules: Reduce the risk of a remote attack on auditd, which
      runs as root, by not building the code that listens for audit messages
      over the network. This will prevent users from using auditd as a
      centralized audit message aggregator, but this feature is rarely used.
  * Don't build against libwrap since only auditd's network listener used it
    - debian/control: Remove libwrap0-dev Build-Dependency
    - debian/rules: Remove --with-libwrap from configure arguments
  * Remove libev-dev Build-Dependency (LP: #1026852)
    - debian/control: The upstream audit sources embed and build against their
      own version of libev. This is not desirable, but there's no reason to
      list libev-dev as a build dependency at this time.
 -- Tyler Hicks <email address hidden> Wed, 06 Feb 2013 13:51:35 -0800

Changed in audit (Ubuntu):
status: New → Fix Released
Tyler Hicks (tyhicks)
Changed in audit (Ubuntu):
status: Fix Released → New
Changed in audit (Ubuntu):
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in libprelude (Ubuntu):
assignee: nobody → Jamie Strandboge (jdstrand)
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

MIR review for libprelude:
 * Builds fine with only main enabled
 * Has a test suite and it is enabled in the build
 * No Ubuntu delta
 * dh_makeshlibs is used, but not dh_makeshlibs -V (would be nice to have this in Debian)
 * Has debian/watch file
 * Update history is slow, but there isn't really much to do
 * The current release is not packaged. 1.0.1 is avilable but this only has 2 bug fixes
 * Will entering main make it harder for the people currently keeping it up to date? no-- should be able to just sync
 * Lintian warnings (lintian ../source/*dsc ../binary/*.deb)
 * Is debian/rules a mess? it's fine
 * there are warnings during the build, but they shouldn't be a problem (in the testsuite, setting variables but not using them, unused functions, etc)
 * Incautious use of malloc/sprintf: spot checked various places and it seems fine-- returns code are checked, string operations are ok
 * Uses of sudo (see audit-packaging.sh) or LD_LIBRARY_PATH (see audit-code.sh)
 * Important bugs (crashers, etc) in Debian or Ubuntu: no
 * Does the package have a CVE history? no
 * binaries are compiled with PIE
 * No initscripts/upstart jobs, dbus services, setuid/fscaps, sudo, cron jobs
 * use of chown() suggests privileged operations, but this seems under the control of the admin (ie, no network services processes untrusted input)

Nothing in this review suggests it needs a security audit. ACK

Changed in libprelude (Ubuntu):
assignee: Jamie Strandboge (jdstrand) → nobody
status: New → Fix Committed
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Audit MIR:

 * Builds fine pulling in only libprelude from universe
 * Has a test suire and it is enabled
 * Uses dh_python2 but not reverse depends on python-audit that are on the CD
 * There is an Ubuntu delta, but Tyler is pushing this to Debian. A small delta may remain to disable the network code, but this is manageable
 * uses a symbols file
 * has a debian/watch file
 * update history is fine
 * current release is packaged
 * Entering main will likely make it harder to maintain, but it is something that is needed
 * Lintian is fine
 * debian/rules is ok
 * Errors/warnings during the build: there are quite a few warnings particularly due to -Wunused-result (especially surrounding asprintf)
 * No important bugs in Debian or Ubuntu
 * There is a CVE history. Patch was simple and easy to fix.
 * Binaries compiled with PIE.
 * Lintian clean
 * one daemon, runs as root, does not process network traffic with new build options
 * no dbus services, setuid/fscaps, sudo/gksu/pkexec or cron jobs

Conditional ACK provided the compiler warnings are fixed or shown to not be a problem for any binaries going to main.

Changed in audit (Ubuntu):
assignee: Jamie Strandboge (jdstrand) → Tyler Hicks (tyhicks)
status: New → In Progress
Revision history for this message
Adam Conrad (adconrad) wrote :

Alright, based on the above comments by Jamie, and the knowledge that Tyler will have packages fixing the warnings for me this weekend, I'm going to pre-promote the lot so nscd can build against it.

Changed in libprelude (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Adam Conrad (adconrad) wrote :

If we want auditd in main, someone will have to decide where it should be seeded, for now, this will just pull in libaudit-dev and libaudit1, as build-deps of glibc, linux, etc.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Here's the (large) debdiff that fixes the important compiler warnings. I've submitted all three patches upstream:

https://www.redhat.com/archives/linux-audit/2013-February/msg00003.html

There are still remaining compiler warnings, but I consider them to be ok:

../../../../src/libev/ev.c:1254:31: warning: 'ev_default_loop_ptr' initialized and declared 'extern' [enabled by default]
../../../../src/libev/ev.c: In function 'pipecb':
../../../../src/libev/ev.c:1900:16: warning: ignoring return value of 'read', declared with attribute warn_unused_result [-Wunused-result]
../../../../src/libev/ev.c:1907:16: warning: ignoring return value of 'read', declared with attribute warn_unused_result [-Wunused-result]

The ev_default_loop_ptr issue is intentional considering this comment on the line triggering the warning:

/* needs to be initialised to make it a definition despite extern */

The ignored return value of 'read' is ok because the function is just slurping a few bytes off of a signal pipe into a dummy buffer, which is ignored. There is no error handling or logging to be done in this situation, even if read() failed.

There are still a number of compiler warnings in the zos-remote audispd plugin. Most of them are harmless warnings, but the main reason why I feel they can be ignored is because the audisp-plugins package is remaining in universe.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package audit - 1:2.2.2-1ubuntu3

---------------
audit (1:2.2.2-1ubuntu3) raring; urgency=low

  * Fix important build warnings (LP: #1026852)
    - debian/patches/fix-asprintf-warnings.patch: Linux asprintf()
      implementations do not provide guarantees around the strp variable upon
      error so its return code must be checked.
    - debian/patches/fix-unused-result-warnings.patch: Be sure to check the
      return code of various important functions and create an appropriate
      error path.
    - debian/patches/fix-discards-const-qualifier-warnings.patch: Fix some
      areas where the const qualifier was not being respected.
 -- Tyler Hicks <email address hidden> Fri, 08 Feb 2013 18:36:06 -0800

Changed in audit (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thanks Tyler :) I added auditd as an on-demand supported install.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

> Thanks Tyler :) I added auditd as an on-demand supported install.

(to the raring supported seed)

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Override component to main
auditd 1:2.2.2-1ubuntu3 in raring amd64: universe/admin/extra -> main
auditd 1:2.2.2-1ubuntu3 in raring armhf: universe/admin/extra -> main
auditd 1:2.2.2-1ubuntu3 in raring i386: universe/admin/extra -> main
auditd 1:2.2.2-1ubuntu3 in raring powerpc: universe/admin/extra -> main
Override [y|N]? y
4 publications overridden.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.