IPC_SET_PERM cleanup

-                       if (axi->osid != 0) {
-                               char *ctx = NULL;
-                               u32 len;
-                               if (selinux_ctxid_to_string(

-                                               axi->osid, &ctx, &len)) {
-                                       audit_log_format(ab, " osid=%u",
-                                                       axi->osid);
-                                       call_panic = 1;
-                               } else
-                                       audit_log_format(ab, " obj=%s", ctx); -                               kfree(ctx);
-                       }

Quoted 10 lines · Linda Knippers

kernel/auditsc.c 2006-05-05 14:48:44.000000000 -0400 renamed from kernel/auditsc.c 2006-05-05 14:29:42.000000000 -0400 +0 -10

-                       if (axi->osid != 0) {
-                               char *ctx = NULL;
-                               u32 len;
-                               if (selinux_ctxid_to_string(
-                                               axi->osid, &ctx, &len)) {
-                                       audit_log_format(ab, " osid=%u",
-                                                       axi->osid);
-                                       call_panic = 1;
-                               } else
-                                       audit_log_format(ab, " obj=%s",

Quoted 2 lines

ctx); - kfree(ctx);
- }

This patch deletes the context string out of this record. Are we losing anything important?

The following patch addresses most of the issues with the IPC_SET_PERM records as described in:
https://www.redhat.com/archives/linux-audit/2006-May/msg00010.html

To summarize, I made the following changes:

1. Changed sys_msgctl() and semctl_down() so that an IPC_SET_PERM record is emitted in the failure case as well as the success case. This matches the behavior in sys_shmctl(). I could simplify the code in sys_msgctl() and semctl_down() slightly but it would mean that in some error cases we could get an IPC_SET_PERM record without an IPC record and that seemed odd.

2. No change to the IPC record type, given no feedback on the backward compatibility question.

3. Removed the qbytes field from the IPC record. It wasn't being set and when audit_ipc_obj() is called from ipcperms(), the information isn't available. If we want the information in the IPC record, more extensive changes will be necessary. Since it only applies to message queues and it isn't really permission related, it doesn't seem worth it.

4. Removed the obj field from the IPC_SET_PERM record. This means that the kern_ipc_perm argument is no longer needed.

5. Replaced the spaces in the IPC_SET_PERM field names with underscores.

I tested this with the lspp.22 kernel on an x86_64 box. Please let me know if you see any issues.

-- ljk

 include/linux/audit.h |    2 +-
 ipc/msg.c             |    9 +++++----
 ipc/sem.c             |    8 +++++---
 ipc/shm.c             |    2 +-
 kernel/auditsc.c      |   22 +++++-----------------
 5 files changed, 17 insertions(+), 26 deletions(-)

kernel/auditsc.c 2006-05-05 14:48:44.000000000 -0400 renamed from kernel/auditsc.c 2006-05-05 14:29:42.000000000 -0400 +5 -17

@@ -665,8 +665,8 @@ static void audit_log_exit(struct audit_
                case AUDIT_IPC: {
                        struct audit_aux_data_ipcctl *axi = (void *)aux;
                        audit_log_format(ab,
-                                " qbytes=%lx iuid=%u igid=%u mode=%x",
-                                axi->qbytes, axi->uid, axi->gid, axi->mode);
+                                "iuid=%u igid=%u mode=%x",
+                                axi->uid, axi->gid, axi->mode);
                        if (axi->osid != 0) {
                                char *ctx = NULL;
                                u32 len;

@@ -684,21 +684,10 @@ static void audit_log_exit(struct audit_
                case AUDIT_IPC_SET_PERM: {
                        struct audit_aux_data_ipcctl *axi = (void *)aux;
                        audit_log_format(ab,
-                               " new qbytes=%lx new iuid=%u new igid=%u new mode=%x",
+                               "new_qbytes=%lx new_iuid=%u new_igid=%u new_mode=%x",
                                axi->qbytes, axi->uid, axi->gid, axi->mode);
-                       if (axi->osid != 0) {
-                               char *ctx = NULL;
-                               u32 len;
-                               if (selinux_ctxid_to_string(
-                                               axi->osid, &ctx, &len)) {
-                                       audit_log_format(ab, " osid=%u",
-                                                       axi->osid);
-                                       call_panic = 1;
-                               } else
-                                       audit_log_format(ab, " obj=%s", ctx);
-                               kfree(ctx);
-                       }
                        break; }
+
                case AUDIT_EXECVE: {
                        struct audit_aux_data_execve *axi = (void *)aux;
                        int i;

@@ -1232,7 +1221,7 @@ int audit_ipc_obj(struct kern_ipc_perm *
  *
  * Returns 0 for success or NULL context or < 0 on error.
  */
-int audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode, struct kern_ipc_perm *ipcp)
+int audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode)
 {
        struct audit_aux_data_ipcctl *ax;
        struct audit_context *context = current->audit_context;

@@ -1248,7 +1237,6 @@ int audit_ipc_set_perm(unsigned long qby
        ax->uid = uid;
        ax->gid = gid;
        ax->mode = mode;
-       selinux_get_ipc_sid(ipcp, &ax->osid);
 
        ax->d.type = AUDIT_IPC_SET_PERM;
        ax->d.next = context->aux;

ipc/msg.c 2006-05-05 14:50:32.000000000 -0400 renamed from ipc/msg.c 2006-05-05 14:30:15.000000000 -0400 +5 -4

@@ -454,6 +454,11 @@ asmlinkage long sys_msgctl (int msqid, i
        err = audit_ipc_obj(ipcp);
        if (err)
                goto out_unlock_up;
+       if (cmd==IPC_SET) {
+               err = audit_ipc_set_perm(setbuf.qbytes, setbuf.uid, setbuf.gid, setbuf.mode);
+               if (err)
+                       goto out_unlock_up;
+       }
 
        err = -EPERM;
        if (current->euid != ipcp->cuid &&

@@ -468,10 +473,6 @@ asmlinkage long sys_msgctl (int msqid, i
        switch (cmd) {
        case IPC_SET:
        {
-               err = audit_ipc_set_perm(setbuf.qbytes, setbuf.uid, setbuf.gid, setbuf.mode, ipcp);
-               if (err)
-                       goto out_unlock_up;
-
                err = -EPERM;
                if (setbuf.qbytes > msg_ctlmnb && !capable(CAP_SYS_RESOURCE))
                        goto out_unlock_up;

ipc/shm.c 2006-05-05 14:51:53.000000000 -0400 renamed from ipc/shm.c 2006-05-05 15:08:23.000000000 -0400 +1 -1

@@ -643,7 +643,7 @@ asmlinkage long sys_shmctl (int shmid, i
                err = audit_ipc_obj(&(shp->shm_perm));
                if (err)
                        goto out_unlock_up;
-               err = audit_ipc_set_perm(0, setbuf.uid, setbuf.gid, setbuf.mode, &(shp->shm_perm));
+               err = audit_ipc_set_perm(0, setbuf.uid, setbuf.gid, setbuf.mode);
                if (err)
                        goto out_unlock_up;
                err=-EPERM;

ipc/sem.c 2006-05-05 14:50:58.000000000 -0400 renamed from ipc/sem.c 2006-05-05 14:30:02.000000000 -0400 +5 -3

@@ -828,6 +828,11 @@ static int semctl_down(int semid, int se
        if (err)
                goto out_unlock;
 
+       if (cmd == IPC_SET) {
+               err = audit_ipc_set_perm(0, setbuf.uid, setbuf.gid, setbuf.mode);
+               if (err)
+                       goto out_unlock;
+       }
        if (current->euid != ipcp->cuid &&
            current->euid != ipcp->uid && !capable(CAP_SYS_ADMIN)) {
                err=-EPERM;

@@ -844,9 +849,6 @@ static int semctl_down(int semid, int se
                err = 0;
                break;
        case IPC_SET:
-               err = audit_ipc_set_perm(0, setbuf.uid, setbuf.gid, setbuf.mode, ipcp);
-               if (err)
-                       goto out_unlock;
                ipcp->uid = setbuf.uid;
                ipcp->gid = setbuf.gid;
                ipcp->mode = (ipcp->mode & ~S_IRWXUGO)

include/linux/audit.h 2006-05-05 14:49:35.000000000 -0400 renamed from include/linux/audit.h 2006-05-05 15:09:42.000000000 -0400 +1 -2

@@ -324,7 +324,7 @@ extern void auditsc_get_stamp(struct aud
 extern int  audit_set_loginuid(struct task_struct *task, uid_t loginuid);
 extern uid_t audit_get_loginuid(struct audit_context *ctx);
 extern int audit_ipc_obj(struct kern_ipc_perm *ipcp);
-extern int audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode, struct kern_ipc_perm  *ipcp);
+extern int audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode);
 extern int audit_bprm(struct linux_binprm *bprm);
 extern int audit_socketcall(int nargs, unsigned long *args);
 extern int audit_sockaddr(int len, void *addr);
 
--

Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

The following patch addresses most of the issues with the IPC_SET_PERM records as described in:
https://www.redhat.com/archives/linux-audit/2006-May/msg00010.html

To summarize, I made the following changes:

1. Changed sys_msgctl() and semctl_down() so that an IPC_SET_PERM record is emitted in the failure case as well as the success case. This matches the behavior in sys_shmctl(). I could simplify the code in sys_msgctl() and semctl_down() slightly but it would mean that in some error cases we could get an IPC_SET_PERM record without an IPC record and that seemed odd.

2. No change to the IPC record type, given no feedback on the backward compatibility question.

3. Removed the qbytes field from the IPC record. It wasn't being set and when audit_ipc_obj() is called from ipcperms(), the information isn't available. If we want the information in the IPC record, more extensive changes will be necessary. Since it only applies to message queues and it isn't really permission related, it doesn't seem worth it.

4. Removed the obj field from the IPC_SET_PERM record. This means that the kern_ipc_perm argument is no longer needed.

5. Replaced the spaces in the IPC_SET_PERM field names with underscores.

Quoted 3 lines · Linda Knippers

The following patch addresses most of the issues with the IPC_SET_PERM records as described in:
https://www.redhat.com/archives/linux-audit/2006-May/msg00010.html

Hi Linda-

I apologize for the delay in my response. I'm pretty much permanently away from Audit-related work, and just now got a chance to respond to this.

First, let me point you to a thread wherein I explained why I made the audit ipc changes that I did (in case you missed it the first time around). It starts here:
https://www.redhat.com/archives/linux-audit/2006-March/msg00088.html

That said, thanks for testing some of this out more thoroughly and posting your findings.

Quoted 9 lines

To summarize, I made the following changes:

1. Changed sys_msgctl() and semctl_down() so that an IPC_SET_PERM record is emitted in the failure case as well as the success case. This matches the behavior in sys_shmctl(). I could simplify the code in sys_msgctl() and semctl_down() slightly but it would mean that in some error cases we could get an IPC_SET_PERM record without an IPC record and that seemed odd.

I think this is ok.

Quoted 2 lines

2. No change to the IPC record type, given no feedback on the backward compatibility question.

I'm not one to speak authoritatively about compatibility issues... But I do prefer the more descriptive AUDIT_IPC_SET_PERM type, as it more accurately explains what the record is. Someone might complain at some point about changing the record type, but there will be considerably more invasive API changes elsewhere between the 2.6.5 and the 2.6.16 kernels (and the RHEL4/RHEL5 kernels).

Quoted 6 lines

3. Removed the qbytes field from the IPC record. It wasn't being set and when audit_ipc_obj() is called from ipcperms(), the information isn't available. If we want the information in the IPC record, more extensive changes will be necessary. Since it only applies to message queues and it isn't really permission related, it doesn't seem worth it.

I agree with you here. However, I resisted the urge to remove the qbytes field when I reworked the ipc audit code as I did not know __why__ this was being saved. I assumed this was required by CAPP for one reason or another. I personally don't find it a very interesting field to capture. But I encourage you to review this with Klaus (or another of the CAPP/LSPP experts).

Quoted 3 lines

4. Removed the obj field from the IPC_SET_PERM record. This means that the kern_ipc_perm argument is no longer needed.

Hmm... This is probably ok, as long as you can __guarantee__ that an IPC record will always closely follow an IPC_SET_PERM (and can be associated together). The object label is needed for LSPP. If that can be found in an associated record, so be it. Looking at your example results, it seems ok.

Quoted 2 lines

5. Replaced the spaces in the IPC_SET_PERM field names with underscores.

Thanks, that was my oversight. There should definitely be underscores rather than spaces.

:-Dustin

Quoted 2 lines · Steve Grubb

This patch deletes the context string out of this record. Are we losing anything important?

I don't think so. I don't think the IPC_SET operations change the sid (at least I don't see it in the code) so its redundant with the obj information that's in the IPC record. If I'm missing it, I hope someone will point it out to me.

Quoted 2 lines · Linda Knippers

2. No change to the IPC record type, given no feedback on the backward compatibility question.

I'm not one to speak authoritatively about compatibility issues... But I do prefer the more descriptive AUDIT_IPC_SET_PERM type, as it more accurately explains what the record is. Someone might complain at some point about changing the record type, but there will be considerably more invasive API changes elsewhere between the 2.6.5 and the 2.6.16 kernels (and the RHEL4/RHEL5 kernels).

Quoted 6 lines

3. Removed the qbytes field from the IPC record. It wasn't being set and when audit_ipc_obj() is called from ipcperms(), the information isn't available. If we want the information in the IPC record, more extensive changes will be necessary. Since it only applies to message queues and it isn't really permission related, it doesn't seem worth it.

I agree with you here. However, I resisted the urge to remove the qbytes field when I reworked the ipc audit code as I did not know __why__ this was being saved. I assumed this was required by CAPP for one reason or another. I personally don't find it a very interesting field to capture. But I encourage you to review this with Klaus (or another of the CAPP/LSPP experts).

I think having a separate record type would be an alternative to the "new_" (or "new ") prefix. If the record types are distinct, it would be possible to use the same field names for current and requested object properties.

I'd rather see the word "new" separated by a space so that searching is better/easier.

How it is easier/better?

I thought we had a discussion in the LSPP call and on the list about spaces vs. underscores?

Quoted 1 line

How it is easier/better?

Much easier for the user. For example, the user might be searching for uid. Should the user have to specify new_uid ? Would they also need to search for old_uid? Would they have to know all the possible variations on uid?

We had this discussion on the audit list back in March while discussing the Audit Parsing Library.

https://www.redhat.com/archives/linux-audit/2006-March/msg00186.html

I thought it was settled at that time. If this was brought up on the LSPP telecon I missed it.

Might make it tougher too. For example, suppose someone wanted to find auid =
520. The user space object manager, dbus, has a auid of -1, and is not what we want. The sender auid is what we want. So, by having a space, we still match the sender auid = 520. The word to the left is to give context to a human reading the raw record, but the search utility should still match the auid value even if it has sender before it. If we combine field names, I think I'll have to maintain a table of field names to types so that ausearch can make sense of the field's contents no matter what the name is.

Quoted 1 line · Linda Knippers

How it is easier/better?

Much easier for the user. For example, the user might be searching for uid. Should the user have to specify new_uid ? Would they also need to search for old_uid? Would they have to know all the possible variations on uid?

If someone is looking for the records for a particular uid, wouldn't they expect to get the records generated by someone with that uid?

Quoted 2 lines

I thought it was settled at that time. If this was brought up on the LSPP telecon I missed it.

It didn't seem setttled, although you were the last to reply.  I think the discussion on the LSPP list is what initiated the mail exchange.

At this point there are already a bunch of uid fields (auid, uid, euid, suid, fsuid, iuid, ouid) in various audit records, and a similar set of guid files, so would you be happier with nuid, ngid, etc?

I even updated the audit parsing specs to include all keywords:
http://people.redhat.com/sgrubb/audit/audit-parse.txt

Does ouid and ogid not fit? I'd like us to define what we need in the parser API and then use it in the audit messages. Ancilliary words like new, old, last, first should not be tied with an underscore. If you find any, let me know.

Quoted 2 lines

If someone is looking for the records for a particular uid, wouldn't

Quoted 1 line

they expect to get the records generated by someone with that uid?

Not necessarily. I would like to present all matches of uid and let them decide what is relavent.

-ui <user id>
Search for an event with the given user ID.

Quoted 3 lines

At this point there are already a bunch of uid fields (auid, uid, euid,

Quoted 2 lines

suid, fsuid, iuid, ouid) in various audit records, and a similar set of guid files, so would you be happier with nuid, ngid, etc?

Does ouid and ogid not fit? I'd like us to define what we need in the parser API and then use it in the audit messages. Ancilliary words like new, old, last, first should not be tied with an underscore. If you find any, let me know.

I'm starting to wonder whether we actually need the IPC_NEW_PERMS record. We don't spell out similar information for chown, for example. In that case, the new owner is a1 field. Do we treat IPC's differently because their argument is a structure pointer?

In any case, if someone truly wanted to get all audit records that had the uid either as part of the subject/object identity and also included all records that had the uid as an argument, they'd need to look at the a* fields for other system calls as well. Since we don't look at the a* arguments for other syscalls, it doesn't seem like we should include the arguments for the IPC syscalls if someone is searching for the records generated by a uid.

Quoted 4 lines · Linda Knippers

I'm starting to wonder whether we actually need the IPC_NEW_PERMS record. We don't spell out similar information for chown, for example. In that case, the new owner is a1 field. Do we treat IPC's differently because their argument is a structure pointer?

Yes, that's the difference. chown takes an integer argument that's already contained in the syscall record, even though "a1" isn't a very descriptive name for it. For ipc_set, the argument is the address of the structure which would be useless, and the extra record makes the content of the structure available.

Quoted 7 lines

In any case, if someone truly wanted to get all audit records that had the uid either as part of the subject/object identity and also included all records that had the uid as an argument, they'd need to look at the a* fields for other system calls as well. Since we don't look at the a* arguments for other syscalls, it doesn't seem like we should include the arguments for the IPC syscalls if someone is searching for the records generated by a uid.

CAPP requires audit records for "All modifications of the values of security attributes" for 5.4.1 Management of Object Security Attributes (FMT_MSA.1). It doesn't specifically require the new values as detail information for the audit record, so if you just want to meet the letter of CAPP you could leave it out, but the record is of course far more useful if you include the information.

How the search on UID works in detail isn't all that important. I think the main issue is that the syscall record contains correct and useful information. For chown, someone who cares about the argument UID can look at the "a1" field, but I don't think it would be necessary to have an easy search method available for that.

If you want something equivalent for ipc_set, having fields with unsearchable names would be fine, which happens to be what you get with the "new_iuid" style field names.

-Klaus

I wasn't actually proposing that we do that.  I was really trying to make the point that including the iuid and new_iuid fields in with the results of a uid search, which is what Steve was proposing as a good thing, doesn't seem right to me.

Maybe I should use a5, a6, ...,

Bottom line, for the search API, I want all similar types to have a common field name. They can have a modifier adjacent to them.

Quoted 1 line

Maybe I should use a5, a6, ...,

Please no. Let's keep it as iuid or ouid.

I'd personally prefer to drop iuid so we can consolidate field types. ouid means "owner's uid".

Quoted 2 lines · Steve Grubb

Bottom line, for the search API, I want all similar types to have a common field name. They can have a modifier adjacent to them.

If that's the way you want to do it, there needs to be a way to get the modifier to disambiguate them.

Is adding "new " modifiers the best way to do that? You could also keep the field names the same and look at the syscall record type to find out which context they get used in.

Quoted 11 lines · Klaus Weidner

Quoted 2 lines · Steve Grubb

Bottom line, for the search API, I want all similar types to have a common field name. They can have a modifier adjacent to them.

If that's the way you want to do it, there needs to be a way to get the modifier to disambiguate them.

Is adding "new " modifiers the best way to do that? You could also keep the field names the same and look at the syscall record type to find out which context they get used in.

A bit more detail...

Here are the current audit records:

type=SYSCALL msg=audit(1146691872.791:94): arch=c000003e syscall=66 success=yes exit=0 a0=10000 a1=1 a2=1 a3=7fff328a7e70 items=0 pid=4327 auid=500 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts2 comm="syscalls" exe="/usr/local/eal3_testing/audit-test/syscalls/syscalls" subj=user_u:system_r:unconfined_t:s0-s0:c0.c255

type=IPC_SET_PERM msg=audit(1146691872.791:94): new qbytes=0 new iuid=501 new igid=0 new mode=0 obj=user_u:system_r:unconfined_t:s0-s0:c0.c255

type=IPC msg=audit(1146691872.791:94): qbytes=5a5a5a5a5a5a5a5a iuid=0 igid=0 mode=1c0 obj=user_u:system_r:unconfined_t:s0-s0:c0.c255

The original patches by Dustin and Linda had used "new_iuid=501" to differentiate the values, which I personally think was fine since it's unlikely that people want to be searching for those.

If you absolutely want to avoid adding new tag names, an alternative would be to get rid of the "new " modifiers, and use the "type=" name to differentiate them. The audit parsing library could then provide an auparse_get_field_type() function so that the clients that care can treat "IPC_SET_PERM" differently from type "IPC".

Something like:

	while (ausearch_next_event(au)) {
		if (auparse_find_field(au, "ouid")
		    && !strcmp(auparse_get_field_type(au), "IPC"))
		{
			printf("ouid=%s\n", auparse_interpret_field(au));
		}
	}

I still think that the new_* field names are fine and don't need fixing, but the "new" modifiers just look wrong.

-Klaus

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Quoted 3 lines

The original patches by Dustin and Linda had used "new_iuid=501" to differentiate the values, which I personally think was fine since it's unlikely that people want to be searching for those.

And if they do, they're easy to find with an ausearch | grep.

Quoted 3 lines

If you absolutely want to avoid adding new tag names, an alternative would be to get rid of the "new " modifiers, and use the "type=" name to differentiate them.

Quoted 5 lines · Linda Knippers

Quoted 3 lines

The original patches by Dustin and Linda had used "new_iuid=501" to differentiate the values, which I personally think was fine since it's unlikely that people want to be searching for those.

And if they do, they're easy to find with an ausearch | grep.

This is at the wrong level. There may be people that are writing programs that want any ouid. I want to stop the proliferation of field names and follow a convention. Forget whether or not you think people will ever want the information. We need a convention and then to follow it.

Quoted 3 lines

Quoted 3 lines

If you absolutely want to avoid adding new tag names, an alternative would be to get rid of the "new " modifiers, and use the "type=" name to differentiate them.

I don't want a proliferation of type names either. I think we have a lot of them and should try to use existing ones where possible.

Quoted 7 lines · Steve Grubb

Quoted 5 lines · Linda Knippers

Quoted 3 lines

The original patches by Dustin and Linda had used "new_iuid=501" to differentiate the values, which I personally think was fine since it's unlikely that people want to be searching for those.

And if they do, they're easy to find with an ausearch | grep.

Quoted 4 lines

This is at the wrong level. There may be people that are writing programs that want any ouid. I want to stop the proliferation of field names and follow a convention. Forget whether or not you think people will ever want the information. We need a convention and then to follow it.

Yes - but "new ouid" is also a different field name from "ouid", and unnecessarily hard to parse, especially since there's currently no well defined concept of name modifiers like "new".

What about a new field in the audit records that is a bitmask, wherein bits are flipped on for each field being a "new" or "updated" field?

Example:

type=IPC_SET_PERM msg=audit(1146863632.117:98): new_qbytes=0 new_iuid=501 new_igid=0 new_mode=0

becomes:

type=IPC_SET_PERM msg=audit(1146863632.117:98) new=1111: qbytes=0 iuid=501 igid=0 mode=0

I'm not sure that's really necessary, the type=IPC_SET_PERM already tells you that these are new values. How about simply the following:

type=IPC_SET_PERM msg=audit(1146863632.117:98): qbytes=0 iuid=501 igid=0 mode=0

Quoted 4 lines

This is at the wrong level. There may be people that are writing programs that want any ouid. I want to stop the proliferation of field names and follow a convention. Forget whether or not you think people will ever want the information. We need a convention and then to follow it.

Yes - but "new ouid" is also a different field name from "ouid", and unnecessarily hard to parse,

especially since there's currently no well defined concept of name modifiers like "new"

the type=IPC_SET_PERM already tells you that these are new values. How about simply the following:

type=IPC_SET_PERM msg=audit(1146863632.117:98): qbytes=0 iuid=501 igid=0 mode=0

Quoted 5 lines · Klaus Weidner

Quoted 4 lines

This is at the wrong level. There may be people that are writing programs that want any ouid. I want to stop the proliferation of field names and follow a convention. Forget whether or not you think people will ever want the information. We need a convention and then to follow it.

Quoted 2 lines

Yes - but "new ouid" is also a different field name from "ouid", and unnecessarily hard to parse,

I am writing the parser. No one else should have to worry about it.

Besides,
we already do this *everywhere* except in this patch. I am just trying to keep the whole thing consistent. If you see anywhere that has new_something or old_something, please let me know.

In all the places I looked, the value given is considered the new value. The old value is given as old=

Some examples:
"audit_rate_limit=%d old=%d by auid=%u"
"audit_backlog_limit=%d old=%d by auid=%u"

But then there is this:
audit_log_format(ab, "login pid=%d uid=%u " "old auid=%u new auid=%u",

Arguably, that could be re-written as:
audit_log_format(ab, "login pid=%d uid=%u " "auid=%u old auid=%u"

Quoted 3 lines

especially since there's currently no well defined concept of name modifiers like "new"

Its used in many places, but you are more likely to run across old. The function in the specs that was intended to do this was:

const char *auparse_get_field_name_aux(auparse_state_t *au) - return supplemental information about the field's name.

-Steve

Quoted 5 lines · Klaus Weidner

the type=IPC_SET_PERM already tells you that these are new values. How about simply the following:

type=IPC_SET_PERM msg=audit(1146863632.117:98): qbytes=0 iuid=501 igid=0 mode=0

How about we change that to ouid & ogid ? Object user ID & Object group ID?

We have existing code we're supporting that doesn't use your parser and we're not planning to re-write our code.

I don't know how many other people are in the same position.  I also think its helpful if the output of ausearch is easily grepable.

I think what these examples show is that there is no consistency.

Quoted 2 lines

"audit_rate_limit=%d old=%d by auid=%u"
"audit_backlog_limit=%d old=%d by auid=%u"

What does "by" signify as a modifier?

Quoted 6 lines

Quoted 2 lines

especially since there's currently no well defined concept of name modifiers like "new"

Its used in many places, but you are more likely to run across old. The function in the specs that was intended to do this was:

Quoted 2 lines

const char *auparse_get_field_name_aux(auparse_state_t *au) - return supplemental information about the field's name.

If I used the APIs then I have to look at the aux information for a bunch of records I don't want because I can't directly search for the ones I do?