* [LTP] [RFC PATCH 1/1] capability: Introduce capability API
2019-08-08 15:38 ` [LTP] [RFC PATCH 1/1] capability: Introduce capability API Richard Palethorpe
@ 2019-08-09 12:27 ` Cyril Hrubis
2019-08-09 14:42 ` Jan Stancek
2019-08-21 11:43 ` Richard Palethorpe
2019-08-15 7:10 ` Li Wang
` (2 subsequent siblings)
3 siblings, 2 replies; 16+ messages in thread
From: Cyril Hrubis @ 2019-08-09 12:27 UTC (permalink / raw)
To: ltp
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.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 16+ messages in thread* [LTP] [RFC PATCH 1/1] capability: Introduce capability API
2019-08-09 12:27 ` Cyril Hrubis
@ 2019-08-09 14:42 ` Jan Stancek
2019-08-21 11:43 ` Richard Palethorpe
1 sibling, 0 replies; 16+ messages in thread
From: Jan Stancek @ 2019-08-09 14:42 UTC (permalink / raw)
To: ltp
----- 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.
^ permalink raw reply [flat|nested] 16+ messages in thread* [LTP] [RFC PATCH 1/1] capability: Introduce capability API
2019-08-09 12:27 ` Cyril Hrubis
2019-08-09 14:42 ` Jan Stancek
@ 2019-08-21 11:43 ` Richard Palethorpe
1 sibling, 0 replies; 16+ messages in thread
From: Richard Palethorpe @ 2019-08-21 11:43 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [RFC PATCH 1/1] capability: Introduce capability API
2019-08-08 15:38 ` [LTP] [RFC PATCH 1/1] capability: Introduce capability API Richard Palethorpe
2019-08-09 12:27 ` Cyril Hrubis
@ 2019-08-15 7:10 ` Li Wang
2019-08-21 11:56 ` Richard Palethorpe
2019-08-22 5:56 ` Yang Xu
2019-08-22 8:41 ` Yang Xu
2019-08-22 14:17 ` [LTP] [PATCH v2 1/2] " Richard Palethorpe
3 siblings, 2 replies; 16+ messages in thread
From: Li Wang @ 2019-08-15 7:10 UTC (permalink / raw)
To: ltp
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),
{},
},
};
--
Regards,
Li Wang
^ permalink raw reply [flat|nested] 16+ messages in thread* [LTP] [RFC PATCH 1/1] capability: Introduce capability API
2019-08-15 7:10 ` Li Wang
@ 2019-08-21 11:56 ` Richard Palethorpe
2019-08-22 5:56 ` Yang Xu
1 sibling, 0 replies; 16+ messages in thread
From: Richard Palethorpe @ 2019-08-21 11:56 UTC (permalink / raw)
To: ltp
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.
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 16+ messages in thread* [LTP] [RFC PATCH 1/1] capability: Introduce capability API
2019-08-15 7:10 ` Li Wang
2019-08-21 11:56 ` Richard Palethorpe
@ 2019-08-22 5:56 ` Yang Xu
1 sibling, 0 replies; 16+ messages in thread
From: Yang Xu @ 2019-08-22 5:56 UTC (permalink / raw)
To: ltp
? 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),
> {},
> },
> };
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [RFC PATCH 1/1] capability: Introduce capability API
2019-08-08 15:38 ` [LTP] [RFC PATCH 1/1] capability: Introduce capability API Richard Palethorpe
2019-08-09 12:27 ` Cyril Hrubis
2019-08-15 7:10 ` Li Wang
@ 2019-08-22 8:41 ` Yang Xu
2019-08-22 9:35 ` Richard Palethorpe
2019-08-22 14:17 ` [LTP] [PATCH v2 1/2] " Richard Palethorpe
3 siblings, 1 reply; 16+ messages in thread
From: Yang Xu @ 2019-08-22 8:41 UTC (permalink / raw)
To: ltp
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)
^ permalink raw reply related [flat|nested] 16+ messages in thread* [LTP] [RFC PATCH 1/1] capability: Introduce capability API
2019-08-22 8:41 ` Yang Xu
@ 2019-08-22 9:35 ` Richard Palethorpe
0 siblings, 0 replies; 16+ messages in thread
From: Richard Palethorpe @ 2019-08-22 9:35 UTC (permalink / raw)
To: ltp
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.
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH v2 1/2] capability: Introduce capability API
2019-08-08 15:38 ` [LTP] [RFC PATCH 1/1] capability: Introduce capability API Richard Palethorpe
` (2 preceding siblings ...)
2019-08-22 8:41 ` Yang Xu
@ 2019-08-22 14:17 ` Richard Palethorpe
2019-08-22 14:17 ` [LTP] [PATCH v2 2/2] capability: library tests Richard Palethorpe
2019-08-23 4:24 ` [LTP] [PATCH v2 1/2] capability: Introduce capability API Yang Xu
3 siblings, 2 replies; 16+ messages in thread
From: Richard Palethorpe @ 2019-08-22 14:17 UTC (permalink / raw)
To: ltp
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
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+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");
+ } 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@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;
+ }
+}
+
+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);
+
+ 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)
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [LTP] [PATCH v2 2/2] capability: library tests
2019-08-22 14:17 ` [LTP] [PATCH v2 1/2] " Richard Palethorpe
@ 2019-08-22 14:17 ` Richard Palethorpe
2019-08-23 4:33 ` Yang Xu
2019-08-23 4:24 ` [LTP] [PATCH v2 1/2] capability: Introduce capability API Yang Xu
1 sibling, 1 reply; 16+ messages in thread
From: Richard Palethorpe @ 2019-08-22 14:17 UTC (permalink / raw)
To: ltp
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
include/lapi/capability.h | 8 +++++
lib/newlib_tests/tst_capability01.c | 50 +++++++++++++++++++++++++++++
lib/newlib_tests/tst_capability02.c | 35 ++++++++++++++++++++
3 files changed, 93 insertions(+)
create mode 100644 lib/newlib_tests/tst_capability01.c
create mode 100644 lib/newlib_tests/tst_capability02.c
diff --git a/include/lapi/capability.h b/include/lapi/capability.h
index 02d7a9fda..dac233d84 100644
--- a/include/lapi/capability.h
+++ b/include/lapi/capability.h
@@ -12,10 +12,18 @@
# include <sys/capability.h>
#endif
+#ifndef CAP_NET_RAW
+# define CAP_NET_RAW 13
+#endif
+
#ifndef CAP_SYS_ADMIN
# define CAP_SYS_ADMIN 21
#endif
+#ifndef CAP_AUDIT_READ
+# define CAP_AUDIT_READ 37
+#endif
+
#ifndef CAP_TO_INDEX
# define CAP_TO_INDEX(x) ((x) >> 5)
#endif
diff --git a/lib/newlib_tests/tst_capability01.c b/lib/newlib_tests/tst_capability01.c
new file mode 100644
index 000000000..1a9cb0568
--- /dev/null
+++ b/lib/newlib_tests/tst_capability01.c
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
+ *
+ * The user or file requires CAP_NET_RAW for this test to work.
+ * e.g use "$ setcap cap_net_raw=pei tst_capability"
+ */
+
+#include <unistd.h>
+#include <sys/types.h>
+
+#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");
+ } 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)
+{
+ if (geteuid() == 0)
+ tst_res(TWARN, "CAP_NET_RAW may be ignored when euid == 0");
+
+ TEST(socket(AF_INET, SOCK_RAW, 1));
+ if (TST_RET < 0)
+ tst_brk(TFAIL | TTERRNO, "Can't create raw socket in setup");
+
+ 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),
+ {}
+ },
+};
diff --git a/lib/newlib_tests/tst_capability02.c b/lib/newlib_tests/tst_capability02.c
new file mode 100644
index 000000000..45e3f2d22
--- /dev/null
+++ b/lib/newlib_tests/tst_capability02.c
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com>
+ */
+
+#include <unistd.h>
+#include <sys/types.h>
+
+#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(TPASS, "Created raw socket");
+ SAFE_CLOSE(TST_RET);
+ } else {
+ tst_res(TFAIL | TTERRNO, "Didn't create raw socket");
+ }
+}
+
+static struct tst_test test = {
+ .test_all = run,
+ .needs_root = 1,
+ .caps = (struct tst_cap []) {
+ TST_CAP(TST_CAP_REQ, CAP_NET_RAW),
+ TST_CAP(TST_CAP_DROP, CAP_AUDIT_READ), /* 64bit capability */
+ TST_CAP(TST_CAP_DROP, CAP_SYS_ADMIN),
+ {}
+ },
+};
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [LTP] [PATCH v2 2/2] capability: library tests
2019-08-22 14:17 ` [LTP] [PATCH v2 2/2] capability: library tests Richard Palethorpe
@ 2019-08-23 4:33 ` Yang Xu
0 siblings, 0 replies; 16+ messages in thread
From: Yang Xu @ 2019-08-23 4:33 UTC (permalink / raw)
To: ltp
on 2019/08/22 22:17, Richard Palethorpe wrote:
> diff --git a/lib/newlib_tests/tst_capability01.c b/lib/newlib_tests/tst_capability01.c
> new file mode 100644
> index 000000000..1a9cb0568
> --- /dev/null
> +++ b/lib/newlib_tests/tst_capability01.c
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2019 Richard Palethorpe<rpalethorpe@suse.com>
> + *
> + * The user or file requires CAP_NET_RAW for this test to work.
> + * e.g use "$ setcap cap_net_raw=pei tst_capability"
> + */
> +
> +#include<unistd.h>
> +#include<sys/types.h>
> +
> +#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");
> + }
> +}
> +
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH v2 1/2] capability: Introduce capability API
2019-08-22 14:17 ` [LTP] [PATCH v2 1/2] " Richard Palethorpe
2019-08-22 14:17 ` [LTP] [PATCH v2 2/2] capability: library tests Richard Palethorpe
@ 2019-08-23 4:24 ` Yang Xu
2019-08-23 8:37 ` Richard Palethorpe
1 sibling, 1 reply; 16+ messages in thread
From: Yang Xu @ 2019-08-23 4:24 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 16+ messages in thread* [LTP] [PATCH v2 1/2] capability: Introduce capability API
2019-08-23 4:24 ` [LTP] [PATCH v2 1/2] capability: Introduce capability API Yang Xu
@ 2019-08-23 8:37 ` Richard Palethorpe
0 siblings, 0 replies; 16+ messages in thread
From: Richard Palethorpe @ 2019-08-23 8:37 UTC (permalink / raw)
To: ltp
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.
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 16+ messages in thread