| Message ID | 20100402160409.705798cf@corrin.poochiereds.net |
|---|---|
| State | New |
| Headers | show |
Yes, it's working again like it did. The previouis version I used was cifs-utils 4.0. Now the cred file is /home/sbon/.smb/moun.cred again, and no problem. It was compiling using the libcap package, automatically. Thanks, Stef 2010/4/2 Jeff Layton <jlayton@samba.org>: > On Fri, 2 Apr 2010 15:12:12 -0400 > Jeff Layton <jlayton@samba.org> wrote: > >> On Fri, 2 Apr 2010 17:11:20 +0200 >> Stef Bon <stefbon@gmail.com> wrote: >> >> > >> > IT's not such a big problem, I've got it working again, but it should >> > be documented. >> > >> > Stef >> >> What was the last version on which this worked? Are you mount.cifs as a >> setuid root program? Is mount.cifs linked against libcap? >> > > Does the attached patch fix the problem? > > -- > Jeff Layton <jlayton@samba.org> >
On Sat, 3 Apr 2010 12:26:38 +0200 Stef Bon <stefbon@gmail.com> wrote: > Yes, > > it's working again like it did. The previouis version I used was cifs-utils 4.0. > > Now the cred file is /home/sbon/.smb/moun.cred again, and no problem. > > It was compiling using the libcap package, automatically. > > Thanks, > > Stef > > > 2010/4/2 Jeff Layton <jlayton@samba.org>: > > On Fri, 2 Apr 2010 15:12:12 -0400 > > Jeff Layton <jlayton@samba.org> wrote: > > > >> On Fri, 2 Apr 2010 17:11:20 +0200 > >> Stef Bon <stefbon@gmail.com> wrote: > >> > > >> > > >> > IT's not such a big problem, I've got it working again, but it should > >> > be documented. > >> > > >> > Stef > >> > >> What was the last version on which this worked? Are you mount.cifs as a > >> setuid root program? Is mount.cifs linked against libcap? > >> > > > > Does the attached patch fix the problem? > > > > -- > > Jeff Layton <jlayton@samba.org> > > > Ok, I've gone ahead and pushed some patches into the git repo that should fix this and still keep CAP_DAC_OVERRIDE exposure to a minimum. Please test it if you're able and let me know whether the problem is still fixed. I'm starting to wonder whether it would be better though to switch this code to use libcap-ng: http://people.redhat.com/sgrubb/libcap-ng/ ...the older libcap is pretty cumbersome.
Yes, I will do that. First I would like to know what this libcap(-ng) is for. I've read the website, but can you give some explanation? The website is mentioning the security and the dropping of privileges. What does this mean in respect to the cifs utils? You're dropping privileges or you don't, that's (not the question) a decision an app makes. Is an extra library required to do so? Stef Bon 2010/4/3 Jeff Layton <jlayton@samba.org>: > On Sat, 3 Apr 2010 12:26:38 +0200 > Stef Bon <stefbon@gmail.com> wrote: > >> Yes, >> >> it's working again like it did. The previouis version I used was cifs-utils 4.0. >> >> Now the cred file is /home/sbon/.smb/moun.cred again, and no problem. >> >> It was compiling using the libcap package, automatically. >> >> Thanks, >> >> Stef >> >> >> 2010/4/2 Jeff Layton <jlayton@samba.org>: >> > On Fri, 2 Apr 2010 15:12:12 -0400 >> > Jeff Layton <jlayton@samba.org> wrote: >> > >> >> On Fri, 2 Apr 2010 17:11:20 +0200 >> >> Stef Bon <stefbon@gmail.com> wrote: >> >> >> >> >> > >> >> > IT's not such a big problem, I've got it working again, but it should >> >> > be documented. >> >> > >> >> > Stef >> >> >> >> What was the last version on which this worked? Are you mount.cifs as a >> >> setuid root program? Is mount.cifs linked against libcap? >> >> >> > >> > Does the attached patch fix the problem? >> > >> > -- >> > Jeff Layton <jlayton@samba.org> >> > >> > > Ok, I've gone ahead and pushed some patches into the git repo that > should fix this and still keep CAP_DAC_OVERRIDE exposure to a minimum. > Please test it if you're able and let me know whether the problem is > still fixed. > > I'm starting to wonder whether it would be better though to switch this > code to use libcap-ng: > > http://people.redhat.com/sgrubb/libcap-ng/ > > ...the older libcap is pretty cumbersome. > > -- > Jeff Layton <jlayton@samba.org> >
On Sat, 3 Apr 2010 15:56:40 +0200 Stef Bon <stefbon@gmail.com> wrote: > Yes, I will do that. > > First I would like to know what this libcap(-ng) is for. > I've read the website, but can you give some explanation? > > The website is mentioning the security and the dropping of privileges. > What does this > mean in respect to the cifs utils? You're dropping privileges or you > don't, that's (not the question) > a decision an app makes. Is an extra library required to do so? > > One way to drop privileges is to setuid() to a non-privileged user. Another is to just explicitly turn off capabilities that you know the process doesn't need. This makes running a process as root less of an "all or nothing" thing. See the capabilities(7) manpage for more info on them. When mount.cifs is run by root, we can't really take the first approach -- that leaves it potentially unable to do things like open cred files and it's unclear to what user you could setuid anyway. libcap and libcap-ng are libraries that make it easier to manage capability sets, but libcap-ng appears to be much simpler to use. The downside is that libcap-ng is fairly recent and a lot of older distros don't have it.
Thanks for the explenation. I've got the recent dev. sources with git, and see the differences in the mount.cifs.c file. (line 325: #ifdef HAVE_LIBCAP) MY first analyse was wrong, that the function access gave an error, but what has changed? Was the implementation of libcap not right, and thus dropping privileges in a wrong manner? But how is it dropping privileges if it is run as root? To what account it's changing then? Stef 2010/4/3 Jeff Layton <jlayton@samba.org>: > On Sat, 3 Apr 2010 15:56:40 +0200 > Stef Bon <stefbon@gmail.com> wrote: > >> Yes, I will do that. >> >> First I would like to know what this libcap(-ng) is for. >> I've read the website, but can you give some explanation? >> >> The website is mentioning the security and the dropping of privileges. >> What does this >> mean in respect to the cifs utils? You're dropping privileges or you >> don't, that's (not the question) >> a decision an app makes. Is an extra library required to do so? >> >> > > One way to drop privileges is to setuid() to a non-privileged user. > Another is to just explicitly turn off capabilities that you know the > process doesn't need. This makes running a process as root less of > an "all or nothing" thing. See the capabilities(7) manpage for more > info on them. > > When mount.cifs is run by root, we can't really take the first approach > -- that leaves it potentially unable to do things like open cred files > and it's unclear to what user you could setuid anyway. > > libcap and libcap-ng are libraries that make it easier to manage > capability sets, but libcap-ng appears to be much simpler to use. The > downside is that libcap-ng is fairly recent and a lot of older distros > don't have it. > > -- > Jeff Layton <jlayton@samba.org> >
On Sat, 3 Apr 2010 22:42:39 +0200 Stef Bon <stefbon@gmail.com> wrote: > Thanks for the explenation. > > I've got the recent dev. sources with git, and see the differences in > the mount.cifs.c file. > (line 325: #ifdef HAVE_LIBCAP) > > MY first analyse was wrong, that the function access gave an error, > but what has changed? The child mount.cifs process no longer had CAP_DAC_OVERRIDE. > Was the implementation of libcap not right, and thus dropping > privileges in a wrong manner? It was dropping CAP_DAC_OVERRIDE which is needed for root to be able to open files to which it doesn't have explicit permission. > But how is it dropping privileges if it is run as root? To what > account it's changing then? > It's not changing uid, it's explicitly dropping capabilities using libcap.
2010/4/4 Jeff Layton <jlayton@samba.org>: > On Sat, 3 Apr 2010 22:42:39 +0200 > Stef Bon <stefbon@gmail.com> wrote: > >> Thanks for the explenation. >> >> I've got the recent dev. sources with git, and see the differences in >> the mount.cifs.c file. >> (line 325: #ifdef HAVE_LIBCAP) >> >> MY first analyse was wrong, that the function access gave an error, >> but what has changed? > > The child mount.cifs process no longer had CAP_DAC_OVERRIDE. > >> Was the implementation of libcap not right, and thus dropping >> privileges in a wrong manner? > > It was dropping CAP_DAC_OVERRIDE which is needed for root to be able to > open files to which it doesn't have explicit permission. > OK, but then the system call fopen (and maybe access?) looks at this value CAP_DAC_OVERRIDE, but to be frankly, I've never heard of this before. (and I'm developing fs with FUSE..) Can you please explain how these system calls look at the cap values/settings? When your app is not using the libcap, the cap values are not set. It still works. Stef >> But how is it dropping privileges if it is run as root? To what >> account it's changing then? >> > > It's not changing uid, it's explicitly dropping capabilities using > libcap. Yes sorry, you've already explained this. Stupid question. Stef
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Sun, 4 Apr 2010 18:40:10 +0200 Stef Bon <stefbon@gmail.com> wrote: > 2010/4/4 Jeff Layton <jlayton@samba.org>: > > On Sat, 3 Apr 2010 22:42:39 +0200 > > Stef Bon <stefbon@gmail.com> wrote: > > > >> Thanks for the explenation. > >> > >> I've got the recent dev. sources with git, and see the differences in > >> the mount.cifs.c file. > >> (line 325: #ifdef HAVE_LIBCAP) > >> > >> MY first analyse was wrong, that the function access gave an error, > >> but what has changed? > > > > The child mount.cifs process no longer had CAP_DAC_OVERRIDE. > > > >> Was the implementation of libcap not right, and thus dropping > >> privileges in a wrong manner? > > > > It was dropping CAP_DAC_OVERRIDE which is needed for root to be able to > > open files to which it doesn't have explicit permission. > > > > OK, but then the system call fopen (and maybe access?) looks at this > value CAP_DAC_OVERRIDE, > but to be frankly, I've never heard of this before. (and I'm > developing fs with FUSE..) > > Can you please explain how these system calls look at the cap values/settings? > Not any better than the capabilities(7) manpage can. ...actually though in reading the manpage, I probably don't need CAP_DAC_OVERRIDE even. CAP_DAC_READ_SEARCH would probably be fine for this...hmm. > When your app is not using the libcap, the cap values are not set. It > still works. > Yes. That's because it doesn't drop any capabilities. It just runs with more privileges than are necessary to perform the mount. - -- Jeff Layton <jlayton@samba.org> -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (GNU/Linux) iEYEARECAAYFAku4/PMACgkQyP0gxQMdzIA1VQCdHONVWvNHqZ5eERwGYS0Y7FdW P9cAn3Yil1XRKv8AufMeu+otKYnY7yZF =O8N7 -----END PGP SIGNATURE-----
Thanks a lot! Stef 2010/4/4 Jeff Layton <jlayton@samba.org>: >> Can you please explain how these system calls look at the cap values/settings? >> > > Not any better than the capabilities(7) manpage can. > > ...actually though in reading the manpage, I probably don't need > CAP_DAC_OVERRIDE even. CAP_DAC_READ_SEARCH would probably be fine for > this...hmm. > >> When your app is not using the libcap, the cap values are not set. It >> still works. >> > > Yes. That's because it doesn't drop any capabilities. It just runs with > more privileges than are necessary to perform the mount. > > - -- > Jeff Layton <jlayton@samba.org> > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2.0.14 (GNU/Linux) > > iEYEARECAAYFAku4/PMACgkQyP0gxQMdzIA1VQCdHONVWvNHqZ5eERwGYS0Y7FdW > P9cAn3Yil1XRKv8AufMeu+otKYnY7yZF > =O8N7 > -----END PGP SIGNATURE----- >
From d652b86adc7e9c62ba71b315e91fdd24af0063d8 Mon Sep 17 00:00:00 2001 From: Jeff Layton <jlayton@samba.org> Date: Fri, 2 Apr 2010 16:02:37 -0400 Subject: [PATCH] mount.cifs: if real uid is 0, child must keep CAP_DAC_OVERRIDE ...otherwise, root may not be able to read credential files. The ideal thing would be to remove it from the effective set, and only turn it on when needed, but for now this should fix the immediate problem. Signed-off-by: Jeff Layton <jlayton@samba.org> --- mount.cifs.c | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-) diff --git a/mount.cifs.c b/mount.cifs.c index ab155e3..7d1fa83 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -1150,7 +1150,7 @@ add_mtab_exit: static int drop_capabilities(int parent) { - int rc = 0; + int rc = 0, ncap; cap_t caps; cap_value_t cap_list[2]; @@ -1168,17 +1168,20 @@ drop_capabilities(int parent) goto free_caps; } - /* parent needs to keep some capabilities */ - if (parent) { - cap_list[0] = CAP_SYS_ADMIN; - cap_list[1] = CAP_DAC_OVERRIDE; - if (cap_set_flag(caps, CAP_PERMITTED, 2, cap_list, CAP_SET) == -1) { + if (parent || getuid() == 0) { + ncap = 1; + cap_list[0] = CAP_DAC_OVERRIDE; + if (parent) { + cap_list[1] = CAP_SYS_ADMIN; + ++ncap; + } + if (cap_set_flag(caps, CAP_PERMITTED, ncap, cap_list, CAP_SET) == -1) { fprintf(stderr, "Unable to set permitted capabilities: %s\n", strerror(errno)); rc = EX_SYSERR; goto free_caps; } - if (cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_list, CAP_SET) == -1) { + if (cap_set_flag(caps, CAP_EFFECTIVE, ncap, cap_list, CAP_SET) == -1) { fprintf(stderr, "Unable to set effective capabilities: %s\n", strerror(errno)); rc = EX_SYSERR; -- 1.6.6.1