Skip to content

Commit

Permalink
apparmor: fix profile verification and enable it
Browse files Browse the repository at this point in the history 
The transition table size was not being set by compat mappings
resulting in the profile verification code not being run. Unfortunately
the checks were also buggy not being correctly updated from the old
accept perms, to the new layout.

Also indicate to userspace that the kernel has the permstable verification
fixes.

BugLink: http://bugs.launchpad.net/bugs/2017903
Fixes: 670f317 ("apparmor: verify permission table indexes")
Signed-off-by: John Johansen <[email protected]>
Reviewed-by: Jon Tourville <[email protected]>
  • Loading branch information
John Johansen committed Jul 6, 2023
1 parent 0bac200 commit 6f442d4
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 25 deletions.
18 changes: 12 additions & 6 deletions security/apparmor/policy_compat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ static struct aa_perms compute_fperms_other(struct aa_dfa *dfa,
*
* Returns: remapped perm table
*/
static struct aa_perms *compute_fperms(struct aa_dfa *dfa)
static struct aa_perms *compute_fperms(struct aa_dfa *dfa,
u32 *size)
{
aa_state_t state;
unsigned int state_count;
Expand All @@ -159,6 +160,7 @@ static struct aa_perms *compute_fperms(struct aa_dfa *dfa)
table = kvcalloc(state_count * 2, sizeof(struct aa_perms), GFP_KERNEL);
if (!table)
return NULL;
*size = state_count * 2;

/* zero init so skip the trap state (state == 0) */
for (state = 1; state < state_count; state++) {
Expand All @@ -169,7 +171,8 @@ static struct aa_perms *compute_fperms(struct aa_dfa *dfa)
return table;
}

static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch)
static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch,
u32 *size)
{
struct aa_perms *perms;
int state;
Expand All @@ -182,6 +185,7 @@ static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch)
perms = kvcalloc(state_count, sizeof(struct aa_perms), GFP_KERNEL);
if (!perms)
return NULL;
*size = state_count;

/* zero init so skip the trap state (state == 0) */
for (state = 1; state < state_count; state++)
Expand Down Expand Up @@ -242,7 +246,8 @@ static struct aa_perms compute_perms_entry(struct aa_dfa *dfa,
return perms;
}

static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version)
static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version,
u32 *size)
{
unsigned int state;
unsigned int state_count;
Expand All @@ -255,6 +260,7 @@ static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version)
table = kvcalloc(state_count, sizeof(struct aa_perms), GFP_KERNEL);
if (!table)
return NULL;
*size = state_count;

/* zero init so skip the trap state (state == 0) */
for (state = 1; state < state_count; state++)
Expand Down Expand Up @@ -289,7 +295,7 @@ static void remap_dfa_accept(struct aa_dfa *dfa, unsigned int factor)
/* TODO: merge different dfa mappings into single map_policy fn */
int aa_compat_map_xmatch(struct aa_policydb *policy)
{
policy->perms = compute_xmatch_perms(policy->dfa);
policy->perms = compute_xmatch_perms(policy->dfa, &policy->size);
if (!policy->perms)
return -ENOMEM;

Expand All @@ -300,7 +306,7 @@ int aa_compat_map_xmatch(struct aa_policydb *policy)

int aa_compat_map_policy(struct aa_policydb *policy, u32 version)
{
policy->perms = compute_perms(policy->dfa, version);
policy->perms = compute_perms(policy->dfa, version, &policy->size);
if (!policy->perms)
return -ENOMEM;

Expand All @@ -311,7 +317,7 @@ int aa_compat_map_policy(struct aa_policydb *policy, u32 version)

int aa_compat_map_file(struct aa_policydb *policy)
{
policy->perms = compute_fperms(policy->dfa);
policy->perms = compute_fperms(policy->dfa, &policy->size);
if (!policy->perms)
return -ENOMEM;

Expand Down
34 changes: 15 additions & 19 deletions security/apparmor/policy_unpack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1135,22 +1135,16 @@ static int verify_header(struct aa_ext *e, int required, const char **ns)
return 0;
}

static bool verify_xindex(int xindex, int table_size)
{
int index, xtype;
xtype = xindex & AA_X_TYPE_MASK;
index = xindex & AA_X_INDEX_MASK;
if (xtype == AA_X_TABLE && index >= table_size)
return false;
return true;
}

/* verify dfa xindexes are in range of transition tables */
static bool verify_dfa_xindex(struct aa_dfa *dfa, int table_size)
/**
* verify_dfa_accept_xindex - verify accept indexes are in range of perms table
* @dfa: the dfa to check accept indexes are in range
* table_size: the permission table size the indexes should be within
*/
static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size)
{
int i;
for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
if (!verify_xindex(ACCEPT_TABLE(dfa)[i], table_size))
if (ACCEPT_TABLE(dfa)[i] >= table_size)
return false;
}
return true;
Expand Down Expand Up @@ -1187,11 +1181,13 @@ static bool verify_perms(struct aa_policydb *pdb)
if (!verify_perm(&pdb->perms[i]))
return false;
/* verify indexes into str table */
if (pdb->perms[i].xindex >= pdb->trans.size)
if ((pdb->perms[i].xindex & AA_X_TYPE_MASK) == AA_X_TABLE &&
(pdb->perms[i].xindex & AA_X_INDEX_MASK) >= pdb->trans.size)
return false;
if (pdb->perms[i].tag >= pdb->trans.size)
if (pdb->perms[i].tag && pdb->perms[i].tag >= pdb->trans.size)
return false;
if (pdb->perms[i].label >= pdb->trans.size)
if (pdb->perms[i].label &&
pdb->perms[i].label >= pdb->trans.size)
return false;
}

Expand All @@ -1213,10 +1209,10 @@ static int verify_profile(struct aa_profile *profile)
if (!rules)
return 0;

if ((rules->file.dfa && !verify_dfa_xindex(rules->file.dfa,
rules->file.trans.size)) ||
if ((rules->file.dfa && !verify_dfa_accept_index(rules->file.dfa,
rules->file.size)) ||
(rules->policy.dfa &&
!verify_dfa_xindex(rules->policy.dfa, rules->policy.trans.size))) {
!verify_dfa_accept_index(rules->policy.dfa, rules->policy.size))) {
audit_iface(profile, NULL, NULL,
"Unpack: Invalid named transition", NULL, -EPROTO);
return -EPROTO;
Expand Down

0 comments on commit 6f442d4

Please sign in to comment.