Bug 152796 - fcntl(2) audit records should not be labeled "file attribute modify"
Summary: fcntl(2) audit records should not be labeled "file attribute modify"
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 8.1-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-03 00:10 UTC by Garrett Wollman
Modified: 2018-06-11 16:17 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Garrett Wollman 2010-12-03 00:10:11 UTC
	/etc/security/audit_class describes class 0x8 as "file
attribute modify".  This seems like a reasonable thing to audit, but
unfortunately, all calls to fcntl(2) -- which does not modify any file
attributes -- are included in this category.  Any program which uses
POSIX-style locking will flood the audit file with spurious audit
records, while the interesting system calls (those that call
VOP_SETATTR) will be buried.  (And for whatever reason, auditreduce(1)
deosn't appear to perform as advertised when given the "-v" flag.)

Fix: 

Move fcntl to a different audit class (probably "other" or
maybe "ioctl").
How-To-Repeat: 
	Enable auditing with class "fm".  praudit /var/audit/current.
Hit ^C when all you see is "fcntl(2)".
Comment 1 jhell 2010-12-03 04:06:29 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

>> Fix:
> 
> 	Move fcntl to a different audit class (probably "other" or
> maybe "ioctl").

To give a little background with this issue, I had addressed once with
Robert,

> On Sat, 6 Feb 2010 11:15, sson@ wrote:
>> 
>> On Feb 6, 2010, at 6:48 AM, Robert N. M. Watson wrote:
>> 
>>> 
>>> On 5 Feb 2010, at 05:28, jhell wrote:
>>> 
>>>> I just wanted to run this by you quick informally before I
>>>> approach any work on audit and changing things around that will
>>>> never get changed possibly due to strict standards.
>>>> 
>>>> I was setting up audit on a stable/7 remote booting X machine
>>>> and wanted to audit any chmod's chflags etc... after final
>>>> configuration I noticed that fcntl(2) is also included in the
>>>> "fm" class but had noticed that ioctl(2) has its own class
>>>> "io".
>>>> 
>>>> So in my case on a Xserver I get high amounts of fcntl(2)
>>>> changes logged. I would say at a guess of at the very least
>>>> 1000 every 5 seconds. The machines these modifications are
>>>> intended to be placed across are up constantly all week long.
>>>> 
>>>> Being that these are file descriptors do you think it would
>>>> possible to change them around and give fcntl(2) its own class
>>>> "ds" or something similar ?
>>>> 
>>>> What do you think would be the best way to approach this matter
>>>> to bring fcntl(2) into its own class ?.
>>> 
>>> Well, I'd be a little worried about using up too many bits, the
>>> class mask size is fixed in the ABI and difficult to expand.
>>> Perhaps what we should do is move to a group named 'ct' or
>>> something to reflect control calls, and put both fcntl and ioctl
>>> in that? Christian and Stacey added the CC line as they may have
>>> thoughts on this as well. One problem with ioctl, of course, is
>>> that some ioctls are queries on pending bytes to sockets and
>>> ttys, and others are administrative commands :-).
>> 
> 
> I agree moving these to something like "ct" would be considerably an
> advantage over the current state that they are in. I'm starting to
> find that the more I delve into this the more twists and turns I am
> coming across as you have pointed out the limits on the mask size
> etc. :(
> 
> What this is starting to make me think about is a way that the admin
> could control certain aspects of audit. Something like the
> audit_control file but only to turn off certain events either
> specified by field 1 or 2 in audit_events and then by user or group.
> Example idea below.
> 

/etc/security/audit_event_control
#           (none)  Record both successful and failed events.
#           +       Record successful events.
#           -       Record failed events.
#           ^       Record neither successful nor failed events.
#           ^+      Do not record successful events.
#           ^-      Do not record failed events.

> 
> # Field 1 event number, field 2 group spec, field 3 user spec # I put
> them in this order since the effect of adding groups # could allow
> the user field to be omitted entirely if the admin # has no users
> he/she wants to specify.
> 
> # Do not audit fcntl events for group xusers except the user macguy. 
> 30:^xusers:macguy
> 
> # No FIPS for Stacey! ;)
> 65535::^stacey
> 
> Just a thought. I am sure that there is something that would really
> effect this functionality and concern for security having a on-disk
> way to exempt certain aspects of auditing but then again audit_users
> already does that on a wider range of audit_events on the class
> itself.
> 
> This could offer the fine grained control that some areas might just
> be looking for.
> 
>> Yes, it is odd that ioctl has a class all to itself.  Of course,
>> how ioctl is used has changed a lot from what it was originally
>> intended for.  I suspect that ioctl was put in its own class for
>> this very reason.  The overuse of the ioctl(2) interface has made
>> it a target for bugs and potential security issues.  I recall that
>> phk@ gave a talk on ioctl and making this argument as well some
>> years back at BSDCan.
>> 
>> WRT putting fcntl(2) in its own class I would agree with Robert.
>> It would better if fcntl() events were moved into another class
>> rather than creating a new one given the limited class mask size.
>> Moving both ioctl and fcntl into a new class like "ct" seems like
>> the right approach.
>> 
> 
> Seconded.
> 
>> On a similar subject I am thinking about adding an AUE_FIPS_AUDIT
>> event as part of FIPS 140-2 (potential) compliance of FreeBSD.  I
>> noticed that the mozilla nss security policy recommends that a new
>> audit class be created (incorrectly): see
>> http://www.mozilla.org/projects/security/pki/nss/fips/secpolicy.pdf
>> I think it might be best if it was just simply added to the "ad"
>> class. Do you see any problem with this?
>> 
> 
> Thanks for the information, I knew this was out there somewhere just
> had not seen too many free man hours to go looking for it yet.  Bring
> on the FIPS compliance!.  As for adding it to the :ad class I really
> can't offer a real opinion on that other than it seems like the right
> approach at this time.
> 
> This class mask size limitation is rather frustrating.

- -- 

 jhell,v
-----BEGIN PGP SIGNATURE-----

iQEcBAEBAgAGBQJM+GzFAAoJEJBXh4mJ2FR+fVcH/0Sw9M4lZimoAL3nG3fc/+JP
GBVpPaIEUINZyBemCDhc++0HLcNSUsWAY7bPj3e1bZn69iMF+Xqq2lM8dOxLDnlo
8zNoYTLeyazZ/IUGMMRFQAj+3dZ0SGOMhwOjNq24q0DC8e0ODcwtqvOkNEmND/cQ
f6PFfipjpVJU/t4TnA8JwcUb34Twwc+KreuP0xGElqqofx+O4EDcyQIBm+tRRTH7
9kpVzbbtivHyrLZem49rxWCtKfwUkJ2JlhUyXbmVRYhp6Iv8F0jt6Atz4OJpRdoV
CMvvoM8HMybZ66/sSwCVwAN4zw/i3rNNm3H9rVAdtWCVGjy7Y8/i7Tr97KkFH3E=
=e40p
-----END PGP SIGNATURE-----
Comment 2 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 07:58:48 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 3 Alan Somers freebsd_committer freebsd_triage 2018-06-11 16:17:53 UTC
I think fcntl should simply be moved into the "no" class, because none of its commands are security-relevant.  It doesn't really affect the file on-disk at all, just the way that the process accesses the file.  It combines the functionality of dup2 ("no" class) with some of the obscure open(2) options (open is audited, but those options aren't treated specially), with sigaction(2) ("no" class), with flock ("fm" class).  It also adds the ability to set file readahead (not security relevant).  The only commands that affect the file itself are the locking commands.  I would argue that, since they're advisory and very frequently used, the file locking operations shouldn't be auditted.