From e5cbf2feeb4217fbb30ed3562028ae2845eccbd0 Mon Sep 17 00:00:00 2001 From: Gerhard Rieger Date: Wed, 19 Jul 2023 21:51:59 +0200 Subject: [PATCH] Restrict option umask to the address it is applied to --- CHANGES | 5 ++ doc/socat.yo | 29 ++++++++-- test.sh | 155 +++++++++++++++++++++++++++++++++++++++++++++++++++ xio-named.c | 7 --- xio-named.h | 1 - xiolayer.c | 1 + xiolayer.h | 2 + xioopen.c | 18 ++++++ xioopts.c | 2 +- xioopts.h | 2 + 10 files changed, 208 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index 1df4df9..bb22299 100644 --- a/CHANGES +++ b/CHANGES @@ -143,6 +143,11 @@ Features: Added option res-nsaddr that overrides /etc/resolv.conf nameserver address based on an undocumented resolver feature. + Option umask now applies only during opening of its very address, not + for the lifetime of the process; the original umask is restored + afterwards. + Tests: UMASK_ON_CREATE UMASK_ON_SYSTEM + Corrections: When a sub process (EXEC, SYSTEM) terminated with exit code other than 0, its last sent data might have been lost depending on timing of read/ diff --git a/doc/socat.yo b/doc/socat.yo index dd4d830..ad5e1b8 100644 --- a/doc/socat.yo +++ b/doc/socat.yo @@ -341,6 +341,7 @@ label(ADDRESS_EXEC)dit(bf(tt(EXEC:))) link(ctty)(OPTION_CTTY), link(setsid)(OPTION_SETSID), link(pipes)(OPTION_PIPES), + link(umask)(OPTION_UMASK), link(login)(OPTION_LOGIN), link(sigint)(OPTION_SIGINT), link(sigquit)(OPTION_SIGQUIT), @@ -1111,6 +1112,7 @@ label(ADDRESS_SHELL)dit(bf(tt(SHELL:))) link(ctty)(OPTION_CTTY), link(setsid)(OPTION_SETSID), link(pipes)(OPTION_PIPES), + link(umask)(OPTION_UMASK), link(sigint)(OPTION_SIGINT), link(sigquit)(OPTION_SIGQUIT)nl() See also: link(EXEC)(ADDRESS_EXEC), link(SYSTEM)(ADDRESS_SYSTEM) @@ -1137,6 +1139,7 @@ label(ADDRESS_SYSTEM)dit(bf(tt(SYSTEM:))) link(ctty)(OPTION_CTTY), link(setsid)(OPTION_SETSID), link(pipes)(OPTION_PIPES), + link(umask)(OPTION_UMASK), link(sigint)(OPTION_SIGINT), link(sigquit)(OPTION_SIGQUIT), link(netns)(OPTION_NETNS)nl() @@ -1503,6 +1506,7 @@ label(ADDRESS_UNIX_RECVFROM)dit(bf(tt(UNIX-RECVFROM:))) See the link(note about RECVFROM addresses)(NOTE_RECVFROM).nl() Useful options: link(fork)(OPTION_FORK)nl() + link(umask)(OPTION_UMASK)nl() See also: link(UNIX-SENDTO)(ADDRESS_UNIX_SENDTO), link(UNIX-RECV)(ADDRESS_UNIX_RECV), @@ -1518,6 +1522,8 @@ label(ADDRESS_UNIX_RECV)dit(bf(tt(UNIX-RECV:))) It can be, e.g., addressed by socat UNIX-SENDTO address peers. It behaves similar to a syslog server.nl() Option groups: link(FD)(GROUP_FD),link(SOCKET)(GROUP_SOCKET),link(NAMED)(GROUP_NAMED),link(UNIX)(GROUP_SOCK_UNIX) nl() + Useful options: + link(umask)(OPTION_UMASK)nl() See also: link(UNIX-SENDTO)(ADDRESS_UNIX_SENDTO), link(UNIX-RECVFROM)(ADDRESS_UNIX_RECVFROM), @@ -1856,11 +1862,6 @@ label(OPTION_PERM_EARLY)dit(bf(tt(perm-early=))) before accessing it, using the code(chmod()) system call. This call might require ownership or root privilege. -label(OPTION_UMASK)dit(bf(tt(umask=))) - Sets the umask of the process to [link(mode_t)(TYPE_MODE_T)] before - accessing the file system entry (useful - with unixdomain() sockets!). This call might affect all further operations - of the socat() process! label(OPTION_UNLINK_EARLY)dit(bf(tt(unlink-early[=]))) Unlinks (removes) the file before opening it and even before applying user-early etc. @@ -1983,6 +1984,24 @@ enddit() startdit()enddit()nl() +label(GROUP_ADDRS)em(bf(General address options)) + +These options may be applied to all address types. They change some process +properties that are restored after opening the address. + +startdit() +label(OPTION_UMASK)dit(bf(tt(umask=))) + Sets the umask of the process to [link(mode_t)(TYPE_MODE_T)] before + opening the address. Useful when file system entries are created or a shell + or program is invoked. Usually the value is specified as octal number.nl() + The processes tt(umask) value is inherited by child processes. + Note: umask is an inverted value: creating a file with umask=0026 results in + permissions 0640. +enddit() + +startdit()enddit()nl() + + label(GROUP_PROCESS)em(bf(PROCESS option group)) Options of this group change the process properties instead of just affecting diff --git a/test.sh b/test.sh index a2255db..b9ff555 100755 --- a/test.sh +++ b/test.sh @@ -17355,6 +17355,161 @@ else listFAIL="$listFAIL $N" namesFAIL="$namesFAIL $NAME" fi + + +# Test the modified umask option, in particular if umask with first address +# (CREATE) does not affect umask of second address, i.e. original umask is +# recovered +NAME=UMASK_ON_CREATE +case "$TESTS" in +*%$N%*|*%functions%*|*%creat%*|*%system%*|*%umask%*|*%$NAME%*) +TEST="$NAME: test restore after CREAT with umask option" +# Run Socat with first address CREAT with modified umask, +# and second address SYSTEM (shell) with umask command +# Check if the file is created with modified umask but shell has original umask +if ! eval $NUMCOND; then :; +elif ! F=$(testfeats CREAT SYSTEM); then + $PRINTF "test $F_n $TEST... ${YELLOW}Feature $F not configured in $SOCAT${NORMAL}\n" $N + numCANT=$((numCANT+1)) + listCANT="$listCANT $N" +elif ! A=$(testaddrs - CREAT SYSTEM); then + $PRINTF "test $F_n $TEST... ${YELLOW}Address $A not available in $SOCAT${NORMAL}\n" $N + numCANT=$((numCANT+1)) + listCANT="$listCANT $N" +elif ! o=$(testoptions umask) >/dev/null; then + $PRINTF "test $F_n $TEST... ${YELLOW}Option $o not available in $SOCAT${NORMAL}\n" $N + numCANT=$((numCANT+1)) + listCANT="$listCANT $N" +else + tf="$td/test$N.stdout" + te="$td/test$N.stderr" + tc="$td/test$N.creat" + tdiff="$td/test$N.diff" + tdebug="$td/test$N.debug" + oumask=$(umask) + # Construct a temp umask differing from original umask + case oumask in + *066) tumask=0026 ;; + *) tumask=0066 ;; + esac + CMD0="$TRACE $SOCAT $opts -U CREAT:$tc,umask=$tumask SYSTEM:umask" + printf "test $F_n $TEST... " $N + $CMD0 >/dev/null 2>"${te}0" + rc0=$? + tperms=$(fileperms $tc) + case $tperms in + 0*) ;; + *) tperms=0$tperms ;; + esac + echo "Original umask: $oumask" >>$tdebug + echo "Temporary umask: $tumask" >>$tdebug + echo "Created umask: $tperms" >>$tdebug + if [ "$rc0" -ne 0 ]; then + $PRINTF "$FAILED\n" + echo "$CMD0 &" + cat "${te}0" >&2 + numFAIL=$((numFAIL+1)) + listFAIL="$listFAIL $N" + namesFAIL="$namesFAIL $NAME" + elif [ $((tumask + tperms - 0666)) -ne 0 ]; then + $PRINTF "$FAILED (umask failed)\n" + echo "$CMD0 &" + cat "${te}0" >&2 + numFAIL=$((numFAIL+1)) + listFAIL="$listFAIL $N" + namesFAIL="$namesFAIL $NAME" + elif ! echo "$oumask" |diff "$tc" - >$tdiff; then + $PRINTF "$FAILED (bad umask)\n" + echo "$CMD0 &" + cat "${te}0" >&2 + numFAIL=$((numFAIL+1)) + listFAIL="$listFAIL $N" + namesFAIL="$namesFAIL $NAME" + else + $PRINTF "$OK\n" + if [ "$VERBOSE" ]; then echo "$CMD0 &"; fi + if [ "$DEBUG" ]; then cat "${te}0" >&2; fi + numOK=$((numOK+1)) + fi +fi # NUMCOND + ;; +esac +N=$((N+1)) + +# Test the modified umask option, in particular if umask with first address +# (SHELL) does not affect umask of second address, i.e. original umask is +# recovered +NAME=UMASK_ON_SYSTEM +case "$TESTS" in +*%$N%*|*%functions%*|*%shell%*|*%umask%*|*%socket%*|*%$NAME%*) +TEST="$NAME: test restore after SHELL with umask option" +# Run Socat with first address SHELL:"cat >file" with modified umask, +# and second address SYSTEM (shell) with umask command. +# Check if the file is created with modified umask but shell has original umask +if ! eval $NUMCOND; then :; +elif ! F=$(testfeats SHELL SYSTEM); then + $PRINTF "test $F_n $TEST... ${YELLOW}Feature $F not configured in $SOCAT${NORMAL}\n" $N + numCANT=$((numCANT+1)) + listCANT="$listCANT $N" +elif ! A=$(testaddrs SHELL SYSTEM); then + $PRINTF "test $F_n $TEST... ${YELLOW}Address $A not available in $SOCAT${NORMAL}\n" $N + numCANT=$((numCANT+1)) + listCANT="$listCANT $N" +elif ! o=$(testoptions umask) >/dev/null; then + $PRINTF "test $F_n $TEST... ${YELLOW}Option $o not available in $SOCAT${NORMAL}\n" $N + numCANT=$((numCANT+1)) + listCANT="$listCANT $N" +else + tf="$td/test$N.stdout" + te="$td/test$N.stderr" + tc="$td/test$N.creat" + tdiff="$td/test$N.diff" + tdebug="$td/test$N.debug" + oumask=$(umask) + # Construct a temp umask differing from original umask + case oumask in + *066) tumask=0026 ;; + *) tumask=0066 ;; + esac + CMD0="$TRACE $SOCAT $opts -U SHELL:\"cat\ >$tc\",umask=$tumask SYSTEM:umask" + printf "test $F_n $TEST... " $N + eval "$CMD0" >/dev/null 2>"${te}0" + rc0=$? + tperms=$(fileperms $tc) + case $tperms in + 0*) ;; + *) tperms=0$tperms ;; + esac + echo "Original umask: $oumask" >>$tdebug + echo "Temporary umask: $tumask" >>$tdebug + echo "Created umask: $tperms" >>$tdebug + if [ "$rc0" -ne 0 ]; then + $PRINTF "$FAILED\n" + echo "$CMD0 &" + cat "${te}0" >&2 + numFAIL=$((numFAIL+1)) + listFAIL="$listFAIL $N" + namesFAIL="$namesFAIL $NAME" + elif [ $((tumask + tperms - 0666)) -ne 0 ]; then + $PRINTF "$FAILED (umask failed)\n" + echo "$CMD0 &" + cat "${te}0" >&2 + numFAIL=$((numFAIL+1)) + listFAIL="$listFAIL $N" + namesFAIL="$namesFAIL $NAME" + elif ! echo "$oumask" |diff "$tc" - >$tdiff; then + $PRINTF "$FAILED (bad umask)\n" + echo "$CMD0 &" + cat "${te}0" >&2 + numFAIL=$((numFAIL+1)) + listFAIL="$listFAIL $N" + namesFAIL="$namesFAIL $NAME" + else + $PRINTF "$OK\n" + if [ "$VERBOSE" ]; then echo "$CMD0 &"; fi + if [ "$DEBUG" ]; then cat "${te}0" >&2; fi + numOK=$((numOK+1)) + fi fi # NUMCOND ;; esac diff --git a/xio-named.c b/xio-named.c index abb9f51..1dec534 100644 --- a/xio-named.c +++ b/xio-named.c @@ -21,7 +21,6 @@ const struct optdesc opt_unlink = { "unlink", NULL, OPT_UNLINK, G const struct optdesc opt_unlink_early= { "unlink-early",NULL, OPT_UNLINK_EARLY,GROUP_NAMED, PH_EARLY, TYPE_BOOL, OFUNC_SPEC }; const struct optdesc opt_unlink_late = { "unlink-late", NULL, OPT_UNLINK_LATE, GROUP_NAMED, PH_PASTOPEN, TYPE_BOOL, OFUNC_SPEC }; const struct optdesc opt_unlink_close = { "unlink-close", NULL, OPT_UNLINK_CLOSE, GROUP_NAMED, PH_LATE, TYPE_BOOL, OFUNC_SPEC }; -const struct optdesc opt_umask = { "umask", NULL, OPT_UMASK, GROUP_NAMED, PH_EARLY, TYPE_MODET, OFUNC_SPEC }; #endif /* WITH_NAMED */ /* applies to filesystem entry all options belonging to phase */ @@ -71,12 +70,6 @@ int applyopts_named(const char *filename, struct opt *opts, unsigned int phase) } } break; - case OPT_UMASK: - if (Umask(opt->value.u_modet) < 0) { - /* linux docu says it always succeeds, but who believes it? */ - Error2("umask("F_mode"): %s", opt->value.u_modet, strerror(errno)); - } - break; default: Error1("applyopts_named(): option \"%s\" not implemented", opt->desc->defname); break; diff --git a/xio-named.h b/xio-named.h index ac83dc3..101bdf7 100644 --- a/xio-named.h +++ b/xio-named.h @@ -14,7 +14,6 @@ extern const struct optdesc opt_unlink; extern const struct optdesc opt_unlink_early; extern const struct optdesc opt_unlink_late; extern const struct optdesc opt_unlink_close; -extern const struct optdesc opt_umask; extern int applyopts_named(const char *filename, struct opt *opts, unsigned int phase); diff --git a/xiolayer.c b/xiolayer.c index 0256c12..6838e17 100644 --- a/xiolayer.c +++ b/xiolayer.c @@ -24,3 +24,4 @@ const struct optdesc opt_intervall = { "interval", NULL, OPT_INTERVALL, GROUP_R const struct optdesc opt_retry = { "retry", NULL, OPT_RETRY, GROUP_RETRY, PH_INIT, TYPE_UINT, OFUNC_EXT, XIO_OFFSETOF(retry), XIO_SIZEOF(retry) }; #endif +const struct optdesc opt_umask = { "umask", NULL, OPT_UMASK, GROUP_ADDR, PH_INIT, TYPE_MODET, OFUNC_SPEC }; diff --git a/xiolayer.h b/xiolayer.h index 5e3c65a..22a52ad 100644 --- a/xiolayer.h +++ b/xiolayer.h @@ -15,5 +15,7 @@ extern const struct optdesc opt_escape; extern const struct optdesc opt_forever; extern const struct optdesc opt_intervall; extern const struct optdesc opt_retry; +extern const struct optdesc opt_umask; +extern const struct optdesc opt_un_umask; #endif /* !defined(__xiolayer_h_included) */ diff --git a/xioopen.c b/xioopen.c index 93dba85..0477824 100644 --- a/xioopen.c +++ b/xioopen.c @@ -621,6 +621,9 @@ int xioopen_single(xiofile_t *xfd, int xioflags) { struct single *sfd = &xfd->stream; const struct addrdesc *addrdesc; const char *modetext[4] = { "none", "read-only", "write-only", "read-write" } ; + /* Values to be saved until xioopen() is finished */ + bool have_umask = false; + mode_t orig_umask, tmp_umask; int result; /* Values to be saved until xioopen() is finished */ #if WITH_RESOLVE && HAVE_RESOLV_H @@ -633,6 +636,8 @@ int xioopen_single(xiofile_t *xfd, int xioflags) { #endif int rc; + /* Apply "temporary" process properties, save value for later restore */ + if (applyopts_single(sfd, sfd->opts, PH_OFFSET) < 0) return -1; @@ -676,10 +681,23 @@ int xioopen_single(xiofile_t *xfd, int xioflags) { } xfd->stream.flags &= (~XIO_ACCMODE); xfd->stream.flags |= (xioflags & XIO_ACCMODE); + + if (retropt_mode(xfd->stream.opts, OPT_UMASK, &tmp_umask) >= 0) { + Info1("changing umask to 0%3o", tmp_umask); + orig_umask = Umask(tmp_umask); + have_umask = true; + } + result = (*addrdesc->func)(xfd->stream.argc, xfd->stream.argv, xfd->stream.opts, xioflags, xfd, addrdesc); + /* Restore process properties */ + if (have_umask) { + Info1("restoring umask to 0%3o", orig_umask); + Umask(orig_umask); + } + #if WITH_RESOLVE && HAVE_RESOLV_H if (do_res) xio_res_restore(&save_res); diff --git a/xioopts.c b/xioopts.c index 9838d87..b326211 100644 --- a/xioopts.c +++ b/xioopts.c @@ -2074,7 +2074,7 @@ int parseopts_table(const char **a, groups_t groups, struct opt **opts, } if (!(ent->desc->group & groups) && !(ent->desc->group & GROUP_ANY) && - !xioopts_ignoregroups) { + (ent->desc->group != GROUP_ADDR) && !xioopts_ignoregroups) { Error1("parseopts_table(): option \"%s\" not supported with this address type", token /*a0*/); Info2("parseopts_table() groups="F_groups_t", ent->group="F_groups_t, diff --git a/xioopts.h b/xioopts.h index 20caa23..00153ea 100644 --- a/xioopts.h +++ b/xioopts.h @@ -148,6 +148,7 @@ enum e_func { /* keep consistent with xiohelp.c:addressgroupnames[] ! */ /* a dummy group */ #define GROUP_NONE 0x00000000 +#define GROUP_ADDR 0x00000000 /* options that apply to all addresses */ #define GROUP_FD 0x00000001 /* everything applyable to a fd */ #define GROUP_FIFO 0x00000002 @@ -862,6 +863,7 @@ enum e_optcode { OPT_UNLINK_CLOSE, OPT_UNLINK_EARLY, OPT_UNLINK_LATE, + OPT_UN_UMASK, OPT_USER, OPT_USER_EARLY, OPT_USER_LATE,