| Message ID | 20190808153825.18363-2-rpalethorpe@suse.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tst API for dropping or requiring capabilities | expand |
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.
----- 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.
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), {}, }, };
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.
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.
δΊ 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), > {}, > }, > }; >
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)
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.
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)