From 2a5658382a4443b105e0badf92701bb7b18c7d00 Mon Sep 17 00:00:00 2001 From: Robert Watson Date: Thu, 2 Jul 2009 09:15:30 +0000 Subject: [PATCH] Clean up a number of aspects of token generation from audit arguments to system calls: - Centralize generation of argument tokens for VM addresses in a macro, ADDR_TOKEN(), and properly encode 64-bit addresses in 64-bit arguments. - Fix up argument numbers across a large number of syscalls so that they match the numeric argument into the system call. - Don't audit the address argument to ioctl(2) or ptrace(2), but do keep generating tokens for mmap(2), minherit(2), since they relate to passing object access across execve(2). Approved by: re (audit argument blanket) Obtained from: TrustedBSD Project MFC after: 1 week --- sys/kern/sys_process.c | 1 - sys/security/audit/audit_bsm.c | 124 +++++++++++++++------------------ 2 files changed, 55 insertions(+), 70 deletions(-) diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c index 3c7b52c1e32c..b8803af0c837 100644 --- a/sys/kern/sys_process.c +++ b/sys/kern/sys_process.c @@ -402,7 +402,6 @@ ptrace(struct thread *td, struct ptrace_args *uap) #endif AUDIT_ARG_PID(uap->pid); AUDIT_ARG_CMD(uap->req); - AUDIT_ARG_ADDR(uap->addr); AUDIT_ARG_VALUE(uap->data); addr = &r; switch (uap->req) { diff --git a/sys/security/audit/audit_bsm.c b/sys/security/audit/audit_bsm.c index dd23c992eb77..e41a33e72e4e 100644 --- a/sys/security/audit/audit_bsm.c +++ b/sys/security/audit/audit_bsm.c @@ -253,7 +253,7 @@ kau_free(struct au_record *rec) } \ } while (0) -#define EXTATTR_TOKENS do { \ +#define EXTATTR_TOKENS(namespace_argnum) do { \ if (ARG_IS_VALID(kar, ARG_VALUE)) { \ switch (ar->ar_arg_value) { \ case EXTATTR_NAMESPACE_USER: \ @@ -263,8 +263,8 @@ kau_free(struct au_record *rec) tok = au_to_text(EXTATTR_NAMESPACE_SYSTEM_STRING);\ break; \ default: \ - tok = au_to_arg32(3, "attrnamespace", \ - ar->ar_arg_value); \ + tok = au_to_arg32((namespace_argnum), \ + "attrnamespace", ar->ar_arg_value); \ break; \ } \ kau_write(rec, tok); \ @@ -276,6 +276,24 @@ kau_free(struct au_record *rec) } \ } while (0) +/* + * Not all pointer arguments to system calls are of interest, but in some + * cases they reflect delegation of rights, such as mmap(2) falled by + * minherit(2) before execve(2), so do the best we can. + */ +#define ADDR_TOKEN(argnum, argname) do { \ + if (ARG_IS_VALID(kar, ARG_ADDR)) { \ + if (sizeof(void *) == sizeof(uint32_t)) \ + tok = au_to_arg32((argnum), (argname), \ + (uint32_t)(uintptr_t)ar->ar_arg_addr); \ + else \ + tok = au_to_arg64((argnum), (argname), \ + (uint64_t)(uintptr_t)ar->ar_arg_addr); \ + kau_write(rec, tok); \ + } \ +} while (0) + + /* * Implement auditing for the auditon() system call. The audit tokens that * are generated depend on the command that was sent into the auditon() @@ -286,21 +304,20 @@ audit_sys_auditon(struct audit_record *ar, struct au_record *rec) { struct au_token *tok; + tok = au_to_arg32(3, "length", ar->ar_arg_len); + kau_write(rec, tok); switch (ar->ar_arg_cmd) { case A_OLDSETPOLICY: if ((size_t)ar->ar_arg_len == sizeof(int64_t)) { - tok = au_to_arg32(3, "length", ar->ar_arg_len); - kau_write(rec, tok); tok = au_to_arg64(2, "policy", ar->ar_arg_auditon.au_policy64); kau_write(rec, tok); break; } /* FALLTHROUGH */ + case A_SETPOLICY: - tok = au_to_arg32(3, "length", ar->ar_arg_len); - kau_write(rec, tok); - tok = au_to_arg32(1, "policy", ar->ar_arg_auditon.au_policy); + tok = au_to_arg32(2, "policy", ar->ar_arg_auditon.au_policy); kau_write(rec, tok); break; @@ -315,8 +332,6 @@ audit_sys_auditon(struct audit_record *ar, struct au_record *rec) case A_OLDSETQCTRL: if ((size_t)ar->ar_arg_len == sizeof(au_qctrl64_t)) { - tok = au_to_arg32(3, "length", ar->ar_arg_len); - kau_write(rec, tok); tok = au_to_arg64(2, "setqctrl:aq_hiwater", ar->ar_arg_auditon.au_qctrl64.aq64_hiwater); kau_write(rec, tok); @@ -335,8 +350,9 @@ audit_sys_auditon(struct audit_record *ar, struct au_record *rec) break; } /* FALLTHROUGH */ + case A_SETQCTRL: - tok = au_to_arg32(3, "setqctrl:aq_hiwater", + tok = au_to_arg32(2, "setqctrl:aq_hiwater", ar->ar_arg_auditon.au_qctrl.aq_hiwater); kau_write(rec, tok); tok = au_to_arg32(2, "setqctrl:aq_lowater", @@ -354,17 +370,15 @@ audit_sys_auditon(struct audit_record *ar, struct au_record *rec) break; case A_SETUMASK: - tok = au_to_arg32(3, "setumask:as_success", + tok = au_to_arg32(2, "setumask:as_success", ar->ar_arg_auditon.au_auinfo.ai_mask.am_success); kau_write(rec, tok); - tok = au_to_arg32(3, "setumask:as_failure", + tok = au_to_arg32(2, "setumask:as_failure", ar->ar_arg_auditon.au_auinfo.ai_mask.am_failure); kau_write(rec, tok); break; case A_SETSMASK: - tok = au_to_arg32(3, "length", ar->ar_arg_len); - kau_write(rec, tok); tok = au_to_arg32(2, "setsmask:as_success", ar->ar_arg_auditon.au_auinfo.ai_mask.am_success); kau_write(rec, tok); @@ -375,23 +389,19 @@ audit_sys_auditon(struct audit_record *ar, struct au_record *rec) case A_OLDSETCOND: if ((size_t)ar->ar_arg_len == sizeof(int64_t)) { - tok = au_to_arg32(3, "length", ar->ar_arg_len); - kau_write(rec, tok); tok = au_to_arg64(2, "setcond", ar->ar_arg_auditon.au_cond64); kau_write(rec, tok); break; } /* FALLTHROUGH */ + case A_SETCOND: - tok = au_to_arg32(3, "length", ar->ar_arg_len); - kau_write(rec, tok); - tok = au_to_arg32(3, "setcond", ar->ar_arg_auditon.au_cond); + tok = au_to_arg32(2, "setcond", ar->ar_arg_auditon.au_cond); kau_write(rec, tok); break; case A_SETCLASS: - tok = au_to_arg32(3, "length", ar->ar_arg_len); kau_write(rec, tok); tok = au_to_arg32(2, "setclass:ec_event", ar->ar_arg_auditon.au_evclass.ec_number); @@ -402,8 +412,6 @@ audit_sys_auditon(struct audit_record *ar, struct au_record *rec) break; case A_SETPMASK: - tok = au_to_arg32(3, "length", ar->ar_arg_len); - kau_write(rec, tok); tok = au_to_arg32(2, "setpmask:as_success", ar->ar_arg_auditon.au_aupinfo.ap_mask.am_success); kau_write(rec, tok); @@ -413,8 +421,6 @@ audit_sys_auditon(struct audit_record *ar, struct au_record *rec) break; case A_SETFSIZE: - tok = au_to_arg32(3, "length", ar->ar_arg_len); - kau_write(rec, tok); tok = au_to_arg32(2, "setfsize:filesize", ar->ar_arg_auditon.au_fstat.af_filesz); kau_write(rec, tok); @@ -530,13 +536,13 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) case AUE_SOCKET: case AUE_SOCKETPAIR: if (ARG_IS_VALID(kar, ARG_SOCKINFO)) { - tok = au_to_arg32(1,"domain", + tok = au_to_arg32(1, "domain", ar->ar_arg_sockinfo.so_domain); kau_write(rec, tok); - tok = au_to_arg32(2,"type", + tok = au_to_arg32(2, "type", ar->ar_arg_sockinfo.so_type); kau_write(rec, tok); - tok = au_to_arg32(3,"protocol", + tok = au_to_arg32(3, "protocol", ar->ar_arg_sockinfo.so_protocol); kau_write(rec, tok); } @@ -734,7 +740,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) case AUE_EACCESS: UPATH1_VNODE1_TOKENS; if (ARG_IS_VALID(kar, ARG_VALUE)) { - tok = au_to_arg32(1, "mode", ar->ar_arg_value); + tok = au_to_arg32(2, "mode", ar->ar_arg_value); kau_write(rec, tok); } break; @@ -784,7 +790,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) case AUE_CLOSE: if (ARG_IS_VALID(kar, ARG_FD)) { - tok = au_to_arg32(2, "fd", ar->ar_arg_fd); + tok = au_to_arg32(1, "fd", ar->ar_arg_fd); kau_write(rec, tok); } UPATH1_VNODE1_TOKENS; @@ -792,7 +798,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) case AUE_CORE: if (ARG_IS_VALID(kar, ARG_SIGNUM)) { - tok = au_to_arg32(0, "signal", ar->ar_arg_signum); + tok = au_to_arg32(1, "signal", ar->ar_arg_signum); kau_write(rec, tok); } UPATH1_VNODE1_TOKENS; @@ -807,7 +813,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) /* extattrctl(2) filename parameter is in upath2/vnode2 */ UPATH2_TOKENS; VNODE2_TOKENS; - EXTATTR_TOKENS; + EXTATTR_TOKENS(4); break; case AUE_EXTATTR_GET_FILE: @@ -819,7 +825,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) case AUE_EXTATTR_LIST_LINK: case AUE_EXTATTR_DELETE_LINK: UPATH1_VNODE1_TOKENS; - EXTATTR_TOKENS; + EXTATTR_TOKENS(2); break; case AUE_EXTATTR_GET_FD: @@ -830,7 +836,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) tok = au_to_arg32(2, "fd", ar->ar_arg_fd); kau_write(rec, tok); } - EXTATTR_TOKENS; + EXTATTR_TOKENS(2); break; case AUE_FEXECVE: @@ -945,11 +951,6 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) tok = au_to_arg32(2, "cmd", ar->ar_arg_cmd); kau_write(rec, tok); } - if (ARG_IS_VALID(kar, ARG_ADDR)) { - tok = au_to_arg32(1, "arg", - (u_int32_t)(uintptr_t)ar->ar_arg_addr); - kau_write(rec, tok); - } if (ARG_IS_VALID(kar, ARG_VNODE1)) FD_VNODE1_TOKENS; else { @@ -995,11 +996,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) break; case AUE_LOADSHFILE: - if (ARG_IS_VALID(kar, ARG_ADDR)) { - tok = au_to_arg32(4, "base addr", - (u_int32_t)(uintptr_t)ar->ar_arg_addr); - kau_write(rec, tok); - } + ADDR_TOKEN(4, "base addr"); UPATH1_VNODE1_TOKENS; break; @@ -1029,11 +1026,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) case AUE_MLOCK: case AUE_MUNLOCK: case AUE_MINHERIT: - if (ARG_IS_VALID(kar, ARG_ADDR)) { - tok = au_to_arg32(1, "addr", - (u_int32_t)(uintptr_t)ar->ar_arg_addr); - kau_write(rec, tok); - } + ADDR_TOKEN(1, "addr"); if (ARG_IS_VALID(kar, ARG_LEN)) { tok = au_to_arg32(2, "len", ar->ar_arg_len); kau_write(rec, tok); @@ -1071,14 +1064,14 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) case AUE_NFS_SVC: if (ARG_IS_VALID(kar, ARG_CMD)) { - tok = au_to_arg32(1, "request", ar->ar_arg_cmd); + tok = au_to_arg32(1, "flags", ar->ar_arg_cmd); kau_write(rec, tok); } break; case AUE_UMOUNT: if (ARG_IS_VALID(kar, ARG_VALUE)) { - tok = au_to_arg32(1, "flags", ar->ar_arg_value); + tok = au_to_arg32(2, "flags", ar->ar_arg_value); kau_write(rec, tok); } UPATH1_VNODE1_TOKENS; @@ -1113,11 +1106,7 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) break; case AUE_RESETSHFILE: - if (ARG_IS_VALID(kar, ARG_ADDR)) { - tok = au_to_arg32(1, "base addr", - (u_int32_t)(uintptr_t)ar->ar_arg_addr); - kau_write(rec, tok); - } + ADDR_TOKEN(1, "base addr"); break; case AUE_OPEN_RC: @@ -1151,11 +1140,6 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) tok = au_to_arg32(1, "request", ar->ar_arg_cmd); kau_write(rec, tok); } - if (ARG_IS_VALID(kar, ARG_ADDR)) { - tok = au_to_arg32(3, "addr", - (u_int32_t)(uintptr_t)ar->ar_arg_addr); - kau_write(rec, tok); - } if (ARG_IS_VALID(kar, ARG_VALUE)) { tok = au_to_arg32(4, "data", ar->ar_arg_value); kau_write(rec, tok); @@ -1172,6 +1156,10 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) tok = au_to_arg32(3, "uid", ar->ar_arg_uid); kau_write(rec, tok); } + if (ARG_IS_VALID(kar, ARG_GID)) { + tok = au_to_arg32(3, "gid", ar->ar_arg_gid); + kau_write(rec, tok); + } UPATH1_VNODE1_TOKENS; break; @@ -1210,14 +1198,14 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) case AUE_SETEGID: if (ARG_IS_VALID(kar, ARG_EGID)) { - tok = au_to_arg32(1, "gid", ar->ar_arg_egid); + tok = au_to_arg32(1, "egid", ar->ar_arg_egid); kau_write(rec, tok); } break; case AUE_SETEUID: if (ARG_IS_VALID(kar, ARG_EUID)) { - tok = au_to_arg32(1, "uid", ar->ar_arg_euid); + tok = au_to_arg32(1, "euid", ar->ar_arg_euid); kau_write(rec, tok); } break; @@ -1315,8 +1303,9 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) tok = au_to_arg32(2, "who", ar->ar_arg_uid); kau_write(rec, tok); } + PROCESS_PID_TOKENS(2); if (ARG_IS_VALID(kar, ARG_VALUE)) { - tok = au_to_arg32(2, "priority", ar->ar_arg_value); + tok = au_to_arg32(3, "priority", ar->ar_arg_value); kau_write(rec, tok); } break; @@ -1509,12 +1498,9 @@ kaudit_to_bsm(struct kaudit_record *kar, struct au_record **pau) break; case AUE_WAIT4: - if (ARG_IS_VALID(kar, ARG_PID)) { - tok = au_to_arg32(0, "pid", ar->ar_arg_pid); - kau_write(rec, tok); - } + PROCESS_PID_TOKENS(1); if (ARG_IS_VALID(kar, ARG_VALUE)) { - tok = au_to_arg32(0, "options", ar->ar_arg_value); + tok = au_to_arg32(3, "options", ar->ar_arg_value); kau_write(rec, tok); } break;