Re: [PATCH v2] libsepol: add missing ibendport port validity check

[William Roberts]

On Mon, Oct 22, 2018 at 11:58 PM Ondrej Mosnacek  wrote:
>
> The kernel checks if the port is in the range 1-255 when loading an
> ibenportcon rule. Add the same check to libsepol.
>
> Fixes: 118c0cd1038e ("libsepol: Add ibendport ocontext handling")
> Signed-off-by: Ondrej Mosnacek 
> ---
>  libsepol/src/policydb.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> Changes in v2:
>  - use UINT8_MAX as the limit for ibendport.port value to emphasize that
>it is an 8-bit value
>
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index db6765ba..96176d80 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -2854,7 +2854,9 @@ static int ocontext_read_selinux(struct 
> policydb_compat_info *info,
> return -1;
> break;
> }
> -   case OCON_IBENDPORT:
> +   case OCON_IBENDPORT: {
> +   uint32_t port;
> +
> rc = next_entry(buf, fp, sizeof(uint32_t) * 
> 2);
> if (rc < 0)
> return -1;
> @@ -2862,6 +2864,10 @@ static int ocontext_read_selinux(struct 
> policydb_compat_info *info,
> if (len == 0 || len > IB_DEVICE_NAME_MAX - 1)
> return -1;
>
> +   port = le32_to_cpu(buf[1]);
> +   if (port > UINT8_MAX || port == 0)
> +   return -1;
> +
> c->u.ibendport.dev_name = malloc(len + 1);
> if (!c->u.ibendport.dev_name)
> return -1;
> @@ -2869,11 +2875,12 @@ static int ocontext_read_selinux(struct 
> policydb_compat_info *info,
> if (rc < 0)
> return -1;
> c->u.ibendport.dev_name[len] = 0;
> -   c->u.ibendport.port = le32_to_cpu(buf[1]);
> +   c->u.ibendport.port = port;
> if (context_read_and_validate
> (>context[0], p, fp))
> return -1;
> break;
> +   }
> case OCON_PORT:
> rc = next_entry(buf, fp, sizeof(uint32_t) * 
> 3);
> if (rc < 0)
> --
> 2.17.2
>

ack. I dropped it on top of https://github.com/SELinuxProject/selinux/pull/105

Thanks
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: cil mlsconstrain

[Ted Toth]

On Tue, Oct 23, 2018 at 9:05 AM Stephen Smalley  wrote:

> On 10/23/2018 09:56 AM, Ted Toth wrote:
> >
> >
> > On Tue, Oct 23, 2018 at 8:39 AM Stephen Smalley  > > wrote:
> >
> > On 10/23/2018 09:33 AM, Ted Toth wrote:
> >  > Is it possible to modify/replace an existing mlsconstrain? In
> > playing
> >  > around I created multiple instances of a mlsconstrain and
> > variations of
> >  > mlsconstrains but haven't figured out how to clean them up as I
> get
> >  > "Error: Unknown keyword delete' when trying to delete my
> experiments.
> >
> > Possibly I misunderstand, but can't you just remove or replace the
> > module that defined it previously?
> >
> >
> > We make some changes to several 'x_*' mls constraints which as far as I
> > know are not part of a module.
>
> They have to live in some module, base or otherwise.
> You can extract the CIL for the module in which you defined them via
> semodule -cE , e.g. semodule -cE base.  Then you can edit
> them in that base.cil or other file and re-insert the updated one.
>
>
That's what I'll do, thanks.


>
> >
> >
> > BTW, selinux mailing list has moved to seli...@vger.kernel.org
> > .
> >
> > Thanks for the reminder now I just need gmail to remember :(
>
>
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: cil mlsconstrain

[Stephen Smalley]

On 10/23/2018 09:56 AM, Ted Toth wrote:



On Tue, Oct 23, 2018 at 8:39 AM Stephen Smalley > wrote:


On 10/23/2018 09:33 AM, Ted Toth wrote:
 > Is it possible to modify/replace an existing mlsconstrain? In
playing
 > around I created multiple instances of a mlsconstrain and
variations of
 > mlsconstrains but haven't figured out how to clean them up as I get
 > "Error: Unknown keyword delete' when trying to delete my experiments.

Possibly I misunderstand, but can't you just remove or replace the
module that defined it previously?


We make some changes to several 'x_*' mls constraints which as far as I 
know are not part of a module.


They have to live in some module, base or otherwise.
You can extract the CIL for the module in which you defined them via 
semodule -cE , e.g. semodule -cE base.  Then you can edit 
them in that base.cil or other file and re-insert the updated one.






BTW, selinux mailing list has moved to seli...@vger.kernel.org
.

Thanks for the reminder now I just need gmail to remember :(


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH v6] selinux: policydb - fix byte order and alignment issues

[Stephen Smalley]

On 10/23/2018 03:02 AM, Ondrej Mosnacek wrote:

Do the LE conversions before doing the Infiniband-related range checks.
The incorrect checks are otherwise causing a failure to load any policy
with an ibendportcon rule on BE systems. This can be reproduced by
running (on e.g. ppc64):

cat >my_module.cil <
Cc: Eli Cohen 
Cc: James Morris 
Cc: Doug Ledford 
Cc:  # 4.13+
Fixes: a806f7a1616f ("selinux: Create policydb version for Infiniband support")
Signed-off-by: Ondrej Mosnacek 


Acked-by: Stephen Smalley 


---
  security/selinux/ss/policydb.c | 51 --
  1 file changed, 36 insertions(+), 15 deletions(-)

Changes in v6:
  - use U8_MAX as the limit for ibendport.port value to emphasize that it
is an 8-bit value

Changes in v5:
  - defer also assignment to 8-bit ibendport.port

Changes in v4:
  - defer assignment to 16-bit struct fields to after the range check

Changes in v3:
  - use separate buffer for the 64-bit subnet_prefix
  - add comments on the byte ordering of subnet_prefix
  - deduplicate the le32_to_cpu() calls from checks

Changes in v2:
  - add reproducer to commit message
  - update e-mail address of James Morris
  - better Cc also the old SELinux ML

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index f4eadd3f7350..b63ef865ce1e 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -2108,6 +2108,7 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
  {
int i, j, rc;
u32 nel, len;
+   __be64 prefixbuf[1];
__le32 buf[3];
struct ocontext *l, *c;
u32 nodebuf[8];
@@ -2217,21 +2218,30 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
goto out;
break;
}
-   case OCON_IBPKEY:
-   rc = next_entry(nodebuf, fp, sizeof(u32) * 4);
+   case OCON_IBPKEY: {
+   u32 pkey_lo, pkey_hi;
+
+   rc = next_entry(prefixbuf, fp, sizeof(u64));
+   if (rc)
+   goto out;
+
+   /* we need to have subnet_prefix in CPU order */
+   c->u.ibpkey.subnet_prefix = 
be64_to_cpu(prefixbuf[0]);
+
+   rc = next_entry(buf, fp, sizeof(u32) * 2);
if (rc)
goto out;
  
-c->u.ibpkey.subnet_prefix = be64_to_cpu(*((__be64 *)nodebuf));

+   pkey_lo = le32_to_cpu(buf[0]);
+   pkey_hi = le32_to_cpu(buf[1]);
  
-if (nodebuf[2] > 0x ||

-   nodebuf[3] > 0x) {
+   if (pkey_lo > U16_MAX || pkey_hi > U16_MAX) {
rc = -EINVAL;
goto out;
}
  
-c->u.ibpkey.low_pkey = le32_to_cpu(nodebuf[2]);

-   c->u.ibpkey.high_pkey = le32_to_cpu(nodebuf[3]);
+   c->u.ibpkey.low_pkey  = pkey_lo;
+   c->u.ibpkey.high_pkey = pkey_hi;
  
  rc = context_read_and_validate(>context[0],

   p,
@@ -2239,7 +2249,10 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
if (rc)
goto out;
break;
-   case OCON_IBENDPORT:
+   }
+   case OCON_IBENDPORT: {
+   u32 port;
+
rc = next_entry(buf, fp, sizeof(u32) * 2);
if (rc)
goto out;
@@ -2249,12 +2262,13 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
if (rc)
goto out;
  
-if (buf[1] > 0xff || buf[1] == 0) {

+   port = le32_to_cpu(buf[1]);
+   if (port > U8_MAX || port == 0) {
rc = -EINVAL;
goto out;
}
  
-c->u.ibendport.port = le32_to_cpu(buf[1]);

+   c->u.ibendport.port = port;
  
  rc = context_read_and_validate(>context[0],

   p,
@@ -2262,7 +2276,8 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
if (rc)
goto out;

Re: cil mlsconstrain

[Ted Toth]

On Tue, Oct 23, 2018 at 8:39 AM Stephen Smalley  wrote:

> On 10/23/2018 09:33 AM, Ted Toth wrote:
> > Is it possible to modify/replace an existing mlsconstrain? In playing
> > around I created multiple instances of a mlsconstrain and variations of
> > mlsconstrains but haven't figured out how to clean them up as I get
> > "Error: Unknown keyword delete' when trying to delete my experiments.
>
> Possibly I misunderstand, but can't you just remove or replace the
> module that defined it previously?
>

We make some changes to several 'x_*' mls constraints which as far as I
know are not part of a module.


> BTW, selinux mailing list has moved to seli...@vger.kernel.org.
>
>
Thanks for the reminder now I just need gmail to remember :(
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: cil mlsconstrain

[Stephen Smalley]

On 10/23/2018 09:33 AM, Ted Toth wrote:
Is it possible to modify/replace an existing mlsconstrain? In playing 
around I created multiple instances of a mlsconstrain and variations of 
mlsconstrains but haven't figured out how to clean them up as I get 
"Error: Unknown keyword delete' when trying to delete my experiments.


Possibly I misunderstand, but can't you just remove or replace the 
module that defined it previously?


BTW, selinux mailing list has moved to seli...@vger.kernel.org.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

cil mlsconstrain

[Ted Toth]

Is it possible to modify/replace an existing mlsconstrain? In playing
around I created multiple instances of a mlsconstrain and variations of
mlsconstrains but haven't figured out how to clean them up as I get "Error:
Unknown keyword delete' when trying to delete my experiments.

Ted
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

[PATCH v6] selinux: policydb - fix byte order and alignment issues

[Ondrej Mosnacek]

Do the LE conversions before doing the Infiniband-related range checks.
The incorrect checks are otherwise causing a failure to load any policy
with an ibendportcon rule on BE systems. This can be reproduced by
running (on e.g. ppc64):

cat >my_module.cil <
Cc: Eli Cohen 
Cc: James Morris 
Cc: Doug Ledford 
Cc:  # 4.13+
Fixes: a806f7a1616f ("selinux: Create policydb version for Infiniband support")
Signed-off-by: Ondrej Mosnacek 
---
 security/selinux/ss/policydb.c | 51 --
 1 file changed, 36 insertions(+), 15 deletions(-)

Changes in v6:
 - use U8_MAX as the limit for ibendport.port value to emphasize that it
   is an 8-bit value 

Changes in v5:
 - defer also assignment to 8-bit ibendport.port

Changes in v4:
 - defer assignment to 16-bit struct fields to after the range check

Changes in v3:
 - use separate buffer for the 64-bit subnet_prefix
 - add comments on the byte ordering of subnet_prefix
 - deduplicate the le32_to_cpu() calls from checks

Changes in v2:
 - add reproducer to commit message
 - update e-mail address of James Morris
 - better Cc also the old SELinux ML

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index f4eadd3f7350..b63ef865ce1e 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -2108,6 +2108,7 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
 {
int i, j, rc;
u32 nel, len;
+   __be64 prefixbuf[1];
__le32 buf[3];
struct ocontext *l, *c;
u32 nodebuf[8];
@@ -2217,21 +2218,30 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
goto out;
break;
}
-   case OCON_IBPKEY:
-   rc = next_entry(nodebuf, fp, sizeof(u32) * 4);
+   case OCON_IBPKEY: {
+   u32 pkey_lo, pkey_hi;
+
+   rc = next_entry(prefixbuf, fp, sizeof(u64));
+   if (rc)
+   goto out;
+
+   /* we need to have subnet_prefix in CPU order */
+   c->u.ibpkey.subnet_prefix = 
be64_to_cpu(prefixbuf[0]);
+
+   rc = next_entry(buf, fp, sizeof(u32) * 2);
if (rc)
goto out;
 
-   c->u.ibpkey.subnet_prefix = 
be64_to_cpu(*((__be64 *)nodebuf));
+   pkey_lo = le32_to_cpu(buf[0]);
+   pkey_hi = le32_to_cpu(buf[1]);
 
-   if (nodebuf[2] > 0x ||
-   nodebuf[3] > 0x) {
+   if (pkey_lo > U16_MAX || pkey_hi > U16_MAX) {
rc = -EINVAL;
goto out;
}
 
-   c->u.ibpkey.low_pkey = le32_to_cpu(nodebuf[2]);
-   c->u.ibpkey.high_pkey = le32_to_cpu(nodebuf[3]);
+   c->u.ibpkey.low_pkey  = pkey_lo;
+   c->u.ibpkey.high_pkey = pkey_hi;
 
rc = context_read_and_validate(>context[0],
   p,
@@ -2239,7 +2249,10 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
if (rc)
goto out;
break;
-   case OCON_IBENDPORT:
+   }
+   case OCON_IBENDPORT: {
+   u32 port;
+
rc = next_entry(buf, fp, sizeof(u32) * 2);
if (rc)
goto out;
@@ -2249,12 +2262,13 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
if (rc)
goto out;
 
-   if (buf[1] > 0xff || buf[1] == 0) {
+   port = le32_to_cpu(buf[1]);
+   if (port > U8_MAX || port == 0) {
rc = -EINVAL;
goto out;
}
 
-   c->u.ibendport.port = le32_to_cpu(buf[1]);
+   c->u.ibendport.port = port;
 
rc = context_read_and_validate(>context[0],
   p,
@@ -2262,7 +2276,8 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,

[PATCH v2] libsepol: add missing ibendport port validity check

[Ondrej Mosnacek]

The kernel checks if the port is in the range 1-255 when loading an
ibenportcon rule. Add the same check to libsepol.

Fixes: 118c0cd1038e ("libsepol: Add ibendport ocontext handling")
Signed-off-by: Ondrej Mosnacek 
---
 libsepol/src/policydb.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

Changes in v2:
 - use UINT8_MAX as the limit for ibendport.port value to emphasize that
   it is an 8-bit value

diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index db6765ba..96176d80 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -2854,7 +2854,9 @@ static int ocontext_read_selinux(struct 
policydb_compat_info *info,
return -1;
break;
}
-   case OCON_IBENDPORT:
+   case OCON_IBENDPORT: {
+   uint32_t port;
+
rc = next_entry(buf, fp, sizeof(uint32_t) * 2);
if (rc < 0)
return -1;
@@ -2862,6 +2864,10 @@ static int ocontext_read_selinux(struct 
policydb_compat_info *info,
if (len == 0 || len > IB_DEVICE_NAME_MAX - 1)
return -1;
 
+   port = le32_to_cpu(buf[1]);
+   if (port > UINT8_MAX || port == 0)
+   return -1;
+
c->u.ibendport.dev_name = malloc(len + 1);
if (!c->u.ibendport.dev_name)
return -1;
@@ -2869,11 +2875,12 @@ static int ocontext_read_selinux(struct 
policydb_compat_info *info,
if (rc < 0)
return -1;
c->u.ibendport.dev_name[len] = 0;
-   c->u.ibendport.port = le32_to_cpu(buf[1]);
+   c->u.ibendport.port = port;
if (context_read_and_validate
(>context[0], p, fp))
return -1;
break;
+   }
case OCON_PORT:
rc = next_entry(buf, fp, sizeof(uint32_t) * 3);
if (rc < 0)
-- 
2.17.2

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH] libsepol: add missing ibendport port validity check

[Ondrej Mosnacek]

On Mon, Oct 22, 2018 at 4:49 PM William Roberts
 wrote:
> On Mon, Oct 22, 2018 at 1:18 AM Ondrej Mosnacek  wrote:
> >
> > The kernel checks if the port is in the range 1-255 when loading an
> > ibenportcon rule. Add the same check to libsepol.
> >
> > Fixes: 118c0cd1038e ("libsepol: Add ibendport ocontext handling")
> > Signed-off-by: Ondrej Mosnacek 
> > ---
> >  libsepol/src/policydb.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> > index db6765ba..e2808b2d 100644
> > --- a/libsepol/src/policydb.c
> > +++ b/libsepol/src/policydb.c
> > @@ -2854,7 +2854,9 @@ static int ocontext_read_selinux(struct 
> > policydb_compat_info *info,
> > return -1;
> > break;
> > }
> > -   case OCON_IBENDPORT:
> > +   case OCON_IBENDPORT: {
> > +   uint32_t port;
> > +
> > rc = next_entry(buf, fp, sizeof(uint32_t) * 
> > 2);
> > if (rc < 0)
> > return -1;
> > @@ -2862,6 +2864,10 @@ static int ocontext_read_selinux(struct 
> > policydb_compat_info *info,
> > if (len == 0 || len > IB_DEVICE_NAME_MAX - 
> > 1)
> > return -1;
> >
> > +   port = le32_to_cpu(buf[1]);
> > +   if (port > 0xff || port == 0)
> > +   return -1;
>
> You switched the other code to using UINT16_MAX, should probably use
> UINT8_MAX here.

Good point. I'll need to update the kernel patch as well.

Thanks,

>
> > +
> > c->u.ibendport.dev_name = malloc(len + 1);
> > if (!c->u.ibendport.dev_name)
> > return -1;
> > @@ -2869,11 +2875,12 @@ static int ocontext_read_selinux(struct 
> > policydb_compat_info *info,
> > if (rc < 0)
> > return -1;
> > c->u.ibendport.dev_name[len] = 0;
> > -   c->u.ibendport.port = le32_to_cpu(buf[1]);
> > +   c->u.ibendport.port = port;
> > if (context_read_and_validate
> > (>context[0], p, fp))
> > return -1;
> > break;
> > +   }
> > case OCON_PORT:
> > rc = next_entry(buf, fp, sizeof(uint32_t) * 
> > 3);
> > if (rc < 0)
> > --
> > 2.17.2
> >

-- 
Ondrej Mosnacek 
Associate Software Engineer, Security Technologies
Red Hat, Inc.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.