Re: [PATCH V9 3/3] audit: add audit by children of executable path
On 15/08/07, Paul Moore wrote: > On Fri, Aug 7, 2015 at 12:03 PM, Richard Guy Briggs wrote: > > On 15/08/07, Paul Moore wrote: > >> On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: > >> > On 15/08/06, Paul Moore wrote: > >> > > >> > > I guess what I'm saying is that I'm not currently convinced that > >> > > there is enough value in this to offset the risk I feel the loop > >> > > presents. I understand the use cases that you are mentioning, the > >> > > are the same as the last time we discussed this, but I'm going to > >> > > need something better than that. > >> > > >> > Can you better describe the loop that concerns you? I don't quite see > >> > it. > >> > >> It would be the only loop in the patch, look at the for loop in > >> audit_filter_rules() which iterates up the process' parent chain. > > > > Sorry, I should reword that... What risk do you see in that loop? It > > works up the task ancestry tree until it triggers, or hits init for that > > PID namespace that terminates the loop. Do you see a risk in the > > numerical pids rolling underneath the loop? > > I suppose there is some risk of PID overlap, and while that is a > concern, it isn't my first. > > My main concern is that a malicious user could add an extra level of > burden to the system by making an absurdly tall process tree and then > hammer the system with trivial, short lived syscalls. Granted, there > are userspace limits which would bound the impact to some extent, but > there is no way to really reduce the risk. You could further put hard > limits on the loop, but what good would that do? Malicious users > would just know to blow past that limit before they did their Evil > Deeds. > > I'll say it again; I'm not completely opposed to something like this - > perhaps in some modified form - but I have yet to see a need for this > functionality that is great enough to counter the risk. I am not going to lobby hard for it. I split this part of the patch out to avoid jeopardizing the acceptability of the rest of the patchset and to isolate it to make it easier to focus on its issues and apply it later once they are addressed. I'll reflect on this concern and see if I can come up with any ways to minimize this danger. This issue is related to the request to list the chain of processes back to the first ancestor in each record. You can make a best effort to record or track the entire chain, but at some point need to put a limit on it to avoid a DoS, at which point there is no point in listing the information since it is incomplete. (Too many "point"s in that last sentence...) > > I *do* notice that find_task_by_vpid(pid_t) must be replaced with > > find_task_by_pid_ns(pid_t, _pid_ns), since task_struct->pid is > > always stored in the initial PID namespace. > > Another thing that needs to be resolved. I've already fixed it in my tree: ptsk = find_task_by_pid_ns(ptsk->parent->pid) _pid_ns) > paul moore - RGB -- Richard Guy Briggs Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On Fri, Aug 7, 2015 at 12:03 PM, Richard Guy Briggs wrote: > On 15/08/07, Paul Moore wrote: >> On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: >> > On 15/08/06, Paul Moore wrote: >> > >> > > I guess what I'm saying is that I'm not currently convinced that >> > > there is enough value in this to offset the risk I feel the loop >> > > presents. I understand the use cases that you are mentioning, the >> > > are the same as the last time we discussed this, but I'm going to >> > > need something better than that. >> > >> > Can you better describe the loop that concerns you? I don't quite see >> > it. >> >> It would be the only loop in the patch, look at the for loop in >> audit_filter_rules() which iterates up the process' parent chain. > > Sorry, I should reword that... What risk do you see in that loop? It > works up the task ancestry tree until it triggers, or hits init for that > PID namespace that terminates the loop. Do you see a risk in the > numerical pids rolling underneath the loop? I suppose there is some risk of PID overlap, and while that is a concern, it isn't my first. My main concern is that a malicious user could add an extra level of burden to the system by making an absurdly tall process tree and then hammer the system with trivial, short lived syscalls. Granted, there are userspace limits which would bound the impact to some extent, but there is no way to really reduce the risk. You could further put hard limits on the loop, but what good would that do? Malicious users would just know to blow past that limit before they did their Evil Deeds. I'll say it again; I'm not completely opposed to something like this - perhaps in some modified form - but I have yet to see a need for this functionality that is great enough to counter the risk. > I *do* notice that find_task_by_vpid(pid_t) must be replaced with > find_task_by_pid_ns(pid_t, _pid_ns), since task_struct->pid is > always stored in the initial PID namespace. Another thing that needs to be resolved. -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On 15/08/07, Paul Moore wrote: > On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: > > On 15/08/06, Paul Moore wrote: > > > > > I guess what I'm saying is that I'm not currently convinced that > > > there is enough value in this to offset the risk I feel the loop > > > presents. I understand the use cases that you are mentioning, the > > > are the same as the last time we discussed this, but I'm going to > > > need something better than that. > > > > Can you better describe the loop that concerns you? I don't quite see > > it. > > It would be the only loop in the patch, look at the for loop in > audit_filter_rules() which iterates up the process' parent chain. Sorry, I should reword that... What risk do you see in that loop? It works up the task ancestry tree until it triggers, or hits init for that PID namespace that terminates the loop. Do you see a risk in the numerical pids rolling underneath the loop? I *do* notice that find_task_by_vpid(pid_t) must be replaced with find_task_by_pid_ns(pid_t, _pid_ns), since task_struct->pid is always stored in the initial PID namespace. > paul moore - RGB -- Richard Guy Briggs Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: > On 15/08/06, Paul Moore wrote: > > > I guess what I'm saying is that I'm not currently convinced that > > there is enough value in this to offset the risk I feel the loop > > presents. I understand the use cases that you are mentioning, the > > are the same as the last time we discussed this, but I'm going to > > need something better than that. > > Can you better describe the loop that concerns you? I don't quite see > it. It would be the only loop in the patch, look at the for loop in audit_filter_rules() which iterates up the process' parent chain. -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On 15/08/06, Paul Moore wrote: > On August 6, 2015 5:11:50 PM Steve Grubb wrote: > > >On Thursday, August 06, 2015 04:24:58 PM Paul Moore wrote: > >> On Wednesday, August 05, 2015 04:29:38 PM Richard Guy Briggs wrote: > >> > This adds the ability to audit the actions of children of a > >> > not-yet-running > >> > process. > >> > > >> > > >> > > >> > This is a split-out of a heavily modified version of a patch originally > >> > submitted by Eric Paris with some ideas from Peter Moody. > >> > > >> > > >> > > >> > Cc: Peter Moody > >> > Cc: Eric Paris > >> > Signed-off-by: Richard Guy Briggs > >> > --- > >> > > >> > include/uapi/linux/audit.h |1 + > >> > kernel/auditfilter.c |5 + > >> > kernel/auditsc.c | 11 +++ > >> > 3 files changed, 17 insertions(+), 0 deletions(-) > >> > >> I'm still not really comfortable with that loop and since there hasn't been > >> a really convincing use case I'm going to pass on this patch for right > >> now. If someone comes up with a *really* compelling case in the future > >> I'll reconsider it. > > > >Its the same reason strace has a -f option. Sometimes you need to also see > >what the children did. For example, maybe you want to audit file access to a > >specific directory and several cgi-bin programs can get there. You could > >write > >a rule for apache and be done. Or maybe, you have an app that lets people > >have > >shell access and you need to see files accessed or connections opened. Or > >maybe > >its a control panel application with helper scripts and you need to see > >changes that its making. Or maybe you have a program that is at risk of being > >compromised and you want to see if someone gets a shell from it. There are a > >lot of cases where it could be useful. > > > >-Steve > > > >-- > >Linux-audit mailing list > >linux-au...@redhat.com > >https://www.redhat.com/mailman/listinfo/linux-audit > > I guess what I'm saying is that I'm not currently convinced that > there is enough value in this to offset the risk I feel the loop > presents. I understand the use cases that you are mentioning, the > are the same as the last time we discussed this, but I'm going to > need something better than that. Can you better describe the loop that concerns you? I don't quite see it. > paul moore - RGB -- Richard Guy Briggs Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On 15/08/07, Paul Moore wrote: On Fri, Aug 7, 2015 at 12:03 PM, Richard Guy Briggs r...@redhat.com wrote: On 15/08/07, Paul Moore wrote: On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: On 15/08/06, Paul Moore wrote: I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that. Can you better describe the loop that concerns you? I don't quite see it. It would be the only loop in the patch, look at the for loop in audit_filter_rules() which iterates up the process' parent chain. Sorry, I should reword that... What risk do you see in that loop? It works up the task ancestry tree until it triggers, or hits init for that PID namespace that terminates the loop. Do you see a risk in the numerical pids rolling underneath the loop? I suppose there is some risk of PID overlap, and while that is a concern, it isn't my first. My main concern is that a malicious user could add an extra level of burden to the system by making an absurdly tall process tree and then hammer the system with trivial, short lived syscalls. Granted, there are userspace limits which would bound the impact to some extent, but there is no way to really reduce the risk. You could further put hard limits on the loop, but what good would that do? Malicious users would just know to blow past that limit before they did their Evil Deeds. I'll say it again; I'm not completely opposed to something like this - perhaps in some modified form - but I have yet to see a need for this functionality that is great enough to counter the risk. I am not going to lobby hard for it. I split this part of the patch out to avoid jeopardizing the acceptability of the rest of the patchset and to isolate it to make it easier to focus on its issues and apply it later once they are addressed. I'll reflect on this concern and see if I can come up with any ways to minimize this danger. This issue is related to the request to list the chain of processes back to the first ancestor in each record. You can make a best effort to record or track the entire chain, but at some point need to put a limit on it to avoid a DoS, at which point there is no point in listing the information since it is incomplete. (Too many points in that last sentence...) I *do* notice that find_task_by_vpid(pid_t) must be replaced with find_task_by_pid_ns(pid_t, init_pid_ns), since task_struct-pid is always stored in the initial PID namespace. Another thing that needs to be resolved. I've already fixed it in my tree: ptsk = find_task_by_pid_ns(ptsk-parent-pid) init_pid_ns) paul moore - RGB -- Richard Guy Briggs rbri...@redhat.com Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On Fri, Aug 7, 2015 at 12:03 PM, Richard Guy Briggs r...@redhat.com wrote: On 15/08/07, Paul Moore wrote: On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: On 15/08/06, Paul Moore wrote: I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that. Can you better describe the loop that concerns you? I don't quite see it. It would be the only loop in the patch, look at the for loop in audit_filter_rules() which iterates up the process' parent chain. Sorry, I should reword that... What risk do you see in that loop? It works up the task ancestry tree until it triggers, or hits init for that PID namespace that terminates the loop. Do you see a risk in the numerical pids rolling underneath the loop? I suppose there is some risk of PID overlap, and while that is a concern, it isn't my first. My main concern is that a malicious user could add an extra level of burden to the system by making an absurdly tall process tree and then hammer the system with trivial, short lived syscalls. Granted, there are userspace limits which would bound the impact to some extent, but there is no way to really reduce the risk. You could further put hard limits on the loop, but what good would that do? Malicious users would just know to blow past that limit before they did their Evil Deeds. I'll say it again; I'm not completely opposed to something like this - perhaps in some modified form - but I have yet to see a need for this functionality that is great enough to counter the risk. I *do* notice that find_task_by_vpid(pid_t) must be replaced with find_task_by_pid_ns(pid_t, init_pid_ns), since task_struct-pid is always stored in the initial PID namespace. Another thing that needs to be resolved. -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On 15/08/06, Paul Moore wrote: On August 6, 2015 5:11:50 PM Steve Grubb sgr...@redhat.com wrote: On Thursday, August 06, 2015 04:24:58 PM Paul Moore wrote: On Wednesday, August 05, 2015 04:29:38 PM Richard Guy Briggs wrote: This adds the ability to audit the actions of children of a not-yet-running process. This is a split-out of a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody. Cc: Peter Moody pe...@hda3.com Cc: Eric Paris epa...@redhat.com Signed-off-by: Richard Guy Briggs r...@redhat.com --- include/uapi/linux/audit.h |1 + kernel/auditfilter.c |5 + kernel/auditsc.c | 11 +++ 3 files changed, 17 insertions(+), 0 deletions(-) I'm still not really comfortable with that loop and since there hasn't been a really convincing use case I'm going to pass on this patch for right now. If someone comes up with a *really* compelling case in the future I'll reconsider it. Its the same reason strace has a -f option. Sometimes you need to also see what the children did. For example, maybe you want to audit file access to a specific directory and several cgi-bin programs can get there. You could write a rule for apache and be done. Or maybe, you have an app that lets people have shell access and you need to see files accessed or connections opened. Or maybe its a control panel application with helper scripts and you need to see changes that its making. Or maybe you have a program that is at risk of being compromised and you want to see if someone gets a shell from it. There are a lot of cases where it could be useful. -Steve -- Linux-audit mailing list linux-au...@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that. Can you better describe the loop that concerns you? I don't quite see it. paul moore - RGB -- Richard Guy Briggs rbri...@redhat.com Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: On 15/08/06, Paul Moore wrote: I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that. Can you better describe the loop that concerns you? I don't quite see it. It would be the only loop in the patch, look at the for loop in audit_filter_rules() which iterates up the process' parent chain. -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On 15/08/07, Paul Moore wrote: On Friday, August 07, 2015 02:37:15 AM Richard Guy Briggs wrote: On 15/08/06, Paul Moore wrote: I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that. Can you better describe the loop that concerns you? I don't quite see it. It would be the only loop in the patch, look at the for loop in audit_filter_rules() which iterates up the process' parent chain. Sorry, I should reword that... What risk do you see in that loop? It works up the task ancestry tree until it triggers, or hits init for that PID namespace that terminates the loop. Do you see a risk in the numerical pids rolling underneath the loop? I *do* notice that find_task_by_vpid(pid_t) must be replaced with find_task_by_pid_ns(pid_t, init_pid_ns), since task_struct-pid is always stored in the initial PID namespace. paul moore - RGB -- Richard Guy Briggs rbri...@redhat.com Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On Wednesday, August 05, 2015 04:29:38 PM Richard Guy Briggs wrote:
> This adds the ability to audit the actions of children of a not-yet-running
> process.
>
> This is a split-out of a heavily modified version of a patch originally
> submitted by Eric Paris with some ideas from Peter Moody.
>
> Cc: Peter Moody
> Cc: Eric Paris
> Signed-off-by: Richard Guy Briggs
> ---
> include/uapi/linux/audit.h |1 +
> kernel/auditfilter.c |5 +
> kernel/auditsc.c | 11 +++
> 3 files changed, 17 insertions(+), 0 deletions(-)
I'm still not really comfortable with that loop and since there hasn't been a
really convincing use case I'm going to pass on this patch for right now. If
someone comes up with a *really* compelling case in the future I'll reconsider
it.
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index e2ca600..55a8dec 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -267,6 +267,7 @@
> #define AUDIT_OBJ_GID110
> #define AUDIT_FIELD_COMPARE 111
> #define AUDIT_EXE112
> +#define AUDIT_EXE_CHILDREN 113
>
> #define AUDIT_ARG0 200
> #define AUDIT_ARG1 (AUDIT_ARG0+1)
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index c662638..802f0cc 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -406,6 +406,7 @@ static int audit_field_valid(struct audit_entry *entry,
> struct audit_field *f) return -EINVAL;
> break;
> case AUDIT_EXE:
> + case AUDIT_EXE_CHILDREN:
> if (f->op != Audit_equal)
> return -EINVAL;
> if (entry->rule.listnr != AUDIT_FILTER_EXIT)
> @@ -547,6 +548,7 @@ static struct audit_entry *audit_data_to_entry(struct
> audit_rule_data *data, entry->rule.filterkey = str;
> break;
> case AUDIT_EXE:
> + case AUDIT_EXE_CHILDREN:
> if (entry->rule.exe || f->val > PATH_MAX)
> goto exit_free;
> str = audit_unpack_string(, , f->val);
> @@ -643,6 +645,7 @@ static struct audit_rule_data
> *audit_krule_to_data(struct audit_krule *krule) audit_pack_string(,
> krule->filterkey);
> break;
> case AUDIT_EXE:
> + case AUDIT_EXE_CHILDREN:
> data->buflen += data->values[i] =
> audit_pack_string(,
> audit_mark_path(krule->exe));
> break;
> @@ -710,6 +713,7 @@ static int audit_compare_rule(struct audit_krule *a,
> struct audit_krule *b) return 1;
> break;
> case AUDIT_EXE:
> + case AUDIT_EXE_CHILDREN:
> /* both paths exist based on above type compare */
> if (strcmp(audit_mark_path(a->exe),
> audit_mark_path(b->exe)))
> @@ -838,6 +842,7 @@ struct audit_entry *audit_dupe_rule(struct audit_krule
> *old) new->filterkey = fk;
> break;
> case AUDIT_EXE:
> + case AUDIT_EXE_CHILDREN:
> err = audit_dupe_exe(new, old);
> break;
> }
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index e9bac2b..4f2b515 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -469,6 +469,17 @@ static int audit_filter_rules(struct task_struct *tsk,
> case AUDIT_EXE:
> result = audit_exe_compare(tsk, rule->exe);
> break;
> + case AUDIT_EXE_CHILDREN:
> + {
> + struct task_struct *ptsk;
> + for (ptsk = tsk; ptsk->parent->pid > 0; ptsk =
> find_task_by_vpid(ptsk->parent->pid)) { + if
(audit_exe_compare(ptsk,
> rule->exe)) {
> + ++result;
> + break;
> + }
> + }
> + }
> + break;
> case AUDIT_UID:
> result = audit_uid_comparator(cred->uid, f->op, f->uid);
> break;
--
paul moore
security @ redhat
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On August 6, 2015 5:11:50 PM Steve Grubb wrote: On Thursday, August 06, 2015 04:24:58 PM Paul Moore wrote: > On Wednesday, August 05, 2015 04:29:38 PM Richard Guy Briggs wrote: > > This adds the ability to audit the actions of children of a > > not-yet-running > > process. > > > > > > > > This is a split-out of a heavily modified version of a patch originally > > submitted by Eric Paris with some ideas from Peter Moody. > > > > > > > > Cc: Peter Moody > > Cc: Eric Paris > > Signed-off-by: Richard Guy Briggs > > --- > > > > include/uapi/linux/audit.h |1 + > > kernel/auditfilter.c |5 + > > kernel/auditsc.c | 11 +++ > > 3 files changed, 17 insertions(+), 0 deletions(-) > > I'm still not really comfortable with that loop and since there hasn't been > a really convincing use case I'm going to pass on this patch for right > now. If someone comes up with a *really* compelling case in the future > I'll reconsider it. Its the same reason strace has a -f option. Sometimes you need to also see what the children did. For example, maybe you want to audit file access to a specific directory and several cgi-bin programs can get there. You could write a rule for apache and be done. Or maybe, you have an app that lets people have shell access and you need to see files accessed or connections opened. Or maybe its a control panel application with helper scripts and you need to see changes that its making. Or maybe you have a program that is at risk of being compromised and you want to see if someone gets a shell from it. There are a lot of cases where it could be useful. -Steve -- Linux-audit mailing list linux-au...@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that. -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On Thursday, August 06, 2015 04:24:58 PM Paul Moore wrote: > On Wednesday, August 05, 2015 04:29:38 PM Richard Guy Briggs wrote: > > This adds the ability to audit the actions of children of a > > not-yet-running > > process. > > > > > > > > This is a split-out of a heavily modified version of a patch originally > > submitted by Eric Paris with some ideas from Peter Moody. > > > > > > > > Cc: Peter Moody > > Cc: Eric Paris > > Signed-off-by: Richard Guy Briggs > > --- > > > > include/uapi/linux/audit.h |1 + > > kernel/auditfilter.c |5 + > > kernel/auditsc.c | 11 +++ > > 3 files changed, 17 insertions(+), 0 deletions(-) > > I'm still not really comfortable with that loop and since there hasn't been > a really convincing use case I'm going to pass on this patch for right > now. If someone comes up with a *really* compelling case in the future > I'll reconsider it. Its the same reason strace has a -f option. Sometimes you need to also see what the children did. For example, maybe you want to audit file access to a specific directory and several cgi-bin programs can get there. You could write a rule for apache and be done. Or maybe, you have an app that lets people have shell access and you need to see files accessed or connections opened. Or maybe its a control panel application with helper scripts and you need to see changes that its making. Or maybe you have a program that is at risk of being compromised and you want to see if someone gets a shell from it. There are a lot of cases where it could be useful. -Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On Thursday, August 06, 2015 04:24:58 PM Paul Moore wrote: On Wednesday, August 05, 2015 04:29:38 PM Richard Guy Briggs wrote: This adds the ability to audit the actions of children of a not-yet-running process. This is a split-out of a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody. Cc: Peter Moody pe...@hda3.com Cc: Eric Paris epa...@redhat.com Signed-off-by: Richard Guy Briggs r...@redhat.com --- include/uapi/linux/audit.h |1 + kernel/auditfilter.c |5 + kernel/auditsc.c | 11 +++ 3 files changed, 17 insertions(+), 0 deletions(-) I'm still not really comfortable with that loop and since there hasn't been a really convincing use case I'm going to pass on this patch for right now. If someone comes up with a *really* compelling case in the future I'll reconsider it. Its the same reason strace has a -f option. Sometimes you need to also see what the children did. For example, maybe you want to audit file access to a specific directory and several cgi-bin programs can get there. You could write a rule for apache and be done. Or maybe, you have an app that lets people have shell access and you need to see files accessed or connections opened. Or maybe its a control panel application with helper scripts and you need to see changes that its making. Or maybe you have a program that is at risk of being compromised and you want to see if someone gets a shell from it. There are a lot of cases where it could be useful. -Steve -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On Wednesday, August 05, 2015 04:29:38 PM Richard Guy Briggs wrote:
This adds the ability to audit the actions of children of a not-yet-running
process.
This is a split-out of a heavily modified version of a patch originally
submitted by Eric Paris with some ideas from Peter Moody.
Cc: Peter Moody pe...@hda3.com
Cc: Eric Paris epa...@redhat.com
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
include/uapi/linux/audit.h |1 +
kernel/auditfilter.c |5 +
kernel/auditsc.c | 11 +++
3 files changed, 17 insertions(+), 0 deletions(-)
I'm still not really comfortable with that loop and since there hasn't been a
really convincing use case I'm going to pass on this patch for right now. If
someone comes up with a *really* compelling case in the future I'll reconsider
it.
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index e2ca600..55a8dec 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -267,6 +267,7 @@
#define AUDIT_OBJ_GID110
#define AUDIT_FIELD_COMPARE 111
#define AUDIT_EXE112
+#define AUDIT_EXE_CHILDREN 113
#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index c662638..802f0cc 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -406,6 +406,7 @@ static int audit_field_valid(struct audit_entry *entry,
struct audit_field *f) return -EINVAL;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
if (f-op != Audit_equal)
return -EINVAL;
if (entry-rule.listnr != AUDIT_FILTER_EXIT)
@@ -547,6 +548,7 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data, entry-rule.filterkey = str;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
if (entry-rule.exe || f-val PATH_MAX)
goto exit_free;
str = audit_unpack_string(bufp, remain, f-val);
@@ -643,6 +645,7 @@ static struct audit_rule_data
*audit_krule_to_data(struct audit_krule *krule) audit_pack_string(bufp,
krule-filterkey);
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
data-buflen += data-values[i] =
audit_pack_string(bufp,
audit_mark_path(krule-exe));
break;
@@ -710,6 +713,7 @@ static int audit_compare_rule(struct audit_krule *a,
struct audit_krule *b) return 1;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
/* both paths exist based on above type compare */
if (strcmp(audit_mark_path(a-exe),
audit_mark_path(b-exe)))
@@ -838,6 +842,7 @@ struct audit_entry *audit_dupe_rule(struct audit_krule
*old) new-filterkey = fk;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
err = audit_dupe_exe(new, old);
break;
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e9bac2b..4f2b515 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -469,6 +469,17 @@ static int audit_filter_rules(struct task_struct *tsk,
case AUDIT_EXE:
result = audit_exe_compare(tsk, rule-exe);
break;
+ case AUDIT_EXE_CHILDREN:
+ {
+ struct task_struct *ptsk;
+ for (ptsk = tsk; ptsk-parent-pid 0; ptsk =
find_task_by_vpid(ptsk-parent-pid)) { + if
(audit_exe_compare(ptsk,
rule-exe)) {
+ ++result;
+ break;
+ }
+ }
+ }
+ break;
case AUDIT_UID:
result = audit_uid_comparator(cred-uid, f-op, f-uid);
break;
--
paul moore
security @ redhat
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V9 3/3] audit: add audit by children of executable path
On August 6, 2015 5:11:50 PM Steve Grubb sgr...@redhat.com wrote: On Thursday, August 06, 2015 04:24:58 PM Paul Moore wrote: On Wednesday, August 05, 2015 04:29:38 PM Richard Guy Briggs wrote: This adds the ability to audit the actions of children of a not-yet-running process. This is a split-out of a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody. Cc: Peter Moody pe...@hda3.com Cc: Eric Paris epa...@redhat.com Signed-off-by: Richard Guy Briggs r...@redhat.com --- include/uapi/linux/audit.h |1 + kernel/auditfilter.c |5 + kernel/auditsc.c | 11 +++ 3 files changed, 17 insertions(+), 0 deletions(-) I'm still not really comfortable with that loop and since there hasn't been a really convincing use case I'm going to pass on this patch for right now. If someone comes up with a *really* compelling case in the future I'll reconsider it. Its the same reason strace has a -f option. Sometimes you need to also see what the children did. For example, maybe you want to audit file access to a specific directory and several cgi-bin programs can get there. You could write a rule for apache and be done. Or maybe, you have an app that lets people have shell access and you need to see files accessed or connections opened. Or maybe its a control panel application with helper scripts and you need to see changes that its making. Or maybe you have a program that is at risk of being compromised and you want to see if someone gets a shell from it. There are a lot of cases where it could be useful. -Steve -- Linux-audit mailing list linux-au...@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that. -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V9 3/3] audit: add audit by children of executable path
This adds the ability to audit the actions of children of a not-yet-running
process.
This is a split-out of a heavily modified version of a patch originally
submitted by Eric Paris with some ideas from Peter Moody.
Cc: Peter Moody
Cc: Eric Paris
Signed-off-by: Richard Guy Briggs
---
include/uapi/linux/audit.h |1 +
kernel/auditfilter.c |5 +
kernel/auditsc.c | 11 +++
3 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index e2ca600..55a8dec 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -267,6 +267,7 @@
#define AUDIT_OBJ_GID 110
#define AUDIT_FIELD_COMPARE111
#define AUDIT_EXE 112
+#define AUDIT_EXE_CHILDREN 113
#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index c662638..802f0cc 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -406,6 +406,7 @@ static int audit_field_valid(struct audit_entry *entry,
struct audit_field *f)
return -EINVAL;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
if (f->op != Audit_equal)
return -EINVAL;
if (entry->rule.listnr != AUDIT_FILTER_EXIT)
@@ -547,6 +548,7 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data,
entry->rule.filterkey = str;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
if (entry->rule.exe || f->val > PATH_MAX)
goto exit_free;
str = audit_unpack_string(, , f->val);
@@ -643,6 +645,7 @@ static struct audit_rule_data *audit_krule_to_data(struct
audit_krule *krule)
audit_pack_string(, krule->filterkey);
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
data->buflen += data->values[i] =
audit_pack_string(,
audit_mark_path(krule->exe));
break;
@@ -710,6 +713,7 @@ static int audit_compare_rule(struct audit_krule *a, struct
audit_krule *b)
return 1;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
/* both paths exist based on above type compare */
if (strcmp(audit_mark_path(a->exe),
audit_mark_path(b->exe)))
@@ -838,6 +842,7 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old)
new->filterkey = fk;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
err = audit_dupe_exe(new, old);
break;
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e9bac2b..4f2b515 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -469,6 +469,17 @@ static int audit_filter_rules(struct task_struct *tsk,
case AUDIT_EXE:
result = audit_exe_compare(tsk, rule->exe);
break;
+ case AUDIT_EXE_CHILDREN:
+ {
+ struct task_struct *ptsk;
+ for (ptsk = tsk; ptsk->parent->pid > 0; ptsk =
find_task_by_vpid(ptsk->parent->pid)) {
+ if (audit_exe_compare(ptsk, rule->exe)) {
+ ++result;
+ break;
+ }
+ }
+ }
+ break;
case AUDIT_UID:
result = audit_uid_comparator(cred->uid, f->op, f->uid);
break;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V9 3/3] audit: add audit by children of executable path
This adds the ability to audit the actions of children of a not-yet-running
process.
This is a split-out of a heavily modified version of a patch originally
submitted by Eric Paris with some ideas from Peter Moody.
Cc: Peter Moody pe...@hda3.com
Cc: Eric Paris epa...@redhat.com
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
include/uapi/linux/audit.h |1 +
kernel/auditfilter.c |5 +
kernel/auditsc.c | 11 +++
3 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index e2ca600..55a8dec 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -267,6 +267,7 @@
#define AUDIT_OBJ_GID 110
#define AUDIT_FIELD_COMPARE111
#define AUDIT_EXE 112
+#define AUDIT_EXE_CHILDREN 113
#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index c662638..802f0cc 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -406,6 +406,7 @@ static int audit_field_valid(struct audit_entry *entry,
struct audit_field *f)
return -EINVAL;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
if (f-op != Audit_equal)
return -EINVAL;
if (entry-rule.listnr != AUDIT_FILTER_EXIT)
@@ -547,6 +548,7 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data,
entry-rule.filterkey = str;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
if (entry-rule.exe || f-val PATH_MAX)
goto exit_free;
str = audit_unpack_string(bufp, remain, f-val);
@@ -643,6 +645,7 @@ static struct audit_rule_data *audit_krule_to_data(struct
audit_krule *krule)
audit_pack_string(bufp, krule-filterkey);
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
data-buflen += data-values[i] =
audit_pack_string(bufp,
audit_mark_path(krule-exe));
break;
@@ -710,6 +713,7 @@ static int audit_compare_rule(struct audit_krule *a, struct
audit_krule *b)
return 1;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
/* both paths exist based on above type compare */
if (strcmp(audit_mark_path(a-exe),
audit_mark_path(b-exe)))
@@ -838,6 +842,7 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old)
new-filterkey = fk;
break;
case AUDIT_EXE:
+ case AUDIT_EXE_CHILDREN:
err = audit_dupe_exe(new, old);
break;
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e9bac2b..4f2b515 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -469,6 +469,17 @@ static int audit_filter_rules(struct task_struct *tsk,
case AUDIT_EXE:
result = audit_exe_compare(tsk, rule-exe);
break;
+ case AUDIT_EXE_CHILDREN:
+ {
+ struct task_struct *ptsk;
+ for (ptsk = tsk; ptsk-parent-pid 0; ptsk =
find_task_by_vpid(ptsk-parent-pid)) {
+ if (audit_exe_compare(ptsk, rule-exe)) {
+ ++result;
+ break;
+ }
+ }
+ }
+ break;
case AUDIT_UID:
result = audit_uid_comparator(cred-uid, f-op, f-uid);
break;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
