commit f2fc8ce0a3b225e5408c9b26476e395ca7109e63 from: Stefan Sperling via: Thomas Adam date: Fri Jan 06 09:33:00 2023 UTC remove the gotsh group requirement from gotd; any user can now connect Repository access is now controlled by access rules in gotd.conf, and concurrent connections to the gotd socket by local users are limited by the listen process. We should keep refining our anti-DoS measures in the future, but at least we have something in place now. ok jamsek, op commit - efe3fd03c530b4b4e583a61d9106ffc560ba9bea commit + f2fc8ce0a3b225e5408c9b26476e395ca7109e63 blob - f5cb18388b02c2401ec5902ed38ff9848ee2ffe7 blob + fb2ac445012f43434ddd2dce19216c1e024287cf --- gotd/gotd.c +++ gotd/gotd.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -141,7 +140,7 @@ unix_socket_listen(const char *unix_socket_path, uid_t } old_umask = umask(S_IXUSR|S_IXGRP|S_IWOTH|S_IROTH|S_IXOTH); - mode = S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP; + mode = S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH; if (bind(fd, (struct sockaddr *)&sun, sizeof(sun)) == -1) { log_warn("bind: %s", unix_socket_path); @@ -174,25 +173,6 @@ unix_socket_listen(const char *unix_socket_path, uid_t } return fd; -} - -static struct group * -match_group(gid_t *groups, int ngroups, const char *unix_group_name) -{ - struct group *gr; - int i; - - for (i = 0; i < ngroups; i++) { - gr = getgrgid(groups[i]); - if (gr == NULL) { - log_warn("getgrgid %d", groups[i]); - continue; - } - if (strcmp(gr->gr_name, unix_group_name) == 0) - return gr; - } - - return NULL; } static uint64_t @@ -2405,10 +2385,7 @@ main(int argc, char **argv) const char *confpath = GOTD_CONF_PATH; char *argv0 = argv[0]; char title[2048]; - gid_t groups[NGROUPS_MAX]; - int ngroups = NGROUPS_MAX; struct passwd *pw = NULL; - struct group *gr = NULL; char *repo_path = NULL; enum gotd_procid proc_id = PROC_GOTD; struct event evsigint, evsigterm, evsighup, evsigusr1; @@ -2491,23 +2468,6 @@ main(int argc, char **argv) getprogname(), pw->pw_name, getprogname()); } - if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1) - log_warnx("group membership list truncated"); - - gr = match_group(groups, ngroups, gotd.unix_group_name); - if (gr == NULL) { - fatalx("cannot start %s: the user running %s " - "must be a secondary member of group %s", - getprogname(), getprogname(), gotd.unix_group_name); - } - if (gr->gr_gid == pw->pw_gid) { - fatalx("cannot start %s: the user running %s " - "must be a secondary member of group %s, but " - "%s is the user's primary group", - getprogname(), getprogname(), gotd.unix_group_name, - gotd.unix_group_name); - } - if (proc_id == PROC_LISTEN && !got_path_is_absolute(gotd.unix_socket_path)) fatalx("bad unix socket path \"%s\": must be an absolute path", @@ -2528,11 +2488,10 @@ main(int argc, char **argv) if (verbosity) { log_info("socket: %s", gotd.unix_socket_path); log_info("user: %s", pw->pw_name); - log_info("secondary group: %s", gr->gr_name); } fd = unix_socket_listen(gotd.unix_socket_path, pw->pw_uid, - gr->gr_gid); + pw->pw_gid); if (fd == -1) { fatal("cannot listen on unix socket %s", gotd.unix_socket_path); blob - a45eb65170386548a9b8a2b7812f7309eebdc6f6 blob + ccde19f88e7b40b792b110a9d7de24f505e67cae --- gotd/gotd.conf.5 +++ gotd/gotd.conf.5 @@ -92,19 +92,6 @@ should listen on. If not specified, the path .Pa /var/run/gotd.sock will be used. -.It Ic unix_group Ar group -Set the -.Ar group , -defined in the -.Xr group 5 -file, which is allowed to access -.Xr gotd 8 -via -.Xr gotsh 1 . -The -.Xr gotd 8 -user must be a secondary member of this group. -If not specified, the group _gotsh will be used. .It Ic user Ar user Set the .Ar user @@ -195,8 +182,7 @@ configuration file. .El .Sh EXAMPLES .Bd -literal -offset indent -# Default unix_group and user values: -unix_group _gotsh +# Run as the default user: user _gotd # This repository can be accessed via ssh://user@example.com/src @@ -228,5 +214,4 @@ connection { .Sh SEE ALSO .Xr got 1 , .Xr gotsh 1 , -.Xr group 5 , .Xr gotd 8 blob - 82485863fa9ea61c5334cb5ecd4b70eac7128cb4 blob + 266fba3b38a61177a81510a5a8753084f64b1667 --- gotd/gotd.h +++ gotd/gotd.h @@ -17,7 +17,6 @@ #define GOTD_UNIX_SOCKET "/var/run/gotd.sock" #define GOTD_UNIX_SOCKET_BACKLOG 10 -#define GOTD_UNIX_GROUP "_gotsh" #define GOTD_USER "_gotd" #define GOTD_CONF_PATH "/etc/gotd.conf" #define GOTD_EMPTY_PATH "/var/empty" @@ -119,7 +118,6 @@ struct gotd_uid_connection_limit { struct gotd { pid_t pid; char unix_socket_path[PATH_MAX]; - char unix_group_name[32]; char user_name[32]; struct gotd_repolist repos; int nrepos; blob - d8319e4be5f8185780729a14a4a706eb136762d2 blob + 2103f5caa2ef025a6e22f6667363ba2268844af0 --- gotd/parse.y +++ gotd/parse.y @@ -104,7 +104,7 @@ typedef struct { %} -%token PATH ERROR ON UNIX_SOCKET UNIX_GROUP USER REPOSITORY PERMIT DENY +%token PATH ERROR ON UNIX_SOCKET USER REPOSITORY PERMIT DENY %token RO RW CONNECTION LIMIT REQUEST TIMEOUT %token STRING @@ -207,17 +207,6 @@ main : UNIX_SOCKET STRING { } free($2); } - | UNIX_GROUP STRING { - if (strlcpy(gotd->unix_group_name, $2, - sizeof(gotd->unix_group_name)) >= - sizeof(gotd->unix_group_name)) { - yyerror("%s: unix group name too long", - __func__); - free($2); - YYERROR; - } - free($2); - } | USER STRING { if (strlcpy(gotd->user_name, $2, sizeof(gotd->user_name)) >= @@ -372,7 +361,6 @@ lookup(char *s) { "ro", RO }, { "rw", RW }, { "timeout", TIMEOUT }, - { "unix_group", UNIX_GROUP }, { "unix_socket", UNIX_SOCKET }, { "user", USER }, }; @@ -715,11 +703,6 @@ parse_config(const char *filename, enum gotd_procid pr fprintf(stderr, "%s: unix socket path too long", __func__); return -1; } - if (strlcpy(gotd->unix_group_name, GOTD_UNIX_GROUP, - sizeof(gotd->unix_group_name)) >= sizeof(gotd->unix_group_name)) { - fprintf(stderr, "%s: unix group name too long", __func__); - return -1; - } if (strlcpy(gotd->user_name, GOTD_USER, sizeof(gotd->user_name)) >= sizeof(gotd->user_name)) { fprintf(stderr, "%s: user name too long", __func__); blob - 436c016514b9a9589c787504047ecff6003477d9 blob + 8e552939759f9d99f850f531a4a7041b8aaa2956 --- gotsh/gotsh.1 +++ gotsh/gotsh.1 @@ -66,13 +66,6 @@ program running on the client machine. .Pp Users running .Nm -must be members of the group which has read/write permission to the -.Xr gotd 8 -unix socket. -The group used for this purpose can be configured in -.Xr gotd.conf 5 . -Users running -.Nm should not have access to Git repositories by means other than accessing the unix socket of .Xr gotd 8 @@ -97,22 +90,24 @@ If not specified, the default path will be used. .El .Sh EXAMPLES -The following .Xr sshd_config 5 -directives are recommended to protect the server machine and any systems -reachable from it via -.Xr ssh 1 -forwarding features. -This example assumes the group called -.Dq _gotsh -has read/write access to the -.Xr gotd 8 -unix socket. +directives such as the following are recommended to protect the server +machine and any systems reachable from it, especially if anonymous users +are allowed to connect: .Bd -literal -offset indent -Match Group _gotsh +Match User anonymous DisableForwarding yes PermitTTY no .Ed +.Pp +It can be convenient to add all relevant users to a common group, such as +.Dq developers , +and then use this group as the Match criteria: +.Bd -literal -offset indent +Match Group developers + DisableForwarding yes + PermitTTY no +.Ed .Sh SEE ALSO .Xr got 1 , .Xr ssh 1 , blob - 0fe6926134b97333a0a4a221f988bb8f85ab7242 blob + c43ea128c1006bf4ed809106ce3ea7d8811bbf90 --- regress/gotd/Makefile +++ regress/gotd/Makefile @@ -25,7 +25,6 @@ GOTD_TEST_USER_HOME!=userinfo $(GOTD_TEST_USER) | awk # gotd.conf parameters GOTD_USER?=got -GOTD_GROUP?=gotsh GOTD_SOCK=${GOTD_DEVUSER_HOME}/gotd.sock .if "${GOT_RELEASE}" == "Yes" @@ -60,7 +59,6 @@ ensure_root: start_gotd_ro: ensure_root @echo 'unix_socket "$(GOTD_SOCK)"' > $(PWD)/gotd.conf - @echo "unix_group $(GOTD_GROUP)" >> $(PWD)/gotd.conf @echo "user $(GOTD_USER)" >> $(PWD)/gotd.conf @echo 'repository "test-repo" {' >> $(PWD)/gotd.conf @echo ' path "$(GOTD_TEST_REPO)"' >> $(PWD)/gotd.conf @@ -71,7 +69,6 @@ start_gotd_ro: ensure_root start_gotd_ro_group: ensure_root @echo 'unix_socket "$(GOTD_SOCK)"' > $(PWD)/gotd.conf - @echo "unix_group $(GOTD_GROUP)" >> $(PWD)/gotd.conf @echo "user $(GOTD_USER)" >> $(PWD)/gotd.conf @echo 'repository "test-repo" {' >> $(PWD)/gotd.conf @echo ' path "$(GOTD_TEST_REPO)"' >> $(PWD)/gotd.conf @@ -83,7 +80,6 @@ start_gotd_ro_group: ensure_root # try a permit rule followed by a deny rule; last matched rule wins start_gotd_ro_denied_user: ensure_root @echo 'unix_socket "$(GOTD_SOCK)"' > $(PWD)/gotd.conf - @echo "unix_group $(GOTD_GROUP)" >> $(PWD)/gotd.conf @echo "user $(GOTD_USER)" >> $(PWD)/gotd.conf @echo 'repository "test-repo" {' >> $(PWD)/gotd.conf @echo ' path "$(GOTD_TEST_REPO)"' >> $(PWD)/gotd.conf @@ -96,7 +92,6 @@ start_gotd_ro_denied_user: ensure_root # try a permit rule followed by a deny rule; last matched rule wins start_gotd_ro_denied_group: ensure_root @echo 'unix_socket "$(GOTD_SOCK)"' > $(PWD)/gotd.conf - @echo "unix_group $(GOTD_GROUP)" >> $(PWD)/gotd.conf @echo "user $(GOTD_USER)" >> $(PWD)/gotd.conf @echo 'repository "test-repo" {' >> $(PWD)/gotd.conf @echo ' path "$(GOTD_TEST_REPO)"' >> $(PWD)/gotd.conf @@ -109,7 +104,6 @@ start_gotd_ro_denied_group: ensure_root # $GOTD_DEVUSER should not equal $GOTD_USER start_gotd_ro_bad_user: ensure_root @echo 'unix_socket "$(GOTD_SOCK)"' > $(PWD)/gotd.conf - @echo "unix_group $(GOTD_GROUP)" >> $(PWD)/gotd.conf @echo "user $(GOTD_USER)" >> $(PWD)/gotd.conf @echo 'repository "test-repo" {' >> $(PWD)/gotd.conf @echo ' path "$(GOTD_TEST_REPO)"' >> $(PWD)/gotd.conf @@ -121,7 +115,6 @@ start_gotd_ro_bad_user: ensure_root # $GOTD_DEVUSER should not be in group wheel start_gotd_ro_bad_group: ensure_root @echo 'unix_socket "$(GOTD_SOCK)"' > $(PWD)/gotd.conf - @echo "unix_group $(GOTD_GROUP)" >> $(PWD)/gotd.conf @echo "user $(GOTD_USER)" >> $(PWD)/gotd.conf @echo 'repository "test-repo" {' >> $(PWD)/gotd.conf @echo ' path "$(GOTD_TEST_REPO)"' >> $(PWD)/gotd.conf @@ -132,7 +125,6 @@ start_gotd_ro_bad_group: ensure_root start_gotd_rw: ensure_root @echo 'unix_socket "$(GOTD_SOCK)"' > $(PWD)/gotd.conf - @echo "unix_group $(GOTD_GROUP)" >> $(PWD)/gotd.conf @echo "user $(GOTD_USER)" >> $(PWD)/gotd.conf @echo 'repository "test-repo" {' >> $(PWD)/gotd.conf @echo ' path "$(GOTD_TEST_REPO)"' >> $(PWD)/gotd.conf blob - 09b2d5993cc7481df50e85fbda4705700de997bd blob + d75faae64b46a7c347ecb77c6631207a3ded44b0 --- regress/gotd/README +++ regress/gotd/README @@ -1,13 +1,12 @@ Running server regression tests requires some manual system preparation. -Two dedicated user accounts and a group must be created. Password login +Two dedicated user accounts must be created. Password login for these users should be disabled. - $ doas groupadd gotsh - $ doas useradd -m -G gotsh got - $ doas useradd -m -G gotsh gotdev + $ doas useradd -m got + $ doas useradd -m gotdev -The above user and group names correspond to defaults used by the test suite. +The above user names correspond to defaults used by the test suite. If needed, the defaults can be overridden on by passing values for the following variables to make(1): GOTD_USER, GOTD_DEVUSER, GOTD_GROUP