Project

General

Profile

Actions

Feature #11875

open

ZFS changes for file access auditing

Added by Gordon Ross over 4 years ago. Updated about 1 year ago.

Status:
New
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
fs_auditing
Gerrit CR:
External Bug:

Description

The ZFS ACL evaluation code needs some enhancements to support file access auditing using System Access Control LIsts (SACLs)
See also (required by) #11873 #11874


Related issues

Related to illumos gate - Feature #11037: SMB File access audit logging (reserve IDs)ClosedGordon Ross2019-05-15

Actions
Blocks illumos gate - Bug #11873: SMB file access audit loggingIn ProgressMatt Barden

Actions
Blocks illumos gate - Feature #11874: NFS file access audit loggingIn ProgressMatt Barden

Actions
Blocked by illumos gate - Bug #14984: vnodetopath could be more reliable for files on ZFSNewMatt Barden

Actions
Actions #1

Updated by Rich Lowe over 4 years ago

  • Project changed from site to illumos gate
Actions #2

Updated by Electric Monk almost 2 years ago

  • Gerrit CR set to 2365
Actions #3

Updated by Matt Barden almost 2 years ago

Today, ZFS supports setting 'AUDIT' and 'ALARM' type entries in a file's ACL. This project gives these ACE types a mechanical behavior: when performing access checks, additional 'success' and 'failure' masks are calculated by combining the bits set in any AUDIT or ALARM-type ACE that matches the user. An AUDIT/ALARM ACE can have a SUCCESSFUL_ACCESS ('S') control bit, a FAILED_ACCESS ('F') control bit, or both, which determines whether it contributes to the 'success' or 'failure' audit mask (or both). The masks are only calculated if the user's credential has preselected the 'AUE_SACL' event.

This mask is passed up to the caller through an extension to the existing per-thread audit control structure. The caller initializes this structure; ZFS manipulates this structure based on the system state and any AUDIT/ALARM ACEs processed during access checks; the caller then reads the structure to get the calculated audit masks. It can then compare these masks against the accesses involved in the operation to decide whether a record needs to be generated. Notably, ZFS does not actually generate any records - its only job is to calculate the masks. See the comment above 'typedef struct t_audit_sacl' in uts/common/c2/audit_kernel.h for the exact mechanics. This strategy was chosen to avoid any changes to VOP_ACCESS or adding any new VOP interfaces. (One day, it would be nice to have a 'VOP_ACCESS_EX' or similar that could directly acquire the audit mask - as well as provide granular access grants)

Additionally, several changes were made to how ZFS handles AUDIT- and ALARM-type ACEs, which are necessary due to ZFS (and the various ACL interfaces) combining what Windows refers to as the DACL and the SACL into a single ACL (I'd like for the SACL/DACL to be split one day, but that's work for another time):
1. A new z_hints/pflags flag 'ZFS_ACL_NO_AUDIT_ACE' was added, which is set when the ACL on an object has no AUDIT/ALARM type ACEs. This is used various places to speed up ACL processing; if we know there are no AUDIT ACEs, we can end processing once any other work has been completed.
2. AUDIT/ALARM ACEs no longer prevent an ACL from being marked 'TRIVIAL' (they no longer prevent a file from being chmod'd in restricted mode).
3. Inheritable AUDIT/ALARM ACEs are no longer discarded in discard mode. Discard mode is meant to prevent permissions from automatically propagating to new objects; any new access to these objects should still be audited.
4. AUDIT/ALARM types are treated separately from ALLOW/DENY types in all cases.
5. A new flag VSA_ACE_SYS was added to vsa_mask. This indicates that the caller has included a System ACL (SACL) in the ACL. During 'create file with specified ACL', if this isn't set, ZFS performs inheritance for AUDIT/ALARM types and adds them to the specified ACL. Currently, this is only set by SMB, and the 'create with ACL' interface isn't exposed to userspace.
6. A new flag VSA_ACE_NOSACL was added to vsa_mask. This indicates that the caller does not understand AUDIT/ALARM type ACEs. It instructs ZFS to filter out AUDIT/ALARM types from any ACL returned to the caller, and to merge any existing AUDIT/ALARM entries into any ACL the caller attempts to set. This is used by the NFS server; neither the client nor the server support AUDIT/ALARM types (though the protocol does), and ZFS can handle the split/merge more efficiently than NFS.

Limitations:
1. There are several reasons why a given call to VOP_ACCESS(), or another interface that performs access checks, might not actually process the ACL (see ATTR_NOACLCHECK). In these cases, the per-thread struct is left unchanged. The caller can detect this and perform an 'alternate' access check that will hopefully cause the ACL to be processed, but if this fails, the caller must assume the request needs to be audited. This could be fixed with a dedicated VOP interface, which would eliminate any ambiguity.
2. A bunch of this work is necessary because neither ZFS nor the VSA interface separate the DACL (allow/deny) from the SACL (audit/alarm).
3. There are no privilege checks for modifying AUDIT/ALARM types, unlike in SMB; having WRITE_ACL access is sufficient. (The SMB server requires a client to have 'SeSecurityPrivilege', which it grants to members of the local Administrators group - smbadm show -m administrators).

Actions #4

Updated by Gordon Ross over 1 year ago

  • Tags set to fs_auditing
Actions #5

Updated by Gordon Ross over 1 year ago

  • Related to Feature #11037: SMB File access audit logging (reserve IDs) added
Actions #6

Updated by Gordon Ross over 1 year ago

  • Assignee changed from Gordon Ross to Matt Barden
Actions #7

Updated by Gordon Ross over 1 year ago

Actions #8

Updated by Gordon Ross over 1 year ago

  • Blocked by Bug #14984: vnodetopath could be more reliable for files on ZFS added
Actions #9

Updated by Bill Sommerfeld over 1 year ago

I'll be looking these changes over starting maybe a week from now but one point lept out at me when reading the summary.

There are no privilege checks for modifying AUDIT/ALARM types, unlike in SMB; having WRITE_ACL access is sufficient. (The SMB server requires a client to have 'SeSecurityPrivilege', which it grants to members of the local Administrators group - smbadm show -m administrators).

This seems like a significant limitation as unwanted alarms are a serious irritant to people on call and unwanted audit records are a denial of service. Is it infeasible to add these checks to ZFS itself? Either of sys_audit or proc_audit (aka PRIV_SYS_AUDIT and PRIV_PROC_AUDIT) would at first glance be the ones to check for.

(Yes, I suspect it would be nasty to check in ZFS that an audit or alarm acl isn't being deleted when an acl is updated..)

Actions #10

Updated by Bill Sommerfeld about 1 year ago

Alternative suggestion:

If a process lacks appropriate privilege, hide the AUDIT/ALARM ace types (just as is proposed for NFS access), including merging them in as appropriate when the editing the ACL.

I think PRIV_PROC_AUDIT is the best fit of the existing privileges, though maybe a new privilege -- PRIV_FILE_AUDIT? -- would be better?

Actions #11

Updated by Matt Barden about 1 year ago

This seems like a significant limitation as unwanted alarms are a serious irritant to people on call and unwanted audit records are a denial of service.

ALARM and AUDIT ACEs give identical functionality; there aren't actually any 'alarms'. Any smb/nfs user can spam audit records by repeatedly accessing any object that legitimately has an audit acl, whether successful or not. An administrator worried about unwanted records due to this feature would not select the 'sa' class for auditing. This is also only a worry if the administrator allows access to the same objects both locally and over smb/nfs, which provides room for administrators to guard against your worries.

Is it infeasible to add these checks to ZFS itself? Either of sys_audit or proc_audit (aka PRIV_SYS_AUDIT and PRIV_PROC_AUDIT) would at first glance be the ones to check for.

Yes; for one, that would prevent unprivileged users from modifying the ACL at all, as the system does not allow updating one without the other (unlike SMB). For another, it would require every SMB and NFS user have these privileges, as file operations are done with the credentials of the client. I'm also not sure either privilege is appropriate here, given their existing definitions - it would require more consideration.

The current iteration of this feature is appropriate only for systems that do not allow non-administrative local access to files accessible over SMB/NFS. As I noted in my original email:

Do note that the current scope of this work is restricted to the in-kernel SMB2+ and NFSv4 servers; for example, this does not apply to local access.

There are many design and policy considerations that need to be made to expand this feature to local users, including what privileges are required for interaction with audit/alarm ACEs, and those decisions should be made at the time the feature is extended to local users; we shouldn't be making decisions now that bind us in the future. I agree that, at that time, audit/alarm type ACEs should be protected by a privilege (new or existing).

Actions #12

Updated by Bill Sommerfeld about 1 year ago

ALARM and AUDIT ACEs give identical functionality

can a downstream system be built that can tell whether an audit event was generated by an ALARM vs an AUDIT acl entry? (If not, that seems like a bug).

Any smb/nfs user can spam audit records by repeatedly accessing any object that legitimately has an audit acl, whether successful or not.

That's arguably not spam if the administrator intended it to be logged.

IMHO the volume of audit trail is such that it is only likely to be used successfully if it is used selectively. Which means only the system administrator should have a say in what gets audited. Say, crown jewels get audited, regular user filesystems don't.

Yes; for one, that would prevent unprivileged users from modifying the ACL at all, as the system does not allow updating one without the other (unlike SMB)

How do ACL updates work for NFS then? Why couldn't the same ACE-merging strategy be used for local ACL updates when the process doing the update lacks privilege to see the audit/alarm ACEs?

There are many design and policy considerations that need to be made to expand this feature to local users, including what privileges are required for interaction with audit/alarm ACEs, and those decisions should be made at the time the feature is extended to local users; we shouldn't be making decisions now that bind us in the future.

I fear that what you're doing now may preclude, or complicate, a clean solution to this problem in the future that works for both local users and remote file access.

Actions #13

Updated by Matt Barden about 1 year ago

can a downstream system be built that can tell whether an audit event was generated by an ALARM vs an AUDIT acl entry? (If not, that seems like a bug).

Audit records do not carry information over what kind of ACL generated them, and that's not a bug. Implementing Alarm functionality is not part of the design goals (even Windows doesn't support adding Alarm ACEs).

That's arguably not spam if the administrator intended it to be logged.

If the administrator intends for SACL-based events to be generated, they already need to take steps to segment local and remote access for that to be effective. As mentioned: local access does not generate audit events (that's for future work).

How do ACL updates work for NFS then? Why couldn't the same ACE-merging strategy be used for local ACL updates when the process doing the update lacks privilege to see the audit/alarm ACEs?

The NFS server does not recognize audit or alarm type ACEs, and returns an error if a client tries to set them (both before and after these changes). That makes it fundamentally different from local access.

I fear that what you're doing now may preclude, or complicate, a clean solution to this problem in the future that works for both local users and remote file access.

The clean solution to your concerns, as well as others associated with local access, is to comprehensively consider the problem of 'how to prevent local users from interfering with attempts to audit them', which would happen as part of implementing local access auditing. Nothing in the current changeset precludes future work in this area - retaining the ability to implement local access auditing in the future was one of the considerations in the design. Doing it piecemeal - particularly for a use case that is not supported by the feature today - will only complicate that work.

Actions

Also available in: Atom PDF