Re: Re: [RFC PATCH v1 3/7] landlock: Log ruleset creation and release

On Fri, Dec 29, 2023 at 06:42:10PM +0100, Mickaël Salaün wrote:

On Fri, Dec 22, 2023 at 05:42:35PM -0500, Paul Moore wrote:

quoted

On Thu, Dec 21, 2023 at 1:45 PM Mickaël Salaün [off-list ref] wrote:

quoted

On Wed, Dec 20, 2023 at 04:22:15PM -0500, Paul Moore wrote:

quoted

On Thu, Sep 21, 2023 at 2:17 AM Mickaël Salaün [off-list ref] wrote:

quoted

Add audit support for ruleset/domain creation and release ...

...

quoted

quoted

For rule addition, several records per landlock_add_rule(2) call.
Example with a path_beneath rule:
- AUDIT_LANDLOCK_RULESET: "id=[ruleset ID] op=add_rule"
- AUDIT_LANDLOCK_PATH: "scope=beneath path=[file path] dev= ino="
- AUDIT_LANDLOCK_ACCESS: "type=fs rights=[bitmask]"

I worry that LANDLOCK_PATH is too much of a duplicate for the existing
PATH record.  Assuming the "scope=" field is important, could it live
in the LANDLOCK_ACCESS record and then you could do away with the
dedicated LANDLOCK_PATH record?  Oh, wait ... this is to record the
policy, not a individual access request, gotcha.  If that is the case
and RULESET, PATH, ACCESS are all used simply to record the policy
information I might suggest creation of an AUDIT_LANDLOCK_POLICY
record that captures all of the above.  If you think that is too
cumbersome, then perhaps you can do the object/access-specific record
type, e.g. AUDIT_LANDLOCK_POLICY_FS and AUDIT_LANDLOCK_POLICY_NET.

OK, what about this records for *one* rule addition event?

- AUDIT_LANDLOCK_RULE: "ruleset=[ruleset ID] rule_type=path_beneath
  allowed_access=[bitmask]"
- AUDIT_PATH: "path=[file path] dev= ino= ..."

However, because struct landlock_path_beneath_attr can evolve and get
new fields which might be differents than the landlock_net_port_attr's
ones, wouldn't it be wiser to use a dedicated AUDIT_LANDLOCK_RULE_FS or
AUDIT_LANDLOCK_RULE_PATH_BENEATH record type? These names are getting a
bit long though, but types match the UAPI.

Hmm, AUDIT_PATH is used when a syscall's argument is a path, but in the
case of Landlock, the arguments are file descriptors.

I can still export audit_copy_inode() to create a synthetic audit_names
struct, and export/call audit_log_name() to create an AUDIT_PATH entry
but I'm not sure it is the best approach. What would you prefer?
Should I use AUDIT_TYPE_NORMAL or create a new one?

[...]

quoted

quoted

For denied FS access:
- AUDIT_LANDLOCK_DENIAL: "id=[domain ID] op=mkdir"
- AUDIT_LANDLOCK_PATH: "scope=exact path=[file path] dev= ino="

I would use a single record type, i.e. AUDIT_LANDLOCK_ACCESS, to
capture both access granted and denied events.  I'd also omit the
dedicated LANDLOCK_PATH record here in favor of the generic PATH
record (see my comments above).

Makes sense for the generic PATH record. We would get this:

- AUDIT_LANDLOCK_ACCESS: "domain=[domain ID] op=mkdir result=denied"
- AUDIT_PATH: "path=[file path] dev= ino= ..."