maruos / vendor_maruos

Common files for Maru OS.
Apache License 2.0
5 stars 15 forks source link

Remove attribute dac_override from type mcprepare #18

Closed bootlessxfly closed 2 years ago

bootlessxfly commented 2 years ago

mcprepare violates a neverallow rule around dac_override_allow. mcprepare simpily needs to be added to dac_override_allow. The issue comes in that type mcprepare is not loaded untill later in the sepolicycheck test. This causes the build to fail with a type not found error when adding mcprepare to dac_override_allow. To get around this, dac_override_allow has been added to mcprepare.te after the declaration of types. I added mcprepare to this custom dac_override_allow to add it as an exception. Then, I added the original dac_override newverallow rule so that the check included mcprepare. Inside of system/sepolicy/public/domain.te, the neverallow policy for dac_overide needs to be removed so that it is reran once mcprepare is recognized. This allows for the check to pass without being disbaled.

While this does require changes to system/sepolicy, this would need to be done regardless as mcprepare would need to be added to domain.te via dac_override_allow.

Ideally, this could have been set and overridden inside of vendor/maruos. I tried and was unable to do so.

Signed-off-by: Chris White bootlessxfly@gmail.com

bootlessxfly commented 2 years ago

There is no maru project for system/sepolicy. It may be a good idea to create one to keep up with changes that can not be handled by vendor/maruos/sepolicy.

Here is the corresponding system/sepolicy branch that corresponds to this fix: https://github.com/bootlessxfly/android_system_sepolicy/tree/mcprepare_dac_override

utzcoz commented 2 years ago

There is no maru project for system/sepolicy. It may be a good idea to create one to keep up with changes that can not be handled by vendor/maruos/sepolicy.

The reason that MaruOS doesn't folk and create system/sepolicy is to reduce the maintaining problem. When we moved most of MaruOS to app or other upper level, we can recheck those MaruOS sepolicy. Maybe we can remove some of them.

bootlessxfly commented 2 years ago

There is no maru project for system/sepolicy. It may be a good idea to create one to keep up with changes that can not be handled by vendor/maruos/sepolicy.

The reason that MaruOS doesn't folk and create system/sepolicy is to reduce the maintaining problem. When we moved most of MaruOS to app or other upper level, we can recheck those MaruOS sepolicy. Maybe we can remove some of them.

That makes sense. An additional project to maintain is probably more hassle than its worth. I mentioned above that we may be able to use something like sed to automatically remove that line whenever we run "repo sync". So I see your point about forking off system/sepolicy.

utzcoz commented 2 years ago

Thanks. I will test it on my pixel phones this weekend.

utzcoz commented 2 years ago

Hi @pdsouza @pintaf , maybe this PR needs your double-checking on your devices.

bootlessxfly commented 2 years ago

LGTM. The device(Pixel XL and Pixel 3a) shows denied log of mcprepare to access dac_override after applying this PR. But mcprepare's function doesn't be affected after removing dac_override from type mcprepare. I think it is safe to remove it to fix other device's problem.

Im still new to the idea of a "dontaudit" rule, but we may do something similar to what was done with mknod with dacoverride. If I am understanding it correctly, this would get rid of the "denied log" messages.

pdsouza commented 2 years ago

Nice, I'll try this out this week and check

pdsouza commented 2 years ago

Had some time this evening and ran a build on my Nexus 5. I see the denied logs for mcprepare for dac_override but it does not impact mcprepare's function. I think this is likely because we have mcprepare set to permissive for now.

Anyways, LGTM!

utzcoz commented 2 years ago

@bootlessxfly Thanks for your work.