[cifs-utils PATCH] cifs.upcall: trim even more capabilities

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[cifs-utils PATCH] cifs.upcall: trim even more capabilities

Jeff Layton-4
We really only need CAP_DAC_READ_SEARCH, not CAP_DAC_OVERRIDE, and
only when we are going to probe the environ file.

Also, fix the non-libcap-ng trim_capabilities prototype.

Signed-off-by: Jeff Layton <[hidden email]>
---
 cifs.upcall.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/cifs.upcall.c b/cifs.upcall.c
index 6d9c427b7032..dae58b919408 100644
--- a/cifs.upcall.c
+++ b/cifs.upcall.c
@@ -70,22 +70,21 @@ typedef enum _sectype {
 
 #ifdef HAVE_LIBCAP_NG
 static int
-trim_capabilities(bool need_ptrace)
+trim_capabilities(bool need_environ)
 {
  capng_clear(CAPNG_SELECT_BOTH);
 
- /*
- * Need PTRACE and DAC_OVERRIDE for environment scraping, SETGID to
- * change gid and grouplist, and SETUID to change uid.
- */
+ /* SETUID and SETGID to change uid, gid, and grouplist */
  if (capng_updatev(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE,
- CAP_SETUID, CAP_SETGID, CAP_DAC_OVERRIDE, -1)) {
+ CAP_SETUID, CAP_SETGID, -1)) {
  syslog(LOG_ERR, "%s: Unable to update capability set: %m\n", __func__);
  return 1;
  }
 
- if (need_ptrace &&
-    capng_update(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE, CAP_SYS_PTRACE)) {
+ /* Need PTRACE and DAC_OVERRIDE for environment scraping */
+ if (need_environ &&
+    capng_updatev(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE,
+ CAP_SYS_PTRACE, CAP_DAC_READ_SEARCH, -1)) {
  syslog(LOG_ERR, "%s: Unable to update capability set: %m\n", __func__);
  return 1;
  }
@@ -109,7 +108,7 @@ drop_all_capabilities(void)
 }
 #else /* HAVE_LIBCAP_NG */
 static int
-trim_capabilities(void)
+trim_capabilities(bool unused)
 {
  return 0;
 }
--
2.9.3


Reply | Threaded
Open this post in threaded view
|

Re: [cifs-utils PATCH] cifs.upcall: trim even more capabilities

Simo Sorce-3
Reviewed-by: Simo Sorce <[hidden email]>

On Thu, 2017-02-16 at 09:59 -0500, Jeff Layton wrote:

> We really only need CAP_DAC_READ_SEARCH, not CAP_DAC_OVERRIDE, and
> only when we are going to probe the environ file.
>
> Also, fix the non-libcap-ng trim_capabilities prototype.
>
> Signed-off-by: Jeff Layton <[hidden email]>
> ---
>  cifs.upcall.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/cifs.upcall.c b/cifs.upcall.c
> index 6d9c427b7032..dae58b919408 100644
> --- a/cifs.upcall.c
> +++ b/cifs.upcall.c
> @@ -70,22 +70,21 @@ typedef enum _sectype {
>  
>  #ifdef HAVE_LIBCAP_NG
>  static int
> -trim_capabilities(bool need_ptrace)
> +trim_capabilities(bool need_environ)
>  {
>   capng_clear(CAPNG_SELECT_BOTH);
>  
> - /*
> - * Need PTRACE and DAC_OVERRIDE for environment scraping, SETGID to
> - * change gid and grouplist, and SETUID to change uid.
> - */
> + /* SETUID and SETGID to change uid, gid, and grouplist */
>   if (capng_updatev(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE,
> - CAP_SETUID, CAP_SETGID, CAP_DAC_OVERRIDE, -1)) {
> + CAP_SETUID, CAP_SETGID, -1)) {
>   syslog(LOG_ERR, "%s: Unable to update capability set: %m\n", __func__);
>   return 1;
>   }
>  
> - if (need_ptrace &&
> -    capng_update(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE, CAP_SYS_PTRACE)) {
> + /* Need PTRACE and DAC_OVERRIDE for environment scraping */
> + if (need_environ &&
> +    capng_updatev(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE,
> + CAP_SYS_PTRACE, CAP_DAC_READ_SEARCH, -1)) {
>   syslog(LOG_ERR, "%s: Unable to update capability set: %m\n", __func__);
>   return 1;
>   }
> @@ -109,7 +108,7 @@ drop_all_capabilities(void)
>  }
>  #else /* HAVE_LIBCAP_NG */
>  static int
> -trim_capabilities(void)
> +trim_capabilities(bool unused)
>  {
>   return 0;
>  }


--
Simo Sorce * Red Hat, Inc * New York


Reply | Threaded
Open this post in threaded view
|

Re: [cifs-utils PATCH] cifs.upcall: trim even more capabilities

Pavel Shilovsky-2
In reply to this post by Jeff Layton-4
2017-02-16 6:59 GMT-08:00 Jeff Layton <[hidden email]>:

> We really only need CAP_DAC_READ_SEARCH, not CAP_DAC_OVERRIDE, and
> only when we are going to probe the environ file.
>
> Also, fix the non-libcap-ng trim_capabilities prototype.
>
> Signed-off-by: Jeff Layton <[hidden email]>
> ---
>  cifs.upcall.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/cifs.upcall.c b/cifs.upcall.c
> index 6d9c427b7032..dae58b919408 100644
> --- a/cifs.upcall.c
> +++ b/cifs.upcall.c
> @@ -70,22 +70,21 @@ typedef enum _sectype {
>
>  #ifdef HAVE_LIBCAP_NG
>  static int
> -trim_capabilities(bool need_ptrace)
> +trim_capabilities(bool need_environ)
>  {
>         capng_clear(CAPNG_SELECT_BOTH);
>
> -       /*
> -        * Need PTRACE and DAC_OVERRIDE for environment scraping, SETGID to
> -        * change gid and grouplist, and SETUID to change uid.
> -        */
> +       /* SETUID and SETGID to change uid, gid, and grouplist */
>         if (capng_updatev(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE,
> -                       CAP_SETUID, CAP_SETGID, CAP_DAC_OVERRIDE, -1)) {
> +                       CAP_SETUID, CAP_SETGID, -1)) {
>                 syslog(LOG_ERR, "%s: Unable to update capability set: %m\n", __func__);
>                 return 1;
>         }
>
> -       if (need_ptrace &&
> -           capng_update(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE, CAP_SYS_PTRACE)) {
> +        /* Need PTRACE and DAC_OVERRIDE for environment scraping */

It seems that the comment above doesn't reflect the proposed change.
Should it be DAC_READ_SEARCH instead?

> +       if (need_environ &&
> +           capng_updatev(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE,
> +                       CAP_SYS_PTRACE, CAP_DAC_READ_SEARCH, -1)) {
>                 syslog(LOG_ERR, "%s: Unable to update capability set: %m\n", __func__);
>                 return 1;
>         }
> @@ -109,7 +108,7 @@ drop_all_capabilities(void)
>  }
>  #else /* HAVE_LIBCAP_NG */
>  static int
> -trim_capabilities(void)
> +trim_capabilities(bool unused)
>  {
>         return 0;
>  }
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to [hidden email]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
Best regards,
Pavel Shilovsky

Reply | Threaded
Open this post in threaded view
|

Re: [cifs-utils PATCH] cifs.upcall: trim even more capabilities

Jeff Layton-4
On Thu, 2017-02-16 at 09:28 -0800, Pavel Shilovsky wrote:

> 2017-02-16 6:59 GMT-08:00 Jeff Layton <[hidden email]>:
> > We really only need CAP_DAC_READ_SEARCH, not CAP_DAC_OVERRIDE, and
> > only when we are going to probe the environ file.
> >
> > Also, fix the non-libcap-ng trim_capabilities prototype.
> >
> > Signed-off-by: Jeff Layton <[hidden email]>
> > ---
> >  cifs.upcall.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/cifs.upcall.c b/cifs.upcall.c
> > index 6d9c427b7032..dae58b919408 100644
> > --- a/cifs.upcall.c
> > +++ b/cifs.upcall.c
> > @@ -70,22 +70,21 @@ typedef enum _sectype {
> >
> >  #ifdef HAVE_LIBCAP_NG
> >  static int
> > -trim_capabilities(bool need_ptrace)
> > +trim_capabilities(bool need_environ)
> >  {
> >         capng_clear(CAPNG_SELECT_BOTH);
> >
> > -       /*
> > -        * Need PTRACE and DAC_OVERRIDE for environment scraping, SETGID to
> > -        * change gid and grouplist, and SETUID to change uid.
> > -        */
> > +       /* SETUID and SETGID to change uid, gid, and grouplist */
> >         if (capng_updatev(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE,
> > -                       CAP_SETUID, CAP_SETGID, CAP_DAC_OVERRIDE, -1)) {
> > +                       CAP_SETUID, CAP_SETGID, -1)) {
> >                 syslog(LOG_ERR, "%s: Unable to update capability set: %m\n", __func__);
> >                 return 1;
> >         }
> >
> > -       if (need_ptrace &&
> > -           capng_update(CAPNG_ADD, CAPNG_PERMITTED|CAPNG_EFFECTIVE, CAP_SYS_PTRACE)) {
> > +        /* Need PTRACE and DAC_OVERRIDE for environment scraping */
>
> It seems that the comment above doesn't reflect the proposed change.
> Should it be DAC_READ_SEARCH instead?
>

Yes! It should and it's fixed in the version in the tree.

Thanks,
--
Jeff Layton <[hidden email]>