diff mbox series

[v4] cil_access_vector_rules: allowx, auditallowx and dontauditx fixes

Message ID 20200912131411.1213573-1-dominick.grift@defensec.nl (mailing list archive)
State Accepted
Delegated to: James Carter
Headers show
Series [v4] cil_access_vector_rules: allowx, auditallowx and dontauditx fixes | expand

Commit Message

Dominick Grift Sept. 12, 2020, 1:14 p.m. UTC
allowx requires a equivalent "allow ioctl" rule to be present
auditallowx requires a equivalent "auditallow" ioctl rule to be present
dontauditx requires atleast one equivalent "allowx" rule to be present (can be a random irrelevant ioctlcmd)

Signed-off-by: Dominick Grift <dominick.grift@defensec.nl>
---
v4: add a "proper" clarification for dontauditx

This just can't be how it was designed to behave, but it "works".

There seem to be no logic to this at all and allowx, auditallowx and dontauditx should probably work the same as neverallowx.
regardless, this behavior should be documented because people will lose their sanity if they have to figure this out on their own.

 secilc/docs/cil_access_vector_rules.md | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Nicolas Iooss Oct. 2, 2020, 7:02 a.m. UTC | #1
On Sat, Sep 12, 2020 at 3:14 PM Dominick Grift
<dominick.grift@defensec.nl> wrote:
>
> allowx requires a equivalent "allow ioctl" rule to be present
> auditallowx requires a equivalent "auditallow" ioctl rule to be present
> dontauditx requires atleast one equivalent "allowx" rule to be present (can be a random irrelevant ioctlcmd)
>
> Signed-off-by: Dominick Grift <dominick.grift@defensec.nl>

It seems that this patch slipped through the review process, as there
could have been some confusion about the repository it was for. It
seems to me that it is for https://github.com/SELinuxProject/selinux,
and its contents looks good. Thanks!

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

If nobody complains, I will apply this patch tonight.
Nicolas

> ---
> v4: add a "proper" clarification for dontauditx
>
> This just can't be how it was designed to behave, but it "works".
>
> There seem to be no logic to this at all and allowx, auditallowx and dontauditx should probably work the same as neverallowx.
> regardless, this behavior should be documented because people will lose their sanity if they have to figure this out on their own.
>
>  secilc/docs/cil_access_vector_rules.md | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/secilc/docs/cil_access_vector_rules.md b/secilc/docs/cil_access_vector_rules.md
> index 9c03c710..683cc28c 100644
> --- a/secilc/docs/cil_access_vector_rules.md
> +++ b/secilc/docs/cil_access_vector_rules.md
> @@ -238,6 +238,8 @@ allowx
>
>  Specifies the access allowed between a source and target type using extended permissions. Unlike the [`allow`](cil_access_vector_rules.md#allow) statement, the statements [`validatetrans`](cil_constraint_statements.md#validatetrans), [`mlsvalidatetrans`](cil_constraint_statements.md#mlsvalidatetrans), [`constrain`](cil_constraint_statements.md#constrain), and [`mlsconstrain`](cil_constraint_statements.md#mlsconstrain) do not limit accesses granted by [`allowx`](cil_access_vector_rules.md#allowx).
>
> +Note that for this to work there must *also* be valid equivalent [`allow`](cil_access_vector_rules.md#allow) rules present.
> +
>  **Rule definition:**
>
>      (allowx source_id target_id|self permissionx_id)
> @@ -274,18 +276,21 @@ Specifies the access allowed between a source and target type using extended per
>
>  These examples show a selection of possible permutations of [`allowx`](cil_access_vector_rules.md#allowx) rules:
>
> +    (allow type_1 type_2 (tcp_socket (ioctl))) ;; pre-requisite
>      (allowx type_1 type_2 (ioctl tcp_socket (range 0x2000 0x20FF)))
>
>      (permissionx ioctl_nodebug (ioctl udp_socket (not (range 0x4000 0x4010))))
> +    (allow type_3 type_4 (udp_socket (ioctl))) ;; pre-requisite
>      (allowx type_3 type_4 ioctl_nodebug)
>
>
> -
>  auditallowx
>  -----------
>
>  Audit the access rights defined if there is a valid [`allowx`](cil_access_vector_rules.md#allowx) rule. It does NOT allow access, it only audits the event.
>
> +Note that for this to work there must *also* be valid equivalent [`auditallow`](cil_access_vector_rules.md#auditallow) rules present.
> +
>  **Rule definition:**
>
>      (auditallowx source_id target_id|self permissionx_id)
> @@ -324,6 +329,7 @@ This example will log an audit event whenever the corresponding [`allowx`](cil_a
>
>      (allowx type_1 type_2 (ioctl tcp_socket (range 0x2000 0x20FF)))
>
> +    (auditallow type_1 type_2 (tcp_socket (ioctl))) ;; pre-requisite
>      (auditallowx type_1 type_2 (ioctl tcp_socket (range 0x2005 0x2010)))
>
>
> @@ -332,6 +338,8 @@ dontauditx
>
>  Do not audit the access rights defined when access denied. This stops excessive log entries for known events.
>
> +Note that for this to work there must *also* be atleast one [`allowx`](cil_access_vector_rules.md#allowx) rule associated with the target type.
> +
>  Note that these rules can be omitted by the CIL compiler command line parameter `-D` or `--disable-dontaudit` flags.
>
>  **Rule definition:**
> @@ -370,6 +378,7 @@ Note that these rules can be omitted by the CIL compiler command line parameter
>
>  This example will not audit the denied access:
>
> +    (allowx type_1 type_2 (ioctl tcp_socket (0x1))) ;; pre-requisite, just some irrelevant random ioctl
>      (dontauditx type_1 type_2 (ioctl tcp_socket (range 0x3000 0x30FF)))
>
>
> @@ -392,7 +401,7 @@ Note that these rules can be over-ridden by the CIL compiler command line parame
>  </colgroup>
>  <tbody>
>  <tr class="odd">
> -<td align="left"><p><code>neverallows</code></p></td>
> +<td align="left"><p><code>neverallowx</code></p></td>
>  <td align="left"><p>The <code>neverallowx</code> keyword.</p></td>
>  </tr>
>  <tr class="even">
> --
> 2.28.0
>
Nicolas Iooss Oct. 3, 2020, 7:55 a.m. UTC | #2
On Fri, Oct 2, 2020 at 9:02 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Sat, Sep 12, 2020 at 3:14 PM Dominick Grift
> <dominick.grift@defensec.nl> wrote:
> >
> > allowx requires a equivalent "allow ioctl" rule to be present
> > auditallowx requires a equivalent "auditallow" ioctl rule to be present
> > dontauditx requires atleast one equivalent "allowx" rule to be present (can be a random irrelevant ioctlcmd)
> >
> > Signed-off-by: Dominick Grift <dominick.grift@defensec.nl>
>
> It seems that this patch slipped through the review process, as there
> could have been some confusion about the repository it was for. It
> seems to me that it is for https://github.com/SELinuxProject/selinux,
> and its contents looks good. Thanks!
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> If nobody complains, I will apply this patch tonight.
> Nicolas

Merged. Thanks!
Nicolas

> > ---
> > v4: add a "proper" clarification for dontauditx
> >
> > This just can't be how it was designed to behave, but it "works".
> >
> > There seem to be no logic to this at all and allowx, auditallowx and dontauditx should probably work the same as neverallowx.
> > regardless, this behavior should be documented because people will lose their sanity if they have to figure this out on their own.
> >
> >  secilc/docs/cil_access_vector_rules.md | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/secilc/docs/cil_access_vector_rules.md b/secilc/docs/cil_access_vector_rules.md
> > index 9c03c710..683cc28c 100644
> > --- a/secilc/docs/cil_access_vector_rules.md
> > +++ b/secilc/docs/cil_access_vector_rules.md
> > @@ -238,6 +238,8 @@ allowx
> >
> >  Specifies the access allowed between a source and target type using extended permissions. Unlike the [`allow`](cil_access_vector_rules.md#allow) statement, the statements [`validatetrans`](cil_constraint_statements.md#validatetrans), [`mlsvalidatetrans`](cil_constraint_statements.md#mlsvalidatetrans), [`constrain`](cil_constraint_statements.md#constrain), and [`mlsconstrain`](cil_constraint_statements.md#mlsconstrain) do not limit accesses granted by [`allowx`](cil_access_vector_rules.md#allowx).
> >
> > +Note that for this to work there must *also* be valid equivalent [`allow`](cil_access_vector_rules.md#allow) rules present.
> > +
> >  **Rule definition:**
> >
> >      (allowx source_id target_id|self permissionx_id)
> > @@ -274,18 +276,21 @@ Specifies the access allowed between a source and target type using extended per
> >
> >  These examples show a selection of possible permutations of [`allowx`](cil_access_vector_rules.md#allowx) rules:
> >
> > +    (allow type_1 type_2 (tcp_socket (ioctl))) ;; pre-requisite
> >      (allowx type_1 type_2 (ioctl tcp_socket (range 0x2000 0x20FF)))
> >
> >      (permissionx ioctl_nodebug (ioctl udp_socket (not (range 0x4000 0x4010))))
> > +    (allow type_3 type_4 (udp_socket (ioctl))) ;; pre-requisite
> >      (allowx type_3 type_4 ioctl_nodebug)
> >
> >
> > -
> >  auditallowx
> >  -----------
> >
> >  Audit the access rights defined if there is a valid [`allowx`](cil_access_vector_rules.md#allowx) rule. It does NOT allow access, it only audits the event.
> >
> > +Note that for this to work there must *also* be valid equivalent [`auditallow`](cil_access_vector_rules.md#auditallow) rules present.
> > +
> >  **Rule definition:**
> >
> >      (auditallowx source_id target_id|self permissionx_id)
> > @@ -324,6 +329,7 @@ This example will log an audit event whenever the corresponding [`allowx`](cil_a
> >
> >      (allowx type_1 type_2 (ioctl tcp_socket (range 0x2000 0x20FF)))
> >
> > +    (auditallow type_1 type_2 (tcp_socket (ioctl))) ;; pre-requisite
> >      (auditallowx type_1 type_2 (ioctl tcp_socket (range 0x2005 0x2010)))
> >
> >
> > @@ -332,6 +338,8 @@ dontauditx
> >
> >  Do not audit the access rights defined when access denied. This stops excessive log entries for known events.
> >
> > +Note that for this to work there must *also* be atleast one [`allowx`](cil_access_vector_rules.md#allowx) rule associated with the target type.
> > +
> >  Note that these rules can be omitted by the CIL compiler command line parameter `-D` or `--disable-dontaudit` flags.
> >
> >  **Rule definition:**
> > @@ -370,6 +378,7 @@ Note that these rules can be omitted by the CIL compiler command line parameter
> >
> >  This example will not audit the denied access:
> >
> > +    (allowx type_1 type_2 (ioctl tcp_socket (0x1))) ;; pre-requisite, just some irrelevant random ioctl
> >      (dontauditx type_1 type_2 (ioctl tcp_socket (range 0x3000 0x30FF)))
> >
> >
> > @@ -392,7 +401,7 @@ Note that these rules can be over-ridden by the CIL compiler command line parame
> >  </colgroup>
> >  <tbody>
> >  <tr class="odd">
> > -<td align="left"><p><code>neverallows</code></p></td>
> > +<td align="left"><p><code>neverallowx</code></p></td>
> >  <td align="left"><p>The <code>neverallowx</code> keyword.</p></td>
> >  </tr>
> >  <tr class="even">
> > --
> > 2.28.0
> >
diff mbox series

Patch

diff --git a/secilc/docs/cil_access_vector_rules.md b/secilc/docs/cil_access_vector_rules.md
index 9c03c710..683cc28c 100644
--- a/secilc/docs/cil_access_vector_rules.md
+++ b/secilc/docs/cil_access_vector_rules.md
@@ -238,6 +238,8 @@  allowx
 
 Specifies the access allowed between a source and target type using extended permissions. Unlike the [`allow`](cil_access_vector_rules.md#allow) statement, the statements [`validatetrans`](cil_constraint_statements.md#validatetrans), [`mlsvalidatetrans`](cil_constraint_statements.md#mlsvalidatetrans), [`constrain`](cil_constraint_statements.md#constrain), and [`mlsconstrain`](cil_constraint_statements.md#mlsconstrain) do not limit accesses granted by [`allowx`](cil_access_vector_rules.md#allowx).
 
+Note that for this to work there must *also* be valid equivalent [`allow`](cil_access_vector_rules.md#allow) rules present.
+
 **Rule definition:**
 
     (allowx source_id target_id|self permissionx_id)
@@ -274,18 +276,21 @@  Specifies the access allowed between a source and target type using extended per
 
 These examples show a selection of possible permutations of [`allowx`](cil_access_vector_rules.md#allowx) rules:
 
+    (allow type_1 type_2 (tcp_socket (ioctl))) ;; pre-requisite
     (allowx type_1 type_2 (ioctl tcp_socket (range 0x2000 0x20FF)))
 
     (permissionx ioctl_nodebug (ioctl udp_socket (not (range 0x4000 0x4010))))
+    (allow type_3 type_4 (udp_socket (ioctl))) ;; pre-requisite
     (allowx type_3 type_4 ioctl_nodebug)
 
 
-
 auditallowx
 -----------
 
 Audit the access rights defined if there is a valid [`allowx`](cil_access_vector_rules.md#allowx) rule. It does NOT allow access, it only audits the event.
 
+Note that for this to work there must *also* be valid equivalent [`auditallow`](cil_access_vector_rules.md#auditallow) rules present.
+
 **Rule definition:**
 
     (auditallowx source_id target_id|self permissionx_id)
@@ -324,6 +329,7 @@  This example will log an audit event whenever the corresponding [`allowx`](cil_a
 
     (allowx type_1 type_2 (ioctl tcp_socket (range 0x2000 0x20FF)))
 
+    (auditallow type_1 type_2 (tcp_socket (ioctl))) ;; pre-requisite
     (auditallowx type_1 type_2 (ioctl tcp_socket (range 0x2005 0x2010)))
 
 
@@ -332,6 +338,8 @@  dontauditx
 
 Do not audit the access rights defined when access denied. This stops excessive log entries for known events.
 
+Note that for this to work there must *also* be atleast one [`allowx`](cil_access_vector_rules.md#allowx) rule associated with the target type.
+
 Note that these rules can be omitted by the CIL compiler command line parameter `-D` or `--disable-dontaudit` flags.
 
 **Rule definition:**
@@ -370,6 +378,7 @@  Note that these rules can be omitted by the CIL compiler command line parameter
 
 This example will not audit the denied access:
 
+    (allowx type_1 type_2 (ioctl tcp_socket (0x1))) ;; pre-requisite, just some irrelevant random ioctl
     (dontauditx type_1 type_2 (ioctl tcp_socket (range 0x3000 0x30FF)))
 
 
@@ -392,7 +401,7 @@  Note that these rules can be over-ridden by the CIL compiler command line parame
 </colgroup>
 <tbody>
 <tr class="odd">
-<td align="left"><p><code>neverallows</code></p></td>
+<td align="left"><p><code>neverallowx</code></p></td>
 <td align="left"><p>The <code>neverallowx</code> keyword.</p></td>
 </tr>
 <tr class="even">