Re: [PATCH v2 0/3] fanotify: Allow user space to pass back additional audit info

[Richard Guy Briggs]

On 2022-04-28 20:44, Richard Guy Briggs wrote:
> The Fanotify API can be used for access control by requesting permission
> event notification. The user space tooling that uses it may have a
> complicated policy that inherently contains additional context for the
> decision. If this information were available in the audit trail, policy
> writers can close the loop on debugging policy. Also, if this additional
> information were available, it would enable the creation of tools that
> can suggest changes to the policy similar to how audit2allow can help
> refine labeled security.
> 
> This patch defines 2 additional fields within the response structure
> returned from user space on a permission event. The first field is 16
> bits for the context type. The context type will describe what the
> meaning is of the second field. The audit system will separate the
> pieces and log them individually.
> 
> The audit function was updated to log the additional information in the
> AUDIT_FANOTIFY record. The following is an example of the new record
> format:
> 
> type=FANOTIFY msg=audit(1600385147.372:590): resp=2 fan_type=1 fan_ctx=17

It might have been a good idea to tag this as RFC...  I have a few
questions:

1. Where did "resp=" come from?  It isn't in the field dictionary.  It
seems like a needless duplication of "res=".  If it isn't, maybe it
should have a "fan_" namespace prefix and become "fan_res="?

2. It appears I'm ok changing the "__u32 response" to "__u16" without
breaking old userspace.  Is this true on all arches?

3. What should be the action if response contains unknown flags or
types?  Is it reasonable to return -EINVAL?

4. Currently, struct fanotify_response has a fixed size, but if future
types get defined that have variable buffer sizes, how would that be
communicated or encoded?

> changelog:
> v1:
> - first version by Steve Grubb <sgrubb@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/2042449.irdbgypaU6@x2
> 
> v2:
> - enhancements suggested by Jan Kara <jack@xxxxxxx>
> - 1/3 change %d to %u in pr_debug
> - 2/3 change response from __u32 to __u16
> - mod struct fanotify_response and fanotify_perm_event add extra_info_type, extra_info_buf
> - extra_info_buf size max FANOTIFY_MAX_RESPONSE_EXTRA_LEN, add struct fanotify_response_audit_rule
> - extend debug statements
> - remove unneeded macros
> - [internal] change interface to finish_permission_event() and process_access_response()
> - 3/3 update format of extra information
> - [internal] change interface to audit_fanotify()
> - change ctx_type= to fan_type=
> Link: https://lore.kernel.org/r/cover.1651174324.git.rgb@xxxxxxxxxx
> 
> Richard Guy Briggs (3):
>   fanotify: Ensure consistent variable type for response
>   fanotify: define struct members to hold response decision context
>   fanotify: Allow audit to use the full permission event response
> 
>  fs/notify/fanotify/fanotify.c      |  5 ++-
>  fs/notify/fanotify/fanotify.h      |  4 +-
>  fs/notify/fanotify/fanotify_user.c | 59 ++++++++++++++++++++----------
>  include/linux/audit.h              |  8 ++--
>  include/linux/fanotify.h           |  3 ++
>  include/uapi/linux/fanotify.h      | 27 +++++++++++++-
>  kernel/auditsc.c                   | 18 +++++++--
>  7 files changed, 94 insertions(+), 30 deletions(-)
> 
> -- 
> 2.27.0
> 

- RGB

--
Richard Guy Briggs <rgb@xxxxxxxxxx>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635