Download raw body.
gotd session should unveil repository read-only during fetches
I have vague plans to split up the "session" process further but I don't
know when I will find time for doing this.
But as a quick fix it occurred to me that we should be using unveil to
prevent modifications to repositories via the session process while
serving fetches. With this change nothing in gotd has write access to
repositories anymore while serving fetches.
A relatively simple change for increased safety, especially when the
server is offering public access.
However, the /tmp directory must remain read-write because it is needed
to generate pack files.
Tests are still passing and I have this running on got.g.o successfully.
ok?
-----------------------------------------------
unveil repositories read-only in gotd session process while serving fetches
diff 3bf0e21f50b11c683f08a06c8ab362fe220adc2b bd09fefe23a9666e4bee568b524c754e9538e0f4
commit - 3bf0e21f50b11c683f08a06c8ab362fe220adc2b
commit + bd09fefe23a9666e4bee568b524c754e9538e0f4
blob - 23fb7de78a4d49528a35ce6f683badab359d83bc
blob + 3f8d5ac1fd5fd5045c444b77b273f009515c0dc4
--- gotd/gotd.c
+++ gotd/gotd.c
@@ -834,7 +834,8 @@ verify_imsg_src(struct gotd_client *client, struct got
return 0;
}
}
- if (proc->type == PROC_SESSION) {
+ if (proc->type == PROC_SESSION_READ ||
+ proc->type == PROC_SESSION_WRITE) {
if (client->session == NULL) {
log_warnx("no session found for uid %d", client->euid);
return 0;
@@ -871,7 +872,8 @@ verify_imsg_src(struct gotd_client *client, struct got
ret = 1;
break;
case GOTD_IMSG_CLIENT_SESSION_READY:
- if (proc->type != PROC_SESSION) {
+ if (proc->type != PROC_SESSION_READ &&
+ proc->type != PROC_SESSION_WRITE) {
err = got_error_fmt(GOT_ERR_BAD_PACKET,
"unexpected \"ready\" signal from PID %d",
proc->pid);
@@ -1446,7 +1448,10 @@ start_child(enum gotd_procid proc_id, const char *repo
case PROC_AUTH:
argv[argc++] = (char *)"-A";
break;
- case PROC_SESSION:
+ case PROC_SESSION_READ:
+ argv[argc++] = (char *)"-s";
+ break;
+ case PROC_SESSION_WRITE:
argv[argc++] = (char *)"-S";
break;
case PROC_REPO_READ:
@@ -1508,7 +1513,10 @@ start_session_child(struct gotd_client *client, struct
if (proc == NULL)
return got_error_from_errno("calloc");
- proc->type = PROC_SESSION;
+ if (client_is_reading(client))
+ proc->type = PROC_SESSION_READ;
+ else
+ proc->type = PROC_SESSION_WRITE;
if (strlcpy(proc->repo_name, repo->name,
sizeof(proc->repo_name)) >= sizeof(proc->repo_name))
fatalx("repository name too long: %s", repo->name);
@@ -1645,8 +1653,13 @@ apply_unveil_repo_readonly(const char *repo_path)
}
static void
-apply_unveil_repo_readonly(const char *repo_path)
+apply_unveil_repo_readonly(const char *repo_path, int need_tmpdir)
{
+ if (need_tmpdir) {
+ if (unveil(GOT_TMPDIR_STR, "rwc") == -1)
+ fatal("unveil %s", GOT_TMPDIR_STR);
+ }
+
if (unveil(repo_path, "r") == -1)
fatal("unveil %s", repo_path);
@@ -1704,7 +1717,7 @@ main(int argc, char **argv)
log_init(1, LOG_DAEMON); /* Log to stderr until daemonized. */
- while ((ch = getopt(argc, argv, "Adf:LnP:RSvW")) != -1) {
+ while ((ch = getopt(argc, argv, "Adf:LnP:RsSvW")) != -1) {
switch (ch) {
case 'A':
proc_id = PROC_AUTH;
@@ -1729,8 +1742,11 @@ main(int argc, char **argv)
case 'R':
proc_id = PROC_REPO_READ;
break;
+ case 's':
+ proc_id = PROC_SESSION_READ;
+ break;
case 'S':
- proc_id = PROC_SESSION;
+ proc_id = PROC_SESSION_WRITE;
break;
case 'v':
if (verbosity < 3)
@@ -1804,7 +1820,7 @@ main(int argc, char **argv)
snprintf(title, sizeof(title), "%s %s",
gotd_proc_names[proc_id], repo_path);
} else if (proc_id == PROC_REPO_READ || proc_id == PROC_REPO_WRITE ||
- proc_id == PROC_SESSION) {
+ proc_id == PROC_SESSION_READ || proc_id == PROC_SESSION_WRITE) {
error = got_repo_pack_fds_open(&pack_fds);
if (error != NULL)
fatalx("cannot open pack tempfiles: %s", error->msg);
@@ -1868,7 +1884,8 @@ main(int argc, char **argv)
auth_main(title, &gotd.repos, repo_path);
/* NOTREACHED */
break;
- case PROC_SESSION:
+ case PROC_SESSION_READ:
+ case PROC_SESSION_WRITE:
#ifndef PROFILE
/*
* The "recvfd" promise is only needed during setup and
@@ -1878,9 +1895,12 @@ main(int argc, char **argv)
"unveil", NULL) == -1)
err(1, "pledge");
#endif
- apply_unveil_repo_readwrite(repo_path);
+ if (proc_id == PROC_SESSION_READ)
+ apply_unveil_repo_readonly(repo_path, 1);
+ else
+ apply_unveil_repo_readwrite(repo_path);
session_main(title, repo_path, pack_fds, temp_fds,
- &gotd.request_timeout);
+ &gotd.request_timeout, proc_id);
/* NOTREACHED */
break;
case PROC_REPO_READ:
@@ -1888,7 +1908,7 @@ main(int argc, char **argv)
if (pledge("stdio rpath recvfd unveil", NULL) == -1)
err(1, "pledge");
#endif
- apply_unveil_repo_readonly(repo_path);
+ apply_unveil_repo_readonly(repo_path, 0);
repo_read_main(title, repo_path, pack_fds, temp_fds);
/* NOTREACHED */
exit(0);
@@ -1897,7 +1917,7 @@ main(int argc, char **argv)
if (pledge("stdio rpath recvfd unveil", NULL) == -1)
err(1, "pledge");
#endif
- apply_unveil_repo_readonly(repo_path);
+ apply_unveil_repo_readonly(repo_path, 0);
repo = gotd_find_repo_by_path(repo_path, &gotd);
if (repo == NULL)
fatalx("no repository for path %s", repo_path);
blob - b08623dd82c94711edc63cc23126b9a581a45c31
blob + 35d6e95f4881eb3c4ce60156fce684fa03070cb4
--- gotd/gotd.h
+++ gotd/gotd.h
@@ -36,7 +36,8 @@ enum gotd_procid {
PROC_GOTD = 0,
PROC_LISTEN,
PROC_AUTH,
- PROC_SESSION,
+ PROC_SESSION_READ,
+ PROC_SESSION_WRITE,
PROC_REPO_READ,
PROC_REPO_WRITE,
PROC_MAX,
blob - 2818e0cdca9ab6d00ae35b461e2289cb82f51255
blob + fb1301e73a4f0c21dffc335b5f0247e24b31a441
--- gotd/session.c
+++ gotd/session.c
@@ -62,6 +62,7 @@ static struct gotd_session {
int *temp_fds;
struct gotd_imsgev parent_iev;
struct timeval request_timeout;
+ enum gotd_procid proc_id;
} gotd_session;
static struct gotd_session_client {
@@ -93,7 +94,7 @@ disconnect(struct gotd_session_client *client)
log_debug("uid %d: disconnecting", client->euid);
if (gotd_imsg_compose_event(&gotd_session.parent_iev,
- GOTD_IMSG_DISCONNECT, PROC_SESSION, -1, NULL, 0) == -1)
+ GOTD_IMSG_DISCONNECT, gotd_session.proc_id, -1, NULL, 0) == -1)
log_warn("imsg compose DISCONNECT");
imsg_clear(&client->repo_child_iev.ibuf);
@@ -126,7 +127,7 @@ disconnect_on_error(struct gotd_session_client *client
log_warnx("uid %d: %s", client->euid, err->msg);
if (err->code != GOT_ERR_EOF) {
imsg_init(&ibuf, client->fd);
- gotd_imsg_send_error(&ibuf, 0, PROC_SESSION, err);
+ gotd_imsg_send_error(&ibuf, 0, gotd_session.proc_id, err);
imsg_clear(&ibuf);
}
@@ -252,7 +253,7 @@ send_ref_update_ok(struct gotd_session_client *client,
len = sizeof(iok) + iok.name_len;
wbuf = imsg_create(&iev->ibuf, GOTD_IMSG_REF_UPDATE_OK,
- PROC_SESSION, gotd_session.pid, len);
+ gotd_session.proc_id, gotd_session.pid, len);
if (wbuf == NULL)
return got_error_from_errno("imsg_create REF_UPDATE_OK");
@@ -271,7 +272,7 @@ send_refs_updated(struct gotd_session_client *client)
send_refs_updated(struct gotd_session_client *client)
{
if (gotd_imsg_compose_event(&client->iev, GOTD_IMSG_REFS_UPDATED,
- PROC_SESSION, -1, NULL, 0) == -1)
+ gotd_session.proc_id, -1, NULL, 0) == -1)
log_warn("imsg compose REFS_UPDATED");
}
@@ -297,7 +298,7 @@ send_ref_update_ng(struct gotd_session_client *client,
len = sizeof(ing) + ing.name_len + ing.reason_len;
wbuf = imsg_create(&iev->ibuf, GOTD_IMSG_REF_UPDATE_NG,
- PROC_SESSION, gotd_session.pid, len);
+ gotd_session.proc_id, gotd_session.pid, len);
if (wbuf == NULL)
return got_error_from_errno("imsg_create REF_UPDATE_NG");
@@ -780,7 +781,7 @@ forward_want(struct gotd_session_client *client, struc
iwant.client_id = client->id;
if (gotd_imsg_compose_event(&client->repo_child_iev, GOTD_IMSG_WANT,
- PROC_SESSION, -1, &iwant, sizeof(iwant)) == -1)
+ gotd_session.proc_id, -1, &iwant, sizeof(iwant)) == -1)
return got_error_from_errno("imsg compose WANT");
return NULL;
@@ -808,7 +809,8 @@ forward_ref_update(struct gotd_session_client *client,
iref->client_id = client->id;
if (gotd_imsg_compose_event(&client->repo_child_iev,
- GOTD_IMSG_REF_UPDATE, PROC_SESSION, -1, iref, datalen) == -1)
+ GOTD_IMSG_REF_UPDATE, gotd_session.proc_id, -1,
+ iref, datalen) == -1)
err = got_error_from_errno("imsg compose REF_UPDATE");
free(iref);
return err;
@@ -832,7 +834,7 @@ forward_have(struct gotd_session_client *client, struc
ihave.client_id = client->id;
if (gotd_imsg_compose_event(&client->repo_child_iev, GOTD_IMSG_HAVE,
- PROC_SESSION, -1, &ihave, sizeof(ihave)) == -1)
+ gotd_session.proc_id, -1, &ihave, sizeof(ihave)) == -1)
return got_error_from_errno("imsg compose HAVE");
return NULL;
@@ -880,7 +882,7 @@ recv_packfile(struct gotd_session_client *client)
/* Send pack pipe end 0 to repo child process. */
if (gotd_imsg_compose_event(&client->repo_child_iev,
- GOTD_IMSG_PACKFILE_PIPE, PROC_SESSION, pipe[0],
+ GOTD_IMSG_PACKFILE_PIPE, gotd_session.proc_id, pipe[0],
&ipipe, sizeof(ipipe)) == -1) {
err = got_error_from_errno("imsg compose PACKFILE_PIPE");
pipe[0] = -1;
@@ -890,7 +892,8 @@ recv_packfile(struct gotd_session_client *client)
/* Send pack pipe end 1 to gotsh(1) (expects just an fd, no data). */
if (gotd_imsg_compose_event(&client->iev,
- GOTD_IMSG_PACKFILE_PIPE, PROC_SESSION, pipe[1], NULL, 0) == -1)
+ GOTD_IMSG_PACKFILE_PIPE, gotd_session.proc_id, pipe[1],
+ NULL, 0) == -1)
err = got_error_from_errno("imsg compose PACKFILE_PIPE");
pipe[1] = -1;
@@ -928,7 +931,7 @@ recv_packfile(struct gotd_session_client *client)
memset(&ifile, 0, sizeof(ifile));
ifile.client_id = client->id;
if (gotd_imsg_compose_event(&client->repo_child_iev,
- GOTD_IMSG_PACKIDX_FILE, PROC_SESSION,
+ GOTD_IMSG_PACKIDX_FILE, gotd_session.proc_id,
idxfd, &ifile, sizeof(ifile)) == -1) {
err = got_error_from_errno("imsg compose PACKIDX_FILE");
idxfd = -1;
@@ -942,7 +945,7 @@ recv_packfile(struct gotd_session_client *client)
ipack.report_status = 1;
if (gotd_imsg_compose_event(&client->repo_child_iev,
- GOTD_IMSG_RECV_PACKFILE, PROC_SESSION, packfd,
+ GOTD_IMSG_RECV_PACKFILE, gotd_session.proc_id, packfd,
&ipack, sizeof(ipack)) == -1) {
err = got_error_from_errno("imsg compose RECV_PACKFILE");
packfd = -1;
@@ -1271,7 +1274,7 @@ list_refs_request(void)
return got_error_from_errno("dup");
if (gotd_imsg_compose_event(iev, GOTD_IMSG_LIST_REFS_INTERNAL,
- PROC_SESSION, fd, &ilref, sizeof(ilref)) == -1) {
+ gotd_session.proc_id, fd, &ilref, sizeof(ilref)) == -1) {
err = got_error_from_errno("imsg compose LIST_REFS_INTERNAL");
close(fd);
return err;
@@ -1450,7 +1453,8 @@ session_main(const char *title, const char *repo_path,
void
session_main(const char *title, const char *repo_path,
- int *pack_fds, int *temp_fds, struct timeval *request_timeout)
+ int *pack_fds, int *temp_fds, struct timeval *request_timeout,
+ enum gotd_procid proc_id)
{
const struct got_error *err = NULL;
struct event evsigint, evsigterm, evsighup, evsigusr1;
@@ -1461,6 +1465,7 @@ session_main(const char *title, const char *repo_path,
gotd_session.temp_fds = temp_fds;
memcpy(&gotd_session.request_timeout, request_timeout,
sizeof(gotd_session.request_timeout));
+ gotd_session.proc_id = proc_id;
err = got_repo_open(&gotd_session.repo, repo_path, NULL, pack_fds);
if (err)
@@ -1497,7 +1502,8 @@ session_main(const char *title, const char *repo_path,
event_set(&gotd_session.parent_iev.ev, gotd_session.parent_iev.ibuf.fd,
EV_READ, session_dispatch, &gotd_session.parent_iev);
if (gotd_imsg_compose_event(&gotd_session.parent_iev,
- GOTD_IMSG_CLIENT_SESSION_READY, PROC_SESSION, -1, NULL, 0) == -1) {
+ GOTD_IMSG_CLIENT_SESSION_READY, gotd_session.proc_id,
+ -1, NULL, 0) == -1) {
err = got_error_from_errno("imsg compose CLIENT_SESSION_READY");
goto done;
}
blob - 671359022739ca8e501ab0fbd98de3e76447490e
blob + de20117ce268c646687a1977e8e0c7086a7fb2d6
--- gotd/session.h
+++ gotd/session.h
@@ -14,4 +14,5 @@ void session_main(const char *, const char *, int *, i
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
-void session_main(const char *, const char *, int *, int *, struct timeval *);
+void session_main(const char *, const char *, int *, int *, struct timeval *,
+ enum gotd_procid);
gotd session should unveil repository read-only during fetches