[RFC,1/1] capability: Introduce capability API
diff mbox series

Message ID 20190808153825.18363-2-rpalethorpe@suse.com
State New
Headers show
Series
  • tst API for dropping or requiring capabilities
Related show

Commit Message

Richard Palethorpe Aug. 8, 2019, 3:38 p.m. UTC
---
 include/tst_capability.h | 56 +++++++++++++++++++++++++++++
 include/tst_test.h       |  6 ++++
 lib/tst_capability.c     | 78 ++++++++++++++++++++++++++++++++++++++++
 lib/tst_test.c           |  3 ++
 4 files changed, 143 insertions(+)
 create mode 100644 include/tst_capability.h
 create mode 100644 lib/tst_capability.c

Comments

Cyril Hrubis Aug. 9, 2019, 12:27 p.m. UTC | #1
Hi!
>  include/tst_capability.h | 56 +++++++++++++++++++++++++++++
>  include/tst_test.h       |  6 ++++
>  lib/tst_capability.c     | 78 ++++++++++++++++++++++++++++++++++++++++
>  lib/tst_test.c           |  3 ++
>  4 files changed, 143 insertions(+)
>  create mode 100644 include/tst_capability.h
>  create mode 100644 lib/tst_capability.c

This is missing a documentation in the test-writing-guidelines I do
expect that you will add that later on.

> diff --git a/include/tst_capability.h b/include/tst_capability.h
> new file mode 100644
> index 000000000..6342b667e
> --- /dev/null
> +++ b/include/tst_capability.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
> + */
> +/**
> + * @file tst_capability.h
> + *
> + * Limited capability operations without libcap.
> + */
> +
> +#include <stdint.h>
> +
> +#include "lapi/syscalls.h"
               ^
	       What is this needed for here?

> +#ifndef TST_CAPABILITY_H
> +#define TST_CAPABILITY_H
> +
> +#ifndef CAP_SYS_ADMIN
> +# define CAP_SYS_ADMIN        21
> +#endif
>
> +#ifndef CAP_TO_MASK
> +# define CAP_TO_MASK(x)      (1 << ((x) & 31))
> +#endif

These bits should probably go to lapi/capability.h

> +#define TST_DROP 1
> +#define TST_REQUIRE 1 << 1

Maybe these should be TST_CAP_DROP and TST_CAP_REQ

> +#define TST_CAP(action, capability) {action, capability, #capability}
> +
> +struct tst_cap_user_header {
> +	uint32_t version;
> +	int pid;
> +};
> +
> +struct tst_cap_user_data {
> +	uint32_t effective;
> +	uint32_t permitted;
> +	uint32_t inheritable;
> +};

This two should probably go to lapi as well.

> +struct tst_cap {
> +	uint32_t action;
> +	uint32_t id;
> +	char *name;
> +};

I wonder if we should build a table of capability names for translation
just as we do errnos and signals instead of storing the name here. But I
guess that unless we will need a function to translate capabilitities
into strings on runtime this approach is simpler.

> +int tst_capget(struct tst_cap_user_header *hdr,
> +	       struct tst_cap_user_data *data);
> +int tst_capset(struct tst_cap_user_header *hdr,
> +	       const struct tst_cap_user_data *data);
> +
> +void tst_cap_action(struct tst_cap *cap);
> +void tst_cap_setup(struct tst_cap *cap);
> +
> +#endif
> diff --git a/include/tst_test.h b/include/tst_test.h
> index cdeaf6ad0..84acf2c59 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -36,6 +36,7 @@
>  #include "tst_sys_conf.h"
>  #include "tst_coredump.h"
>  #include "tst_buffers.h"
> +#include "tst_capability.h"
>  
>  /*
>   * Reports testcase result.
> @@ -206,6 +207,11 @@ struct tst_test {
>  	 * NULL-terminated array to be allocated buffers.
>  	 */
>  	struct tst_buffers *bufs;
> +
> +	/*
> +	 * NULL-terminated array of capability settings
> +	 */
> +	struct tst_cap *caps;
>  };
>  
>  /*
> diff --git a/lib/tst_capability.c b/lib/tst_capability.c
> new file mode 100644
> index 000000000..d229491ae
> --- /dev/null
> +++ b/lib/tst_capability.c
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
> + */
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_capability.h"
> +
> +int tst_capget(struct tst_cap_user_header *hdr,
> +	       struct tst_cap_user_data *data)
> +{
> +	return tst_syscall(__NR_capget, hdr, data);
> +}
> +
> +int tst_capset(struct tst_cap_user_header *hdr,
> +	       const struct tst_cap_user_data *data)
> +{
> +	return tst_syscall(__NR_capset, hdr, data);
> +}
> +
> +void tst_cap_action(struct tst_cap *cap)
> +{
> +	struct tst_cap_user_header hdr = {
> +		.version = 0x20080522,
> +		.pid = tst_syscall(__NR_gettid),
> +	};
> +	struct tst_cap_user_data cur = { 0 };
> +	struct tst_cap_user_data new = { 0 };
> +	uint32_t mask = CAP_TO_MASK(cap->id);
> +	uint32_t act = cap->action;
> +
> +	if (tst_capget(&hdr, &cur))
> +		tst_brk(TBROK | TTERRNO, "tst_capget()");
> +
> +	new = cur;
> +
> +	switch (act) {
> +	case TST_DROP:
> +		if (cur.effective & mask) {
> +			tst_res(TINFO, "Dropping %s(%d)",
> +				cap->name, cap->id);
> +			new.effective &= ~mask;
> +			new.permitted &= ~mask;
> +			new.inheritable &= ~mask;
> +		}
> +		break;
> +	case TST_REQUIRE:
> +		if (cur.permitted ^ mask) {
> +			tst_brk(TCONF, "Need %s(%d)",
> +				cap->name, cap->id);
> +		} else if (cur.effective ^ mask) {
> +			tst_res(TINFO, "Permitting %s(%d)",
> +				cap->name, cap->id);
> +			new.effective |= mask;
> +			new.inheritable |= mask;
> +		}
> +		break;
> +	default:
> +		tst_brk(TBROK, "Unrecognised action %d", cap->action);
> +	}
> +
> +	if (cur.effective != new.effective) {
> +		if (tst_capset(&hdr, &new))
> +			tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name);
> +	} else {
> +		tst_res(TINFO, "No capability changes needed");
> +	}
> +}
> +
> +void tst_cap_setup(struct tst_cap *caps)
> +{
> +	struct tst_cap *cap;
> +
> +	for (cap = caps; cap->action; cap++) {
> +		tst_cap_action(cap);
> +	}

No need for the curly braces here :-).

> +}
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 8dc71dbb3..62e54d071 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -893,6 +893,9 @@ static void do_test_setup(void)
>  
>  	if (main_pid != getpid())
>  		tst_brk(TBROK, "Runaway child in setup()!");
> +
> +	if (tst_test->caps)
> +		tst_cap_setup(tst_test->caps);
>  }

Other than the minor things and missing docs this patch looks good to me.
Jan Stancek Aug. 9, 2019, 2:42 p.m. UTC | #2
----- Original Message -----
> Hi!
> >  include/tst_capability.h | 56 +++++++++++++++++++++++++++++
> >  include/tst_test.h       |  6 ++++
> >  lib/tst_capability.c     | 78 ++++++++++++++++++++++++++++++++++++++++
> >  lib/tst_test.c           |  3 ++
> >  4 files changed, 143 insertions(+)
> >  create mode 100644 include/tst_capability.h
> >  create mode 100644 lib/tst_capability.c
> 
> This is missing a documentation in the test-writing-guidelines I do
> expect that you will add that later on.
> 
> > diff --git a/include/tst_capability.h b/include/tst_capability.h
> > new file mode 100644
> > index 000000000..6342b667e
> > --- /dev/null
> > +++ b/include/tst_capability.h
> > @@ -0,0 +1,56 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
> > + */
> > +/**
> > + * @file tst_capability.h
> > + *
> > + * Limited capability operations without libcap.
> > + */
> > +
> > +#include <stdint.h>
> > +
> > +#include "lapi/syscalls.h"
>                ^
> 	       What is this needed for here?
> 
> > +#ifndef TST_CAPABILITY_H
> > +#define TST_CAPABILITY_H
> > +
> > +#ifndef CAP_SYS_ADMIN
> > +# define CAP_SYS_ADMIN        21
> > +#endif
> >
> > +#ifndef CAP_TO_MASK
> > +# define CAP_TO_MASK(x)      (1 << ((x) & 31))
> > +#endif
> 
> These bits should probably go to lapi/capability.h
> 
> > +#define TST_DROP 1
> > +#define TST_REQUIRE 1 << 1
> 
> Maybe these should be TST_CAP_DROP and TST_CAP_REQ
> 
> > +#define TST_CAP(action, capability) {action, capability, #capability}
> > +
> > +struct tst_cap_user_header {
> > +	uint32_t version;
> > +	int pid;
> > +};
> > +
> > +struct tst_cap_user_data {
> > +	uint32_t effective;
> > +	uint32_t permitted;
> > +	uint32_t inheritable;
> > +};
> 
> This two should probably go to lapi as well.
> 
> > +struct tst_cap {
> > +	uint32_t action;
> > +	uint32_t id;
> > +	char *name;
> > +};
> 
> I wonder if we should build a table of capability names for translation
> just as we do errnos and signals instead of storing the name here. But I
> guess that unless we will need a function to translate capabilitities
> into strings on runtime this approach is simpler.
> 
> > +int tst_capget(struct tst_cap_user_header *hdr,
> > +	       struct tst_cap_user_data *data);
> > +int tst_capset(struct tst_cap_user_header *hdr,
> > +	       const struct tst_cap_user_data *data);
> > +
> > +void tst_cap_action(struct tst_cap *cap);
> > +void tst_cap_setup(struct tst_cap *cap);
> > +
> > +#endif
> > diff --git a/include/tst_test.h b/include/tst_test.h
> > index cdeaf6ad0..84acf2c59 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -36,6 +36,7 @@
> >  #include "tst_sys_conf.h"
> >  #include "tst_coredump.h"
> >  #include "tst_buffers.h"
> > +#include "tst_capability.h"
> >  
> >  /*
> >   * Reports testcase result.
> > @@ -206,6 +207,11 @@ struct tst_test {
> >  	 * NULL-terminated array to be allocated buffers.
> >  	 */
> >  	struct tst_buffers *bufs;
> > +
> > +	/*
> > +	 * NULL-terminated array of capability settings
> > +	 */
> > +	struct tst_cap *caps;
> >  };
> >  
> >  /*
> > diff --git a/lib/tst_capability.c b/lib/tst_capability.c
> > new file mode 100644
> > index 000000000..d229491ae
> > --- /dev/null
> > +++ b/lib/tst_capability.c
> > @@ -0,0 +1,78 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
> > + */
> > +
> > +#define TST_NO_DEFAULT_MAIN
> > +#include "tst_test.h"
> > +#include "tst_capability.h"
> > +
> > +int tst_capget(struct tst_cap_user_header *hdr,
> > +	       struct tst_cap_user_data *data)
> > +{
> > +	return tst_syscall(__NR_capget, hdr, data);
> > +}
> > +
> > +int tst_capset(struct tst_cap_user_header *hdr,
> > +	       const struct tst_cap_user_data *data)
> > +{
> > +	return tst_syscall(__NR_capset, hdr, data);
> > +}
> > +
> > +void tst_cap_action(struct tst_cap *cap)
> > +{
> > +	struct tst_cap_user_header hdr = {
> > +		.version = 0x20080522,
> > +		.pid = tst_syscall(__NR_gettid),
> > +	};
> > +	struct tst_cap_user_data cur = { 0 };
> > +	struct tst_cap_user_data new = { 0 };
> > +	uint32_t mask = CAP_TO_MASK(cap->id);
> > +	uint32_t act = cap->action;
> > +
> > +	if (tst_capget(&hdr, &cur))
> > +		tst_brk(TBROK | TTERRNO, "tst_capget()");
> > +
> > +	new = cur;
> > +
> > +	switch (act) {
> > +	case TST_DROP:
> > +		if (cur.effective & mask) {
> > +			tst_res(TINFO, "Dropping %s(%d)",
> > +				cap->name, cap->id);
> > +			new.effective &= ~mask;
> > +			new.permitted &= ~mask;
> > +			new.inheritable &= ~mask;
> > +		}
> > +		break;
> > +	case TST_REQUIRE:
> > +		if (cur.permitted ^ mask) {
> > +			tst_brk(TCONF, "Need %s(%d)",
> > +				cap->name, cap->id);
> > +		} else if (cur.effective ^ mask) {
> > +			tst_res(TINFO, "Permitting %s(%d)",
> > +				cap->name, cap->id);
> > +			new.effective |= mask;
> > +			new.inheritable |= mask;
> > +		}
> > +		break;
> > +	default:
> > +		tst_brk(TBROK, "Unrecognised action %d", cap->action);
> > +	}
> > +
> > +	if (cur.effective != new.effective) {
> > +		if (tst_capset(&hdr, &new))
> > +			tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name);
> > +	} else {
> > +		tst_res(TINFO, "No capability changes needed");
> > +	}
> > +}
> > +
> > +void tst_cap_setup(struct tst_cap *caps)
> > +{
> > +	struct tst_cap *cap;
> > +
> > +	for (cap = caps; cap->action; cap++) {
> > +		tst_cap_action(cap);
> > +	}
> 
> No need for the curly braces here :-).
> 
> > +}
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 8dc71dbb3..62e54d071 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -893,6 +893,9 @@ static void do_test_setup(void)
> >  
> >  	if (main_pid != getpid())
> >  		tst_brk(TBROK, "Runaway child in setup()!");
> > +
> > +	if (tst_test->caps)
> > +		tst_cap_setup(tst_test->caps);
> >  }
> 
> Other than the minor things and missing docs this patch looks good to me.

+1, but please rebase it on latest master.
Li Wang Aug. 15, 2019, 7:10 a.m. UTC | #3
On Thu, Aug 8, 2019 at 11:39 PM Richard Palethorpe <rpalethorpe@suse.com> wrote:
>
> ---
>  include/tst_capability.h | 56 +++++++++++++++++++++++++++++
>  include/tst_test.h       |  6 ++++
>  lib/tst_capability.c     | 78 ++++++++++++++++++++++++++++++++++++++++
>  lib/tst_test.c           |  3 ++
>  4 files changed, 143 insertions(+)
>  create mode 100644 include/tst_capability.h
>  create mode 100644 lib/tst_capability.c
>
> diff --git a/include/tst_capability.h b/include/tst_capability.h
> new file mode 100644
> index 000000000..6342b667e
> --- /dev/null
> +++ b/include/tst_capability.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
> + */
> +/**
> + * @file tst_capability.h
> + *
> + * Limited capability operations without libcap.
> + */
> +
> +#include <stdint.h>
> +
> +#include "lapi/syscalls.h"
> +
> +#ifndef TST_CAPABILITY_H
> +#define TST_CAPABILITY_H
> +
> +#ifndef CAP_SYS_ADMIN
> +# define CAP_SYS_ADMIN        21
> +#endif
> +
> +#ifndef CAP_TO_MASK
> +# define CAP_TO_MASK(x)      (1 << ((x) & 31))
> +#endif
> +
> +#define TST_DROP 1
> +#define TST_REQUIRE 1 << 1
> +
> +#define TST_CAP(action, capability) {action, capability, #capability}
> +
> +struct tst_cap_user_header {
> +       uint32_t version;
> +       int pid;
> +};
> +
> +struct tst_cap_user_data {
> +       uint32_t effective;
> +       uint32_t permitted;
> +       uint32_t inheritable;
> +};
> +
> +struct tst_cap {
> +       uint32_t action;
> +       uint32_t id;
> +       char *name;
> +};
> +
> +int tst_capget(struct tst_cap_user_header *hdr,
> +              struct tst_cap_user_data *data);
> +int tst_capset(struct tst_cap_user_header *hdr,
> +              const struct tst_cap_user_data *data);
> +
> +void tst_cap_action(struct tst_cap *cap);
> +void tst_cap_setup(struct tst_cap *cap);
> +
> +#endif
> diff --git a/include/tst_test.h b/include/tst_test.h
> index cdeaf6ad0..84acf2c59 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -36,6 +36,7 @@
>  #include "tst_sys_conf.h"
>  #include "tst_coredump.h"
>  #include "tst_buffers.h"
> +#include "tst_capability.h"
>
>  /*
>   * Reports testcase result.
> @@ -206,6 +207,11 @@ struct tst_test {
>          * NULL-terminated array to be allocated buffers.
>          */
>         struct tst_buffers *bufs;
> +
> +       /*
> +        * NULL-terminated array of capability settings
> +        */
> +       struct tst_cap *caps;
>  };
>
>  /*
> diff --git a/lib/tst_capability.c b/lib/tst_capability.c
> new file mode 100644
> index 000000000..d229491ae
> --- /dev/null
> +++ b/lib/tst_capability.c
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
> + */
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_capability.h"
> +
> +int tst_capget(struct tst_cap_user_header *hdr,
> +              struct tst_cap_user_data *data)
> +{
> +       return tst_syscall(__NR_capget, hdr, data);
> +}
> +
> +int tst_capset(struct tst_cap_user_header *hdr,
> +              const struct tst_cap_user_data *data)
> +{
> +       return tst_syscall(__NR_capset, hdr, data);
> +}
> +
> +void tst_cap_action(struct tst_cap *cap)
> +{
> +       struct tst_cap_user_header hdr = {
> +               .version = 0x20080522,
> +               .pid = tst_syscall(__NR_gettid),
> +       };
> +       struct tst_cap_user_data cur = { 0 };
> +       struct tst_cap_user_data new = { 0 };
> +       uint32_t mask = CAP_TO_MASK(cap->id);
> +       uint32_t act = cap->action;
> +
> +       if (tst_capget(&hdr, &cur))
> +               tst_brk(TBROK | TTERRNO, "tst_capget()");
> +
> +       new = cur;
> +
> +       switch (act) {
> +       case TST_DROP:
> +               if (cur.effective & mask) {
> +                       tst_res(TINFO, "Dropping %s(%d)",
> +                               cap->name, cap->id);
> +                       new.effective &= ~mask;
> +                       new.permitted &= ~mask;
> +                       new.inheritable &= ~mask;
> +               }
> +               break;
> +       case TST_REQUIRE:
> +               if (cur.permitted ^ mask) {
> +                       tst_brk(TCONF, "Need %s(%d)",
> +                               cap->name, cap->id);
> +               } else if (cur.effective ^ mask) {
> +                       tst_res(TINFO, "Permitting %s(%d)",
> +                               cap->name, cap->id);
> +                       new.effective |= mask;
> +                       new.inheritable |= mask;
> +               }
> +               break;
> +       default:
> +               tst_brk(TBROK, "Unrecognised action %d", cap->action);
> +       }
> +
> +       if (cur.effective != new.effective) {
> +               if (tst_capset(&hdr, &new))
> +                       tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name);

It does not work for this simple cap_test.c, did I miss anything?

# whoami
root

# ./cap_test
tst_test.c:1111: INFO: Timeout per run is 0h 05m 00s
tst_capability.c:42: INFO: Dropping CAP_SYS_ADMIN(21)
tst_capability.c:65: BROK: tst_capset(CAP_SYS_ADMIN): EPERM

# ./cap_test
tst_test.c:1111: INFO: Timeout per run is 0h 05m 00s
tst_capability.c:51: CONF: Need CAP_SYS_ADMIN(21)

# cat cap_test.c
#include "tst_test.h"
#include "linux/capability.h"

static void do_test(void)
{
        tst_res(TPASS, "Hello");
}

static struct tst_test test = {
        .test_all = do_test,
        .needs_root = 1,
        .caps = (struct tst_cap []) {
//                TST_CAP(TST_DROP, CAP_SYS_ADMIN),
                TST_CAP(TST_REQUIRE, CAP_SYS_ADMIN),
                {},
        },
};
Richard Palethorpe Aug. 21, 2019, 11:43 a.m. UTC | #4
Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

>> +
>> +struct tst_cap_user_header {
>> +	uint32_t version;
>> +	int pid;
>> +};
>> +
>> +struct tst_cap_user_data {
>> +	uint32_t effective;
>> +	uint32_t permitted;
>> +	uint32_t inheritable;
>> +};
>
> This two should probably go to lapi as well.

Ah, but the naming of the original structures is so bad.

typedef struct __user_cap_header_struct {
        ...
} __user *cap_user_header_t.

Well not necessarily bad, but against current naming guidelines. I would
rather just use our own definitions and keep them separate from any
fallback definitions for test writers.

>
>> +struct tst_cap {
>> +	uint32_t action;
>> +	uint32_t id;
>> +	char *name;
>> +};
>
> I wonder if we should build a table of capability names for translation
> just as we do errnos and signals instead of storing the name here. But I
> guess that unless we will need a function to translate capabilitities
> into strings on runtime this approach is simpler.

I suppose if we want to print all the current capabilities then we need
this. Which I can imagine would be useful, but I would rather wait until
someone actually needs it.


--
Thank you,
Richard.
Richard Palethorpe Aug. 21, 2019, 11:56 a.m. UTC | #5
hi,

> It does not work for this simple cap_test.c, did I miss anything?
>
> # whoami
> root
>
> # ./cap_test
> tst_test.c:1111: INFO: Timeout per run is 0h 05m 00s
> tst_capability.c:42: INFO: Dropping CAP_SYS_ADMIN(21)
> tst_capability.c:65: BROK: tst_capset(CAP_SYS_ADMIN): EPERM

Not sure why this fails for you.

>
> # ./cap_test
> tst_test.c:1111: INFO: Timeout per run is 0h 05m 00s
> tst_capability.c:51: CONF: Need CAP_SYS_ADMIN(21)

Ah, I probably got the binary ops wrong.

>
> # cat cap_test.c
> #include "tst_test.h"
> #include "linux/capability.h"
>
> static void do_test(void)
> {
>         tst_res(TPASS, "Hello");
> }
>
> static struct tst_test test = {
>         .test_all = do_test,
>         .needs_root = 1,
>         .caps = (struct tst_cap []) {
> //                TST_CAP(TST_DROP, CAP_SYS_ADMIN),
>                 TST_CAP(TST_REQUIRE, CAP_SYS_ADMIN),
>                 {},
>         },
> };

I will add a test like this, thanks.
Yang Xu Aug. 22, 2019, 5:56 a.m. UTC | #6
于 2019/08/15 15:10, Li Wang 写道:
> On Thu, Aug 8, 2019 at 11:39 PM Richard Palethorpe<rpalethorpe@suse.com>  wrote:
>> ---
>>   include/tst_capability.h | 56 +++++++++++++++++++++++++++++
>>   include/tst_test.h       |  6 ++++
>>   lib/tst_capability.c     | 78 ++++++++++++++++++++++++++++++++++++++++
>>   lib/tst_test.c           |  3 ++
>>   4 files changed, 143 insertions(+)
>>   create mode 100644 include/tst_capability.h
>>   create mode 100644 lib/tst_capability.c
>>
>> diff --git a/include/tst_capability.h b/include/tst_capability.h
>> new file mode 100644
>> index 000000000..6342b667e
>> --- /dev/null
>> +++ b/include/tst_capability.h
>> @@ -0,0 +1,56 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2019 Richard Palethorpe<rpalethorpe@suse.com>
>> + */
>> +/**
>> + * @file tst_capability.h
>> + *
>> + * Limited capability operations without libcap.
>> + */
>> +
>> +#include<stdint.h>
>> +
>> +#include "lapi/syscalls.h"
>> +
>> +#ifndef TST_CAPABILITY_H
>> +#define TST_CAPABILITY_H
>> +
>> +#ifndef CAP_SYS_ADMIN
>> +# define CAP_SYS_ADMIN        21
>> +#endif
>> +
>> +#ifndef CAP_TO_MASK
>> +# define CAP_TO_MASK(x)      (1<<  ((x)&  31))
>> +#endif
>> +
>> +#define TST_DROP 1
>> +#define TST_REQUIRE 1<<  1
>> +
>> +#define TST_CAP(action, capability) {action, capability, #capability}
>> +
>> +struct tst_cap_user_header {
>> +       uint32_t version;
>> +       int pid;
>> +};
>> +
>> +struct tst_cap_user_data {
>> +       uint32_t effective;
>> +       uint32_t permitted;
>> +       uint32_t inheritable;
>> +};
>> +
>> +struct tst_cap {
>> +       uint32_t action;
>> +       uint32_t id;
>> +       char *name;
>> +};
>> +
>> +int tst_capget(struct tst_cap_user_header *hdr,
>> +              struct tst_cap_user_data *data);
>> +int tst_capset(struct tst_cap_user_header *hdr,
>> +              const struct tst_cap_user_data *data);
>> +
>> +void tst_cap_action(struct tst_cap *cap);
>> +void tst_cap_setup(struct tst_cap *cap);
>> +
>> +#endif
>> diff --git a/include/tst_test.h b/include/tst_test.h
>> index cdeaf6ad0..84acf2c59 100644
>> --- a/include/tst_test.h
>> +++ b/include/tst_test.h
>> @@ -36,6 +36,7 @@
>>   #include "tst_sys_conf.h"
>>   #include "tst_coredump.h"
>>   #include "tst_buffers.h"
>> +#include "tst_capability.h"
>>
>>   /*
>>    * Reports testcase result.
>> @@ -206,6 +207,11 @@ struct tst_test {
>>           * NULL-terminated array to be allocated buffers.
>>           */
>>          struct tst_buffers *bufs;
>> +
>> +       /*
>> +        * NULL-terminated array of capability settings
>> +        */
>> +       struct tst_cap *caps;
>>   };
>>
>>   /*
>> diff --git a/lib/tst_capability.c b/lib/tst_capability.c
>> new file mode 100644
>> index 000000000..d229491ae
>> --- /dev/null
>> +++ b/lib/tst_capability.c
>> @@ -0,0 +1,78 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2019 Richard Palethorpe<rpalethorpe@suse.com>
>> + */
>> +
>> +#define TST_NO_DEFAULT_MAIN
>> +#include "tst_test.h"
>> +#include "tst_capability.h"
>> +
>> +int tst_capget(struct tst_cap_user_header *hdr,
>> +              struct tst_cap_user_data *data)
>> +{
>> +       return tst_syscall(__NR_capget, hdr, data);
>> +}
>> +
>> +int tst_capset(struct tst_cap_user_header *hdr,
>> +              const struct tst_cap_user_data *data)
>> +{
>> +       return tst_syscall(__NR_capset, hdr, data);
>> +}
>> +
>> +void tst_cap_action(struct tst_cap *cap)
>> +{
>> +       struct tst_cap_user_header hdr = {
>> +               .version = 0x20080522,
>> +               .pid = tst_syscall(__NR_gettid),
>> +       };
>> +       struct tst_cap_user_data cur = { 0 };
>> +       struct tst_cap_user_data new = { 0 };
>> +       uint32_t mask = CAP_TO_MASK(cap->id);
>> +       uint32_t act = cap->action;
>> +
>> +       if (tst_capget(&hdr,&cur))
>> +               tst_brk(TBROK | TTERRNO, "tst_capget()");
>> +
>> +       new = cur;
>> +
>> +       switch (act) {
>> +       case TST_DROP:
>> +               if (cur.effective&  mask) {
>> +                       tst_res(TINFO, "Dropping %s(%d)",
>> +                               cap->name, cap->id);
>> +                       new.effective&= ~mask;
>> +                       new.permitted&= ~mask;
>> +                       new.inheritable&= ~mask;
>> +               }
>> +               break;
>> +       case TST_REQUIRE:
>> +               if (cur.permitted ^ mask) {
>> +                       tst_brk(TCONF, "Need %s(%d)",
>> +                               cap->name, cap->id);
>> +               } else if (cur.effective ^ mask) {
>> +                       tst_res(TINFO, "Permitting %s(%d)",
>> +                               cap->name, cap->id);
>> +                       new.effective |= mask;
>> +                       new.inheritable |= mask;
>> +               }
>> +               break;
>> +       default:
>> +               tst_brk(TBROK, "Unrecognised action %d", cap->action);
>> +       }
>> +
>> +       if (cur.effective != new.effective) {
>> +               if (tst_capset(&hdr,&new))
>> +                       tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name);
> It does not work for this simple cap_test.c, did I miss anything?
>
> # whoami
> root
>
> # ./cap_test
> tst_test.c:1111: INFO: Timeout per run is 0h 05m 00s
> tst_capability.c:42: INFO: Dropping CAP_SYS_ADMIN(21)
> tst_capability.c:65: BROK: tst_capset(CAP_SYS_ADMIN): EPERM
>
Hi Li
I have tried it and have the same failure. The _LINUX_CAPABILITY_VERSION_3 seem not support on my system causes fail.
If I use _LINUX_CAPABILITY_VERSION_1, cap_test will pass.  I am still looking into _LINUX_CAPABILITY_VERSION_3 dependence.

> # ./cap_test
> tst_test.c:1111: INFO: Timeout per run is 0h 05m 00s
> tst_capability.c:51: CONF: Need CAP_SYS_ADMIN(21)
>
> # cat cap_test.c
> #include "tst_test.h"
> #include "linux/capability.h"
>
> static void do_test(void)
> {
>          tst_res(TPASS, "Hello");
> }
>
> static struct tst_test test = {
>          .test_all = do_test,
>          .needs_root = 1,
>          .caps = (struct tst_cap []) {
> //                TST_CAP(TST_DROP, CAP_SYS_ADMIN),
>                  TST_CAP(TST_REQUIRE, CAP_SYS_ADMIN),
>                  {},
>          },
> };
>
Yang Xu Aug. 22, 2019, 8:41 a.m. UTC | #7
on 2019/08/08 23:38, Richard Palethorpe wrote:

> ---
>   include/tst_capability.h | 56 +++++++++++++++++++++++++++++
>   include/tst_test.h       |  6 ++++
>   lib/tst_capability.c     | 78 ++++++++++++++++++++++++++++++++++++++++
>   lib/tst_test.c           |  3 ++
>   4 files changed, 143 insertions(+)
>   create mode 100644 include/tst_capability.h
>   create mode 100644 lib/tst_capability.c
>
> diff --git a/include/tst_capability.h b/include/tst_capability.h
> new file mode 100644
> index 000000000..6342b667e
> --- /dev/null
> +++ b/include/tst_capability.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe<rpalethorpe@suse.com>
> + */
> +/**
> + * @file tst_capability.h
> + *
> + * Limited capability operations without libcap.
> + */
> +
> +#include<stdint.h>
> +
> +#include "lapi/syscalls.h"
> +
> +#ifndef TST_CAPABILITY_H
> +#define TST_CAPABILITY_H
> +
> +#ifndef CAP_SYS_ADMIN
> +# define CAP_SYS_ADMIN        21
> +#endif
> +
> +#ifndef CAP_TO_MASK
> +# define CAP_TO_MASK(x)      (1<<  ((x)&  31))
> +#endif
> +
> +#define TST_DROP 1
> +#define TST_REQUIRE 1<<  1
> +
> +#define TST_CAP(action, capability) {action, capability, #capability}
> +
> +struct tst_cap_user_header {
> +	uint32_t version;
> +	int pid;
> +};
> +
> +struct tst_cap_user_data {
> +	uint32_t effective;
> +	uint32_t permitted;
> +	uint32_t inheritable;
> +};
> +
> +struct tst_cap {
> +	uint32_t action;
> +	uint32_t id;
> +	char *name;
> +};
> +
> +int tst_capget(struct tst_cap_user_header *hdr,
> +	       struct tst_cap_user_data *data);
> +int tst_capset(struct tst_cap_user_header *hdr,
> +	       const struct tst_cap_user_data *data);
> +
> +void tst_cap_action(struct tst_cap *cap);
> +void tst_cap_setup(struct tst_cap *cap);
> +
> +#endif
> diff --git a/include/tst_test.h b/include/tst_test.h
> index cdeaf6ad0..84acf2c59 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -36,6 +36,7 @@
>   #include "tst_sys_conf.h"
>   #include "tst_coredump.h"
>   #include "tst_buffers.h"
> +#include "tst_capability.h"
>
>   /*
>    * Reports testcase result.
> @@ -206,6 +207,11 @@ struct tst_test {
>   	 * NULL-terminated array to be allocated buffers.
>   	 */
>   	struct tst_buffers *bufs;
> +
> +	/*
> +	 * NULL-terminated array of capability settings
> +	 */
> +	struct tst_cap *caps;
>   };
>
>   /*
> diff --git a/lib/tst_capability.c b/lib/tst_capability.c
> new file mode 100644
> index 000000000..d229491ae
> --- /dev/null
> +++ b/lib/tst_capability.c
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe<rpalethorpe@suse.com>
> + */
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_capability.h"
> +
> +int tst_capget(struct tst_cap_user_header *hdr,
> +	       struct tst_cap_user_data *data)
> +{
> +	return tst_syscall(__NR_capget, hdr, data);
> +}
> +
> +int tst_capset(struct tst_cap_user_header *hdr,
> +	       const struct tst_cap_user_data *data)
> +{
> +	return tst_syscall(__NR_capset, hdr, data);
> +}
> +
> +void tst_cap_action(struct tst_cap *cap)
> +{
> +	struct tst_cap_user_header hdr = {
> +		.version = 0x20080522,
> +		.pid = tst_syscall(__NR_gettid),
> +	};
Hi Richard

If we use _LINUX_CAPABILITY_VERSION_1, kernel will report the following warning: `cap_test' uses 32-bit capabilities (legacy support in use)

_LINUX_CAPABILITY_VERSION_2 has been deprecated since kernel 2.6.25, so we can only use _LINUX_CAPABILITY_VERSION_3.

But _LINUX_CAPABILITY_VERSION_3 uses 64-bit capabilities as man-page said, effective defined as uint32_t in tst_cap_usr_data is not enough.
I guess we need to define cur[2] ,new[2] and compare. Also, it can slove the EPERM failure as Li wang's cap_test.c found.
  
ps: I changed  kernel code to track this problem.
diff --git a/security/commoncap.c b/security/commoncap.c
index f4ee0ae106b2..291eb4e71031 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -247,24 +247,31 @@ int cap_capset(struct cred *new,
         if (cap_inh_is_capped()&&
             !cap_issubset(*inheritable,
                           cap_combine(old->cap_inheritable,
-                                     old->cap_permitted)))
+                                     old->cap_permitted))) {
                 /* incapable of using this inheritable set */
+               printk("xuyang 0\n");
                 return -EPERM;
+       }

         if (!cap_issubset(*inheritable,
                           cap_combine(old->cap_inheritable,
-                                     old->cap_bset)))
+                                     old->cap_bset))) {
                 /* no new pI capabilities outside bounding set */
+               printk("xuyang 1\n");
                 return -EPERM;
+       }

         /* verify restrictions on target's new Permitted set */
-       if (!cap_issubset(*permitted, old->cap_permitted))
+       if (!cap_issubset(*permitted, old->cap_permitted)) {
+               printk("xuyang  2\n");
                 return -EPERM;
+       }

         /* verify the _new_Effective_ is a subset of the _new_Permitted_ */
-       if (!cap_issubset(*effective, *permitted))
+       if (!cap_issubset(*effective, *permitted)) {
+               printk("xuyang 3\n");
                 return -EPERM;
-
+       }
         new->cap_effective   = *effective;
         new->cap_inheritable = *inheritable;

#./cap_test  (dmesg will report "xuyang 3",return EPERM if use version 3)

Thanks
Yang Xu

> +	struct tst_cap_user_data cur = { 0 };
> +	struct tst_cap_user_data new = { 0 };
> +	uint32_t mask = CAP_TO_MASK(cap->id);
> +	uint32_t act = cap->action;
> +
> +	if (tst_capget(&hdr,&cur))
> +		tst_brk(TBROK | TTERRNO, "tst_capget()");
> +
> +	new = cur;
> +
> +	switch (act) {
> +	case TST_DROP:
> +		if (cur.effective&  mask) {
> +			tst_res(TINFO, "Dropping %s(%d)",
> +				cap->name, cap->id);
> +			new.effective&= ~mask;
> +			new.permitted&= ~mask;
> +			new.inheritable&= ~mask;
> +		}
> +		break;
> +	case TST_REQUIRE:
> +		if (cur.permitted ^ mask) {
> +			tst_brk(TCONF, "Need %s(%d)",
> +				cap->name, cap->id);
> +		} else if (cur.effective ^ mask) {
> +			tst_res(TINFO, "Permitting %s(%d)",
> +				cap->name, cap->id);
> +			new.effective |= mask;
> +			new.inheritable |= mask;
> +		}
> +		break;
> +	default:
> +		tst_brk(TBROK, "Unrecognised action %d", cap->action);
> +	}
> +
> +	if (cur.effective != new.effective) {
> +		if (tst_capset(&hdr,&new))
> +			tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name);
> +	} else {
> +		tst_res(TINFO, "No capability changes needed");
> +	}
> +}
> +
> +void tst_cap_setup(struct tst_cap *caps)
> +{
> +	struct tst_cap *cap;
> +
> +	for (cap = caps; cap->action; cap++) {
> +		tst_cap_action(cap);
> +	}
> +}
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 8dc71dbb3..62e54d071 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -893,6 +893,9 @@ static void do_test_setup(void)
>
>   	if (main_pid != getpid())
>   		tst_brk(TBROK, "Runaway child in setup()!");
> +
> +	if (tst_test->caps)
> +		tst_cap_setup(tst_test->caps);
>   }
>
>   static void do_cleanup(void)
Richard Palethorpe Aug. 22, 2019, 9:35 a.m. UTC | #8
Hello,
> Hi Richard
>
> If we use _LINUX_CAPABILITY_VERSION_1, kernel will report the following warning: `cap_test' uses 32-bit capabilities (legacy support in use)
>
> _LINUX_CAPABILITY_VERSION_2 has been deprecated since kernel 2.6.25, so we can only use _LINUX_CAPABILITY_VERSION_3.
>
> But _LINUX_CAPABILITY_VERSION_3 uses 64-bit capabilities as man-page said, effective defined as uint32_t in tst_cap_usr_data is not enough.
> I guess we need to define cur[2] ,new[2] and compare. Also, it can slove the EPERM failure as Li wang's cap_test.c found.
>  ps: I changed  kernel code to track this problem.
> diff --git a/security/commoncap.c b/security/commoncap.c
> index f4ee0ae106b2..291eb4e71031 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -247,24 +247,31 @@ int cap_capset(struct cred *new,
>         if (cap_inh_is_capped()&&
>             !cap_issubset(*inheritable,
>                           cap_combine(old->cap_inheritable,
> -                                     old->cap_permitted)))
> +                                     old->cap_permitted))) {
>                 /* incapable of using this inheritable set */
> +               printk("xuyang 0\n");
>                 return -EPERM;
> +       }
>
>         if (!cap_issubset(*inheritable,
>                           cap_combine(old->cap_inheritable,
> -                                     old->cap_bset)))
> +                                     old->cap_bset))) {
>                 /* no new pI capabilities outside bounding set */
> +               printk("xuyang 1\n");
>                 return -EPERM;
> +       }
>
>         /* verify restrictions on target's new Permitted set */
> -       if (!cap_issubset(*permitted, old->cap_permitted))
> +       if (!cap_issubset(*permitted, old->cap_permitted)) {
> +               printk("xuyang  2\n");
>                 return -EPERM;
> +       }
>
>         /* verify the _new_Effective_ is a subset of the _new_Permitted_ */
> -       if (!cap_issubset(*effective, *permitted))
> +       if (!cap_issubset(*effective, *permitted)) {
> +               printk("xuyang 3\n");
>                 return -EPERM;
> -
> +       }
>         new->cap_effective   = *effective;
>         new->cap_inheritable = *inheritable;
>
> #./cap_test  (dmesg will report "xuyang 3",return EPERM if use version 3)
>
> Thanks
> Yang Xu

Yes, sorry I should have said earlier. I am converting it to use 64bit
capabilities. Also I have created some tests for this and will try to
use the upper bits.

Patch
diff mbox series

diff --git a/include/tst_capability.h b/include/tst_capability.h
new file mode 100644
index 000000000..6342b667e
--- /dev/null
+++ b/include/tst_capability.h
@@ -0,0 +1,56 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
+ */
+/**
+ * @file tst_capability.h
+ *
+ * Limited capability operations without libcap.
+ */
+
+#include <stdint.h>
+
+#include "lapi/syscalls.h"
+
+#ifndef TST_CAPABILITY_H
+#define TST_CAPABILITY_H
+
+#ifndef CAP_SYS_ADMIN
+# define CAP_SYS_ADMIN        21
+#endif
+
+#ifndef CAP_TO_MASK
+# define CAP_TO_MASK(x)      (1 << ((x) & 31))
+#endif
+
+#define TST_DROP 1
+#define TST_REQUIRE 1 << 1
+
+#define TST_CAP(action, capability) {action, capability, #capability}
+
+struct tst_cap_user_header {
+	uint32_t version;
+	int pid;
+};
+
+struct tst_cap_user_data {
+	uint32_t effective;
+	uint32_t permitted;
+	uint32_t inheritable;
+};
+
+struct tst_cap {
+	uint32_t action;
+	uint32_t id;
+	char *name;
+};
+
+int tst_capget(struct tst_cap_user_header *hdr,
+	       struct tst_cap_user_data *data);
+int tst_capset(struct tst_cap_user_header *hdr,
+	       const struct tst_cap_user_data *data);
+
+void tst_cap_action(struct tst_cap *cap);
+void tst_cap_setup(struct tst_cap *cap);
+
+#endif
diff --git a/include/tst_test.h b/include/tst_test.h
index cdeaf6ad0..84acf2c59 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -36,6 +36,7 @@ 
 #include "tst_sys_conf.h"
 #include "tst_coredump.h"
 #include "tst_buffers.h"
+#include "tst_capability.h"
 
 /*
  * Reports testcase result.
@@ -206,6 +207,11 @@  struct tst_test {
 	 * NULL-terminated array to be allocated buffers.
 	 */
 	struct tst_buffers *bufs;
+
+	/*
+	 * NULL-terminated array of capability settings
+	 */
+	struct tst_cap *caps;
 };
 
 /*
diff --git a/lib/tst_capability.c b/lib/tst_capability.c
new file mode 100644
index 000000000..d229491ae
--- /dev/null
+++ b/lib/tst_capability.c
@@ -0,0 +1,78 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
+ */
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_capability.h"
+
+int tst_capget(struct tst_cap_user_header *hdr,
+	       struct tst_cap_user_data *data)
+{
+	return tst_syscall(__NR_capget, hdr, data);
+}
+
+int tst_capset(struct tst_cap_user_header *hdr,
+	       const struct tst_cap_user_data *data)
+{
+	return tst_syscall(__NR_capset, hdr, data);
+}
+
+void tst_cap_action(struct tst_cap *cap)
+{
+	struct tst_cap_user_header hdr = {
+		.version = 0x20080522,
+		.pid = tst_syscall(__NR_gettid),
+	};
+	struct tst_cap_user_data cur = { 0 };
+	struct tst_cap_user_data new = { 0 };
+	uint32_t mask = CAP_TO_MASK(cap->id);
+	uint32_t act = cap->action;
+
+	if (tst_capget(&hdr, &cur))
+		tst_brk(TBROK | TTERRNO, "tst_capget()");
+
+	new = cur;
+
+	switch (act) {
+	case TST_DROP:
+		if (cur.effective & mask) {
+			tst_res(TINFO, "Dropping %s(%d)",
+				cap->name, cap->id);
+			new.effective &= ~mask;
+			new.permitted &= ~mask;
+			new.inheritable &= ~mask;
+		}
+		break;
+	case TST_REQUIRE:
+		if (cur.permitted ^ mask) {
+			tst_brk(TCONF, "Need %s(%d)",
+				cap->name, cap->id);
+		} else if (cur.effective ^ mask) {
+			tst_res(TINFO, "Permitting %s(%d)",
+				cap->name, cap->id);
+			new.effective |= mask;
+			new.inheritable |= mask;
+		}
+		break;
+	default:
+		tst_brk(TBROK, "Unrecognised action %d", cap->action);
+	}
+
+	if (cur.effective != new.effective) {
+		if (tst_capset(&hdr, &new))
+			tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name);
+	} else {
+		tst_res(TINFO, "No capability changes needed");
+	}
+}
+
+void tst_cap_setup(struct tst_cap *caps)
+{
+	struct tst_cap *cap;
+
+	for (cap = caps; cap->action; cap++) {
+		tst_cap_action(cap);
+	}
+}
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 8dc71dbb3..62e54d071 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -893,6 +893,9 @@  static void do_test_setup(void)
 
 	if (main_pid != getpid())
 		tst_brk(TBROK, "Runaway child in setup()!");
+
+	if (tst_test->caps)
+		tst_cap_setup(tst_test->caps);
 }
 
 static void do_cleanup(void)