Messages in this thread | | | From | Paul Moore <> | Date | Tue, 20 Sep 2016 16:49:52 -0400 | Subject | Re: [PATCH] lsm,audit,selinux: Introduce a new audit data type LSM_AUDIT_DATA_FILE |
| |
On Mon, Sep 19, 2016 at 2:00 PM, Paul Moore <paul@paul-moore.com> wrote: > On Fri, Sep 9, 2016 at 11:37 AM, Vivek Goyal <vgoyal@redhat.com> wrote: >> Hi, >> >> Right now LSM_AUDIT_DATA_PATH type contains "struct path" in union "u" of >> common_audit_data. This information is used to print path of file >> at the same time it is also used to get to dentry and inode. And this >> inode information is used to get to superblock and device and print >> device information. >> >> This does not work well for layered filesystems like overlay where dentry >> contained in path is overlay dentry and not the real dentry of underlying >> file system. That means inode retrieved from dentry is also overlay >> inode and not the real inode. >> >> seliux helpers like file_path_has_perm() are doing checks on inode retrieved >> from file_inode(). This returns the real inode and not the overlay inode. >> That means we are doing check on real inode but for audit purposes we >> are printing details of overlay inode and that can be confusing while >> debugging. >> >> Hence, introduce a new type LSM_AUDIT_DATA_FILE which carries file >> information and inode retrieved is real inode using file_inode(). That >> way right avc denied information is given to user. >> >> For example, following is one example avc before the patch. >> >> type=AVC msg=audit(1473360868.399:214): avc: denied { read open } for pid=1765 comm="cat" path="/root/git/selinux-testsuite/tests/overlay/container1/merged/readfile" dev="overlay" ino=21443 scontext=unconfined_u:unconfined_r:test_overlay_client_t:s0:c10,c20 tcontext=unconfined_u:object_r:test_overlay_files_ro_t:s0 tclass=file permissive=0 >> >> It looks as follows after the patch. >> >> type=AVC msg=audit(1473360017.388:282): avc: denied { read open } for pid=2530 comm="cat" path="/root/git/selinux-testsuite/tests/overlay/container1/merged/readfile" dev="dm-0" ino=2377915 scontext=unconfined_u:unconfined_r:test_overlay_client_t:s0:c10,c20 tcontext=unconfined_u:object_r:test_overlay_files_ro_t:s0 tclass=file permissive=0 >> >> Notice that now dev information points to "dm-0" device instead of "overlay" >> device. This makes it clear that check failed on underlying inode and not >> on the overlay inode. >> >> Signed-off-by: Vivek Goyal <vgoyal@redhat.com> >> --- >> include/linux/lsm_audit.h | 2 ++ >> security/lsm_audit.c | 13 +++++++++++++ >> security/selinux/hooks.c | 16 ++++++++-------- >> 3 files changed, 23 insertions(+), 8 deletions(-) > > Hi Vivek, > > Sorry for the delay, this fell through the my normal filters as it > didn't go to the SELinux and/or audit mailing list. However, this > patch looks reasonable to me and I think it is something we want in > 4.9 with the rest of the LSM/overlayfs bits. I'm building a test > kernel right now and assuming all goes well, I'll push this up to > James since it looks like we've got another week before the merge > window opens.
FYI, the testing went well so I just sent another pull request to James for v4.9.
-- paul moore www.paul-moore.com
| |