diff options
author | Andrew G. Morgan <morgan@kernel.org> | 2021-09-07 13:14:54 -0700 |
---|---|---|
committer | Andrew G. Morgan <morgan@kernel.org> | 2021-09-07 13:14:54 -0700 |
commit | b35370f7f65387c02c0542d6c36144ca0a0e5efd (patch) | |
tree | 9600028dc42978fb857a799ad7e4e561d3f7695c | |
parent | 8434c10a690f3352ff5d8cb011859502718a60b7 (diff) | |
download | libcap2-b35370f7f65387c02c0542d6c36144ca0a0e5efd.tar.gz |
Implement --strict capsh argument.
Up to this point, capsh hides some complexity concerning raising
the CAP_SETPCAP in order to raise inheritable and drop bounding
set values. This made it harder to explain some aspects of
inheritance, and I ran into that detail writing this:
https://sites.google.com/site/fullycapable/why-didnt-that-work#h.z7rwbcazhr4r
Refactored capsh.c to clean up some buggy code, and also fix some
documentation, including reference to the --strict argument.
Signed-off-by: Andrew G. Morgan <morgan@kernel.org>
-rw-r--r-- | doc/capsh.1 | 27 | ||||
-rw-r--r-- | progs/capsh.c | 137 | ||||
-rwxr-xr-x | progs/quicktest.sh | 40 |
3 files changed, 109 insertions, 95 deletions
diff --git a/doc/capsh.1 b/doc/capsh.1 index 42637ab..60d3ad2 100644 --- a/doc/capsh.1 +++ b/doc/capsh.1 @@ -176,7 +176,7 @@ the current process. In all cases, is deactivated when an .BR exec () is performed. See -.B \-\-secbits +.BR \-\-secbits and \-\-mode for ways to disable this feature. .TP .BI \-\-secbits= N @@ -184,16 +184,17 @@ Set the security-bits for the program. This is done using the .BR prctl (2) .B PR_SET_SECUREBITS -operation. -The list of supported bits and their meaning can be found in -the +operation. The list of supported bits and their meaning can be found +in the .B <sys/secbits.h> header file. The program will list these bits via the .B \-\-print -command. -The argument is expressed as a numeric bitmask, -in any of the formats permitted by +command. The argument is expressed as a numeric bitmask, in any of +the formats permitted by .BR strtoul (3). +An alternative to this bit-twiddling is embedded in the +.B \-\-mode* +commandline arguments. .TP .BI \-\-chroot= /some/path Execute the @@ -228,6 +229,18 @@ capability makes available to a running program. Note, instead of \fIcap_xxx\fP, one can provide a decimal number and \fBcapsh\fP will look up the corresponding capability's description. .TP +.BI \-\-shell =/full/path +This option changes the shell that is invoked when the argument +\fB==\fP is encountered. +.TP +.BI \-\-strict +This option toggles the suppression of subsequent attempts to fixup +\fB\-\-caps=\fP and \fB\-\-inh=\fP arguments. That is, when the +prevailing Effective flag does not contain \fBCAP_SETPCAP\fB the to be +raised Inheritable Flag values (in strict mode) are limited to those +in the Permitted set. The strict mode defaults to off. Supplying this +argument an even number of times restores this default behavior. +.TP .BI \-\-suggest= phrase Scan each of the textual descriptions of capabilities, known to \fBcapsh\fP, and display all descriptions that include \fIphrase\fP. diff --git a/progs/capsh.c b/progs/capsh.c index be86cd7..efd1f56 100644 --- a/progs/capsh.c +++ b/progs/capsh.c @@ -191,29 +191,42 @@ static void arg_print(void) static const cap_value_t raise_setpcap[1] = { CAP_SETPCAP }; static const cap_value_t raise_chroot[1] = { CAP_SYS_CHROOT }; -static void push_pcap(cap_t *orig_p, cap_t *raised_for_setpcap_p) +static cap_t will_need_setpcap(int strict) { - /* - * We need to do this here because --inh=XXX may have reset - * orig and it isn't until we are within the --drop code that - * we know what the prevailing (orig) pI value is. - */ - *orig_p = cap_get_proc(); - if (NULL == *orig_p) { + cap_flag_value_t enabled; + cap_t raised = NULL; + + if (strict) { + return NULL; + } + + raised = cap_get_proc(); + if (raised == NULL) { perror("Capabilities not available"); exit(1); } - - *raised_for_setpcap_p = cap_dup(*orig_p); - if (NULL == *raised_for_setpcap_p) { - fprintf(stderr, "modification requires CAP_SETPCAP\n"); + if (cap_get_flag(raised, CAP_SETPCAP, CAP_EFFECTIVE, &enabled) != 0) { + perror("Unable to check CAP_EFFECTIVE CAP_SETPCAP value"); exit(1); } - if (cap_set_flag(*raised_for_setpcap_p, CAP_EFFECTIVE, 1, - raise_setpcap, CAP_SET) != 0) { - perror("unable to select CAP_SETPCAP"); + if (enabled != CAP_SET) { + cap_set_flag(raised, CAP_EFFECTIVE, 1, raise_setpcap, CAP_SET); + } else { + /* no need to raise - since already raised */ + cap_free(raised); + raised = NULL; + } + return raised; +} + +static void push_pcap(int strict, cap_t *orig_p, cap_t *raised_for_setpcap_p) +{ + *orig_p = cap_get_proc(); + if (NULL == *orig_p) { + perror("Capabilities not available"); exit(1); } + *raised_for_setpcap_p = will_need_setpcap(strict); } static void pop_pcap(cap_t orig, cap_t raised_for_setpcap) @@ -222,23 +235,24 @@ static void pop_pcap(cap_t orig, cap_t raised_for_setpcap) cap_free(orig); } -static void arg_drop(const char *arg_names) +static void arg_drop(int strict, const char *arg_names) { char *ptr; cap_t orig, raised_for_setpcap; char *names; - push_pcap(&orig, &raised_for_setpcap); + push_pcap(strict, &orig, &raised_for_setpcap); if (strcmp("all", arg_names) == 0) { unsigned j = 0; while (CAP_IS_SUPPORTED(j)) { int status; - if (cap_set_proc(raised_for_setpcap) != 0) { + if (raised_for_setpcap != NULL && + cap_set_proc(raised_for_setpcap) != 0) { perror("unable to raise CAP_SETPCAP for BSET changes"); exit(1); } status = cap_drop_bound(j); - if (cap_set_proc(orig) != 0) { + if (raised_for_setpcap != NULL && cap_set_proc(orig) != 0) { perror("unable to lower CAP_SETPCAP post BSET change"); exit(1); } @@ -271,12 +285,13 @@ static void arg_drop(const char *arg_names) fprintf(stderr, "capability [%s] is unknown to libcap\n", ptr); exit(1); } - if (cap_set_proc(raised_for_setpcap) != 0) { + if (raised_for_setpcap != NULL && + cap_set_proc(raised_for_setpcap) != 0) { perror("unable to raise CAP_SETPCAP for BSET changes"); exit(1); } status = cap_drop_bound(cap); - if (cap_set_proc(orig) != 0) { + if (raised_for_setpcap != NULL && cap_set_proc(orig) != 0) { perror("unable to lower CAP_SETPCAP post BSET change"); exit(1); } @@ -292,23 +307,15 @@ static void arg_drop(const char *arg_names) static void arg_change_amb(const char *arg_names, cap_flag_value_t set) { char *ptr; - cap_t orig, raised_for_setpcap; + cap_t orig; char *names; - push_pcap(&orig, &raised_for_setpcap); + orig = cap_get_proc(); if (strcmp("all", arg_names) == 0) { unsigned j = 0; while (CAP_IS_SUPPORTED(j)) { int status; - if (cap_set_proc(raised_for_setpcap) != 0) { - perror("unable to raise CAP_SETPCAP for AMBIENT changes"); - exit(1); - } status = cap_set_ambient(j, set); - if (cap_set_proc(orig) != 0) { - perror("unable to lower CAP_SETPCAP post AMBIENT change"); - exit(1); - } if (status != 0) { char *name_ptr; @@ -320,7 +327,7 @@ static void arg_change_amb(const char *arg_names, cap_flag_value_t set) } j++; } - pop_pcap(orig, raised_for_setpcap); + cap_free(orig); return; } @@ -338,22 +345,14 @@ static void arg_change_amb(const char *arg_names, cap_flag_value_t set) fprintf(stderr, "capability [%s] is unknown to libcap\n", ptr); exit(1); } - if (cap_set_proc(raised_for_setpcap) != 0) { - perror("unable to raise CAP_SETPCAP for AMBIENT changes"); - exit(1); - } status = cap_set_ambient(cap, set); - if (cap_set_proc(orig) != 0) { - perror("unable to lower CAP_SETPCAP post AMBIENT change"); - exit(1); - } if (status != 0) { fprintf(stderr, "failed to %s ambient [%s=%u]\n", set == CAP_CLEAR ? "clear":"raise", ptr, cap); exit(1); } } - pop_pcap(orig, raised_for_setpcap); + cap_free(orig); free(names); } @@ -444,6 +443,7 @@ int main(int argc, char *argv[], char *envp[]) { pid_t child; unsigned i; + int strict = 0; const char *shell = SHELL; child = 0; @@ -461,7 +461,7 @@ int main(int argc, char *argv[], char *envp[]) for (i=1; i<argc; ++i) { if (!strncmp("--drop=", argv[i], 7)) { - arg_drop(argv[i]+7); + arg_drop(strict, argv[i]+7); } else if (!strncmp("--dropped=", argv[i], 10)) { cap_value_t cap; if (cap_from_name(argv[i]+10, &cap) < 0) { @@ -476,7 +476,7 @@ int main(int argc, char *argv[], char *envp[]) } } else if (!strcmp("--has-ambient", argv[i])) { if (!CAP_AMBIENT_SUPPORTED()) { - fprintf(stderr, "ambient set not supported\n"); + perror("ambient set not supported"); exit(1); } } else if (!strncmp("--addamb=", argv[i], 9)) { @@ -485,7 +485,7 @@ int main(int argc, char *argv[], char *envp[]) arg_change_amb(argv[i]+9, CAP_CLEAR); } else if (!strncmp("--noamb", argv[i], 7)) { if (cap_reset_ambient() != 0) { - fprintf(stderr, "failed to reset ambient set\n"); + perror("failed to reset ambient set"); exit(1); } } else if (!strncmp("--inh=", argv[i], 6)) { @@ -493,24 +493,11 @@ int main(int argc, char *argv[], char *envp[]) char *text; char *ptr; - all = cap_get_proc(); - if (all == NULL) { - perror("Capabilities not available"); - exit(1); - } + push_pcap(strict, &all, &raised_for_setpcap); if (cap_clear_flag(all, CAP_INHERITABLE) != 0) { perror("libcap:cap_clear_flag() internal error"); exit(1); } - - raised_for_setpcap = cap_dup(all); - if ((raised_for_setpcap != NULL) - && (cap_set_flag(raised_for_setpcap, CAP_EFFECTIVE, 1, - raise_setpcap, CAP_SET) != 0)) { - cap_free(raised_for_setpcap); - raised_for_setpcap = NULL; - } - text = cap_to_text(all, NULL); cap_free(all); if (text == NULL) { @@ -527,13 +514,13 @@ int main(int argc, char *argv[], char *envp[]) } else { strcpy(ptr, text); } + cap_free(text); all = cap_from_text(ptr); if (all == NULL) { perror("Fatal error internalizing capabilities"); exit(1); } - cap_free(text); free(ptr); if (raised_for_setpcap != NULL) { @@ -551,45 +538,29 @@ int main(int argc, char *argv[], char *envp[]) perror("Unable to set inheritable capabilities"); exit(1); } - /* - * Since status is based on orig, we don't want to restore - * the previous value of 'all' again here! - */ - cap_free(all); + } else if (!strcmp("--strict", argv[i])) { + strict = !strict; } else if (!strncmp("--caps=", argv[i], 7)) { cap_t all, raised_for_setpcap; - raised_for_setpcap = cap_get_proc(); - if (raised_for_setpcap == NULL) { - perror("Capabilities not available"); - exit(1); - } - - if ((raised_for_setpcap != NULL) - && (cap_set_flag(raised_for_setpcap, CAP_EFFECTIVE, 1, - raise_setpcap, CAP_SET) != 0)) { - cap_free(raised_for_setpcap); - raised_for_setpcap = NULL; - } - + raised_for_setpcap = will_need_setpcap(strict); all = cap_from_text(argv[i]+7); if (all == NULL) { fprintf(stderr, "unable to interpret [%s]\n", argv[i]); exit(1); } - if (raised_for_setpcap != NULL) { /* - * This is only for the case that pP does not contain - * the requested change to pI.. Failing here is not - * indicative of the cap_set_proc(all) failing (always). + * This is actually only for the case that pP does not + * contain the requested change to pI.. Failing here + * is not always indicative of the cap_set_proc(all) + * failing. */ (void) cap_set_proc(raised_for_setpcap); cap_free(raised_for_setpcap); raised_for_setpcap = NULL; } - if (cap_set_proc(all) != 0) { fprintf(stderr, "Unable to set capabilities [%s]\n", argv[i]); exit(1); @@ -598,7 +569,6 @@ int main(int argc, char *argv[], char *envp[]) * Since status is based on orig, we don't want to restore * the previous value of 'all' again here! */ - cap_free(all); } else if (!strcmp("--modes", argv[i])) { cap_mode_t c; @@ -1122,6 +1092,7 @@ int main(int argc, char *argv[], char *envp[]) " --print display capability relevant state\n" " --secbits=<n> write a new value for securebits\n" " --shell=/xx/yy use /xx/yy instead of " SHELL " for --\n" + " --strict toggle --caps, --drop and --inh fixups\n" " --suggest=text search cap descriptions for text\n" " --supports=xxx exit 1 if capability xxx unsupported\n" " --uid=<n> set uid to <n> (hint: id <username>)\n" diff --git a/progs/quicktest.sh b/progs/quicktest.sh index ebb7567..776b175 100755 --- a/progs/quicktest.sh +++ b/progs/quicktest.sh @@ -130,7 +130,22 @@ fail_capsh --secbits=47 --print -- -c "./capsh --uid=$nouid" pass_capsh --secbits=0x2f --print -- -c "./privileged --uid=$nouid" # observe that the bounding set can be used to suppress this forced capability -fail_capsh --drop=cap_setuid --secbits=0x2f --print -- -c "./privileged --uid=$nouid" +fail_capsh --drop=cap_setuid --secbits=0x2f --print -- \ + -c "./privileged --uid=$nouid" + +# observe that effective cap_setpcap is required to drop bset +fail_capsh --caps="=ep cap_setpcap-ep" --drop=cap_setuid --current +pass_capsh --strict --caps="cap_setpcap=ep" --drop=cap_setuid --current +fail_capsh --strict --caps="cap_setpcap=p" --drop=cap_setuid --current +fail_capsh --strict --caps="=ep cap_setpcap-e" --drop=cap_setuid --current + +# observe that effective cap_setpcap is required to raise non-p bits +fail_capsh --strict --caps="cap_setpcap=p" --inh=cap_chown --current +# non-strict mode and capsh figures it out +pass_capsh --caps="cap_setpcap=p" --inh=cap_chown --current + +# permitted bits can be raised in inheritable flag without being effective. +pass_capsh --strict --caps="cap_chown=p" --inh=cap_chown --current # change the way the capability is obtained (make it inheritable) ./setcap cap_setuid,cap_setgid=ei ./privileged @@ -199,7 +214,8 @@ echo "no capabilities [\$caps] for this shell script" exit 1 EOF /bin/chmod +x hack.sh - pass_capsh --keep=1 --uid=$nouid --inh=cap_setuid --addamb=cap_setuid -- ./hack.sh + pass_capsh --keep=1 --uid=$nouid --inh=cap_setuid --addamb=cap_setuid -- \ + ./hack.sh /bin/rm -f hack.sh @@ -207,11 +223,24 @@ EOF # This is sort of the opposite of privileged - it should ensure that # the file can never acquire privilege by the ambient method. ./setcap = ./privileged - fail_capsh --keep=1 --uid=$nouid --inh=cap_setuid --addamb=cap_setuid -- -c "./privileged --print --uid=1" + fail_capsh --keep=1 --uid=$nouid --inh=cap_setuid --addamb=cap_setuid -- \ + -c "./privileged --print --uid=1" + + pass_capsh --keep=1 --uid=$nouid --strict \ + --caps="cap_setuid=p cap_setpcap=ep" \ + --inh=cap_setuid --addamb=cap_setuid --current + + # No effective capabilities are needed to raise or lower ambient values. + pass_capsh --keep=1 --uid=$nouid --strict --caps="cap_setuid=p" \ + --inh=cap_setuid --addamb=cap_setuid --current + pass_capsh --keep=1 --uid=$nouid --strict --iab="!^cap_setuid" \ + --caps="cap_setuid=pi" --current --delamb=cap_setuid --current + # finally remove the capability from the privileged binary and try again. ./setcap -r ./privileged - pass_capsh --keep=1 --uid=$nouid --inh=cap_setuid --addamb=cap_setuid -- -c "./privileged --print --uid=1" + pass_capsh --keep=1 --uid=$nouid --inh=cap_setuid --addamb=cap_setuid -- \ + -c "./privileged --print --uid=1" # validate IAB setting with an ambient capability pass_capsh --iab='!%cap_chown,^cap_setpcap,cap_setuid' @@ -250,7 +279,8 @@ if [ -f ../go/compare-cap ]; then echo "FAILED to execute go binary" exit 1 fi - LD_LIBRARY_PATH=../libcap ./compare-cap 2>&1 | grep "skipping file cap tests" + LD_LIBRARY_PATH=../libcap ./compare-cap 2>&1 | \ + grep "skipping file cap tests" if [ $? -eq 0 ]; then echo "FAILED not engaging file cap tests" fi |