On Wed, Mar 4, 2020 at 9:25 AM Stephen Smalley <stephen.smalley.work@xxxxxxxxx> wrote: > > On Wed, Mar 4, 2020 at 4:07 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > I played with this a bit by recompiling the local binary policy with > > secilc and then comparing the CIL of both binary policies (I used this > > script [1]) and the results are a bit confusing... There is no > > difference in result between -X 0 and -X 1 [2] and in both cases it > > removes some unused attributes (those are only referenced from > > neverallow rules) that were in the original policy > > (/etc/selinux/targeted/policy/policy.31 from my Fedora 31 machine), > > but not in the one recompiled via checkpolicy -C + secilc... At least > > I was able to confirm that secilc -X 2 really removes the attributes > > that have only one type and reduces the policy size by a few > > kilobytes. > > > > I suspect that the reason for the unremoved attributes in the policy > > built by semodule are due to a bug in libsepol: It seems that when it > > starts with a cildb that has the neverallow rules in the input policy > > + has disable_neverallow set, it removes the rules but not the > > attributes that are used only in them. Only when it reads the policy > > again, it identifies these unused attributes (since there are no > > longer any neverallow rules in the input) and removes them > > unconditionally. It could be something else, but if I'm right then I > > think libsepol should be fixed to remove the unused attributes right > > away. I don't dare digging into the CIL code to investigate it, though > > ;) > > James will have to confirm the details but IIRC we had to keep > attributes in the policy > when they are referenced by a neverallow in order to avoid breaking > Android because > it uses the attribute definition from the policy as part of CTS > validation of OEM policies > (it extracts the neverallows from the AOSP policy, extracts the binary > policy from the device, > and checks the neverallows against the device policy). > Steve is correct, we keep attributes that appear in neverallow rules to avoid breaking Android. We also keep attributes that appear in typeattributesets for attributes that appear in neverallow rules. See commits 67b410e80f0917bf1b378997cac0dddf1e6406f7 (libsepol/cil: Keep attributes used by generated attributes in neverallow rules) and 0be23c3f15fdbef35a57d8586aeeae9b1f7606cc (libsepol/cil: Add ability to expand some attributes in binary policy) for more details. Jim
