2024-04-05 16:27:26

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 2/2] selinux: add support for xperms in conditional policies

From: Christian Göttsche <[email protected]>

Add support for extended permission rules in conditional policies.
Currently the kernel accepts such rules already, but evaluating a
security decision will hit a BUG() in
services_compute_xperms_decision(). Thus reject extended permission
rules in conditional policies for current policy versions.

Add a new policy version for this feature.

Signed-off-by: Christian Göttsche <[email protected]>
---
Userspace patches are available at:
https://github.com/SELinuxProject/selinux/pull/432

Maybe the policy version 34 can be reused for the prefix/suffix filetrans
feature to avoid two new versions?
---
security/selinux/include/security.h | 3 ++-
security/selinux/ss/avtab.c | 12 ++++++++++--
security/selinux/ss/avtab.h | 2 +-
security/selinux/ss/conditional.c | 2 +-
security/selinux/ss/policydb.c | 5 +++++
security/selinux/ss/services.c | 11 +++++++----
security/selinux/ss/services.h | 2 +-
7 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 289bf9233f71..3a385821c574 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -46,10 +46,11 @@
#define POLICYDB_VERSION_INFINIBAND 31
#define POLICYDB_VERSION_GLBLUB 32
#define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */
+#define POLICYDB_VERSION_COND_XPERMS 34 /* extended permissions in conditional policies */

/* Range of policy versions we understand*/
#define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE
-#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS
+#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COND_XPERMS

/* Mask for just the mount related flags */
#define SE_MNTMASK 0x0f
diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index 2ad98732d052..bc7f1aa3ebfb 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -339,7 +339,7 @@ static const uint16_t spec_order[] = {
int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
int (*insertf)(struct avtab *a, const struct avtab_key *k,
const struct avtab_datum *d, void *p),
- void *p)
+ void *p, bool conditional)
{
__le16 buf16[4];
u16 enabled;
@@ -457,6 +457,14 @@ int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
"was specified\n",
vers);
return -EINVAL;
+ } else if ((vers < POLICYDB_VERSION_COND_XPERMS) &&
+ (key.specified & AVTAB_XPERMS) &&
+ conditional) {
+ pr_err("SELinux: avtab: policy version %u does not "
+ "support extended permissions rules in conditional "
+ "policies and one was specified\n",
+ vers);
+ return -EINVAL;
} else if (key.specified & AVTAB_XPERMS) {
memset(&xperms, 0, sizeof(struct avtab_extended_perms));
rc = next_entry(&xperms.specified, fp, sizeof(u8));
@@ -523,7 +531,7 @@ int avtab_read(struct avtab *a, void *fp, struct policydb *pol)
goto bad;

for (i = 0; i < nel; i++) {
- rc = avtab_read_item(a, fp, pol, avtab_insertf, NULL);
+ rc = avtab_read_item(a, fp, pol, avtab_insertf, NULL, false);
if (rc) {
if (rc == -ENOMEM)
pr_err("SELinux: avtab: out of memory\n");
diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
index 8e8820484c55..b48c15b3698c 100644
--- a/security/selinux/ss/avtab.h
+++ b/security/selinux/ss/avtab.h
@@ -107,7 +107,7 @@ struct policydb;
int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
int (*insert)(struct avtab *a, const struct avtab_key *k,
const struct avtab_datum *d, void *p),
- void *p);
+ void *p, bool conditional);

int avtab_read(struct avtab *a, void *fp, struct policydb *pol);
int avtab_write_item(struct policydb *p, const struct avtab_node *cur,
diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index d53c34021dbe..ed4606e3af5d 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -349,7 +349,7 @@ static int cond_read_av_list(struct policydb *p, void *fp,
for (i = 0; i < len; i++) {
data.dst = &list->nodes[i];
rc = avtab_read_item(&p->te_cond_avtab, fp, p, cond_insertf,
- &data);
+ &data, true);
if (rc) {
kfree(list->nodes);
list->nodes = NULL;
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 383f3ae82a73..3ba5506a3fff 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -155,6 +155,11 @@ static const struct policydb_compat_info policydb_compat[] = {
.sym_num = SYM_NUM,
.ocon_num = OCON_NUM,
},
+ {
+ .version = POLICYDB_VERSION_COND_XPERMS,
+ .sym_num = SYM_NUM,
+ .ocon_num = OCON_NUM,
+ },
};

static const struct policydb_compat_info *
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index e88b1b6c4adb..57f09f830a06 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -944,9 +944,10 @@ static void avd_init(struct selinux_policy *policy, struct av_decision *avd)
}

void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
- struct avtab_node *node)
+ const struct avtab_node *node)
{
unsigned int i;
+ u16 specified;

if (node->datum.u.xperms->specified == AVTAB_XPERMS_IOCTLFUNCTION) {
if (xpermd->driver != node->datum.u.xperms->driver)
@@ -959,7 +960,9 @@ void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
BUG();
}

- if (node->key.specified == AVTAB_XPERMS_ALLOWED) {
+ specified = node->key.specified & ~(AVTAB_ENABLED | AVTAB_ENABLED_OLD);
+
+ if (specified == AVTAB_XPERMS_ALLOWED) {
xpermd->used |= XPERMS_ALLOWED;
if (node->datum.u.xperms->specified == AVTAB_XPERMS_IOCTLDRIVER) {
memset(xpermd->allowed->p, 0xff,
@@ -970,7 +973,7 @@ void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
xpermd->allowed->p[i] |=
node->datum.u.xperms->perms.p[i];
}
- } else if (node->key.specified == AVTAB_XPERMS_AUDITALLOW) {
+ } else if (specified == AVTAB_XPERMS_AUDITALLOW) {
xpermd->used |= XPERMS_AUDITALLOW;
if (node->datum.u.xperms->specified == AVTAB_XPERMS_IOCTLDRIVER) {
memset(xpermd->auditallow->p, 0xff,
@@ -981,7 +984,7 @@ void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
xpermd->auditallow->p[i] |=
node->datum.u.xperms->perms.p[i];
}
- } else if (node->key.specified == AVTAB_XPERMS_DONTAUDIT) {
+ } else if (specified == AVTAB_XPERMS_DONTAUDIT) {
xpermd->used |= XPERMS_DONTAUDIT;
if (node->datum.u.xperms->specified == AVTAB_XPERMS_IOCTLDRIVER) {
memset(xpermd->dontaudit->p, 0xff,
diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h
index 93358e7a649c..a6d8a06fd13c 100644
--- a/security/selinux/ss/services.h
+++ b/security/selinux/ss/services.h
@@ -38,7 +38,7 @@ struct convert_context_args {
void services_compute_xperms_drivers(struct extended_perms *xperms,
struct avtab_node *node);
void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
- struct avtab_node *node);
+ const struct avtab_node *node);

int services_convert_context(struct convert_context_args *args,
struct context *oldc, struct context *newc,
--
2.43.0



2024-04-05 16:29:51

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 1/2] selinux: constify source policy in cond_policydb_dup()

From: Christian Göttsche <[email protected]>

cond_policydb_dup() duplicates conditional parts of an existing policy.
Declare the source policy const, since it should not be modified.

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/ss/conditional.c | 12 ++++++------
security/selinux/ss/conditional.h | 2 +-
security/selinux/ss/hashtab.c | 9 +++++----
security/selinux/ss/hashtab.h | 4 ++--
4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index e868fc403d75..d53c34021dbe 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -603,7 +603,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
}
}

-static int cond_dup_av_list(struct cond_av_list *new, struct cond_av_list *orig,
+static int cond_dup_av_list(struct cond_av_list *new, const struct cond_av_list *orig,
struct avtab *avtab)
{
u32 i;
@@ -626,7 +626,7 @@ static int cond_dup_av_list(struct cond_av_list *new, struct cond_av_list *orig,
}

static int duplicate_policydb_cond_list(struct policydb *newp,
- struct policydb *origp)
+ const struct policydb *origp)
{
int rc;
u32 i;
@@ -643,7 +643,7 @@ static int duplicate_policydb_cond_list(struct policydb *newp,

for (i = 0; i < origp->cond_list_len; i++) {
struct cond_node *newn = &newp->cond_list[i];
- struct cond_node *orign = &origp->cond_list[i];
+ const struct cond_node *orign = &origp->cond_list[i];

newp->cond_list_len++;

@@ -683,7 +683,7 @@ static int cond_bools_destroy(void *key, void *datum, void *args)
return 0;
}

-static int cond_bools_copy(struct hashtab_node *new, struct hashtab_node *orig,
+static int cond_bools_copy(struct hashtab_node *new, const struct hashtab_node *orig,
void *args)
{
struct cond_bool_datum *datum;
@@ -710,7 +710,7 @@ static int cond_bools_index(void *key, void *datum, void *args)
}

static int duplicate_policydb_bools(struct policydb *newdb,
- struct policydb *orig)
+ const struct policydb *orig)
{
struct cond_bool_datum **cond_bool_array;
int rc;
@@ -743,7 +743,7 @@ void cond_policydb_destroy_dup(struct policydb *p)
cond_policydb_destroy(p);
}

-int cond_policydb_dup(struct policydb *new, struct policydb *orig)
+int cond_policydb_dup(struct policydb *new, const struct policydb *orig)
{
cond_policydb_init(new);

diff --git a/security/selinux/ss/conditional.h b/security/selinux/ss/conditional.h
index b972ce40db18..8827715bad75 100644
--- a/security/selinux/ss/conditional.h
+++ b/security/selinux/ss/conditional.h
@@ -79,6 +79,6 @@ void cond_compute_xperms(struct avtab *ctab, struct avtab_key *key,
struct extended_perms_decision *xpermd);
void evaluate_cond_nodes(struct policydb *p);
void cond_policydb_destroy_dup(struct policydb *p);
-int cond_policydb_dup(struct policydb *new, struct policydb *orig);
+int cond_policydb_dup(struct policydb *new, const struct policydb *orig);

#endif /* _CONDITIONAL_H_ */
diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c
index 754bedbde133..836642f789ab 100644
--- a/security/selinux/ss/hashtab.c
+++ b/security/selinux/ss/hashtab.c
@@ -136,11 +136,12 @@ void hashtab_stat(struct hashtab *h, struct hashtab_info *info)
}
#endif /* CONFIG_SECURITY_SELINUX_DEBUG */

-int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
+int hashtab_duplicate(struct hashtab *new, const struct hashtab *orig,
int (*copy)(struct hashtab_node *new,
- struct hashtab_node *orig, void *args),
+ const struct hashtab_node *orig, void *args),
int (*destroy)(void *k, void *d, void *args), void *args)
{
+ const struct hashtab_node *orig_cur;
struct hashtab_node *cur, *tmp, *tail;
u32 i;
int rc;
@@ -155,12 +156,12 @@ int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,

for (i = 0; i < orig->size; i++) {
tail = NULL;
- for (cur = orig->htable[i]; cur; cur = cur->next) {
+ for (orig_cur = orig->htable[i]; orig_cur; orig_cur = orig_cur->next) {
tmp = kmem_cache_zalloc(hashtab_node_cachep,
GFP_KERNEL);
if (!tmp)
goto error;
- rc = copy(tmp, cur, args);
+ rc = copy(tmp, orig_cur, args);
if (rc) {
kmem_cache_free(hashtab_node_cachep, tmp);
goto error;
diff --git a/security/selinux/ss/hashtab.h b/security/selinux/ss/hashtab.h
index 5f74dcc1360f..deba82d78c3a 100644
--- a/security/selinux/ss/hashtab.h
+++ b/security/selinux/ss/hashtab.h
@@ -136,9 +136,9 @@ void hashtab_destroy(struct hashtab *h);
int hashtab_map(struct hashtab *h, int (*apply)(void *k, void *d, void *args),
void *args);

-int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
+int hashtab_duplicate(struct hashtab *new, const struct hashtab *orig,
int (*copy)(struct hashtab_node *new,
- struct hashtab_node *orig, void *args),
+ const struct hashtab_node *orig, void *args),
int (*destroy)(void *k, void *d, void *args), void *args);

#ifdef CONFIG_SECURITY_SELINUX_DEBUG
--
2.43.0


2024-04-30 22:55:20

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 1/2] selinux: constify source policy in cond_policydb_dup()

On Apr 5, 2024 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <[email protected]> wrote:
>
> cond_policydb_dup() duplicates conditional parts of an existing policy.
> Declare the source policy const, since it should not be modified.
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/ss/conditional.c | 12 ++++++------
> security/selinux/ss/conditional.h | 2 +-
> security/selinux/ss/hashtab.c | 9 +++++----
> security/selinux/ss/hashtab.h | 4 ++--
> 4 files changed, 14 insertions(+), 13 deletions(-)

I had to do some line length fixups, but otherwise this looked good
to me, merged into selinux/dev. Thanks!

--
paul-moore.com