From 88b0d668394c22a1097b0cb80347d839abbf804d Mon Sep 17 00:00:00 2001 From: Riccardo Murri Date: Tue, 30 May 2017 16:20:48 +0200 Subject: [PATCH 01/12] Add Makefile target for debug compilation --- Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index bda7689..1e128e8 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ -CFLAGS ?= -Wall -Werror -g +CFLAGS ?= -Wall -Werror LDFLAGS ?= PROG := su-exec @@ -13,5 +13,8 @@ $(PROG): $(SRCS) $(PROG)-static: $(SRCS) $(CC) $(CFLAGS) -o $@ $^ -static $(LDFLAGS) +$(PROG)-debug: $(SRCS) + $(CC) -g $(CFLAGS) -o $@ $^ $(LDFLAGS) + clean: - rm -f $(PROG) $(PROG)-static + rm -f $(PROG) $(PROG)-static $(PROG)-debug From f4d37b2f1cf50b9aa1cc2f2650bfae758dfe85b0 Mon Sep 17 00:00:00 2001 From: Riccardo Murri Date: Tue, 30 May 2017 16:22:35 +0200 Subject: [PATCH 02/12] Strip non-debug binaries. This brings down `su-exec` to 11kB and `su-exec-static` to 853kB on my Ubuntu 16.04 x86_64. --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 1e128e8..0eead54 100644 --- a/Makefile +++ b/Makefile @@ -9,9 +9,11 @@ all: $(PROG) $(PROG): $(SRCS) $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) + strip $@ $(PROG)-static: $(SRCS) $(CC) $(CFLAGS) -o $@ $^ -static $(LDFLAGS) + strip $@ $(PROG)-debug: $(SRCS) $(CC) -g $(CFLAGS) -o $@ $^ $(LDFLAGS) From cc18e89d7338c798f5447a1119355bf67ba14bed Mon Sep 17 00:00:00 2001 From: Andrew Ball Date: Tue, 30 May 2017 13:48:36 -0500 Subject: [PATCH 03/12] Add functionality for "make install" with dynamic linking to libc. Static linking to GNU libc is not really allowed, and I am using GNU libc for my purposes (running Java on CentOS and Fedora). Being able to do "make install" makes packaging su-exec as an RPM easier. --- Makefile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Makefile b/Makefile index bda7689..75919ea 100644 --- a/Makefile +++ b/Makefile @@ -5,6 +5,9 @@ LDFLAGS ?= PROG := su-exec SRCS := $(PROG).c +PREFIX := /usr/local +INSTALL_DIR := $(PREFIX)/bin + all: $(PROG) $(PROG): $(SRCS) @@ -13,5 +16,8 @@ $(PROG): $(SRCS) $(PROG)-static: $(SRCS) $(CC) $(CFLAGS) -o $@ $^ -static $(LDFLAGS) +install: + install -m 0755 $(PROG) $(INSTALL_DIR) + clean: rm -f $(PROG) $(PROG)-static From 5a4e10b56fe8d1f9820f3aab5636ed1f94d9f4e6 Mon Sep 17 00:00:00 2001 From: Riccardo Murri Date: Wed, 31 May 2017 13:38:06 +0200 Subject: [PATCH 04/12] git ignore also the `-static` and `-debug` variants of exe file --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index ac47793..94e08f0 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,5 @@ *.dSYM/ su-exec +su-exec-static +su-exec-debug From 41cda4e5a30adaf5b56d6143d53ead11439980d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20L=C3=B8vdal?= Date: Sat, 14 Oct 2017 13:19:16 +0200 Subject: [PATCH 05/12] Add support for DESTDIR install prefix --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5698707..2532c7f 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,7 @@ $(PROG)-debug: $(SRCS) $(CC) -g $(CFLAGS) -o $@ $^ $(LDFLAGS) install: - install -m 0755 $(PROG) $(INSTALL_DIR) + install -m 0755 $(PROG) $(DESTDIR)$(INSTALL_DIR) clean: rm -f $(PROG) $(PROG)-static $(PROG)-debug From 4912a85c95b0d471256947117d52cca28846d827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20L=C3=B8vdal?= Date: Sat, 14 Oct 2017 13:47:21 +0200 Subject: [PATCH 06/12] Make sure install dir exists --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 2532c7f..bf4f3d7 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,7 @@ $(PROG)-debug: $(SRCS) $(CC) -g $(CFLAGS) -o $@ $^ $(LDFLAGS) install: + install -d 0755 $(DESTDIR)$(INSTALL_DIR) install -m 0755 $(PROG) $(DESTDIR)$(INSTALL_DIR) clean: From 4176c2cf71b7313ba947f0434f91d776caeaee14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20L=C3=B8vdal?= Date: Sat, 14 Oct 2017 21:22:16 +0200 Subject: [PATCH 07/12] Add man page http://www.linuxjournal.com/article/1158 used as reference. --- Makefile | 3 +++ su-exec.1 | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 su-exec.1 diff --git a/Makefile b/Makefile index bf4f3d7..b70490a 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,7 @@ SRCS := $(PROG).c PREFIX := /usr/local INSTALL_DIR := $(PREFIX)/bin +MAN_DIR := $(PREFIX)/share/man/man8 all: $(PROG) @@ -24,6 +25,8 @@ $(PROG)-debug: $(SRCS) install: install -d 0755 $(DESTDIR)$(INSTALL_DIR) install -m 0755 $(PROG) $(DESTDIR)$(INSTALL_DIR) + install -d 0755 $(DESTDIR)$(MAN_DIR) + install -m 0644 su-exec.1 $(DESTDIR)$(MAN_DIR) clean: rm -f $(PROG) $(PROG)-static $(PROG)-debug diff --git a/su-exec.1 b/su-exec.1 new file mode 100644 index 0000000..47e809b --- /dev/null +++ b/su-exec.1 @@ -0,0 +1,59 @@ +.TH SU-EXEC 8 "14 Oct 2017" + +.SH NAME +su-exec \- change user id and group id before executing a program + +.SH SYNOPSIS +\fBsu-exec\fP \fIuser-spec\fP \fIcommand\fP [ \fIarguments...\fP ] + +.SH DESCRIPTION +\fBsu-exec\fP executes a program with modified privileges. The program +will be exceuted directly and not run as a child, like su and sudo does, +which avoids TTY and signal issues. + +Notice that su-exec depends on being run by the root user, non-root +users do not have permission to change uid/gid. + +.SH OPTIONS +.TP +\fIuser-spec\fP +is either a user name (e.g. \fBnobody\fP) or user name and group name +separated with colon (e.g. \fBnobody:ftp\fP). Numeric uid/gid values +can be used instead of names. + +.TP +\fIcommand\fP +is the program to execute. Can be either absolute or relative path. + +.SH EXAMPLES + +.TP +Execute httpd as user \fIapache\fP and gid value 1000 with the two specified arguments: + +$ \fBsu-exec apache:1000 /usr/sbin/httpd -f /opt/www/httpd.conf\fP + +.SH ENVIRONMENT VARIABLES + +.TP +\fBHOME\fP +Is updated to the value matching the user entry in \fC/etc/passwd\fP. + +.TP +\fBPATH\fP +Is used for searching for the program to execute. + +Since su-exec is not running as a suid binary, the dynamic linker or +libc will not strip or ignore variables like LD_LIBRARY_PATH etc. + +.SH EXIT STATUS +.TP +\fB1\fP +If \fbsu-exec\fR fails to change priveledges or execute the program it +will return \fB1\fP. In the successfull case the exit value will be +whatever the executed program returns. + +.SH "SEE ALSO" +su(1), runuser(8), sudo(8), gosu(1) + +.SH BUGS +\fBUSER\fP and \fBLOGNAME\fP environmental variables are not updated. From d50ad68002797177532056da048a59075ddc286b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20L=C3=B8vdal?= Date: Sun, 15 Oct 2017 00:51:30 +0200 Subject: [PATCH 08/12] Add option for license printing Addresses https://github.com/ncopa/su-exec/issues/7 --- .gitignore | 1 + Makefile | 19 ++++++++++++------- su-exec.1 | 10 ++++++++++ su-exec.c | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 94e08f0..bbf718a 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,4 @@ su-exec su-exec-static su-exec-debug +license.inc diff --git a/Makefile b/Makefile index b70490a..d53af98 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,7 @@ LDFLAGS ?= PROG := su-exec SRCS := $(PROG).c +INCS := license.inc PREFIX := /usr/local INSTALL_DIR := $(PREFIX)/bin @@ -11,16 +12,19 @@ MAN_DIR := $(PREFIX)/share/man/man8 all: $(PROG) -$(PROG): $(SRCS) - $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) +license.inc: LICENSE + xxd -i $^ > $@ + +$(PROG): $(SRCS) $(INCS) + $(CC) $(CFLAGS) -o $@ $(SRCS) $(LDFLAGS) strip $@ -$(PROG)-static: $(SRCS) - $(CC) $(CFLAGS) -o $@ $^ -static $(LDFLAGS) +$(PROG)-static: $(SRCS) $(INCS) + $(CC) $(CFLAGS) -o $@ $(SRCS) -static $(LDFLAGS) strip $@ -$(PROG)-debug: $(SRCS) - $(CC) -g $(CFLAGS) -o $@ $^ $(LDFLAGS) +$(PROG)-debug: $(SRCS) $(INCS) + $(CC) -g $(CFLAGS) -o $@ $(SRCS) $(LDFLAGS) install: install -d 0755 $(DESTDIR)$(INSTALL_DIR) @@ -29,4 +33,5 @@ install: install -m 0644 su-exec.1 $(DESTDIR)$(MAN_DIR) clean: - rm -f $(PROG) $(PROG)-static $(PROG)-debug + rm -f $(PROG) $(PROG)-static $(PROG)-debug $(INCS) + diff --git a/su-exec.1 b/su-exec.1 index 47e809b..67cf2be 100644 --- a/su-exec.1 +++ b/su-exec.1 @@ -6,6 +6,8 @@ su-exec \- change user id and group id before executing a program .SH SYNOPSIS \fBsu-exec\fP \fIuser-spec\fP \fIcommand\fP [ \fIarguments...\fP ] +\fBsu-exec\fP \fI-l\fP + .SH DESCRIPTION \fBsu-exec\fP executes a program with modified privileges. The program will be exceuted directly and not run as a child, like su and sudo does, @@ -25,6 +27,10 @@ can be used instead of names. \fIcommand\fP is the program to execute. Can be either absolute or relative path. +.TP +\fI-l\fP +Print license information and exits. + .SH EXAMPLES .TP @@ -46,6 +52,10 @@ Since su-exec is not running as a suid binary, the dynamic linker or libc will not strip or ignore variables like LD_LIBRARY_PATH etc. .SH EXIT STATUS +.TP +\fB0\fP +When printing license information. + .TP \fB1\fP If \fbsu-exec\fR fails to change priveledges or execute the program it diff --git a/su-exec.c b/su-exec.c index 176bbf2..a3c2655 100644 --- a/su-exec.c +++ b/su-exec.c @@ -11,14 +11,24 @@ #include #include + static char *argv0; static void usage(int exitcode) { printf("Usage: %s user-spec command [args]\n", argv0); + printf("Usage: %s -l\n\tShows license.\n", argv0); exit(exitcode); } +#include "license.inc" +static void print_license() +{ + unsigned int i; + for (i=0; i Date: Sat, 14 Oct 2017 22:27:54 +0200 Subject: [PATCH 09/12] Ignore vim swap files --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index bbf718a..016e97d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ +# Vim swap files +.*.swp + # Object files *.o *.ko From 62cc5b0d7c4fc71965670e63a320d07022008fc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20L=C3=B8vdal?= Date: Sun, 15 Oct 2017 00:19:27 +0200 Subject: [PATCH 10/12] Use exit code 1 in case of too few arguments --- su-exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/su-exec.c b/su-exec.c index a3c2655..40962b8 100644 --- a/su-exec.c +++ b/su-exec.c @@ -43,7 +43,7 @@ int main(int argc, char *argv[]) return 0; } if (argc < 3) - usage(0); + usage(1); user = argv[1]; group = strchr(user, ':'); From 161380c565ff480c86f1770d04ac1d7b4530cdf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20L=C3=B8vdal?= Date: Sun, 15 Oct 2017 01:02:07 +0200 Subject: [PATCH 11/12] Use EXIT_SUCCESS + EXIT_FAILURE --- su-exec.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/su-exec.c b/su-exec.c index 40962b8..fa1bbe3 100644 --- a/su-exec.c +++ b/su-exec.c @@ -40,10 +40,10 @@ int main(int argc, char *argv[]) argv0 = argv[0]; if (argc == 2 && strcmp(argv[1], "-l") == 0) { print_license(); - return 0; + return EXIT_SUCCESS; } if (argc < 3) - usage(1); + usage(EXIT_FAILURE); user = argv[1]; group = strchr(user, ':'); @@ -88,7 +88,7 @@ int main(int argc, char *argv[]) if (pw == NULL) { if (setgroups(1, &gid) < 0) - err(1, "setgroups(%i)", gid); + err(EXIT_FAILURE, "setgroups(%i)", gid); } else { int ngroups = 0; gid_t *glist = NULL; @@ -98,24 +98,24 @@ int main(int argc, char *argv[]) if (r >= 0) { if (setgroups(ngroups, glist) < 0) - err(1, "setgroups"); + err(EXIT_FAILURE, "setgroups"); break; } glist = realloc(glist, ngroups * sizeof(gid_t)); if (glist == NULL) - err(1, "malloc"); + err(EXIT_FAILURE, "malloc"); } } if (setgid(gid) < 0) - err(1, "setgid(%i)", gid); + err(EXIT_FAILURE, "setgid(%i)", gid); if (setuid(uid) < 0) - err(1, "setuid(%i)", uid); + err(EXIT_FAILURE, "setuid(%i)", uid); execvp(cmdargv[0], cmdargv); - err(1, "%s", cmdargv[0]); + err(EXIT_FAILURE, "%s", cmdargv[0]); - return 1; + return EXIT_FAILURE; } From 898b79af4cd0839c22528a4f01b1fa35ee1ae5fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20L=C3=B8vdal?= Date: Sun, 15 Oct 2017 14:04:07 +0200 Subject: [PATCH 12/12] Print an extra, descriptive warning for EPERM The far most likely cause for EPERM is that people try to run su-exec as a non-root user, so help them understand this is not possible. --- su-exec.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/su-exec.c b/su-exec.c index fa1bbe3..955ff5d 100644 --- a/su-exec.c +++ b/su-exec.c @@ -29,6 +29,26 @@ static void print_license() putchar(LICENSE[i]); } +#ifdef linux +#define WARNING_setgroups " (on Linux you need CAP_SETGID)" +#define WARNING_setgid " (on Linux you need CAP_SETGID)" +#define WARNING_setuid " (on Linux you need CAP_SETUID)" +#else +#define WARNING_setgroups "" +#define WARNING_setgid "" +#define WARNING_setuid "" +#endif + +static void print_eperm_warning(const char *extra_warn) +{ + int saved_errno; + if (errno != EPERM) + return; + saved_errno = errno; + fprintf(stderr, "Insufficient privilege, %s needs to run as root%s.\n", argv0, extra_warn); + errno = saved_errno; +} + int main(int argc, char *argv[]) { char *user, *group, **cmdargv; @@ -87,8 +107,10 @@ int main(int argc, char *argv[]) } if (pw == NULL) { - if (setgroups(1, &gid) < 0) + if (setgroups(1, &gid) < 0) { + print_eperm_warning(WARNING_setgroups); err(EXIT_FAILURE, "setgroups(%i)", gid); + } } else { int ngroups = 0; gid_t *glist = NULL; @@ -97,8 +119,10 @@ int main(int argc, char *argv[]) int r = getgrouplist(pw->pw_name, gid, glist, &ngroups); if (r >= 0) { - if (setgroups(ngroups, glist) < 0) + if (setgroups(ngroups, glist) < 0) { + print_eperm_warning(WARNING_setgroups); err(EXIT_FAILURE, "setgroups"); + } break; } @@ -108,11 +132,15 @@ int main(int argc, char *argv[]) } } - if (setgid(gid) < 0) + if (setgid(gid) < 0) { + print_eperm_warning(WARNING_setgid); err(EXIT_FAILURE, "setgid(%i)", gid); + } - if (setuid(uid) < 0) + if (setuid(uid) < 0) { + print_eperm_warning(WARNING_setuid); err(EXIT_FAILURE, "setuid(%i)", uid); + } execvp(cmdargv[0], cmdargv); err(EXIT_FAILURE, "%s", cmdargv[0]);