Paul, could you please answer this question so I can move forward? :)

On Thu, Sep 13, 2018 at 10:13 AM Paul Moore <paul(a)paul-moore.com&gt; wrote: > On Thu, Sep 13, 2018 at 9:58 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > > Paul, could you please answer this question so I can move forward? :) > > Yep, sorry for the delay ... I just went back over the original problem, your proposed fix, and all of the discussion in this thread. Sadly, I don't think the patch you have proposed is the right fix. As Steve has pointed out, the CWD path is the working directory from which the current process was executed. I believe we should log the full path, or as complete a path as possible, in the nametype=CWD PATH records. While the nametype=PARENT PATH records have a connection with some of the other PATH records (e.g. DELETE and CREATE), the nametype=PARENT PATH records are independent of the current working directory, although they sometimes may be the same; in the cases where they are the same, this is purely a coincidence and is due to operation being performed, not something that should be seen as a flaw. From what I can tell, there are issues involving the nametype=PARENT PATH records, especially when it comes to the *at() syscalls, but no issue where the nametype=CWD PATH records have been wrong, is that correct?

> > > > case 0: > > > > /* name was specified as a relative path and the > > > > * directory component is the cwd */ > > > > - audit_log_d_path(ab, " name=", &context->pwd); > > > > + audit_log_untrustedstring(ab, "."); > > > > > > This isn't a comprehensive review, I only gave this a quick look, but > > > considering you are only logging "." above we can safely use > > > audit_log_string() and safe a few cycles. > > > > I used audit_log_untrustedstring() to maintain the current norm that > > the name= field would always contain a quoted string (either in > > double-quotes or hex-escaped). I don't know if such consistency is > > important for parsing in userspace (it should probably be robust > > enough to handle any format), but I wanted to be on the safe side. > > In this particular case there should be no visible difference in the > resulting record and audit_log_string() is going to have less overhead > in the kernel. OK, so should I just replace it with: audit_log_string(ab, "."); or should I still escape the path manually: audit_log_string(ab, "\".\""); ?

On Wed, Sep 19, 2018 at 7:01 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > On Wed, Sep 19, 2018 at 3:35 AM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > On Thu, Sep 13, 2018 at 10:13 AM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > > On Thu, Sep 13, 2018 at 9:58 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > > > > Paul, could you please answer this question so I can move forward? :) > > > > > > Yep, sorry for the delay ... > > > > I just went back over the original problem, your proposed fix, and all > > of the discussion in this thread. > > > > Sadly, I don't think the patch you have proposed is the right fix. > > > > As Steve has pointed out, the CWD path is the working directory from > > which the current process was executed. I believe we should log the > > full path, or as complete a path as possible, in the nametype=CWD PATH > > records. While the nametype=PARENT PATH records have a connection > > with some of the other PATH records (e.g. DELETE and CREATE), the > > nametype=PARENT PATH records are independent of the current working > > directory, although they sometimes may be the same; in the cases where > > they are the same, this is purely a coincidence and is due to > > operation being performed, not something that should be seen as a > > flaw. > > > > From what I can tell, there are issues involving the nametype=PARENT > > PATH records, especially when it comes to the *at() syscalls, but no > > issue where the nametype=CWD PATH records have been wrong, is that > > correct? > > Sorry, but I think you are completely misunderstanding the problem... > I tried to explain it several times in different ways, but apparently > I'm still not doing it right... > > First of all, there is no "nametype=CWD" PATH record. There is only > the classic CWD record that is associated to every syscall and I don't > touch that one at all. The information in the CWD record is perfectly > fine. Yes, that was a casualty of me looking at too many audit logs too late in the day, I mistakenly typed it as a nametype PATH record when CWD is its own record type. My apologies. > Let me try to demonstrate it with some more verbose examples. (TL;DR: > The info in the CWD record is correct, but it is being abused in > audit_log_name().) > > EXAMPLE #1 (The non-edge case): > 1. A userspace process calls rename("dir1/file1", "dir2/file2") with > CWD set to "/home/user". > 2. The syscall causes four calls to __audit_inode(), which generate > four 'struct audit_names' objects with the following information > (maybe not in this specific order, but that doesn't matter): > .name = "dir1/file1", .type = AUDIT_TYPE_PARENT, .name_len = 5 > .name = "dir1/file1", .type = AUDIT_TYPE_DELETE, .name_len = AUDIT_NAME_FULL > .name = "dir2/file2", .type = AUDIT_TYPE_PARENT, .name_len = 5 > .name = "dir2/file2", .type = AUDIT_TYPE_CREATE, .name_len = AUDIT_NAME_FULL > 3. At the end of the syscall, audit_log_name() is called on each of > these objects and produces the following PATH records (simplifed): > nametype=PARENT name="dir1/" > nametype=DELETE name="dir1/file1" > nametype=PARENT name="dir2/" > nametype=CREATE name="dir2/file2" > > Notice that for the PARENT objects the .name_len is truncated to only > the directory component. > > EXAMPLE #2 (The single-path-component case): > 1. A userspace process calls rename("file1", "file2") with CWD set to > "/home/user". > 2. The 'struct audit_names' objects will now be: > .name = "file1", .type = AUDIT_TYPE_PARENT, .name_len = 0 > .name = "file1", .type = AUDIT_TYPE_DELETE, .name_len = AUDIT_NAME_FULL > .name = "file2", .type = AUDIT_TYPE_PARENT, .name_len = 0 > .name = "file2", .type = AUDIT_TYPE_CREATE, .name_len = AUDIT_NAME_FULL > 3. At the end of the syscall, audit_log_name() is called on each of > these objects and produces the following PATH records (simplifed): > nametype=PARENT name="/home/user" > nametype=DELETE name="file1" > nametype=PARENT name="/home/user" > nametype=CREATE name="file2" > > Notice that in this case, the "clever" logic in audit_log_name() > wanted to avoid logging an empty path (name="") in the PARENT records, > so it instead put the CWD path in there ("/home/user"). In this case > this is perfectly valid (although could be a bit surprising that there > is suddenly a full path instead of a relative one), since the full > path of "file1" is actually "/home/user/file1". A quick comment on the "surprising" nature of seeing the "/home/user" path in the PARENT record instead of "/home/user/file1" - the PARENT record is the file's parent directory (as you mention above), so it would be surprising to see "/home/user/file1" in the PARENT record, seeing just "/home/user" is valid and could be expected.

> EXAMPLE #3 (The non-edge renameat(2) case): > 1. A userspace process calls the following syscalls (with CWD set to > "/home/user"): > int srcfd = open("/some/path1", O_DIRECTORY | O_PATH); > int dstfd = open("/another/path2", O_DIRECTORY | O_PATH); > renameat(srcfd, "dir1/file1", dstfd, "dir2/file2"); > 2. The 'name', 'type' and 'name_len' fields of the 'struct > audit_names' objects will now be exactly the same as in EXAMPLE #1: > .name = "dir1/file1", .type = AUDIT_TYPE_PARENT, .name_len = 5 > .name = "dir1/file1", .type = AUDIT_TYPE_DELETE, .name_len = AUDIT_NAME_FULL > .name = "dir2/file2", .type = AUDIT_TYPE_PARENT, .name_len = 5 > .name = "dir2/file2", .type = AUDIT_TYPE_CREATE, .name_len = AUDIT_NAME_FULL > 3. The type and name information in the PATH records will be also > exactly the same: > nametype=PARENT name="dir1/" > nametype=DELETE name="dir1/file1" > nametype=PARENT name="dir2/" > nametype=CREATE name="dir2/file2" > > Even in this case the information in the records is correct (although > there is no information telling us that "dir1/..." actually > corresponds to "/some/path1/dir1/..." and "dir2/..." actually > corresponds to "/another/path2/dir2/..."). Yeah, I'm starting to think we should always log the absolute path in the PARENT record. > So far so good, but now we are coming to... > > EXAMPLE #4 (The single-path-component renameat(2) case): > 1. A userspace process calls the following syscalls (with CWD set to > "/home/user"): > int srcfd = open("/some/path1", O_DIRECTORY | O_PATH); > int dstfd = open("/another/path2", O_DIRECTORY | O_PATH); > renameat(srcfd, "file1", dstfd, "file2"); > 2. The 'name', 'type' and 'name_len' fields of the 'struct > audit_names' objects will now be exactly the same as in EXAMPLE #2: > .name = "file1", .type = AUDIT_TYPE_PARENT, .name_len = 0 > .name = "file1", .type = AUDIT_TYPE_DELETE, .name_len = AUDIT_NAME_FULL > .name = "file2", .type = AUDIT_TYPE_PARENT, .name_len = 0 > .name = "file2", .type = AUDIT_TYPE_CREATE, .name_len = AUDIT_NAME_FULL > 3. The type and name information in the PATH records will be also > exactly the same: > nametype=PARENT name="/home/user" > nametype=DELETE name="file1" > nametype=PARENT name="/home/user" > nametype=CREATE name="file2" > > ...and HERE is the problem. The parent of "file1" is not "/home/user", > it is "/some/path1", and the parent of "file2" is also not > "/home/user", it is "/another/path2". > > The proposed fix simply changes the handling of the name_len == 0 case > to output "." instead of the CWD. This doesn't solve the wider problem > that we don't have the dirfd path information (GHAK #9), but it at > least makes the situation in EXAMPLE #4 *not worse* than in EXAMPLE #3 > (i.e. it fixes the less demanding GHAK #95). As an additional minor > benefit it also brings a bit more consistency - with it the PATH > records will contain relative (resp. absolute) paths if and only if > the corresponding path given to the syscall was relative (resp. > absolute). > > I hope this finally clears things up. Yes, it does, thanks. My apologies for getting tangled up in the CWD logging. My current thinking is that logging relative paths in the nametype=PARENT PATH record is a mistake. Yes, I understand there are some cases where this information will be the same as the CWD information, and that could add some additional log overhead, but as tricky as path resolution can be I think this is the safest option and the one I would like to see us pursue. This will likely require some extra work for the *at() syscalls, but those aren't reported correctly right now as discussed here and elsewhere. I would expect the non-PARENT PATH records to stay as they are, in other words this should only affect things which are *not* (.name_len == AUDIT_NAME_FULL).

> BTW, you still didn't answer my brief question from a few replies > above (quoted below with context): FYI, I didn't answer it in my previous email because I thought logging the "." as a PATH name was wrong, so that bit of your patch wasn't desirable regardless of the quotes. However, to answer your question in a general sense, the usual guidance is to follow existing convention and that would indicate that the 'name' field value in the PATH record should be enclosed in double quotes.

Hopefully that answers your question, but if you're still unclear let me know. -- paul moore www.paul-moore.com--

On Wed, Oct 31, 2018 at 4:54 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > On Wed, Sep 19, 2018 at 5:44 PM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > On Wed, Sep 19, 2018 at 7:01 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > > > On Wed, Sep 19, 2018 at 3:35 AM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > > > On Thu, Sep 13, 2018 at 10:13 AM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > > > > On Thu, Sep 13, 2018 at 9:58 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > > > > > > Paul, could you please answer this question so I can move forward? :) > > > > > > > > > > Yep, sorry for the delay ... > > > > > > > > I just went back over the original problem, your proposed fix, and all > > > > of the discussion in this thread. > > > > > > > > Sadly, I don't think the patch you have proposed is the right fix. > > > > > > > > As Steve has pointed out, the CWD path is the working directory from > > > > which the current process was executed. I believe we should log the > > > > full path, or as complete a path as possible, in the nametype=CWD PATH > > > > records. While the nametype=PARENT PATH records have a connection > > > > with some of the other PATH records (e.g. DELETE and CREATE), the > > > > nametype=PARENT PATH records are independent of the current working > > > > directory, although they sometimes may be the same; in the cases where > > > > they are the same, this is purely a coincidence and is due to > > > > operation being performed, not something that should be seen as a > > > > flaw. > > > > > > > > From what I can tell, there are issues involving the nametype=PARENT > > > > PATH records, especially when it comes to the *at() syscalls, but no > > > > issue where the nametype=CWD PATH records have been wrong, is that > > > > correct? > > > > > > Sorry, but I think you are completely misunderstanding the problem... > > > I tried to explain it several times in different ways, but apparently > > > I'm still not doing it right... > > > > > > First of all, there is no "nametype=CWD" PATH record. There is only > > > the classic CWD record that is associated to every syscall and I don't > > > touch that one at all. The information in the CWD record is perfectly > > > fine. > > > > Yes, that was a casualty of me looking at too many audit logs too late > > in the day, I mistakenly typed it as a nametype PATH record when CWD > > is its own record type. My apologies. > > > > > Let me try to demonstrate it with some more verbose examples. (TL;DR: > > > The info in the CWD record is correct, but it is being abused in > > > audit_log_name().) > > > > > > EXAMPLE #1 (The non-edge case): > > > 1. A userspace process calls rename("dir1/file1", "dir2/file2") with > > > CWD set to "/home/user". > > > 2. The syscall causes four calls to __audit_inode(), which generate > > > four 'struct audit_names' objects with the following information > > > (maybe not in this specific order, but that doesn't matter): > > > .name = "dir1/file1", .type = AUDIT_TYPE_PARENT, .name_len = 5 > > > .name = "dir1/file1", .type = AUDIT_TYPE_DELETE, .name_len = AUDIT_NAME_FULL > > > .name = "dir2/file2", .type = AUDIT_TYPE_PARENT, .name_len = 5 > > > .name = "dir2/file2", .type = AUDIT_TYPE_CREATE, .name_len = AUDIT_NAME_FULL > > > 3. At the end of the syscall, audit_log_name() is called on each of > > > these objects and produces the following PATH records (simplifed): > > > nametype=PARENT name="dir1/" > > > nametype=DELETE name="dir1/file1" > > > nametype=PARENT name="dir2/" > > > nametype=CREATE name="dir2/file2" > > > > > > Notice that for the PARENT objects the .name_len is truncated to only > > > the directory component. > > > > > > EXAMPLE #2 (The single-path-component case): > > > 1. A userspace process calls rename("file1", "file2") with CWD set to > > > "/home/user". > > > 2. The 'struct audit_names' objects will now be: > > > .name = "file1", .type = AUDIT_TYPE_PARENT, .name_len = 0 > > > .name = "file1", .type = AUDIT_TYPE_DELETE, .name_len = AUDIT_NAME_FULL > > > .name = "file2", .type = AUDIT_TYPE_PARENT, .name_len = 0 > > > .name = "file2", .type = AUDIT_TYPE_CREATE, .name_len = AUDIT_NAME_FULL > > > 3. At the end of the syscall, audit_log_name() is called on each of > > > these objects and produces the following PATH records (simplifed): > > > nametype=PARENT name="/home/user" > > > nametype=DELETE name="file1" > > > nametype=PARENT name="/home/user" > > > nametype=CREATE name="file2" > > > > > > Notice that in this case, the "clever" logic in audit_log_name() > > > wanted to avoid logging an empty path (name="") in the PARENT records, > > > so it instead put the CWD path in there ("/home/user"). In this case > > > this is perfectly valid (although could be a bit surprising that there > > > is suddenly a full path instead of a relative one), since the full > > > path of "file1" is actually "/home/user/file1". > > > > A quick comment on the "surprising" nature of seeing the "/home/user" > > path in the PARENT record instead of "/home/user/file1" - the PARENT > > record is the file's parent directory (as you mention above), so it > > would be surprising to see "/home/user/file1" in the PARENT record, > > seeing just "/home/user" is valid and could be expected. > > Wait, no... I meant it is surprising that there is suddenly a full > path to directory ("/home/user") instead of a relative one (which > would be "." in this case). This happens if and only if the original > relative path has just a single component, which is a strange > condition for anyone who doesn't know how the audit_log_name() > function is implemented. The fact that the PARENT record has a path > to the parent si obviously logical and sound, I have no problem with > that :) > > > > > > EXAMPLE #3 (The non-edge renameat(2) case): > > > 1. A userspace process calls the following syscalls (with CWD set to > > > "/home/user"): > > > int srcfd = open("/some/path1", O_DIRECTORY | O_PATH); > > > int dstfd = open("/another/path2", O_DIRECTORY | O_PATH); > > > renameat(srcfd, "dir1/file1", dstfd, "dir2/file2"); > > > 2. The 'name', 'type' and 'name_len' fields of the 'struct > > > audit_names' objects will now be exactly the same as in EXAMPLE #1: > > > .name = "dir1/file1", .type = AUDIT_TYPE_PARENT, .name_len = 5 > > > .name = "dir1/file1", .type = AUDIT_TYPE_DELETE, .name_len = AUDIT_NAME_FULL > > > .name = "dir2/file2", .type = AUDIT_TYPE_PARENT, .name_len = 5 > > > .name = "dir2/file2", .type = AUDIT_TYPE_CREATE, .name_len = AUDIT_NAME_FULL > > > 3. The type and name information in the PATH records will be also > > > exactly the same: > > > nametype=PARENT name="dir1/" > > > nametype=DELETE name="dir1/file1" > > > nametype=PARENT name="dir2/" > > > nametype=CREATE name="dir2/file2" > > > > > > Even in this case the information in the records is correct (although > > > there is no information telling us that "dir1/..." actually > > > corresponds to "/some/path1/dir1/..." and "dir2/..." actually > > > corresponds to "/another/path2/dir2/..."). > > > > Yeah, I'm starting to think we should always log the absolute path in > > the PARENT record. > > > > > So far so good, but now we are coming to... > > > > > > EXAMPLE #4 (The single-path-component renameat(2) case): > > > 1. A userspace process calls the following syscalls (with CWD set to > > > "/home/user"): > > > int srcfd = open("/some/path1", O_DIRECTORY | O_PATH); > > > int dstfd = open("/another/path2", O_DIRECTORY | O_PATH); > > > renameat(srcfd, "file1", dstfd, "file2"); > > > 2. The 'name', 'type' and 'name_len' fields of the 'struct > > > audit_names' objects will now be exactly the same as in EXAMPLE #2: > > > .name = "file1", .type = AUDIT_TYPE_PARENT, .name_len = 0 > > > .name = "file1", .type = AUDIT_TYPE_DELETE, .name_len = AUDIT_NAME_FULL > > > .name = "file2", .type = AUDIT_TYPE_PARENT, .name_len = 0 > > > .name = "file2", .type = AUDIT_TYPE_CREATE, .name_len = AUDIT_NAME_FULL > > > 3. The type and name information in the PATH records will be also > > > exactly the same: > > > nametype=PARENT name="/home/user" > > > nametype=DELETE name="file1" > > > nametype=PARENT name="/home/user" > > > nametype=CREATE name="file2" > > > > > > ...and HERE is the problem. The parent of "file1" is not "/home/user", > > > it is "/some/path1", and the parent of "file2" is also not > > > "/home/user", it is "/another/path2". > > > > > > The proposed fix simply changes the handling of the name_len == 0 case > > > to output "." instead of the CWD. This doesn't solve the wider problem > > > that we don't have the dirfd path information (GHAK #9), but it at > > > least makes the situation in EXAMPLE #4 *not worse* than in EXAMPLE #3 > > > (i.e. it fixes the less demanding GHAK #95). As an additional minor > > > benefit it also brings a bit more consistency - with it the PATH > > > records will contain relative (resp. absolute) paths if and only if > > > the corresponding path given to the syscall was relative (resp. > > > absolute). > > > > > > I hope this finally clears things up. > > > > Yes, it does, thanks. My apologies for getting tangled up in the CWD logging. > > > > My current thinking is that logging relative paths in the > > nametype=PARENT PATH record is a mistake. Yes, I understand there are > > some cases where this information will be the same as the CWD > > information, and that could add some additional log overhead, but as > > tricky as path resolution can be I think this is the safest option and > > the one I would like to see us pursue. This will likely require some > > extra work for the *at() syscalls, but those aren't reported correctly > > right now as discussed here and elsewhere. > > > > I would expect the non-PARENT PATH records to stay as they are, in > > other words this should only affect things which are *not* (.name_len > > == AUDIT_NAME_FULL). > > Well, logging (correct) absolute path in *all* PATH records would > certainly solve the problem (and also GHAK #9) and considering all the > problems with relative paths it might even be the most reasonable > solution. However, doing so only in the case of PARENT records > doesn't seem like a good idea to me... Consider the first two > arguments of a renameat(2) syscall with olddirfd pointing to > "/some/dir" and an oldpath of "subdir/file". We would get PATH > records like this: > > nametype=PARENT path="/some/dir/subdir" > nametype=DELETE path="subdir/file" > [...] Let's reset this discussion a bit ... if we abolish relative paths and make everything absolute, is there even a need to log PARENT?

On Tue, Nov 6, 2018 at 3:09 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > On Tue, Nov 6, 2018 at 12:30 AM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > Let's reset this discussion a bit ... if we abolish relative paths and > > make everything absolute, is there even a need to log PARENT? > > If there ever was such need, then this won't change when we switch to > absolute paths. The PATH records contain some fields (inode, dev, obj, > ...) that can be different for the child and parent and I would say > these are the only new information that the PARENT records provide > over the corresponding CREATE/DELETE records. Sigh. Of course the inode information is going to be different between the object in question and the parent, they are different filesystem objects. Ask your self the bigger question: does the PARENT record provide me any security relevant information related to the filesystem object that is being accessed?

With the messed up state of path name auditing, the PARENT records are useful when trying to recreate the full path used by the process to access a given filesystem object (transient as it may be, the path name can still be useful after the fact). If we switch to always recording absolute path names, why do we care about recording the PARENT filesystem object at all (both the path and the inode information)?

On Tue, Nov 13, 2018 at 10:25 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt;

> On Tue, Nov 6, 2018 at 9:19 PM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > On Tue, Nov 6, 2018 at 3:09 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt;

> > > On Tue, Nov 6, 2018 at 12:30 AM Paul Moore <paul(a)paul-moore.com&gt;

> > > > Let's reset this discussion a bit ... if we abolish relative paths > > > > and make everything absolute, is there even a need to log PARENT?

> > > If there ever was such need, then this won't change when we switch to > > > absolute paths. The PATH records contain some fields (inode, dev, > > > obj, that can be different for the child and parent and I would say > > > these are the only new information that the PARENT records provide > > > over the corresponding CREATE/DELETE records. > > > > Sigh. Of course the inode information is going to be different > > between the object in question and the parent, they are different > > filesystem objects. Ask your self the bigger question: does the > > PARENT record provide me any security relevant information related to > > the filesystem object that is being accessed? > > I would say it does. Consider e.g. the "mode" and "obj" fields. When > you move (rename) a file from one directory to another (which is the > main, if not the only, case when a PARENT record is emitted), then you > are usually more interested in the values for the parent directory > than the file itself (that's what determines if you can move the > file). I disagree on the importance of the mode/obj of the parent in a rename operation. From my perspective I really only care about the filesystem object that is being moved and if it succeeded or not. The idea that you care more about the parent than the object being moved makes no sense to me at all.

> For example, assume you have a rule that logs whenever some sensitive > file gets moved. You do not expect that to happen because you set the > file/directory permissions and labels so that it can't be done by > anyone unauthorized. But something goes wrong, the permissions/labels > get changed somehow ... In which case you should be watching for changes to the filesystem metadata which affect access rights. That is how you should catch changes to permissions on a filesystem object as it gives you information about the change as well as the subject information of the user/process which made the change.

> ... and a bad actor leverages the situation to move > the file. Then later you want to investigate this security incident > and as part of it you want to know what permissions were set on the > directories involved that had allowed the file to be moved, because > this may give you a useful lead. With PARENT records, you get such > information, without them you don't. If you only have that information in the parent record then you are missing half the story, and it may be the important half as the interesting bit of information in this example is the identity of the user/process which was able to change permissions to enable the rename to take place. Unless Steve provides evidence of some compelling certification requirement which necessitates the need for a parent record, I see no reason to keep it.

On Tuesday, November 13, 2018 11:30:55 AM EST Paul Moore wrote: > On Tue, Nov 13, 2018 at 10:25 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > > On Tue, Nov 6, 2018 at 9:19 PM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > > On Tue, Nov 6, 2018 at 3:09 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt; wrote: > > > > On Tue, Nov 6, 2018 at 12:30 AM Paul Moore <paul(a)paul-moore.com&gt; wrote: > > > > > Let's reset this discussion a bit ... if we abolish relative paths > > > > > and make everything absolute, is there even a need to log PARENT? I believe that Al Viro has said that sometime paths are not resolvable in the future. For example process A opens a file. It passes the descriptor to another process using scm rights. Directory tree is deleted. Process B receives the descriptor. Process A exits. Process B is now accessing what?

> > > > If there ever was such need, then this won't change when we switch to > > > > absolute paths. The PATH records contain some fields (inode, dev, > > > > obj, that can be different for the child and parent and I would say > > > > these are the only new information that the PARENT records provide > > > > over the corresponding CREATE/DELETE records. > > > > > > Sigh. Of course the inode information is going to be different > > > between the object in question and the parent, they are different > > > filesystem objects. Ask your self the bigger question: does the > > > PARENT record provide me any security relevant information related to > > > the filesystem object that is being accessed? > > > > I would say it does. Consider e.g. the "mode" and "obj" fields. When > > you move (rename) a file from one directory to another (which is the > > main, if not the only, case when a PARENT record is emitted), then you > > are usually more interested in the values for the parent directory > > than the file itself (that's what determines if you can move the > > file). > > I disagree on the importance of the mode/obj of the parent in a rename > operation. From my perspective I really only care about the > filesystem object that is being moved and if it succeeded or not. The > idea that you care more about the parent than the object being moved > makes no sense to me at all. The mode is really not important.

> > For example, assume you have a rule that logs whenever some sensitive > > file gets moved. You do not expect that to happen because you set the > > file/directory permissions and labels so that it can't be done by > > anyone unauthorized. But something goes wrong, the permissions/labels > > get changed somehow ... > > In which case you should be watching for changes to the filesystem > metadata which affect access rights. That is how you should catch > changes to permissions on a filesystem object as it gives you > information about the change as well as the subject information of the > user/process which made the change. Right. You would watch for attribute changes on a directory. > > ... and a bad actor leverages the situation to move > > the file. Then later you want to investigate this security incident > > and as part of it you want to know what permissions were set on the > > directories involved that had allowed the file to be moved, because > > this may give you a useful lead. With PARENT records, you get such > > information, without them you don't. > > If you only have that information in the parent record then you are > missing half the story, and it may be the important half as the > interesting bit of information in this example is the identity of the > user/process which was able to change permissions to enable the rename > to take place. > > Unless Steve provides evidence of some compelling certification > requirement which necessitates the need for a parent record, I see no > reason to keep it. Certification does not care about parent records. What is cares about is being able to say what the object was that is acted upon. So, that would be device and inode. But that is not nice for people. So, we would also like to know the path name. If parent record are necessary because of the *at syscalls, then that may be the only purpose. And they do not need to be emitted each time. If we also have a full path, then they are not needed. If we have a relative path, then CWD is needed, not the parent.

I also understand that at times full path resolution may not work out due to directory permissions blocking access at a deeper level of the directory tree. In those cases, we probably do want a PARENT record for the failed access attempt. I think that discussion may have prompted creation of PARENT records a long time ago. But at the same time, I also mentioned that the path was passed as an arguement and we could always emit that...but we do not have any other information such as mode or security label. In summary, certification does not say we need PARENT directories of the object. We need the object. And we only need help when its not clear what the object was.

On Thu, Aug 2, 2018 at 8:03 PM Paul Moore <paul(a)paul-moore.com&gt; wrote: > On Thu, Aug 2, 2018 at 7:45 AM Ondrej Mosnacek <omosnace(a)redhat.com&gt;

> > When a relative path has just a single component and we want to emit a > > nametype=PARENT record, the current implementation just reports the > > full CWD path (which is alrady available in the audit context).

> > This is wrong for three reasons: > > 1. Wasting log space for redundant data (CWD path is already in the CWD > > record).

> > 2. Inconsistency with other PATH records (if a relative PARENT > > directory path contains at least one component, only the verbatim > > relative path is logged). > > 3. In some syscalls (e.g. openat(2)) the relative path may not even be > > relative to the CWD, but to another directory specified as a file > > descriptor. In that case the logged path is simply plain wrong.

> > This patch modifies this behavior to simply report "." in the > > aforementioned case, which is equivalent to an "empty" directory path > > and can be concatenated with the actual base directory path (CWD or > > dirfd from openat(2)-like syscall) once support for its logging is > > added later. In the meantime, defaulting to CWD as base directory on > > relative paths (as already done by the userspace tools) will be enough > > to achieve results equivalent to the current behavior. > > I have to ask the obvious question, if we already have the necessary > parent path in the CWD record, why do we need a nametype=parent PATH > record anyway?

> Can we safely remove it or will that cause problems for Steve's userspace > tools?