On Jun 7 2019, at 12:48 am, John Johansen <john.johan...@canonical.com> wrote:
On 6/6/19 1:21 AM, Abhishek Vijeev wrote:
Hi,

I'm looking for some help with modifying AppArmor's kernel code. Kindly let me 
know whether this is the right forum for such discussions (as I didn't think it 
would be appropriate to ask for help via the 'Issues' tab on GitLab).

Onto my problem. Basically, I'm trying to add a custom field to 'struct 
aa_profile' found in <linux_kernel_path>/security/apparmor/include/policy.h and 
set this field to  a value of my choice. To accomplish this, I have added a 
single line of code to the 'unpack_profile( )' function found in 
<linux_kernel_path>/security/apparmor/policy_unpack.c.  However, a kernel that 
has been compiled with this single extra line of code fails to boot. The boot 
process halts at 'A start job is running for AppArmor initialization'.

the is odd, the addition of a field to aa_profile should not stop apparmor from 
functioning. There are several fields in there that are kernel only and have 
nothing to do with loaded policy. Also unless the field is to do with loaded 
policy/can or will be set be loaded policy a better place to initialize fields 
is in aa_alloc_profile in policy.c


For greater clarity, here is the structure after adding my custom field,

struct aa_profile {
struct aa_policy base;
struct aa_profile __rcu *parent;

struct aa_ns *ns;
const char *rename;

const char *attach;
struct aa_dfa *xmatch;
int xmatch_len;
enum audit_mode audit;
long mode;
u32 path_flags;
const char *disconnected;
int size;

struct aa_policydb policy;
struct aa_file_rules file;
struct aa_caps caps;

int xattr_count;
char **xattrs;

struct aa_rlimit rlimits;

struct aa_loaddata *rawdata;
unsigned char *hash;
char *dirname;
struct dentry *dents[AAFS_PROF_SIZEOF];
struct rhashtable *data;
struct aa_label label;
/*
* Custom field:
*/
int custom_field;
};

I would avoid putting data fields after label. The label structure is 
technically variable length, though its use in profile is always fixed to a 
single entry + the null sentinel, so you should be safe but its bad form 
knowing that field is naturally variable length.

Thanks for this extremely valuable information John. It has fixed the problem. 
Adding the custom field before 'label' has resulted in the kernel successfully 
booting, and the field being set to the required value. Honestly, we would 
haven't been able to resolve the issue, had we not known about this.



and here is the line of code to set this field (added at the end of 
'unpack_profile( )'),

          profile -> custom_field = 10;


where are you making the assignment in unpack_profile? Does profile exist yet? 
unpack_profile() will allocate after unpacking the profiles name. Assigning 
profile->custom_field before the profile is allocated would cause the kernel to 
oops and could cause things to hang.

Yes, I am making the assignment only after the profile has been allocated 
(which I presume occurs at line 677 of this file for reference - 
https://github.com/torvalds/linux/blob/master/security/apparmor/policy_unpack.c<https://link.getmailspring.com/link/77f13bd1-f8fb-41d4-a853-d9ff2221a...@getmailspring.com/0?redirect=https%3A%2F%2Fgithub.com%2Ftorvalds%2Flinux%2Fblob%2Fmaster%2Fsecurity%2Fapparmor%2Fpolicy_unpack.c&recipient=YXBwYXJtb3JAbGlzdHMudWJ1bnR1LmNvbQ%3D%3D>
 )

I'm not sure if I'm doing something fundamentally wrong with trying to modify 
the structure. I do understand that AppArmor verifies each policy's 
cryptographic hash, and suspect that a hash mismatch renders the kernel 
un-bootable. However, if the code that generates the hash and the code that 
calculates and verifies the hash at kernel boot are oblivious of the custom 
field, why would a mismatch occur?


It does compute a policy hash for the loaded profile but that is not used by 
the kernel to verify what it loads, the hash values are used for policy dedup 
and made available to userspace so userspace can do quick verifications of what 
is in the kernel (dumping large amounts of policy back out is much slower than 
reading a few hashes). Their is no mismatch on hashes to render the kernel 
unbootable, apparmor does not do secure boot, integrity is done through the ima 
subsystem, and secure boot to other parts of the kernel. AppArmor does validate 
policy that is being loaded and ensures that the data can not be used to crash 
the kernel, but a failure in a validation check will result in the policy being 
rejected from loaded not a failure to boot. It is possible you could configure 
boot to abort on failure to load policy but I haven't seen that done for a long 
time.

Thank you for this detailed explanation. I'm sure it's going to come in handy 
soon.

So to be clear the policy verification is not causing a mismatch that is 
resulting in the hang. My immediate guess is you are assigning 
profile->custom_field before profile exists.


I'd be grateful if you could kindly provide me with some insight into the root 
cause of this problem as well as how to resolve it. Do let me know if I can 
provide any additional information to help clarify the problem.

Thank you,
Abhishek.
Sent from Mailspring

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to