On Fri, Mar 11, 2022 at 12:11 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Wed, Mar 9, 2022 at 4:13 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Tue, Mar 1, 2022 at 12:05 AM David Anderson <dvander@xxxxxxxxxx> wrote: > > > On Mon, Feb 28, 2022 at 5:09 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > ... > > > >> This patchset may not have been The Answer, but surely there is > > >> something we can do to support this use-case. > > > > > > Yup exactly, and we still need patches 3 & 4 to deal with this. My current plan is to try and rework our sepolicy (we have some ideas on how it could be made compatible with how overlayfs works). If that doesn't pan out we'll revisit these patches and think harder about how to deal with the coherency issues. > > > > Can you elaborate a bit more on the coherency issues? Is this the dir > > cache issue that is alluded to in the patchset? Anything else that > > has come up on review? > > > > Before I start looking at the dir cache in any detail, did you have > > any thoughts on how to resolve the problems that have arisen? > > David, Vivek, Amir, Miklos, or anyone for that matter, can you please > go into more detail on the cache issues? I *think* I may have found a > potential solution for an issue that could arise when the credential > override is not in place, but I'm not certain it's the only issue :) > Hi Paul, In this thread I claimed that the authors of the patches did not present a security model for overlayfs, such as the one currently in overlayfs.rst. If we had a model we could have debated its correctness and review its implementation. As a proof that there is no solid model, I gave an *example* regarding the overlay readdir cache. When listing a merged dir, meaning, a directory containing entries from several overlay layers, ovl_permission() is called to check user's permission, but ovl_permission() does not currently check permissions to read all layers, because that is not the current overlayfs model. Overlayfs has a readdir cache, so without override_cred, a user with high credentials can populate the readdir cache and then a user will fewer credentials, not enough to access the lower layers, but enough to access the upper most layer, will pass ovl_permission() check and be allowed to read from readdir cache. This specific problem can be solved in several ways - disable readdir cache with override_cred=off, check all layers in ovl_permission(). That's not my point. My point is that I provided a proof that the current model of override_cred=off is flawed and it is up to the authors of the patch to fix the model and provide the analysis of overlayfs code to prove the model's correctness. The core of the matter is there is no easy way to "merge" the permissions from all layers into a single permission blob that could be checked once. Maybe the example I gave is the only flaw in the model, maybe not I am not sure. I will be happy to help you in review of a model and the solution that you may have found. Thanks, Amir.
