Bug 13565 - vfs_audit log does not show full path names
Summary: vfs_audit log does not show full path names
Status: ASSIGNED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: 4.7.1
Hardware: x64 Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Björn Baumbach
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-07 18:17 UTC by Carl Byington
Modified: 2019-10-01 07:21 UTC (History)
4 users (show)

See Also:


Attachments
git-am fix for 4.9.next, 4.8.next, 4.7.next (7.60 KB, patch)
2018-08-27 21:41 UTC, Jeremy Allison
asn: review+
Details
patch for v4-10 and v4-11 cherry-picked from master (with some adjustments) (11.56 KB, patch)
2019-09-18 15:52 UTC, Björn Baumbach
bbaumbach: review? (jra)
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Carl Byington 2018-08-07 18:17:24 UTC
vfs_audit log used to show the full path name. I am not sure when that
changed, but now open only logs the last component (like basename).
Rename still logs both old and new full pathnames. Is there some config
entry that needs to be set to get the full pathnames logged?


[global]
    full_audit:priority = notice
    full_audit:facility = local1
    full_audit:success = open rename
    full_audit:failure = connect
    full_audit:prefix = %u|%I|%S

[sname]
    path = /home/usr
    vfs objects = full_audit


Actual results:
ryan|$IP|sname|rename|ok|a/b/c.tmp|a/b/c.xlsx
ryan|$IP|sname|open|ok|r|c.xlsx

Expected results:
ryan|$IP|sname|rename|ok|a/b/c.tmp|a/b/c.xlsx
ryan|$IP|sname|open|ok|r|a/b/c.xlsx


With the current code, we don't know which one of the possibly many
c.xlsx files were read.

Looking at vfs_full_audit.c, smb_full_audit_open() and
smb_full_audit_rename() are very similar, using smb_fname_str_do_log()
to format the file name string for logging. Apparently the difference is
at a higher level. Can we assume that the filename should be prefixed
with the current directory, or might the current directory have been
changed by the time the audit log is called?
Comment 1 Andreas Schneider 2018-08-22 14:52:09 UTC
Jeremy, could you please look into this issue?
Comment 2 Jeremy Allison 2018-08-22 15:50:03 UTC
(In reply to Andreas Schneider from comment #1)

> Jeremy, could you please look into this issue?

After you reviewed my gluster dirpath patch on the list please :-).
Comment 3 Jeremy Allison 2018-08-27 21:41:37 UTC
Created attachment 14449 [details]
git-am fix for 4.9.next, 4.8.next, 4.7.next

Cherry-picked from master.
Comment 4 Andreas Schneider 2018-08-28 13:19:11 UTC
Karolin, could you please apply the patch the the relevant branches? Thanks!
Comment 5 Karolin Seeger 2018-08-31 08:32:30 UTC
(In reply to Andreas Schneider from comment #4)
Pushed to autobuild-v4-{9,8,7}-test.
Comment 6 Stefan Metzmacher 2018-09-05 11:33:57 UTC
(In reply to Karolin Seeger from comment #5)

This fails autobuild consistently for the samba-systemkrb5 target:

[781(4)/2304 at 10s] samba3.rpc.lsa.lookupsids krb5 with old ccache ncacn_np with [smb2] (ktest)
smbtorture 4.8.6-DEVELOPERBUILD
Using seed 1536146480
WARNING!: Failed to connect to remote server: ncacn_np:LOCALKTEST6[,smb2] NT_STATUS_CONNECTION_DISCONNECTED

UNEXPECTED(failure): samba3.rpc.lsa.lookupsids krb5 with old ccache ncacn_np with [smb2] .lsa.LookupSidsReply(ktest)
REASON: Exception: Exception: Setup failed: ../source4/torture/rpc/rpc.c:310: status was NT_STATUS_CONNECTION_DISCONNECTED, expected NT_STATUS_OK: Error connecting to server

FAILED (1 failures, 0 errors and 0 unexpected successes in 0 testsuites)
Comment 7 Jeremy Allison 2018-09-05 19:00:50 UTC
Metze, did you post this comment on the right bug ? This (samba-systemkrb5) doesn't seem related to the vfs_audit_log code ?

Jeremy.
Comment 8 Karolin Seeger 2018-09-10 07:34:25 UTC
(In reply to Jeremy Allison from comment #7)
At least I can confirm that autobuild-v4-8-test and autobuild-v4-7-test failed ~10 times with these patches and does not without them.
Comment 9 Stefan Metzmacher 2018-09-10 08:58:34 UTC
(In reply to Jeremy Allison from comment #7)

I guess that this environment just compiles as the first task
and it fails. I guess it would also fail in any other environment.
Comment 10 Carl Byington 2019-06-07 21:31:52 UTC
Any progress on this? It looks like the patch never made it into 4.8
Comment 11 Björn Baumbach 2019-09-13 09:45:37 UTC
I've verified that the patches are included in current Samba releases >=4.9.0.

The fix will not go into 4.8 anymore, because 4.8 is in security fixes only mode and will be discontinued very soon.

I close the bug as resolved, now.

Best regards,
Björn
Comment 12 Björn Baumbach 2019-09-16 12:16:52 UTC
I re-open this bug because the issue is not fixed completely.

Not all calls use the smb_fname_str_do_log() function. For example "realpath" and "opendir" does still not log the full path name.

Björn
Comment 13 Björn Baumbach 2019-09-18 15:52:19 UTC
Created attachment 15475 [details]
patch for v4-10 and v4-11 cherry-picked from master (with some adjustments)

I've attached the picked patch from master.

Without this patch the different vfs calls log a mix of full path names, paths relative to the share root or just the affected file name.

With the patch the full path names should be used always.
Comment 14 Andreas Schneider 2019-09-26 10:21:56 UTC
Karolin, could you please apply the patch to 4.10 and 4.11? Thanks!
Comment 15 Karolin Seeger 2019-09-26 11:54:52 UTC
(In reply to Andreas Schneider from comment #14)
Pushed to autobuild-v4-{11,10}-test.
Comment 16 Stefan Metzmacher 2019-09-26 17:26:15 UTC
Comment on attachment 15475 [details]
patch for v4-10 and v4-11 cherry-picked from master (with some adjustments)

It seems that the backports are missing a patch that's only in master:

It fails like this:

../../source3/modules/vfs_full_audit.c: In function ‘smb_full_audit_setxattr’:
../../source3/modules/vfs_full_audit.c:2794:30: error: passing argument 1 of ‘smb_fname_str_do_log’ from incompatible pointer type [-Werror=incompatible-pointer-types]
         smb_fname_str_do_log(handle->conn, smb_fname),
                              ^~~~~~
../../source3/modules/vfs_full_audit.c:663:20: note: expected ‘const struct smb_filename *’ but argument is of type ‘struct connection_struct *’
 static const char *smb_fname_str_do_log(const struct smb_filename *cwd,
                    ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

I found this in samba-static.stderr
Comment 17 Karolin Seeger 2019-10-01 07:21:20 UTC
Removed for now from 4.10 and 4.11, re-assigning to Björn.