From: Subrata M. <su...@li...> - 2008-07-11 15:31:54
|
On Fri, 2008-07-11 at 07:47 -0500, Serge E. Hallyn wrote: > Please apply this patch (in place of, not on top of the earlier > version). It eliminates the potential for many false negatives. > > thanks, > -serge > > 1. Use cap_compare to compare capability sets instead of > comparing the far less reliable text representations. > > 2. pI' tests were failing bc I started with empty pI. Fill > pI before those tests. > > 3. Check for libcap-2.11 or later (by checking for cap_compare()) > > Signed-off-by: Serge Hallyn <se...@us...> Thanks. Applied. Regards-- Subrata > --- > Makefile | 3 +- > check_simple_capset.c | 6 +++- > filecapstest.sh | 2 +- > verify_caps_exec.c | 70 +++++++++++++++++++++++-------------------------- > 4 files changed, 41 insertions(+), 40 deletions(-) > > diff --git a/Makefile b/Makefile > index 7bcdea0..52b254a 100644 > --- a/Makefile > +++ b/Makefile > @@ -38,7 +38,8 @@ checkforlibcap: > chmod a+rx $@ > else > @echo "setcap or xattr headers not installed. Please install libcap from"; \ > - echo "ftp://ftp.kernel.org/pub/linux/libs/security/linux-privs/libcap2"; \ > + echo "ftp://ftp.kernel.org/pub/linux/libs/security/linux-privs/libcap2."; \ > + echo "You must use libcap-2.11 or newer."; \ > echo "Then make clean in ltp or ltp/testcases/kernel/security/filecaps, and recompile ltp." > checkforlibcap: > echo false > $@ > diff --git a/check_simple_capset.c b/check_simple_capset.c > index 3c1a057..74f7b1a 100644 > --- a/check_simple_capset.c > +++ b/check_simple_capset.c > @@ -24,12 +24,16 @@ > > int main() > { > - cap_t caps; > + cap_t caps, caps2; > int ret; > > caps = cap_from_text("cap_setpcap+ep"); > + caps2 = cap_from_text("cap_setpcap+ep"); > ret = cap_set_proc(caps); > + ret = cap_compare(caps, caps2); > + printf("Caps were %s the same\n", ret ? "not" : ""); > > cap_free(caps); > + cap_free(caps2); > return ret; > } > diff --git a/filecapstest.sh b/filecapstest.sh > index 8103c03..3cc7bec 100755 > --- a/filecapstest.sh > +++ b/filecapstest.sh > @@ -22,7 +22,7 @@ > checkforlibcap > ret=$? > if [ $ret -ne 0 ]; then > - echo setcap not installed. Please install libcap from > + echo setcap not installed. Please install libcap-2.11 or newer from > echo ftp://ftp.kernel.org/pub/linux/libs/security/linux-privs/libcap2 > exit 1 > fi > diff --git a/verify_caps_exec.c b/verify_caps_exec.c > index 50f18d4..15d56c6 100644 > --- a/verify_caps_exec.c > +++ b/verify_caps_exec.c > @@ -151,19 +151,13 @@ void read_from_fifo(char *buf) > close(fd); > } > > -int compare_caps(char *buf1, char *buf2) > -{ > - int res; > - > - res = strcmp(buf1, buf2) == 0; > - return res; > -} > - > -int fork_drop_and_exec(int keepperms, char *capstxt) > +int fork_drop_and_exec(int keepperms, cap_t expected_caps) > { > int pid; > int ret = 0; > char buf[200], *p; > + char *capstxt; > + cap_t actual_caps; > static int seqno = 0; > > pid = fork(); > @@ -179,7 +173,9 @@ int fork_drop_and_exec(int keepperms, char *capstxt) > ret = execlp(TSTPATH, TSTPATH, buf, NULL); > perror("execl"); > tst_resm(TFAIL, "%s: exec failed\n", __FUNCTION__); > + capstxt = cap_to_text(expected_caps, NULL); > snprintf(buf, 200, "failed to run as %s\n", capstxt); > + cap_free(capstxt); > write_to_fifo(buf); > tst_exit(1); > } else { > @@ -198,12 +194,16 @@ int fork_drop_and_exec(int keepperms, char *capstxt) > tst_resm(TFAIL, "got a bad message from print_caps\n"); > tst_exit(1); > } > - tst_resm(TINFO, "Expected to run as .%s., ran as .%s..\n", > - capstxt, p); > - if (strcmp(p, capstxt) != 0) { > + actual_caps = cap_from_text(p); > + if (cap_compare(actual_caps, expected_caps) != 0) { > + capstxt = cap_to_text(expected_caps, NULL); > + tst_resm(TINFO, "Expected to run as .%s., ran as .%s..\n", > + capstxt, p); > tst_resm(TINFO, "those are not the same\n"); > + cap_free(capstxt); > ret = -1; > } > + cap_free(actual_caps); > seqno++; > } > return ret; > @@ -240,9 +240,7 @@ int caps_actually_set_test(void) > tst_resm(TINFO, "%d\n", whichcap); > continue; > } > - capstxt = cap_to_text(fcap, NULL); > - ret = fork_drop_and_exec(DROP_PERMS, capstxt); > - cap_free(capstxt); > + ret = fork_drop_and_exec(DROP_PERMS, fcap); > if (ret) { > tst_resm(TINFO, "Failed CAP_PERMITTED=%d CAP_EFFECTIVE=0\n", > whichcap); > @@ -262,14 +260,7 @@ int caps_actually_set_test(void) > tst_resm(TINFO, "%d\n", whichcap); > continue; > } > - capstxt = cap_to_text(fcap, NULL); > - if (strcmp(capstxt, "=")==0) { > - tst_resm(TINFO, "%s: libcap doesn't know about cap %d, not running\n", > - __FUNCTION__, whichcap); > - ret = 0; > - } else > - ret = fork_drop_and_exec(DROP_PERMS, capstxt); > - cap_free(capstxt); > + ret = fork_drop_and_exec(DROP_PERMS, fcap); > if (ret) { > tst_resm(TINFO, "Failed CAP_PERMITTED=%d CAP_EFFECTIVE=1\n", > whichcap); > @@ -285,6 +276,15 @@ int caps_actually_set_test(void) > capvalue[0] = i; > cap_set_flag(cap_fullpi, CAP_INHERITABLE, 1, capvalue, CAP_SET); > } > + > + /* > + * For the inheritable tests, we want to make sure pI starts > + * filled. > + */ > + ret = cap_set_proc(cap_fullpi); > + if (ret) > + tst_resm(TINFO, "Could not fill pI. pI tests will fail.\n"); > + > /* > * next try each bit in fI > * The first two attemps have the bit which is in fI in pI. > @@ -295,6 +295,7 @@ int caps_actually_set_test(void) > * no bits to be inherited from the original process. > */ > for (whichcap=0; whichcap < NUM_CAPS; whichcap++) { > + cap_t cmpcap; > capvalue[0] = whichcap; > > /* > @@ -315,9 +316,7 @@ int caps_actually_set_test(void) > tst_resm(TINFO, "%d\n", whichcap); > continue; > } > - capstxt = cap_to_text(pcap, NULL); > - ret = fork_drop_and_exec(KEEP_PERMS, capstxt); > - cap_free(capstxt); > + ret = fork_drop_and_exec(KEEP_PERMS, pcap); > if (ret) { > tst_resm(TINFO, "Failed with_perms CAP_INHERITABLE=%d " > "CAP_EFFECTIVE=0\n", whichcap); > @@ -340,14 +339,13 @@ int caps_actually_set_test(void) > tst_resm(TINFO, "%d\n", whichcap); > continue; > } > - capstxt = cap_to_text(pcap, NULL); > - if (strcmp(capstxt, "=")==0) { > - tst_resm(TINFO, "%s: libcap doesn't know about cap %d, not running\n", > - __FUNCTION__, whichcap); > - ret = 0; > - } else > - ret = fork_drop_and_exec(KEEP_PERMS, capstxt); > - cap_free(capstxt); > + /* The actual result will be a full pI, with > + * pE and pP containing just whichcap. */ > + cmpcap = cap_dup(cap_fullpi); > + cap_set_flag(cmpcap, CAP_PERMITTED, 1, capvalue, CAP_SET); > + cap_set_flag(cmpcap, CAP_EFFECTIVE, 1, capvalue, CAP_SET); > + ret = fork_drop_and_exec(KEEP_PERMS, cmpcap); > + cap_free(cmpcap); > if (ret) { > tst_resm(TINFO, "Failed with_perms CAP_INHERITABLE=%d " > "CAP_EFFECTIVE=1\n", whichcap); > @@ -361,9 +359,7 @@ int caps_actually_set_test(void) > * pI'=pP'=pE'=0 > */ > cap_clear(pcap); > - capstxt = cap_to_text(pcap, NULL); > - ret = fork_drop_and_exec(DROP_PERMS, capstxt); > - cap_free(capstxt); > + ret = fork_drop_and_exec(DROP_PERMS, pcap); > if (ret) { > tst_resm(TINFO, "Failed without_perms CAP_INHERITABLE=%d", > whichcap); |