On Mon, 2017-02-13 at 07:52 -0500, Jeff Layton wrote: > On Mon, 2017-02-13 at 05:02 -0500, Simo Sorce wrote: > > On Sat, 2017-02-11 at 10:16 -0500, Jeff Layton wrote: > > > On Sat, 2017-02-11 at 08:41 -0500, Jeff Layton wrote: > > > > Chad reported that he was seeing a regression in cifs-utils- > > > > 6.6. > > > > Prior > > > > to that, cifs.upcall was able to find credcaches in non-default > > > > FILE: > > > > locations, but with the rework of that code, that ability was > > > > lost. > > > > > > > > Unfortunately, the krb5 library design doesn't really take into > > > > account > > > > the fact that we might need to find a credcache in a process > > > > that > > > > isn't > > > > descended from the session. > > > > > > > > When the kernel does an upcall, it passes several bits of info > > > > about the > > > > task that initiated the upcall. One of those things is the PID > > > > (the > > > > tgid, in particular). We can use that info to reach into the > > > > /proc/<pid>/environ file for the process, and grab whatever > > > > value > > > > of > > > > KRB5CCNAME is there. This patch adds this ability to > > > > cifs.upcall. > > > > > > > > I'm not 100% convinced that this is a good idea however, so for > > > > now, > > > > this is disabled unless the command line has a '-e' switch. > > > > Anyone > > > > wishing to play with this should edit their /etc/request- > > > > key.conf > > > > files > > > > accordingly. > > > > > > > > > > Meant to put a Reported-by: here for Chad. I'll do that before > > > merging. > > > > > > > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxx> > > > > > > > > --- > > > > cifs.upcall.8.in | 10 ++++ > > > > cifs.upcall.c | 148 > > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++--- > > > > 2 files changed, 152 insertions(+), 6 deletions(-) > > > > > > > > > > FWIW, this patch works as expected in cursory, light testing. > > > This > > > should be superior to the earlier method where we scanned in /tmp > > > as > > > well -- more efficient, and also able to find non-FILE: > > > credcaches, > > > as > > > well as ones that lie in other places besides /tmp. > > > > > > My main concern is that we have to do the environ file scraping > > > while > > > privileged, and that we end up trusting some info from a userland > > > process that triggered the upcall in KRB5CCNAME. Does this open > > > any > > > potential attack vectors that I'm not considering? > > > > Well you are operating as the user however I have not seen a setgid > > operation, does this mean cifs.upcall is running with group id 0 ? > > > > This may give access to files the original user should not be able > > to > > access or cause files to be created with improper permission. > > > > Yeah, ick. The kernel doesn't send down the gid, so we can't just add > that in directly... > > We could do a getpwuid(uid), and then setgid(pw_gid). That might be > problematic for applications that have done a setgid(), but those > ought to be very rare. For the CIFS/NFS use case I think this is good enough, if you did setgid and the credential file can only be accessed with that gid... well tough luck, I mean it must be *exceedingly* rare. For tht matter we do not have either all the secondary gids, and the ccache may be (only) readable by one of them too ... > > Another thing people may need to pay attention to is SELinux > > context, I > > am not sure what selinux context cifs.upcall runs into the > > difference > > may matter in some cases, but in those I think proper SELinux > > policy > > should be devised, I see no need to change how cifs.upcall > > operates. > > > > Yeah, that's a different matter. It would be nice to confine > cifs.upcall, but we'd need an SELinux policy for that. Honestly, I'd > rather see us move to gssproxy before spending time on that. Yes, I think the above approach is good enough. Simo. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
