On Tue, Dec 4, 2018 at 9:40 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > On 12/3/18 6:27 PM, Paul Moore wrote: > > On Thu, Nov 29, 2018 at 5:22 PM Daniel Walsh <dwalsh@xxxxxxxxxx> wrote: > >> On 11/29/18 2:47 PM, Miklos Szeredi wrote: > >>> On Thu, Nov 29, 2018 at 5:14 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > >>> > >>>> Possibly I misunderstood you, but I don't think we want to copy-up on > >>>> permission denial, as that would still allow the mounter to read/write > >>>> special files or execute regular files to which it would normally be > >>>> denied access, because the copy would inherit the context specified by > >>>> the mounter in the context mount case. It still represents an > >>>> escalation of privilege for the mounter. In contrast, the copy-up on > >>>> write behavior does not allow the mounter to do anything it could not do > >>>> already (i.e. read from the lower, write to the upper). > >>> Let's get this straight: when file is copied up, it inherits label > >>> from context=, not from label of lower file? > >> > >> Yes, in the case of context mount, it will get the context mount directory. > >> > >> In the case of not context mount, it should maintain the label of the > >> lower. > >> > >>> Next question: permission to change metadata is tied to permission to > >>> open? Is it possible that open is denied, but metadata can be > >>> changed? > >> > >> Yes, SElinux handles open differently then setattr. Although I am not > >> sure if any tools handle this. > >> > >>> DAC model allows this: metadata change is tied to ownership, not mode > >>> bits. And different capability flag. > >>> > >>> If the same is true for MAC, then the pre-v4.20-rc1 is already > >>> susceptible to the privilege escalation you describe, right? > >> > >> After talking to Vivek, I am not sure their is a privilege escallation. > > > > More on this below, but this thread doesn't have me convinced, and we > > are at -rc5 right now. We need to come to some decision on this soon > > because we are running out of time before v4.20 is released with this > > code. > > > >> For device nodes, the mounter has to have the ability to create the > >> devicenode with the context mount, if he can do this, then he can do it > >> with or without Overlay. This might lead to users making mistakes on > >> security, but the model is sound. And I think this stands even in the > >> case of the lower is mounted NODEV and the upper is not. If the mounter > >> can create a device on the upper with a particular label, then he does > >> not need the lower. > > > > The problem I have when looking at the current code is that permission > > is given, regardless of what is requested, for any special_file() on > > an overlayfs mount. > > > > It also looks like the mounter's creds are used when checking > > permissions regardless of the file has been copied up or not; I would > > expect that the mounter's permissions would only used when checking > > permissions against the lower inode, no? > > No, that's never been the model as far as I know. mounter's permissions > are checked to the underlying inode, whether upper or lower. client's > permissions are only checked to the overlay inode. upper and lower are > logically backing store - upper for writes and lower for reads from > unmodified files. Now, in theory, upper should always be labeled the > same as overlay, so client check against overlay should already imply > client access to upper, unless someone has manually relabeled upper > outside of the overlay. Okay, thanks for the clarification on the model. This seems a little odd at first, but I'm trying to convince myself that it makes sense. -- paul moore www.paul-moore.com
