[v2,1/2] capability: Introduce capability API

Allow users to easily ensure particular capabilities are either present or not
present during testing without requiring libcap.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---

V2:
* Add docs
* Support 64bit capabilities
* Restructure code in various ways
* Only try to change the effective set. Inheritable has too many issues.
* Add tests

It occurred to me that if the user wants to spawn child processes then they
will either need to run as root or we need to do something quite complex. On
my system I am not able to set anything in the inheritable set as a normal
user and I don't know about ambient capabilities. These are set through a
different API, so we should consider carefully if we want to try support
setting those. Especially as it may just fail on most setups.

For the most part this is just useful for dropping CAP_SYS_ADMIN or failing
with a more accurate error message than "needs root".

 doc/test-writing-guidelines.txt |  78 ++++++++++++++++++++++
 include/lapi/capability.h       |  27 ++++++++
 include/tst_capability.h        |  48 ++++++++++++++
 include/tst_test.h              |   6 ++
 lib/tst_capability.c            | 110 ++++++++++++++++++++++++++++++++
 lib/tst_test.c                  |   3 +
 6 files changed, 272 insertions(+)
 create mode 100644 include/lapi/capability.h
 create mode 100644 include/tst_capability.h
 create mode 100644 lib/tst_capability.c

on 2019/08/22 22:17, Richard Palethorpe wrote:

> Allow users to easily ensure particular capabilities are either present or not
> present during testing without requiring libcap.
>
> Signed-off-by: Richard Palethorpe<rpalethorpe@suse.com>
> ---
>
> V2:
> * Add docs
> * Support 64bit capabilities
> * Restructure code in various ways
> * Only try to change the effective set. Inheritable has too many issues.
> * Add tests
>
> It occurred to me that if the user wants to spawn child processes then they
> will either need to run as root or we need to do something quite complex. On
> my system I am not able to set anything in the inheritable set as a normal
> user and I don't know about ambient capabilities. These are set through a
> different API, so we should consider carefully if we want to try support
> setting those. Especially as it may just fail on most setups.
>
> For the most part this is just useful for dropping CAP_SYS_ADMIN or failing
> with a more accurate error message than "needs root".
>
>   doc/test-writing-guidelines.txt |  78 ++++++++++++++++++++++
>   include/lapi/capability.h       |  27 ++++++++
>   include/tst_capability.h        |  48 ++++++++++++++
>   include/tst_test.h              |   6 ++
>   lib/tst_capability.c            | 110 ++++++++++++++++++++++++++++++++
>   lib/tst_test.c                  |   3 +
>   6 files changed, 272 insertions(+)
>   create mode 100644 include/lapi/capability.h
>   create mode 100644 include/tst_capability.h
>   create mode 100644 lib/tst_capability.c
>
> diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
> index 573dd08d9..e4463a443 100644
> --- a/doc/test-writing-guidelines.txt
> +++ b/doc/test-writing-guidelines.txt
> @@ -1699,6 +1699,84 @@ struct tst_test test = {
>   };
>   -------------------------------------------------------------------------------
>
> +2.2.31 Adding and removing capabilities
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
As Jan Stancek said, it should be rebased on lastest master.

> +
> +Some tests may require the presence or absence of particular
> +capabilities. Using the API provided by 'tst_capability.h' the test author can
> +try to ensure that some capabilities are either present or absent during the
> +test.
> +
> +For example; below we try to create a raw socket, which requires
> +CAP_NET_ADMIN. During setup we should be able to do it, then during run it
> +should be impossible. The LTP capability library should drop CAP_NET_RAW
> +(assuming we have it) after setup completes.
> +
> +[source,c]
> +--------------------------------------------------------------------------------
> +#include "tst_test.h"
> +#include "tst_capability.h"
> +#include "tst_safe_net.h"
> +
> +#include "lapi/socket.h"
> +
> +static void run(void)
> +{
> +	TEST(socket(AF_INET, SOCK_RAW, 1));
> +	if (TST_RET>  -1) {
> +		tst_res(TFAIL, "Created raw socket");
                             SAFE_CLOSE(TST_RET);

> +	} else if (TST_ERR != EPERM) {
> +		tst_res(TBROK | TTERRNO,
> +			"Failed to create socket for wrong reason");
> +	} else {
> +		tst_res(TPASS | TTERRNO, "Didn't create raw socket");
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	TEST(socket(AF_INET, SOCK_RAW, 1));
> +	if (TST_RET<  0)
> +		tst_brk(TCONF | TTERRNO, "We don't have CAP_NET_RAW to begin with");
> +
> +	SAFE_CLOSE(TST_RET);
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = run,
> +	.caps = (struct tst_cap []) {
> +		TST_CAP(TST_CAP_DROP, CAP_NET_RAW),
> +		{}
> +	},
> +};
> +--------------------------------------------------------------------------------
> +
> +Look at the test struct at the bottom. We have filled in the 'caps' field with
> +a NULL terminated array containing a single 'tst_cap'. This indicates to the
> +library that we should drop CAP_NET_RAW if we have it. The capability will be
> +dropped in between 'setup' and 'run'.
> +
> +[source,c]
> +--------------------------------------------------------------------------------
> +static struct tst_test test = {
> +	.test_all = run,
> +	.caps = (struct tst_cap []) {
> +		TST_CAP(TST_CAP_REQ, CAP_NET_RAW),
> +		TST_CAP(TST_CAP_DROP, CAP_SYS_ADMIN),
> +		{}
> +	},
> +};
> +--------------------------------------------------------------------------------
> +
> +Here we request 'CAP_NET_RAW', but drop 'CAP_SYS_ADMIN'. If the capability is
> +in the permitted set, but not the effective set, the library will try to
> +permit it. If it is not in the permitted set, then it will fail with 'TCONF'.
> +
> +This API does not require 'libcap' to be installed. However it has limited
> +features relative to 'libcap'. It only tries to add or remove capabilities
> +from the effective set. This means that tests which need to spawn child
> +processes with various capabilities will probably need 'libcap'.
>
>   2.3 Writing a testcase in shell
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/include/lapi/capability.h b/include/lapi/capability.h
> new file mode 100644
> index 000000000..02d7a9fda
> --- /dev/null
> +++ b/include/lapi/capability.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe<rpalethorpe@suse.com>
> + */
> +
> +#ifndef LAPI_CAPABILITY_H
> +#define LAPI_CAPABILITY_H
> +
> +#include "config.h"
> +
> +#ifdef HAVE_SYS_CAPABILITY_H
> +# include<sys/capability.h>
> +#endif
> +
> +#ifndef CAP_SYS_ADMIN
> +# define CAP_SYS_ADMIN        21
> +#endif
> +
> +#ifndef CAP_TO_INDEX
> +# define CAP_TO_INDEX(x)     ((x)>>  5)
> +#endif
> +
> +#ifndef CAP_TO_MASK
> +# define CAP_TO_MASK(x)      (1<<  ((x)&  31))
> +#endif
> +
> +#endif
> diff --git a/include/tst_capability.h b/include/tst_capability.h
> new file mode 100644
> index 000000000..396679f2e
> --- /dev/null
> +++ b/include/tst_capability.h
> @@ -0,0 +1,48 @@
> +/* 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.
> + */
> +
> +#ifndef TST_CAPABILITY_H
> +#define TST_CAPABILITY_H
> +
> +#include<stdint.h>
> +
> +#include "lapi/capability.h"
> +
> +#define TST_CAP_DROP 1
> +#define TST_CAP_REQ  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 2e07ff16b..6eb558396 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -35,6 +35,7 @@
>   #include "tst_path_has_mnt_flags.h"
>   #include "tst_sys_conf.h"
>   #include "tst_coredump.h"
> +#include "tst_capability.h"
>
>   /*
>    * Reports testcase result.
> @@ -200,6 +201,11 @@ struct tst_test {
>   	 * test.
>   	 */
>   	const char *const *needs_kconfigs;
> +
> +	/*
> +	 * 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..7e6c56e1d
> --- /dev/null
> +++ b/lib/tst_capability.c
> @@ -0,0 +1,110 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe<rpalethorpe@suse.com>
> + */
> +
> +#include<string.h>
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_capability.h"
> +
> +#include "lapi/syscalls.h"
> +
> +/**
> + * Get the capabilities as decided by hdr.
> + *
> + * Note that the memory pointed to by data should be large enough to store two
> + * structs.
> + */
> +int tst_capget(struct tst_cap_user_header *hdr,
> +	       struct tst_cap_user_data *data)
> +{
> +	return tst_syscall(__NR_capget, hdr, data);
> +}
> +
> +/**
> + * Set the capabilities as decided by hdr and data
> + *
> + * Note that the memory pointed to by data should be large enough to store two
> + * structs.
> + */
> +int tst_capset(struct tst_cap_user_header *hdr,
> +	       const struct tst_cap_user_data *data)
> +{
> +	return tst_syscall(__NR_capset, hdr, data);
> +}
> +
> +static void do_cap_drop(uint32_t *set, uint32_t mask, const struct tst_cap *cap)
> +{
> +	if (*set&  mask) {
> +		tst_res(TINFO, "Dropping %s(%d)", cap->name, cap->id);
> +		*set&= ~mask;
> +	}
> +}
If we drop a cap twice, the second drop reports nothing. I think we should print it as below:

as below:

@ -40,7 +40,10 @@ static void do_cap_drop(uint32_t *set, uint32_t mask, const struct tst_cap *cap)
	if (*set&  mask) {
		tst_res(TINFO, "Dropping %s(%d)", cap->name, cap->id);
		*set&= ~mask;
-       }
+       } else
+               tst_res(TINFO,
+                       "%s(%d) has not been in effective set before dropping",
+                       cap->name, cap->id);



> +
> +static void do_cap_req(uint32_t *set, uint32_t mask, const struct tst_cap *cap)
> +{
> +	if (!(*set&  mask))
> +		tst_brk(TCONF, "Need %s(%d)", cap->name, cap->id);
> +
here has logic problem.  :-)  If set has not the cap, we should use set |mask instead of tst_brk.

code should be as below:

            if ((*set&  mask)) {
                  tst_res(TINFO,
                        "%s(%d) has been in effective set before requring",
                        cap->name, cap->id);
                return;
        } else {
                 tst_res(TINFO, "Permitting %s(%d)", cap->name, cap->id);
                 *set |= mask;

         }

> +	if (!(*set&  mask)) {
> +		tst_res(TINFO, "Permitting %s(%d)", cap->name, cap->id);
> +		*set |= mask;
> +	}
> +}
> +
> +/**
> + * Add, check or remove capabilities
> + *
> + * Takes a NULL terminated array of structs which describe whether some
> + * capabilities are needed or not.
> + *
> + * It will attempt to drop or add capabilities to the effective set. It will
> + * try to detect if this is needed and whether it can or can't be done. If it
> + * clearly can not add a privilege to the effective set then it will return
> + * TCONF. However it may fail for some other reason and return TBROK.
> + *
> + * This only tries to change the effective set. Some tests may need to change
> + * the inheritable and ambient sets, so that child processes retain some
> + * capability.
> + */
> +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[2] = { {0} };
> +	struct tst_cap_user_data new[2] = { {0} };
> +	uint32_t act = cap->action;
> +	uint32_t *set =&new[CAP_TO_INDEX(cap->id)].effective;
> +	uint32_t mask = CAP_TO_MASK(cap->id);
> +
> +	if (tst_capget(&hdr, cur))
> +		tst_brk(TBROK | TTERRNO, "tst_capget()");
> +
> +	memcpy(new, cur, sizeof(new));
> +
> +	switch (act) {
> +	case TST_CAP_DROP:
> +		do_cap_drop(set, mask, cap);
> +		break;
> +	case TST_CAP_REQ:
> +		do_cap_req(set, mask, cap);
> +		break;
> +	default:
> +		tst_brk(TBROK, "Unrecognised action %d", cap->action);
> +	}
> +
> +	if (memcmp(cur, new, sizeof(new))&&  tst_capset(&hdr, new))
> +		tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name);
> +}
> +
> +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 245e287fa..8d5fbd1f9 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -888,6 +888,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)
other than the minor things, this patch looks ok.

Hello,

Yang Xu <xuyang2018.jy@cn.fujitsu.com> writes:

> on 2019/08/22 22:17, Richard Palethorpe wrote:
>
>> diff --git a/lib/tst_capability.c b/lib/tst_capability.c
>> new file mode 100644
>> index 000000000..7e6c56e1d
>> --- /dev/null
>> +++ b/lib/tst_capability.c
>> @@ -0,0 +1,110 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2019 Richard Palethorpe<rpalethorpe@suse.com>
>> + */
>> +
>> +#include<string.h>
>> +
>> +#define TST_NO_DEFAULT_MAIN
>> +#include "tst_test.h"
>> +#include "tst_capability.h"
>> +
>> +#include "lapi/syscalls.h"
>> +
>> +/**
>> + * Get the capabilities as decided by hdr.
>> + *
>> + * Note that the memory pointed to by data should be large enough to store two
>> + * structs.
>> + */
>> +int tst_capget(struct tst_cap_user_header *hdr,
>> +	       struct tst_cap_user_data *data)
>> +{
>> +	return tst_syscall(__NR_capget, hdr, data);
>> +}
>> +
>> +/**
>> + * Set the capabilities as decided by hdr and data
>> + *
>> + * Note that the memory pointed to by data should be large enough to store two
>> + * structs.
>> + */
>> +int tst_capset(struct tst_cap_user_header *hdr,
>> +	       const struct tst_cap_user_data *data)
>> +{
>> +	return tst_syscall(__NR_capset, hdr, data);
>> +}
>> +
>> +static void do_cap_drop(uint32_t *set, uint32_t mask, const struct tst_cap *cap)
>> +{
>> +	if (*set&  mask) {
>> +		tst_res(TINFO, "Dropping %s(%d)", cap->name, cap->id);
>> +		*set&= ~mask;
>> +	}
>> +}
> If we drop a cap twice, the second drop reports nothing. I think we should print it as below:
>
> as below:
>
> @ -40,7 +40,10 @@ static void do_cap_drop(uint32_t *set, uint32_t mask, const struct tst_cap *cap)
> 	if (*set&  mask) {
> 		tst_res(TINFO, "Dropping %s(%d)", cap->name, cap->id);
> 		*set&= ~mask;
> -       }
> +       } else
> +               tst_res(TINFO,
> +                       "%s(%d) has not been in effective set before dropping",
> +                       cap->name, cap->id);
>

I think it should only print something if an action is required. Stating
that we already have or don't have a capability is noise. If it prints
nothing this tells the user the same thing.

>
>
>> +
>> +static void do_cap_req(uint32_t *set, uint32_t mask, const struct tst_cap *cap)
>> +{
>> +	if (!(*set&  mask))
>> +		tst_brk(TCONF, "Need %s(%d)", cap->name, cap->id);
>> +
> here has logic problem.  :-)  If set has not the cap, we should use
> set |mask instead of tst_brk.

Ah, I see I broke my original logic here completely which is why this
makes no sense. xD

>
> code should be as below:
>
>            if ((*set&  mask)) {
>                  tst_res(TINFO,
>                        "%s(%d) has been in effective set before requring",
>                        cap->name, cap->id);
>                return;
>        } else {
>                 tst_res(TINFO, "Permitting %s(%d)", cap->name, cap->id);
>                 *set |= mask;
>
>         }
>
>> +	if (!(*set&  mask)) {
>> +		tst_res(TINFO, "Permitting %s(%d)", cap->name, cap->id);
>> +		*set |= mask;
>> +	}
>> +}
>> +
>> +/**
>> + * Add, check or remove capabilities
>> + *
>> + * Takes a NULL terminated array of structs which describe whether some
>> + * capabilities are needed or not.
>> + *
>> + * It will attempt to drop or add capabilities to the effective set. It will
>> + * try to detect if this is needed and whether it can or can't be done. If it
>> + * clearly can not add a privilege to the effective set then it will return
>> + * TCONF. However it may fail for some other reason and return TBROK.
>> + *
>> + * This only tries to change the effective set. Some tests may need to change
>> + * the inheritable and ambient sets, so that child processes retain some
>> + * capability.
>> + */
>> +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[2] = { {0} };
>> +	struct tst_cap_user_data new[2] = { {0} };
>> +	uint32_t act = cap->action;
>> +	uint32_t *set =&new[CAP_TO_INDEX(cap->id)].effective;
>> +	uint32_t mask = CAP_TO_MASK(cap->id);
>> +
>> +	if (tst_capget(&hdr, cur))
>> +		tst_brk(TBROK | TTERRNO, "tst_capget()");
>> +
>> +	memcpy(new, cur, sizeof(new));
>> +
>> +	switch (act) {
>> +	case TST_CAP_DROP:
>> +		do_cap_drop(set, mask, cap);
>> +		break;
>> +	case TST_CAP_REQ:
>> +		do_cap_req(set, mask, cap);
>> +		break;
>> +	default:
>> +		tst_brk(TBROK, "Unrecognised action %d", cap->action);
>> +	}
>> +
>> +	if (memcmp(cur, new, sizeof(new))&&  tst_capset(&hdr, new))
>> +		tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name);
>> +}
>> +
>> +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 245e287fa..8d5fbd1f9 100644
>> --- a/lib/tst_test.c
>> +++ b/lib/tst_test.c
>> @@ -888,6 +888,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)
> other than the minor things, this patch looks ok.