* [PATCH ghak122 v1] audit: store event sockaddr in case of no rules @ 2020-07-03 17:17 Richard Guy Briggs 2020-07-08 22:49 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2020-07-03 17:17 UTC (permalink / raw) To: Linux-Audit Mailing List; +Cc: Richard Guy Briggs, eparis When there are no rules present, the event SOCKADDR record is not generated due to audit_dummy_context() generated at syscall entry from audit_n_rules. Store this information if there is a context present to store it so that mandatory events are more complete (startup, LSMs...). Please see the upstream issue https://github.com/linux-audit/audit-kernel/issues/122 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- Passes audit-testsuite. include/linux/audit.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 03c4035a532b..07fecd99741a 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -448,7 +448,7 @@ static inline int audit_socketcall_compat(int nargs, u32 *args) static inline int audit_sockaddr(int len, void *addr) { - if (unlikely(!audit_dummy_context())) + if (audit_context()) return __audit_sockaddr(len, addr); return 0; } -- 1.8.3.1 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-03 17:17 [PATCH ghak122 v1] audit: store event sockaddr in case of no rules Richard Guy Briggs @ 2020-07-08 22:49 ` Paul Moore 2020-07-13 17:40 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2020-07-08 22:49 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Fri, Jul 3, 2020 at 1:18 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > When there are no rules present, the event SOCKADDR record is not > generated due to audit_dummy_context() generated at syscall entry from > audit_n_rules. Store this information if there is a context present to > store it so that mandatory events are more complete (startup, LSMs...). > > Please see the upstream issue > https://github.com/linux-audit/audit-kernel/issues/122 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > Passes audit-testsuite. > > include/linux/audit.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Do we have any certification requirements driving this change? I ask because if we make this change, why not do the same for PATH records? The problem of course is that doing this for both is going to be costly, the PATH records in particular seem like they would raise a performance regression. I agree it would be nice to have this information, but I fear that gating this on audit_dummy_context() is the right thing to do unless there is a strong requirement that we always record this information. > diff --git a/include/linux/audit.h b/include/linux/audit.h > index 03c4035a532b..07fecd99741a 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -448,7 +448,7 @@ static inline int audit_socketcall_compat(int nargs, u32 *args) > > static inline int audit_sockaddr(int len, void *addr) > { > - if (unlikely(!audit_dummy_context())) > + if (audit_context()) > return __audit_sockaddr(len, addr); > return 0; > } > -- > 1.8.3.1 -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-08 22:49 ` Paul Moore @ 2020-07-13 17:40 ` Richard Guy Briggs 2020-07-13 17:55 ` Casey Schaufler 2020-07-13 22:30 ` Paul Moore 0 siblings, 2 replies; 14+ messages in thread From: Richard Guy Briggs @ 2020-07-13 17:40 UTC (permalink / raw) To: Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 2020-07-08 18:49, Paul Moore wrote: > On Fri, Jul 3, 2020 at 1:18 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > When there are no rules present, the event SOCKADDR record is not > > generated due to audit_dummy_context() generated at syscall entry from > > audit_n_rules. Store this information if there is a context present to > > store it so that mandatory events are more complete (startup, LSMs...). > > > > Please see the upstream issue > > https://github.com/linux-audit/audit-kernel/issues/122 > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > --- > > Passes audit-testsuite. > > > > include/linux/audit.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Do we have any certification requirements driving this change? I ask > because if we make this change, why not do the same for PATH records? I filed the issue because I noticed the SOCKADDR record missing from configuration events required for certification. > The problem of course is that doing this for both is going to be > costly, the PATH records in particular seem like they would raise a > performance regression. I had a minor concern about performance for SOCKADDR. I filed SOCKADDR because I noticed it missing, but intended to file others as noticed. I agree the PATH records would have a larger performance concern, but if they are required to support event records that are required for certification then either we store them when the context is present or generate them retroactively once a required event record is generated. In the case of SOCKADDR it may be possible to store that information once the required record has been generated rather than whenever there is a valid audit context, but it is currently collected earlier than config records are emitted. > I agree it would be nice to have this information, but I fear that > gating this on audit_dummy_context() is the right thing to do unless > there is a strong requirement that we always record this information. That would have been great feedback when the issue was filed. However, there may be a middle ground as descirbed above. > > diff --git a/include/linux/audit.h b/include/linux/audit.h > > index 03c4035a532b..07fecd99741a 100644 > > --- a/include/linux/audit.h > > +++ b/include/linux/audit.h > > @@ -448,7 +448,7 @@ static inline int audit_socketcall_compat(int nargs, u32 *args) > > > > static inline int audit_sockaddr(int len, void *addr) > > { > > - if (unlikely(!audit_dummy_context())) > > + if (audit_context()) > > return __audit_sockaddr(len, addr); > > return 0; > > } > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-13 17:40 ` Richard Guy Briggs @ 2020-07-13 17:55 ` Casey Schaufler 2020-07-13 20:02 ` Richard Guy Briggs 2020-07-13 22:30 ` Paul Moore 1 sibling, 1 reply; 14+ messages in thread From: Casey Schaufler @ 2020-07-13 17:55 UTC (permalink / raw) To: Richard Guy Briggs, Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 7/13/2020 10:40 AM, Richard Guy Briggs wrote: > On 2020-07-08 18:49, Paul Moore wrote: >> On Fri, Jul 3, 2020 at 1:18 PM Richard Guy Briggs <rgb@redhat.com> wrote: >>> When there are no rules present, the event SOCKADDR record is not >>> generated due to audit_dummy_context() generated at syscall entry from >>> audit_n_rules. Store this information if there is a context present to >>> store it so that mandatory events are more complete (startup, LSMs...). I don't know for sure, but this looks a lot like the issues I have had to address for LSM stacking in [PATCH v18 20/23] Audit: Add new record for multiple process LSM attributes I don't know if the approach I took will help here, or be acceptable at all for that matter. But it might be worth taking a look. >>> >>> Please see the upstream issue >>> https://github.com/linux-audit/audit-kernel/issues/122 >>> >>> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> >>> --- >>> Passes audit-testsuite. >>> >>> include/linux/audit.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >> Do we have any certification requirements driving this change? I ask >> because if we make this change, why not do the same for PATH records? > I filed the issue because I noticed the SOCKADDR record missing from > configuration events required for certification. > >> The problem of course is that doing this for both is going to be >> costly, the PATH records in particular seem like they would raise a >> performance regression. > I had a minor concern about performance for SOCKADDR. I filed SOCKADDR > because I noticed it missing, but intended to file others as noticed. > > I agree the PATH records would have a larger performance concern, but if > they are required to support event records that are required for > certification then either we store them when the context is present or > generate them retroactively once a required event record is generated. > > In the case of SOCKADDR it may be possible to store that information > once the required record has been generated rather than whenever there > is a valid audit context, but it is currently collected earlier than > config records are emitted. > >> I agree it would be nice to have this information, but I fear that >> gating this on audit_dummy_context() is the right thing to do unless >> there is a strong requirement that we always record this information. > That would have been great feedback when the issue was filed. > However, there may be a middle ground as descirbed above. > >>> diff --git a/include/linux/audit.h b/include/linux/audit.h >>> index 03c4035a532b..07fecd99741a 100644 >>> --- a/include/linux/audit.h >>> +++ b/include/linux/audit.h >>> @@ -448,7 +448,7 @@ static inline int audit_socketcall_compat(int nargs, u32 *args) >>> >>> static inline int audit_sockaddr(int len, void *addr) >>> { >>> - if (unlikely(!audit_dummy_context())) >>> + if (audit_context()) >>> return __audit_sockaddr(len, addr); >>> return 0; >>> } >> paul moore > - RGB > > -- > Richard Guy Briggs <rgb@redhat.com> > Sr. S/W Engineer, Kernel Security, Base Operating Systems > Remote, Ottawa, Red Hat Canada > IRC: rgb, SunRaycer > Voice: +1.647.777.2635, Internal: (81) 32635 > > -- > Linux-audit mailing list > Linux-audit@redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit > -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-13 17:55 ` Casey Schaufler @ 2020-07-13 20:02 ` Richard Guy Briggs 2020-07-13 23:08 ` Casey Schaufler 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2020-07-13 20:02 UTC (permalink / raw) To: Casey Schaufler; +Cc: Eric Paris, Linux-Audit Mailing List On 2020-07-13 10:55, Casey Schaufler wrote: > On 7/13/2020 10:40 AM, Richard Guy Briggs wrote: > > On 2020-07-08 18:49, Paul Moore wrote: > >> On Fri, Jul 3, 2020 at 1:18 PM Richard Guy Briggs <rgb@redhat.com> wrote: > >>> When there are no rules present, the event SOCKADDR record is not > >>> generated due to audit_dummy_context() generated at syscall entry from > >>> audit_n_rules. Store this information if there is a context present to > >>> store it so that mandatory events are more complete (startup, LSMs...). > > I don't know for sure, but this looks a lot like the issues > I have had to address for LSM stacking in > > [PATCH v18 20/23] Audit: Add new record for multiple process LSM attributes > > I don't know if the approach I took will help here, or be > acceptable at all for that matter. But it might be worth taking > a look. The first thing I notice is audit_stamp_context() looks pretty similar to audit_alloc_local() in [PATCH ghak90 V9 07/13] audit: add support for non-syscall auxiliary records which creates a local context for limited use by the primary record and local auxiliary records (contid) and is immediately tossed after those records. One use case is network namespace events that are not (yet) tied to any user context ("[PATCH ghak90 V9 10/13] audit: add support for containerid to network namespaces"). The other is for user records that don't need to be tied to the userspace caller ("[PATCH ghak90 V9 08/13] audit: add containerid support for user records"). How does this tie in with this situation? I would ideally like to retroactively capture the sockaddr information in the audit context struct (to avoid the overhead of always capturing sockaddr) to trigger a syscall record on exit along with a sockaddr record once a manadatory record has been triggered (such as audit or netfilter config, LSM event, etc...), depending only on audit_enabled. There could be other substructures of audit context that would need filling out after it is determined that an audit event has happenned and information needs to be collected to support its reporting. > >>> Please see the upstream issue > >>> https://github.com/linux-audit/audit-kernel/issues/122 > >>> > >>> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > >>> --- > >>> Passes audit-testsuite. > >>> > >>> include/linux/audit.h | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >> Do we have any certification requirements driving this change? I ask > >> because if we make this change, why not do the same for PATH records? > > I filed the issue because I noticed the SOCKADDR record missing from > > configuration events required for certification. > > > >> The problem of course is that doing this for both is going to be > >> costly, the PATH records in particular seem like they would raise a > >> performance regression. > > I had a minor concern about performance for SOCKADDR. I filed SOCKADDR > > because I noticed it missing, but intended to file others as noticed. > > > > I agree the PATH records would have a larger performance concern, but if > > they are required to support event records that are required for > > certification then either we store them when the context is present or > > generate them retroactively once a required event record is generated. > > > > In the case of SOCKADDR it may be possible to store that information > > once the required record has been generated rather than whenever there > > is a valid audit context, but it is currently collected earlier than > > config records are emitted. > > > >> I agree it would be nice to have this information, but I fear that > >> gating this on audit_dummy_context() is the right thing to do unless > >> there is a strong requirement that we always record this information. > > That would have been great feedback when the issue was filed. > > However, there may be a middle ground as descirbed above. > > > >>> diff --git a/include/linux/audit.h b/include/linux/audit.h > >>> index 03c4035a532b..07fecd99741a 100644 > >>> --- a/include/linux/audit.h > >>> +++ b/include/linux/audit.h > >>> @@ -448,7 +448,7 @@ static inline int audit_socketcall_compat(int nargs, u32 *args) > >>> > >>> static inline int audit_sockaddr(int len, void *addr) > >>> { > >>> - if (unlikely(!audit_dummy_context())) > >>> + if (audit_context()) > >>> return __audit_sockaddr(len, addr); > >>> return 0; > >>> } > >> paul moore > > - RGB - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-13 20:02 ` Richard Guy Briggs @ 2020-07-13 23:08 ` Casey Schaufler 2020-07-14 0:11 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Casey Schaufler @ 2020-07-13 23:08 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On 7/13/2020 1:02 PM, Richard Guy Briggs wrote: > On 2020-07-13 10:55, Casey Schaufler wrote: >> On 7/13/2020 10:40 AM, Richard Guy Briggs wrote: >>> On 2020-07-08 18:49, Paul Moore wrote: >>>> On Fri, Jul 3, 2020 at 1:18 PM Richard Guy Briggs <rgb@redhat.com> wrote: >>>>> When there are no rules present, the event SOCKADDR record is not >>>>> generated due to audit_dummy_context() generated at syscall entry from >>>>> audit_n_rules. Store this information if there is a context present to >>>>> store it so that mandatory events are more complete (startup, LSMs...). >> I don't know for sure, but this looks a lot like the issues >> I have had to address for LSM stacking in >> >> [PATCH v18 20/23] Audit: Add new record for multiple process LSM attributes >> >> I don't know if the approach I took will help here, or be >> acceptable at all for that matter. But it might be worth taking >> a look. > The first thing I notice is audit_stamp_context() looks pretty similar > to audit_alloc_local() in Sure does, doesn't it. > [PATCH ghak90 V9 07/13] audit: add support for non-syscall auxiliary records > > which creates a local context for limited use by the primary record and > local auxiliary records (contid) and is immediately tossed after those > records. One use case is network namespace events that are not (yet) > tied to any user context ("[PATCH ghak90 V9 10/13] audit: add support > for containerid to network namespaces"). The other is for user records > that don't need to be tied to the userspace caller ("[PATCH ghak90 V9 > 08/13] audit: add containerid support for user records"). > > How does this tie in with this situation? In the case where subject LSM context information is being provided and there is more than one LSM active that uses subject contexts (e.g. AppArmor and SELinux, or AppArmor and Smack) the audit event can't use the old "subj=xyz" field. Instead, the field will be "subj=?" and a new record added to the event that contains all the subject contexts. As is true in the cases you have here, not all the events involved are syscall events. > I would ideally like to > retroactively capture the sockaddr information in the audit context > struct (to avoid the overhead of always capturing sockaddr) to trigger a > syscall record on exit along with a sockaddr record once a manadatory > record has been triggered (such as audit or netfilter config, LSM event, > etc...), depending only on audit_enabled. There could be other > substructures of audit context that would need filling out after it is > determined that an audit event has happenned and information needs to be > collected to support its reporting. I'm not sure that I completely understand all that you're looking at, but it does appear that I could switch to using your audit_alloc_local(). Do you think that has achieved its final form? -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-13 23:08 ` Casey Schaufler @ 2020-07-14 0:11 ` Paul Moore 2020-07-14 0:28 ` Casey Schaufler 2020-07-14 1:08 ` Richard Guy Briggs 0 siblings, 2 replies; 14+ messages in thread From: Paul Moore @ 2020-07-14 0:11 UTC (permalink / raw) To: Casey Schaufler; +Cc: Richard Guy Briggs, Eric Paris, Linux-Audit Mailing List On Mon, Jul 13, 2020 at 7:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > ... but it does appear that I could switch to using your audit_alloc_local(). In my opinion, linking the audit container ID and LSM stacking patchsets would seem like a very big mistake, especially since the consolidation you are describing could be done after the fact without any disruption to the kernel/userspace interface. I would strongly encourage both patchsets to remain self-contained if at all possible so as to not jeopardize each other. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-14 0:11 ` Paul Moore @ 2020-07-14 0:28 ` Casey Schaufler 2020-07-14 0:47 ` Paul Moore 2020-07-14 1:08 ` Richard Guy Briggs 1 sibling, 1 reply; 14+ messages in thread From: Casey Schaufler @ 2020-07-14 0:28 UTC (permalink / raw) To: Paul Moore; +Cc: Richard Guy Briggs, Eric Paris, Linux-Audit Mailing List On 7/13/2020 5:11 PM, Paul Moore wrote: > On Mon, Jul 13, 2020 at 7:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> ... but it does appear that I could switch to using your audit_alloc_local(). > In my opinion, linking the audit container ID and LSM stacking > patchsets would seem like a very big mistake, especially since the > consolidation you are describing could be done after the fact without > any disruption to the kernel/userspace interface. I would strongly > encourage both patchsets to remain self-contained if at all possible > so as to not jeopardize each other. Whatever helps the review/ack process best works for me. I will leave audit_stamp_context() as is unless there is other feedback that leads it to be changed. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-14 0:28 ` Casey Schaufler @ 2020-07-14 0:47 ` Paul Moore 0 siblings, 0 replies; 14+ messages in thread From: Paul Moore @ 2020-07-14 0:47 UTC (permalink / raw) To: Casey Schaufler; +Cc: Richard Guy Briggs, Eric Paris, Linux-Audit Mailing List On Mon, Jul 13, 2020 at 8:28 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 7/13/2020 5:11 PM, Paul Moore wrote: > > On Mon, Jul 13, 2020 at 7:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > >> ... but it does appear that I could switch to using your audit_alloc_local(). > > In my opinion, linking the audit container ID and LSM stacking > > patchsets would seem like a very big mistake, especially since the > > consolidation you are describing could be done after the fact without > > any disruption to the kernel/userspace interface. I would strongly > > encourage both patchsets to remain self-contained if at all possible > > so as to not jeopardize each other. > > Whatever helps the review/ack process best works for me. I will leave > audit_stamp_context() as is unless there is other feedback that leads > it to be changed. Thanks, I think that is the best path forward for both patchsets. If/when both patchsets land in the kernel I'm happy to revisit this. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-14 0:11 ` Paul Moore 2020-07-14 0:28 ` Casey Schaufler @ 2020-07-14 1:08 ` Richard Guy Briggs 2020-07-14 1:19 ` Paul Moore 1 sibling, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2020-07-14 1:08 UTC (permalink / raw) To: Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 2020-07-13 20:11, Paul Moore wrote: > On Mon, Jul 13, 2020 at 7:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > ... but it does appear that I could switch to using your audit_alloc_local(). > > In my opinion, linking the audit container ID and LSM stacking > patchsets would seem like a very big mistake, especially since the > consolidation you are describing could be done after the fact without > any disruption to the kernel/userspace interface. I would strongly > encourage both patchsets to remain self-contained if at all possible > so as to not jeopardize each other. I see no need to link them. The audit_alloc_local() patch could stand on its own to be used by either patchset and doesn't need to be included in the contid patchset. There is no mention of contid in it. Patches 8 and 11 depend on it so as long as it is already upstream that's fine. Of course, we could send a fixup patch after both patchsets are accepted upstream to merge the functionality of the two. > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-14 1:08 ` Richard Guy Briggs @ 2020-07-14 1:19 ` Paul Moore 2020-07-14 2:37 ` Casey Schaufler 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2020-07-14 1:19 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Mon, Jul 13, 2020 at 9:08 PM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2020-07-13 20:11, Paul Moore wrote: > > On Mon, Jul 13, 2020 at 7:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > > ... but it does appear that I could switch to using your audit_alloc_local(). > > > > In my opinion, linking the audit container ID and LSM stacking > > patchsets would seem like a very big mistake, especially since the > > consolidation you are describing could be done after the fact without > > any disruption to the kernel/userspace interface. I would strongly > > encourage both patchsets to remain self-contained if at all possible > > so as to not jeopardize each other. > > I see no need to link them. The audit_alloc_local() patch could stand > on its own to be used by either patchset and doesn't need to be included > in the contid patchset. There is no mention of contid in it. Patches 8 > and 11 depend on it so as long as it is already upstream that's fine. Let me be clear, please don't do this. I really dislike that we need audit_alloc_local(), but we do because computers are awful things and audit is perhaps even more awful. Someday I'll make my peace with audit_alloc_local(), and/or it will become something more useful through consolidation, but now is not the time to push on this issue considering where the audit container ID patchset is at. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-14 1:19 ` Paul Moore @ 2020-07-14 2:37 ` Casey Schaufler 0 siblings, 0 replies; 14+ messages in thread From: Casey Schaufler @ 2020-07-14 2:37 UTC (permalink / raw) To: Paul Moore, Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On 7/13/2020 6:19 PM, Paul Moore wrote: > On Mon, Jul 13, 2020 at 9:08 PM Richard Guy Briggs <rgb@redhat.com> wrote: >> On 2020-07-13 20:11, Paul Moore wrote: >>> On Mon, Jul 13, 2020 at 7:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >>>> ... but it does appear that I could switch to using your audit_alloc_local(). >>> In my opinion, linking the audit container ID and LSM stacking >>> patchsets would seem like a very big mistake, especially since the >>> consolidation you are describing could be done after the fact without >>> any disruption to the kernel/userspace interface. I would strongly >>> encourage both patchsets to remain self-contained if at all possible >>> so as to not jeopardize each other. >> I see no need to link them. The audit_alloc_local() patch could stand >> on its own to be used by either patchset and doesn't need to be included >> in the contid patchset. There is no mention of contid in it. Patches 8 >> and 11 depend on it so as long as it is already upstream that's fine. > Let me be clear, please don't do this. I really dislike that we need > audit_alloc_local(), but we do because computers are awful things and > audit is perhaps even more awful. Computers *are* awful, and getting awfuller. Audit is awful beyond comprehension because the people who wrote the audit section of the Orange Book didn't talk to the people who wrote the access control section, and neither talked with anyone who hadn't worked on MULTICS. I wrote three UNIX audit systems, helped out on several others and derailed the POSIX effort completely. I have tried to avoid understanding Linux audit so far, but have finally gotten to the point where I have to know something about it. It's far from the worst I've seen. There are aspects I find peculiar, but I have my (rather firm and old fashioned) notions. > Someday I'll make my peace with audit_alloc_local(), and/or it will > become something more useful through consolidation, but now is not the > time to push on this issue considering where the audit container ID > patchset is at. And now I'm coming in with a similar notion for stacking. Thanks for your forbearance. We know there's lots on your plate, and that we're coming at you from all directions. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-13 17:40 ` Richard Guy Briggs 2020-07-13 17:55 ` Casey Schaufler @ 2020-07-13 22:30 ` Paul Moore 2020-07-13 22:37 ` Steve Grubb 1 sibling, 1 reply; 14+ messages in thread From: Paul Moore @ 2020-07-13 22:30 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Mon, Jul 13, 2020 at 1:40 PM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2020-07-08 18:49, Paul Moore wrote: > > On Fri, Jul 3, 2020 at 1:18 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > When there are no rules present, the event SOCKADDR record is not > > > generated due to audit_dummy_context() generated at syscall entry from > > > audit_n_rules. Store this information if there is a context present to > > > store it so that mandatory events are more complete (startup, LSMs...). > > > > > > Please see the upstream issue > > > https://github.com/linux-audit/audit-kernel/issues/122 > > > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > --- > > > Passes audit-testsuite. > > > > > > include/linux/audit.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Do we have any certification requirements driving this change? I ask > > because if we make this change, why not do the same for PATH records? > > I filed the issue because I noticed the SOCKADDR record missing from > configuration events required for certification. I guess my original question wasn't very clear, let me try again ... Do we have any certification requirements for this that require the SOCKADDR record without an explicit audit configuration that would capture/generate the sockaddr information? It's been a while since I've been involved in a certification effort, but if I remember correctly those efforts required a specific audit configuration to be compliant (file watches, syscall rules, etc.). If there is a certification requirement for this, it might be a good idea to include it in the commit description. I don't believe we've been very good about doing that in the past, but it seems like something that would be worthwhile. > > The problem of course is that doing this for both is going to be > > costly, the PATH records in particular seem like they would raise a > > performance regression. > > I had a minor concern about performance for SOCKADDR. I filed SOCKADDR > because I noticed it missing, but intended to file others as noticed. > > I agree the PATH records would have a larger performance concern, but if > they are required to support event records that are required for > certification then either we store them when the context is present or > generate them retroactively once a required event record is generated. > > In the case of SOCKADDR it may be possible to store that information > once the required record has been generated rather than whenever there > is a valid audit context, but it is currently collected earlier than > config records are emitted. Before we go too far on this, let's track down the answer to the question above - is this more an issue of having the proper, certification-compliant audit configuration loaded into the kernel? > > I agree it would be nice to have this information, but I fear that > > gating this on audit_dummy_context() is the right thing to do unless > > there is a strong requirement that we always record this information. > > That would have been great feedback when the issue was filed. > However, there may be a middle ground as descirbed above. I've mentioned before that while the GH issues are nice for tracking issues, technical discussions should happen on list ... like they are now. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules 2020-07-13 22:30 ` Paul Moore @ 2020-07-13 22:37 ` Steve Grubb 0 siblings, 0 replies; 14+ messages in thread From: Steve Grubb @ 2020-07-13 22:37 UTC (permalink / raw) To: Paul Moore; +Cc: Richard Guy Briggs, Eric Paris, Linux-Audit Mailing List On Monday, July 13, 2020 6:30:51 PM EDT Paul Moore wrote: > On Mon, Jul 13, 2020 at 1:40 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2020-07-08 18:49, Paul Moore wrote: > > > On Fri, Jul 3, 2020 at 1:18 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > When there are no rules present, the event SOCKADDR record is not > > > > generated due to audit_dummy_context() generated at syscall entry > > > > from > > > > audit_n_rules. Store this information if there is a context present > > > > to > > > > store it so that mandatory events are more complete (startup, > > > > LSMs...). > > > > > > > > Please see the upstream issue > > > > https://github.com/linux-audit/audit-kernel/issues/122 > > > > > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > > --- > > > > Passes audit-testsuite. > > > > > > > > include/linux/audit.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > Do we have any certification requirements driving this change? I ask > > > because if we make this change, why not do the same for PATH records? > > > > I filed the issue because I noticed the SOCKADDR record missing from > > configuration events required for certification. > > I guess my original question wasn't very clear, let me try again ... > > Do we have any certification requirements for this that require the > SOCKADDR record without an explicit audit configuration that would > capture/generate the sockaddr information? No. There is no need to include either the SYSCALL or SOCKADDR record when logging an audit config change event because it will always be sendto and netlink. I suppose this is being done for consistency and not due to certification. We just need the usual minimal information logged and nothing else. -Steve > It's been a while since > I've been involved in a certification effort, but if I remember > correctly those efforts required a specific audit configuration to be > compliant (file watches, syscall rules, etc.). > > If there is a certification requirement for this, it might be a good > idea to include it in the commit description. I don't believe we've > been very good about doing that in the past, but it seems like > something that would be worthwhile. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-07-14 2:37 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-03 17:17 [PATCH ghak122 v1] audit: store event sockaddr in case of no rules Richard Guy Briggs 2020-07-08 22:49 ` Paul Moore 2020-07-13 17:40 ` Richard Guy Briggs 2020-07-13 17:55 ` Casey Schaufler 2020-07-13 20:02 ` Richard Guy Briggs 2020-07-13 23:08 ` Casey Schaufler 2020-07-14 0:11 ` Paul Moore 2020-07-14 0:28 ` Casey Schaufler 2020-07-14 0:47 ` Paul Moore 2020-07-14 1:08 ` Richard Guy Briggs 2020-07-14 1:19 ` Paul Moore 2020-07-14 2:37 ` Casey Schaufler 2020-07-13 22:30 ` Paul Moore 2020-07-13 22:37 ` Steve Grubb
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).