<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> <p style="margin-top:0;margin-bottom:0"></p> <div>Thank you for the explanation John.</div> <div><br> </div> <div>I have attached the files we have modified. Every piece of code that we inserted is enclosed</div> <div>within comment lines 'Custom code begin' and 'Custom code end' so that it's easy for you to find. Here</div> <div>is a brief description of the changes made:</div> <div><br> </div> <div>AppArmor Parser (user-space) - We modified the grammar of AppArmor's parser to include additional </div> <div><span style="white-space:pre"></span> grammar rules. These rules store data in class Profile</div> <div><br> </div> <div>a) profile.h - 2 new structure definitions to store our custom data</div> <div><span style="white-space:pre"></span> - class Profile now contains a member 'clabel' </div> <div><span style="white-space:pre"></span></div> <div>b) parser_interface.c - Added code to sd_serialize_profile( ) that serializes the additional custom</div> <div><span style="white-space:pre"></span> data we added to class Profile during the parsing phase</div> <div><span style="white-space:pre"></span></div> <div><span style="white-space:pre"></span></div> <div>AppArmor LSM (kernel) : </div> <div><br> </div> <div><br> </div> <div>a) include/policy.h - 2 new structure definitions</div> <div><span style="white-space:pre"></span> - struct aa_profile now contains a member 'clabel'</div> <div><br> </div> <div>b) policy_unpack.c<span style="white-space:pre"> </span>- Added code to unpack_profile( ) that unpacks the serialized object sent from </div> <div><span style="white-space:pre"></span>user-space, and allocates kernel memory for the security structures added to </div> <div><span style="white-space:pre"></span>aa_profile - namely, a label string and a linked list containing allow permissions</div> <div><span style="white-space:pre"></span></div> <div>c) policy.c - Added code to function aa_free_profile( ) that frees the allocated memory </div> <br> <p></p> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> John Johansen <john.johansen@canonical.com><br> <b>Sent:</b> 27 July 2019 00:10:14<br> <b>To:</b> Abhishek Vijeev <abhishekvijeev@iisc.ac.in>; apparmor@lists.ubuntu.com <apparmor@lists.ubuntu.com><br> <b>Cc:</b> Rakesh Rajan Beck <rakeshbeck@iisc.ac.in><br> <b>Subject:</b> Re: [apparmor] Questions about AppArmor's Kernel Code</font> <div> </div> </div> <div class="BodyFragment"><font size="2"><span style="font-size:11pt;"> <div class="PlainText">On 7/26/19 5:56 AM, Abhishek Vijeev wrote:<br> > Hi,<br> > <br> > <br> > I have a few questions about AppArmor's kernel code and would be grateful if you could kindly answer them. <br> > <br> > <br> > 1) Why does AppArmor maintain two separate security blobs in cred->security as well as task-security for processes? For a simple project that requires associating a security context with every task, would it suffice to use just one of these?<br> > <br> the task->security field is used to store task specific information, that is not used for general mediation. Currently the information stored their is for the change_hat and change_onexec apis and some info to track what the confinement was when no-newprivs was applied to the task.<br> <br> cred->security is used to store the subjects label (type) for mediation.<br> <br> Before the task->security field was reintroduce all the information was stored off the cred in a intermediate structure. Doing so would cause use of the change_hat and change_onexec api to change the cred of the task even when the confinement had not changed. The switch to using the task->security field was pre 4.18<br> <br> > <br> > 2) There has been a change in the way security blobs are accessed from kernel version 4.18 to 5.2. I see that in v5.2, the security blob's address is obtained by adding the size of the blob to the start address. Why has this change been made? (For reference: <a href="https://github.com/torvalds/linux/blob/master/security/apparmor/include/cred.h#L24">https://github.com/torvalds/linux/blob/master/security/apparmor/include/cred.h#L24</a>)<br> > <br> see Casey's answer<br> <br> > <br> > 3) I tried adding a custom field (pointer to a custom structure) to struct aa_profile, at exactly this point - <a href="https://github.com/torvalds/linux/blob/master/security/apparmor/include/policy.h#L144">https://github.com/torvalds/linux/blob/master/security/apparmor/include/policy.h#L144</a>. I have taken care to allocate and free memory for the pointer at the appropriate places (allocation is performed here - <a href="https://github.com/torvalds/linux/blob/master/security/apparmor/policy_unpack.c#L671">https://github.com/torvalds/linux/blob/master/security/apparmor/policy_unpack.c#L671</a> and freeing is performed here - <a href="https://github.com/torvalds/linux/blob/master/security/apparmor/policy.c#L205">https://github.com/torvalds/linux/blob/master/security/apparmor/policy.c#L205</a>). However, while booting the kernel, it crashes at the function 'security_prepare_creds( )', which I presume invokes 'apparmor_cred_prepare( )'. If I was, to assume for a moment that there are no bugs with my memory allocation code, is there any other reason why such a crash might have occurred? I have attached the kernel crash log file with this email for your kind reference. <br> > <br> <br> I know the code points but to be able to comment beyond vague guesses I need to see your changes. I can give you the warning to not add your field after the current last field,<br> <br> struct aa_label label;<br> <br> as it has a variable length field. While that is always 2 entries when its embedded in the profile the compiler will end up treating it as zero length over lapping your new field with the start of the variable length array.<br> <br> I do have a patch to address this using a union but I haven't landed it yet.<br> </div> </span></font></div> </body> </html>