From 3b7bda2f804cf3148958658fe37b389e08419934 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 9 Dec 2014 13:27:28 +0100 Subject: [PATCH 01/29] ggated: Ignore SIGPIPE to prevent DoS ... by a single prematurely closed client connection. Reported to security-officer@FreeBSD.org on 2014-12-09. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 7cacbf58037e..bdb24d19b94c 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -1053,6 +1053,7 @@ main(int argc, char *argv[]) pidfile_write(pfh); signal(SIGCHLD, SIG_IGN); + signal(SIGPIPE, SIG_IGN); sfd = socket(AF_INET, SOCK_STREAM, 0); if (sfd == -1) -- 2.39.1 From d8b8298b54f5383883c4d7d329cff80f0ee4fbce Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 9 Dec 2014 13:47:31 +0100 Subject: [PATCH 02/29] ggated: Remove connection if the initial packet couldn't be sent Should help to mitigate DoS after flooding ggated with incomplete requests: error: accept(): Software caused connection abort. error: Exiting. Reported to security-officer@FreeBSD.org on 2014-12-09. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index bdb24d19b94c..95d167d062f9 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -956,6 +956,7 @@ handshake(struct sockaddr *from, int sfd) if (data == -1) { sendfail(sfd, errno, "Error while sending initial packet: %s.", strerror(errno)); + connection_remove(conn); return (0); } -- 2.39.1 From cb236d0dec05da2dc6b27a9b7d4464925c8592cb Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 9 Dec 2014 14:09:24 +0100 Subject: [PATCH 03/29] ggated: Continue if accept() is interrupted or the remote connection is lost Reported to security-officer@FreeBSD.org on 2014-12-09. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 95d167d062f9..72720bfe2ba7 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -1078,9 +1078,11 @@ main(int argc, char *argv[]) for (;;) { fromlen = sizeof(from); tmpsfd = accept(sfd, &from, &fromlen); - if (tmpsfd == -1) + if (tmpsfd == -1) { + if (errno == EINTR || errno == ECONNABORTED) + continue; g_gate_xlog("accept(): %s.", strerror(errno)); - + } if (got_sighup) { got_sighup = 0; exports_get(); -- 2.39.1 From 3207a45b54e346c8ffe320e4c51b8f00ceb5d515 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 9 Dec 2014 14:16:47 +0100 Subject: [PATCH 04/29] ggated: Prevent c_diskfd leaks through connection_remove() Should help against DoS: [...] debug: Connection created [127.0.0.1, /tank/scratch/testfile]. debug: New connection created (token=2197914058). debug: exports[/tank/scratch/testfile2]: Path mismatch. debug: Sending initial packet. error: accept(): Too many open files. error: Exiting. Reported to security-officer@FreeBSD.org on 2014-12-09. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 72720bfe2ba7..1fddbabd650d 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -527,6 +527,8 @@ connection_remove(struct ggd_connection *conn) close(conn->c_sendfd); if (conn->c_recvfd != -1) close(conn->c_recvfd); + if (conn->c_diskfd != -1) + close(conn->c_diskfd); free(conn->c_path); free(conn); } -- 2.39.1 From 651de4f613d74cc76fb4e0b4cb77ebf8f2a9f16f Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 9 Dec 2014 15:52:39 +0100 Subject: [PATCH 05/29] ggated: Check for connection_add() failures properly Prevents a socket leak. Reported to security-officer@FreeBSD.org on 2014-12-09. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 1fddbabd650d..f49949d510c2 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -918,7 +918,7 @@ handshake(struct sockaddr *from, int sfd) */ g_gate_log(LOG_DEBUG, "Found existing connection (token=%lu).", (unsigned long)conn->c_token); - if (connection_add(conn, &cinit, from, sfd) == -1) { + if (connection_add(conn, &cinit, from, sfd) == EEXIST) { connection_remove(conn); return (0); } -- 2.39.1 From d171845d006b05939265000878866419a0aaf148 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Thu, 2 Apr 2015 15:24:58 +0200 Subject: [PATCH 06/29] ggated recv_thread(): Do not queue incomplete WRITE requests Verifying that g_gate_recv()'s return code isn't -1 is insufficient as it's a thin wrapper arround recv(2) which, quoting its man page, "may still return less data than requested if a signal is caught, an error or disconnect occurs, or the next data to be received is of a different type than that returned". Previously incomplete WRITE requests would be scheduled with partially uninitialized memory, potentially resulting in file system corruption or, worse, bogus data being later on returned as valid. Security impact: A MITM may cause data corruption by disrupting the connection from ggatec's send_thread() to ggated's recv_thread() at the right point in time. This does not require access to the plain text traffic but if encryption is involved the attacker would have to guess that it's ggate traffic and disrupt connections blindly, hoping that some of the disruptions trigger the bug. The issue was discovered after ZFS on the ggatec side reported checksum errors which weren't reproducible on the ggated side where ZFS had received and checksummed bogus data. The ggate traffic was tunneled through SSH and Tor with sshd running as Tor location hidden service. Reported to security-officer@FreeBSD.org on 2015-04-05. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index f49949d510c2..203bf4474a14 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -684,6 +684,9 @@ recv_thread(void *arg) if (data == -1) { g_gate_xlog("Error while receiving data: %s.", strerror(errno)); + } else if ((uint32_t)data != req->r_length) { + g_gate_xlog("Received %d bytes of data while " + "expecting %u.", data, req->r_length); } } -- 2.39.1 From fdafcbf6c6b9ed0855ce20000f6b191c8631078b Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Thu, 2 Apr 2015 12:09:40 +0200 Subject: [PATCH 07/29] ggated recv_thread(): Do not queue requests with invalid values ... that would cause abort()s when read by the disk_thread() later on. From ggatec's point of view it doesn't make a difference as the connection will get closed either way, but at least the admin on the server side doesn't have to deal with core dumps. Security impact: An authenticated attacker may intentionally cause the ggated process that handles the attacker's connection to core dump and thus use more disk space than intentionally provisioned by the server admin. Without the following patch ggated core dumps may require more than 100 GB of disk space. Reported to security-officer@FreeBSD.org on 2015-04-05. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 203bf4474a14..935b289868ea 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -668,6 +668,23 @@ recv_thread(void *arg) g_gate_log(LOG_DEBUG, "%s: offset=%" PRIu64 " length=%" PRIu32, __func__, req->r_offset, req->r_length); + /* + * Reject requests that violate assertions in disk_thread(). + */ + if (req->r_cmd != GGATE_CMD_READ && + req->r_cmd != GGATE_CMD_WRITE) { + g_gate_xlog("Request contains invalid command."); + } + if (req->r_offset + req->r_length > + (uintmax_t)conn->c_mediasize) { + g_gate_xlog("Request out of bounds."); + } + if (req->r_offset % conn->c_sectorsize != 0 || + req->r_length % conn->c_sectorsize != 0) { + g_gate_xlog("Request length or offset does " + "not fit sector size."); + } + /* * Allocate memory for data. */ -- 2.39.1 From 784a0dcac2633db22ecb9d996de9515577c80d93 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Thu, 2 Apr 2015 19:52:54 +0200 Subject: [PATCH 08/29] ggated recv_thread(): Reject request with more than MAXPHYS bytes of data .. to limit the amount of memory we (try to) allocate on behalf of the client without knowing whether or not the client actually intents to use it. MAXPHYS is the hardcoded limit in ggatec so anything above it is suspicious and could be a DoS attempt. This commit forces users who like to tune MAXPHYS to make sure the value used by ggated is not below the one used by ggatec. While not ideal, this seems preferable to the DoS risk. Reported to security-officer@FreeBSD.org on 2015-04-05. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 935b289868ea..d5d8d564253c 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -685,6 +685,16 @@ recv_thread(void *arg) "not fit sector size."); } + /* + * Limit the amount of memory we allocate on behalf of + * the client. MAXPHYS is the hard limit in ggatec, + * values above it are thus pretty suspicious. + */ + if (req->r_length > MAXPHYS) { + g_gate_xlog("Request length above MAXPHYS: %u > %u", + (unsigned)req->r_length, MAXPHYS); + } + /* * Allocate memory for data. */ -- 2.39.1 From 7c4fa606049a6fcfb36d0c44adea3b46fcf6277e Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Fri, 24 Apr 2015 14:04:31 +0200 Subject: [PATCH 09/29] ggatec: Add support for SOCKS5 with domain names Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.c | 104 +++++++++++++++++++++++++++++++++++-- 1 file changed, 99 insertions(+), 5 deletions(-) diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index 0de8504ce3c4..df00a144dca8 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -67,6 +67,8 @@ static unsigned flags = 0; static int force = 0; static unsigned queue_size = G_GATE_QUEUE_SIZE; static unsigned port = G_GATE_PORT; +static char *socks_dest = NULL; +static unsigned dest_port = 3080; static off_t mediasize; static unsigned sectorsize = 0; static unsigned timeout = G_GATE_TIMEOUT; @@ -82,9 +84,11 @@ usage(void) fprintf(stderr, "usage: %s create [-nv] [-o ] [-p port] " "[-q queue_size] [-R rcvbuf] [-S sndbuf] [-s sectorsize] " - "[-t timeout] [-u unit] \n", getprogname()); + "[-t timeout] [-T :] [-u unit] \n", + getprogname()); fprintf(stderr, " %s rescue [-nv] [-o ] [-p port] " - "[-R rcvbuf] [-S sndbuf] <-u unit> \n", getprogname()); + "[-R rcvbuf] [-S sndbuf] [-T :] <-u unit> " + " \n", getprogname()); fprintf(stderr, " %s destroy [-f] <-u unit>\n", getprogname()); fprintf(stderr, " %s list [-v] [-u unit]\n", getprogname()); exit(EXIT_FAILURE); @@ -286,6 +290,69 @@ recv_thread(void *arg __unused) pthread_exit(NULL); } +static void +negotiate_socks_connection(int sfd) +{ + struct negotiation_request { + char version; + char nmethods; + char method; + } neg_request; + struct socks_request { + char version; + char cmd; + char reserved; + char address_type; + char host_length; + char dest[255 + 2]; + } socks_request; + char response[10]; + size_t request_length; + size_t host_length; + + host_length = strlen(socks_dest); + + neg_request.version = '\x05'; + neg_request.nmethods = '\x01'; /* We support one method: */ + neg_request.method = '\x00'; /* no authentication */ + + g_gate_log(LOG_DEBUG, "Starting SOCKS negotiation."); + if (g_gate_send(sfd, &neg_request, sizeof(neg_request), MSG_NOSIGNAL) == -1) + g_gate_xlog("Failed to send SOCKS negotiation request."); + + if (g_gate_recv(sfd, &response, sizeof(response), MSG_WAITALL) != 2) + g_gate_xlog("Failed to read SOCKS negotiation response."); + + if (response[0] != '\x05' || response[1] != '\x00') + g_gate_xlog("SOCKS negotiation failed."); + + g_gate_log(LOG_DEBUG, "Negotiated SOCKS5. " + "Requesting connection to %s:%d.", socks_dest, dest_port); + + socks_request.version = '\x05'; + socks_request.cmd = '\x01'; /* Connect */ + socks_request.reserved = '\x00'; + socks_request.address_type = '\x03'; /* Address is domain name */; + socks_request.host_length = (char)host_length; + strncpy(socks_request.dest, socks_dest, host_length); + socks_request.dest[host_length] = (char)((dest_port >> 8) & 0xff); + socks_request.dest[host_length + 1] = (char)(dest_port & 0xff); + request_length = sizeof(socks_request) - sizeof(socks_request.dest) + + host_length + 2; + + if (g_gate_send(sfd, &socks_request, request_length, MSG_NOSIGNAL) == -1) + g_gate_xlog("Failed to send SOCKS5 request."); + + if (g_gate_recv(sfd, &response, sizeof(response), MSG_WAITALL) != sizeof(response)) + g_gate_xlog("Failed to read SOCKS5 response."); + + if (response[0] != '\x05' || response[1] != '\x00') + g_gate_xlog("Failed to SOCKS5 connect to %s:%d", + socks_dest, dest_port); + + g_gate_log(LOG_INFO, "Connected to: %s:%d.", socks_dest, dest_port); +} + static int handshake(int dir) { @@ -324,6 +391,9 @@ handshake(int dir) g_gate_log(LOG_INFO, "Connected to the server: %s:%d.", host, port); + if (socks_dest != NULL) + negotiate_socks_connection(sfd); + /* * Create and send version packet. */ @@ -503,8 +573,13 @@ g_gatec_create(void) ggioc.gctl_maxcount = queue_size; ggioc.gctl_timeout = timeout; ggioc.gctl_unit = unit; - snprintf(ggioc.gctl_info, sizeof(ggioc.gctl_info), "%s:%u %s", host, - port, path); + if (socks_dest != NULL) + snprintf(ggioc.gctl_info, sizeof(ggioc.gctl_info), + "socks5://%s:%u -> %s:%u %s", host, + port, socks_dest, dest_port, path); + else + snprintf(ggioc.gctl_info, sizeof(ggioc.gctl_info), "%s:%u %s", + host, port, path); g_gate_ioctl(G_GATE_CMD_CREATE, &ggioc); if (unit == -1) { printf("%s%u\n", G_GATE_PROVIDER_NAME, ggioc.gctl_unit); @@ -563,8 +638,9 @@ main(int argc, char *argv[]) argv += 1; for (;;) { int ch; + char *p; - ch = getopt(argc, argv, "fno:p:q:R:S:s:t:u:v"); + ch = getopt(argc, argv, "fno:p:q:R:S:s:t:T:u:v"); if (ch == -1) break; switch (ch) { @@ -632,6 +708,24 @@ main(int argc, char *argv[]) if (sectorsize == 0 && errno != 0) errx(EXIT_FAILURE, "Invalid sectorsize."); break; + case 'T': + if (action != CREATE && action != RESCUE) + usage(); + socks_dest = optarg; + p = strchr(socks_dest, ':'); + if (p != NULL) { + errno = 0; + *p = '\0'; + p++; + dest_port = strtoul(p, NULL, 10); + if (dest_port == 0 && errno != 0) + errx(EXIT_FAILURE, + "Invalid socks5t port: %s.", p); + } + if (strlen(socks_dest) > (size_t)255) + errx(EXIT_FAILURE, + "Socks destination address too long."); + break; case 't': if (action != CREATE) usage(); -- 2.39.1 From 7d72285247ce868fcc35679303439d89f39dd857 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Fri, 24 Apr 2015 15:26:42 +0200 Subject: [PATCH 10/29] ggatec.8: Document SOCKS5 support Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.8 | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sbin/ggate/ggatec/ggatec.8 b/sbin/ggate/ggatec/ggatec.8 index 6f761dcfd99b..ba3c9ecb6f0b 100644 --- a/sbin/ggate/ggatec/ggatec.8 +++ b/sbin/ggate/ggatec/ggatec.8 @@ -41,6 +41,7 @@ .Op Fl R Ar rcvbuf .Op Fl S Ar sndbuf .Op Fl s Ar sectorsize +.Op Fl T Ar remote_target:port .Op Fl t Ar timeout .Op Fl u Ar unit .Ar host @@ -53,6 +54,7 @@ .Op Fl p Ar port .Op Fl R Ar rcvbuf .Op Fl S Ar sndbuf +.Op Fl T Ar remote_target:port .Fl u Ar unit .Ar host .Ar path @@ -137,6 +139,9 @@ Sector size for .Nm ggate provider. If not specified, it is taken from the device, or set to 512 bytes for files. +.It Fl T Ar remote_host:port +Use SOCK5 to open connection to remote_host:port before switching +to the ggated protocol. .It Fl t Ar timeout Number of seconds to wait before an I/O request will be canceled. Default is 0, which means no timeout. @@ -167,6 +172,14 @@ server# ggated client# ggatec create -o ro server /dev/cd0 ggate0 client# mount_cd9660 /dev/ggate0 /cdrom + +.Ed +Connect to 127.0.1.1:9050, SOCKS5-negotiate a connection to +the Tor location hidden service czdqtfrgvizltdal.onion:1312 +and access a ZVOL: +.Bd -literal -offset indent +# ggatec create -T czdqtfrgvizltdal.onion:1312 -p 9050 \\ + 127.0.1.1 /dev/zvol/dpool/ggated/czdqtfrgvizltdal.eli .Ed .Sh SEE ALSO .Xr geom 4 , -- 2.39.1 From 7082f08bcaac74889032b15e5b27f75820acf57f Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 27 Apr 2015 19:10:17 +0200 Subject: [PATCH 11/29] ggatec: Reject unexpected GGATE commands in recv_thread() Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index df00a144dca8..989eed6ed2c5 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -266,6 +266,10 @@ recv_thread(void *arg __unused) break; } } + if (ggio.gctl_cmd != GGATE_CMD_READ && + ggio.gctl_cmd != GGATE_CMD_WRITE) { + g_gate_xlog("Unexpected GGATE_CMD: %d", ggio.gctl_cmd); + } if (ggio.gctl_error == 0 && ggio.gctl_cmd == GGATE_CMD_READ) { numbytesprocd = g_gate_recv(recvfd, ggio.gctl_data, -- 2.39.1 From 1d5a6c0dfd225161426b545b80fe2e545947b6d5 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 27 Apr 2015 19:15:18 +0200 Subject: [PATCH 12/29] ggatec: Log if the remote side signals errors Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index 989eed6ed2c5..78daab7d50b1 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -271,6 +271,12 @@ recv_thread(void *arg __unused) g_gate_xlog("Unexpected GGATE_CMD: %d", ggio.gctl_cmd); } + if (ggio.gctl_error != 0) { + g_gate_log(LOG_ERR, + "Remote side signaled error %d: %s.", + ggio.gctl_error, strerror(ggio.gctl_error)); + } + if (ggio.gctl_error == 0 && ggio.gctl_cmd == GGATE_CMD_READ) { numbytesprocd = g_gate_recv(recvfd, ggio.gctl_data, ggio.gctl_length, MSG_WAITALL); -- 2.39.1 From 4242b181dbd63c4c80a70c6246bfa375e3a5ca5d Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 27 Apr 2015 19:53:30 +0200 Subject: [PATCH 13/29] ggate[cd]: Improve BIO_FLUSH support (XXX) Let ggated transform BIO_FLUSH requests into fsync() calls. Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.c | 5 ++++- sbin/ggate/ggated/ggated.c | 21 +++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index 78daab7d50b1..19b90081d772 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -163,7 +163,9 @@ send_thread(void *arg __unused) hdr.gh_cmd = GGATE_CMD_WRITE; break; case BIO_FLUSH: + g_gate_log(LOG_DEBUG, "FLUSH request"); hdr.gh_cmd = GGATE_CMD_FLUSH; + assert(ggio.gctl_length == 0); break; default: g_gate_log(LOG_NOTICE, "Unknown gctl_cmd: %i", @@ -267,7 +269,8 @@ recv_thread(void *arg __unused) } } if (ggio.gctl_cmd != GGATE_CMD_READ && - ggio.gctl_cmd != GGATE_CMD_WRITE) { + ggio.gctl_cmd != GGATE_CMD_WRITE && + ggio.gctl_cmd != GGATE_CMD_FLUSH) { g_gate_xlog("Unexpected GGATE_CMD: %d", ggio.gctl_cmd); } diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index d5d8d564253c..820984add062 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -672,8 +672,10 @@ recv_thread(void *arg) * Reject requests that violate assertions in disk_thread(). */ if (req->r_cmd != GGATE_CMD_READ && - req->r_cmd != GGATE_CMD_WRITE) { - g_gate_xlog("Request contains invalid command."); + req->r_cmd != GGATE_CMD_WRITE && + req->r_cmd != GGATE_CMD_FLUSH) { + g_gate_xlog("Request contains invalid command: %d", + req->r_cmd); } if (req->r_offset + req->r_length > (uintmax_t)conn->c_mediasize) { @@ -696,9 +698,10 @@ recv_thread(void *arg) } /* - * Allocate memory for data. + * Allocate memory for data, except when flushing. */ - req->r_data = malloc_waitok(req->r_length); + req->r_data = req->r_cmd != GGATE_CMD_FLUSH ? + malloc_waitok(req->r_length) : NULL; /* * Receive data to write for WRITE request. @@ -758,6 +761,9 @@ disk_thread(void *arg) /* * Check the request. */ + assert(req->r_cmd == GGATE_CMD_READ || + req->r_cmd == GGATE_CMD_WRITE || + req->r_cmd == GGATE_CMD_FLUSH); assert(req->r_offset + req->r_length <= (uintmax_t)conn->c_mediasize); assert((req->r_offset % conn->c_sectorsize) == 0); assert((req->r_length % conn->c_sectorsize) == 0); @@ -782,9 +788,12 @@ disk_thread(void *arg) req->r_data = NULL; break; case GGATE_CMD_FLUSH: - data = fsync(fd); - if (data != 0) + g_gate_log(LOG_DEBUG, "Flushing"); + if (fsync(fd)) { req->r_error = errno; + g_gate_log(LOG_ERR, "Flushing failed: %s", + strerror(errno)); + } break; default: g_gate_log(LOG_DEBUG, "Unsupported request: %i", req->r_cmd); -- 2.39.1 From 8ab0848baa07bc414aadb1ebd1c39731c2302cbc Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Wed, 29 Apr 2015 12:44:56 +0200 Subject: [PATCH 14/29] ggatec: Log the command type for hdr packets (when debugging) ... and provide more details about failed requests. Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.c | 10 +++++++--- sbin/ggate/shared/ggate.c | 16 ++++++++++++++++ sbin/ggate/shared/ggate.h | 1 + 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index 19b90081d772..2e08189ac220 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -182,7 +182,8 @@ send_thread(void *arg __unused) g_gate_swap2n_hdr(&hdr); numbytesprocd = g_gate_send(sendfd, &hdr, sizeof(hdr), MSG_NOSIGNAL); - g_gate_log(LOG_DEBUG, "Sent hdr packet."); + g_gate_log(LOG_DEBUG, "Sent hdr packet (%s).", + g_gate_cmd2str(hdr.gh_cmd)); g_gate_swap2h_hdr(&hdr); if (reconnect) break; @@ -248,7 +249,8 @@ recv_thread(void *arg __unused) pthread_kill(sendtd, SIGUSR1); break; } - g_gate_log(LOG_DEBUG, "Received hdr packet."); + g_gate_log(LOG_DEBUG, "Received hdr packet (%s).", + g_gate_cmd2str(hdr.gh_cmd)); ggio.gctl_seq = hdr.gh_seq; ggio.gctl_cmd = hdr.gh_cmd; @@ -276,7 +278,9 @@ recv_thread(void *arg __unused) if (ggio.gctl_error != 0) { g_gate_log(LOG_ERR, - "Remote side signaled error %d: %s.", + "%s for %d bytes at offset %d failed. " + "Error %d: %s.", g_gate_cmd2str(ggio.gctl_cmd), + ggio.gctl_length, ggio.gctl_offset, ggio.gctl_error, strerror(ggio.gctl_error)); } diff --git a/sbin/ggate/shared/ggate.c b/sbin/ggate/shared/ggate.c index 2f544f7a2c9c..6197540d9678 100644 --- a/sbin/ggate/shared/ggate.c +++ b/sbin/ggate/shared/ggate.c @@ -409,3 +409,19 @@ g_gate_str2ip(const char *str) return (INADDR_NONE); return (((struct in_addr *)(void *)hp->h_addr)->s_addr); } + +const char * +g_gate_cmd2str(int cmd) +{ + + switch (cmd) { + case GGATE_CMD_READ: + return ("GGATE_CMD_READ"); + case GGATE_CMD_WRITE: + return ("GGATE_CMD_WRITE"); + case GGATE_CMD_FLUSH: + return ("GGATE_CMD_FLUSH"); + } + + return ("unknown (invalid?) GGATE command"); +} diff --git a/sbin/ggate/shared/ggate.h b/sbin/ggate/shared/ggate.h index d399b247cd75..846abb7f30bd 100644 --- a/sbin/ggate/shared/ggate.h +++ b/sbin/ggate/shared/ggate.h @@ -114,6 +114,7 @@ void g_gate_socket_settings(int sfd); void g_gate_list(int unit, int verbose); #endif in_addr_t g_gate_str2ip(const char *str); +const char *g_gate_cmd2str(int cmd); /* * g_gate_swap2h_* - functions swap bytes to host byte order (from big endian). -- 2.39.1 From 4aa1151441ff09545c538df536168dfe2696201a Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Wed, 6 May 2015 15:55:08 +0200 Subject: [PATCH 15/29] ggated disk_thread(): Include the command in the debug output Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 820984add062..85c91f412d72 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -768,8 +768,10 @@ disk_thread(void *arg) assert((req->r_offset % conn->c_sectorsize) == 0); assert((req->r_length % conn->c_sectorsize) == 0); - g_gate_log(LOG_DEBUG, "%s: offset=%" PRIu64 " length=%" PRIu32, - __func__, req->r_offset, req->r_length); + g_gate_log(LOG_DEBUG, + "%s: cmd=%s offset=%" PRIu64 " length=%" PRIu32, + __func__, g_gate_cmd2str(req->r_cmd), req->r_offset, + req->r_length); /* * Do the request. -- 2.39.1 From 984fdadf389a369326b1222fa18bd34233ea2ab0 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 5 May 2015 17:39:16 +0200 Subject: [PATCH 16/29] ggate[cd]: Add BIO_DELETE support On the ggated side the requests are translated into writes of zeros which ZFS will convert into BIO_DELETE requests again when zle or another type of compression is enabled. Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.c | 5 ++++ sbin/ggate/ggated/ggated.c | 47 ++++++++++++++++++++++++++++++++------ sbin/ggate/shared/ggate.c | 2 ++ sbin/ggate/shared/ggate.h | 1 + 4 files changed, 48 insertions(+), 7 deletions(-) diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index 2e08189ac220..f6ba77a9d034 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -162,6 +162,10 @@ send_thread(void *arg __unused) case BIO_WRITE: hdr.gh_cmd = GGATE_CMD_WRITE; break; + case BIO_DELETE: + g_gate_log(LOG_DEBUG, "DELETE request"); + hdr.gh_cmd = GGATE_CMD_DELETE; + break; case BIO_FLUSH: g_gate_log(LOG_DEBUG, "FLUSH request"); hdr.gh_cmd = GGATE_CMD_FLUSH; @@ -272,6 +276,7 @@ recv_thread(void *arg __unused) } if (ggio.gctl_cmd != GGATE_CMD_READ && ggio.gctl_cmd != GGATE_CMD_WRITE && + ggio.gctl_cmd != GGATE_CMD_DELETE && ggio.gctl_cmd != GGATE_CMD_FLUSH) { g_gate_xlog("Unexpected GGATE_CMD: %d", ggio.gctl_cmd); } diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 85c91f412d72..cd36dc853c2a 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -651,6 +651,7 @@ recv_thread(void *arg) * Get header packet. */ req = malloc_waitok(sizeof(*req)); + memset(req, 0, sizeof(*req)); data = g_gate_recv(fd, &req->r_hdr, sizeof(req->r_hdr), MSG_WAITALL); if (data == 0) { @@ -673,6 +674,7 @@ recv_thread(void *arg) */ if (req->r_cmd != GGATE_CMD_READ && req->r_cmd != GGATE_CMD_WRITE && + req->r_cmd != GGATE_CMD_DELETE && req->r_cmd != GGATE_CMD_FLUSH) { g_gate_xlog("Request contains invalid command: %d", req->r_cmd); @@ -692,21 +694,16 @@ recv_thread(void *arg) * the client. MAXPHYS is the hard limit in ggatec, * values above it are thus pretty suspicious. */ - if (req->r_length > MAXPHYS) { + if (req->r_length > MAXPHYS && req->r_cmd != GGATE_CMD_DELETE) { g_gate_xlog("Request length above MAXPHYS: %u > %u", (unsigned)req->r_length, MAXPHYS); } - /* - * Allocate memory for data, except when flushing. - */ - req->r_data = req->r_cmd != GGATE_CMD_FLUSH ? - malloc_waitok(req->r_length) : NULL; - /* * Receive data to write for WRITE request. */ if (req->r_cmd == GGATE_CMD_WRITE) { + req->r_data = malloc_waitok(req->r_length); g_gate_log(LOG_DEBUG, "Waiting for %u bytes of data...", req->r_length); data = g_gate_recv(fd, req->r_data, req->r_length, @@ -733,6 +730,34 @@ recv_thread(void *arg) } } +static ssize_t +delete_range(int fd, size_t length, off_t offset) +{ + static char zeros[MAXPHYS]; + size_t written; + + written = 0; + + do + { + int ret; + size_t bytes_left; + size_t chunk_size; + + bytes_left = length - written; + chunk_size = bytes_left > MAXPHYS ? MAXPHYS : bytes_left; + ret = pwrite(fd, zeros, chunk_size, offset + written); + if (ret == -1) + return (written); + written += ret; + } while (written < length); + + g_gate_log(LOG_DEBUG, "Overwritten %u bytes at offset %jd with zeros", + written, (intmax_t)offset); + + return (written); +} + static void * disk_thread(void *arg) { @@ -763,6 +788,7 @@ disk_thread(void *arg) */ assert(req->r_cmd == GGATE_CMD_READ || req->r_cmd == GGATE_CMD_WRITE || + req->r_cmd == GGATE_CMD_DELETE || req->r_cmd == GGATE_CMD_FLUSH); assert(req->r_offset + req->r_length <= (uintmax_t)conn->c_mediasize); assert((req->r_offset % conn->c_sectorsize) == 0); @@ -779,9 +805,16 @@ disk_thread(void *arg) data = 0; switch (req->r_cmd) { case GGATE_CMD_READ: + assert(req->r_data == NULL); + req->r_data = malloc_waitok(req->r_length); data = pread(fd, req->r_data, req->r_length, req->r_offset); break; + case GGATE_CMD_DELETE: + data = delete_range(fd, req->r_length, + req->r_offset); + assert((size_t)data <= req->r_length); + break; case GGATE_CMD_WRITE: data = pwrite(fd, req->r_data, req->r_length, req->r_offset); diff --git a/sbin/ggate/shared/ggate.c b/sbin/ggate/shared/ggate.c index 6197540d9678..d6be4948d9c6 100644 --- a/sbin/ggate/shared/ggate.c +++ b/sbin/ggate/shared/ggate.c @@ -419,6 +419,8 @@ g_gate_cmd2str(int cmd) return ("GGATE_CMD_READ"); case GGATE_CMD_WRITE: return ("GGATE_CMD_WRITE"); + case GGATE_CMD_DELETE: + return ("GGATE_CMD_DELETE"); case GGATE_CMD_FLUSH: return ("GGATE_CMD_FLUSH"); } diff --git a/sbin/ggate/shared/ggate.h b/sbin/ggate/shared/ggate.h index 846abb7f30bd..f2b602670088 100644 --- a/sbin/ggate/shared/ggate.h +++ b/sbin/ggate/shared/ggate.h @@ -58,6 +58,7 @@ #define GGATE_CMD_READ 0 #define GGATE_CMD_WRITE 1 #define GGATE_CMD_FLUSH 3 +#define GGATE_CMD_DELETE 4 extern int g_gate_devfd; extern int g_gate_verbose; -- 2.39.1 From c97af9f4f8f6e6f9b08b76cae3e47dfef27d89bd Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Wed, 29 Apr 2015 10:55:40 +0200 Subject: [PATCH 17/29] ggated send_thread(): Assert that we only send data for read requests Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index cd36dc853c2a..f6a4f653c084 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -907,6 +907,7 @@ send_thread(void *arg) g_gate_log(LOG_DEBUG, "Sent hdr packet."); g_gate_swap2h_hdr(&req->r_hdr); if (req->r_data != NULL) { + assert(req->r_cmd == GGATE_CMD_READ); data = g_gate_send(fd, req->r_data, req->r_length, 0); if (data != (ssize_t)req->r_length) { g_gate_xlog("Error while sending data: %s.", -- 2.39.1 From 91757a485bde61b9d2ac48f961f37ea7a8a1673e Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 4 May 2015 18:00:04 +0200 Subject: [PATCH 18/29] ggated: Open the listening socket CLOEXEC Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index f6a4f653c084..50ce8fd3d5d4 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -1133,7 +1133,7 @@ main(int argc, char *argv[]) signal(SIGCHLD, SIG_IGN); signal(SIGPIPE, SIG_IGN); - sfd = socket(AF_INET, SOCK_STREAM, 0); + sfd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0); if (sfd == -1) g_gate_xlog("Cannot open stream socket: %s.", strerror(errno)); bzero(&serv, sizeof(serv)); -- 2.39.1 From d0e7909eacc2177d6d03e08f4d7895c526b52db3 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 4 May 2015 18:31:46 +0200 Subject: [PATCH 19/29] ggated: Fix another socket leak Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 50ce8fd3d5d4..6631a23d8b93 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -344,6 +344,11 @@ exports_check(struct ggd_export *ex, struct g_gate_cinit *cinit, return (EPERM); } } + if (conn->c_diskfd != -1) { + g_gate_log(LOG_DEBUG, "Requested file %s is already open: %d", + ex->e_path, conn->c_diskfd); + return(0); + } if ((conn->c_flags & GGATE_FLAG_RDONLY) != 0) flags = O_RDONLY; else if ((conn->c_flags & GGATE_FLAG_WRONLY) != 0) -- 2.39.1 From 92f2031b25323925497d93f9017845d41ce9f5b0 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Thu, 30 Apr 2015 11:52:06 +0200 Subject: [PATCH 20/29] ggated recv_thread(): In case of read-only files, only accept read commands Accepting write commands etc. is not a security problem because the file descriptor isn't writeable anyway, but accepting requests other than reads could hide client bugs. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 6631a23d8b93..fef9e831a4ff 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -694,6 +694,12 @@ recv_thread(void *arg) "not fit sector size."); } + if ((conn->c_flags & GGATE_FLAG_RDONLY) != 0 + && req->r_cmd != GGATE_CMD_READ) { + g_gate_xlog("%s request received for read-only file", + g_gate_cmd2str(req->r_cmd)); + } + /* * Limit the amount of memory we allocate on behalf of * the client. MAXPHYS is the hard limit in ggatec, -- 2.39.1 From dd032649c27890fd76bba5d3cc373cdf22b64c48 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Sun, 3 May 2015 14:02:02 +0200 Subject: [PATCH 21/29] ggatec: Add log-to-file support Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.c | 5 ++++- sbin/ggate/shared/ggate.c | 26 +++++++++++++++++++++----- sbin/ggate/shared/ggate.h | 1 + 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index f6ba77a9d034..86a8f147c562 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -662,7 +662,7 @@ main(int argc, char *argv[]) int ch; char *p; - ch = getopt(argc, argv, "fno:p:q:R:S:s:t:T:u:v"); + ch = getopt(argc, argv, "fl:no:p:q:R:S:s:t:T:u:v"); if (ch == -1) break; switch (ch) { @@ -671,6 +671,9 @@ main(int argc, char *argv[]) usage(); force = 1; break; + case 'l': + g_gate_open_log(optarg); + break; case 'n': if (action != CREATE && action != RESCUE) usage(); diff --git a/sbin/ggate/shared/ggate.c b/sbin/ggate/shared/ggate.c index d6be4948d9c6..084db7304e53 100644 --- a/sbin/ggate/shared/ggate.c +++ b/sbin/ggate/shared/ggate.c @@ -28,6 +28,7 @@ * $FreeBSD$ */ +#define _WITH_DPRINTF #include #include #include @@ -61,13 +62,23 @@ int g_gate_devfd = -1; int g_gate_verbose = 0; +static int g_gate_logfd = -1; +void +g_gate_open_log(const char *logfile) +{ + + g_gate_logfd = open(logfile, O_CREAT | O_WRONLY | O_APPEND, S_IWUSR |S_IRUSR); + if (g_gate_logfd == -1) { + g_gate_xlog("Failed to open %s: %s", logfile, strerror(errno)); + } +} void g_gate_vlog(int priority, const char *message, va_list ap) { - if (g_gate_verbose) { + if (g_gate_verbose || g_gate_logfd != -1) { const char *prefix; switch (priority) { @@ -89,10 +100,15 @@ g_gate_vlog(int priority, const char *message, va_list ap) default: prefix = "unknown"; } - - printf("%s: ", prefix); - vprintf(message, ap); - printf("\n"); + if (g_gate_logfd == -1) { + printf("%s: ", prefix); + vprintf(message, ap); + printf("\n"); + } else if (g_gate_verbose || priority != LOG_DEBUG) { + dprintf(g_gate_logfd, "%s: ", prefix); + vdprintf(g_gate_logfd, message, ap); + dprintf(g_gate_logfd, "\n"); + } } else { if (priority != LOG_DEBUG) vsyslog(priority, message, ap); diff --git a/sbin/ggate/shared/ggate.h b/sbin/ggate/shared/ggate.h index f2b602670088..7b72e6e192f2 100644 --- a/sbin/ggate/shared/ggate.h +++ b/sbin/ggate/shared/ggate.h @@ -97,6 +97,7 @@ struct g_gate_hdr { uint16_t gh_error; /* error value (0 if ok) */ } __packed; +void g_gate_open_log(const char *logfile); void g_gate_vlog(int priority, const char *message, va_list ap); void g_gate_log(int priority, const char *message, ...); void g_gate_xvlog(const char *message, va_list ap) __dead2; -- 2.39.1 From dbc0270e8af4de492082e20144e5431e5e2d5e9d Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Thu, 30 Apr 2015 13:52:39 +0200 Subject: [PATCH 22/29] ggate[cd]: Add Jail and Capsicum support The capsicum support for ggatec is incomplete and only enabled if the -c flag is used as it currently prevents ggatec from reconnecting which is very inconvenient. Obtained from: ElectroBSD --- sbin/ggate/ggatec/ggatec.8 | 8 +++ sbin/ggate/ggatec/ggatec.c | 37 ++++++++-- sbin/ggate/ggated/ggated.c | 5 ++ sbin/ggate/shared/ggate.c | 137 +++++++++++++++++++++++++++++++++++++ sbin/ggate/shared/ggate.h | 2 + 5 files changed, 182 insertions(+), 7 deletions(-) diff --git a/sbin/ggate/ggatec/ggatec.8 b/sbin/ggate/ggatec/ggatec.8 index ba3c9ecb6f0b..f4bf7cf43902 100644 --- a/sbin/ggate/ggatec/ggatec.8 +++ b/sbin/ggate/ggatec/ggatec.8 @@ -33,6 +33,7 @@ .Sh SYNOPSIS .Nm .Cm create +.Op Fl c .Op Fl n .Op Fl v .Op Fl o Cm ro | wo | rw @@ -48,6 +49,7 @@ .Ar path .Nm .Cm rescue +.Op Fl c .Op Fl n .Op Fl v .Op Fl o Cm ro | wo | rw @@ -104,6 +106,12 @@ providers. .Pp Available options: .Bl -tag -width ".Fl s Cm ro | wo | rw" +.It Fl c +Enter capsicum sandbox. +Currently this prevents +.Nm ggatec +from reconnecting which is somewhat inconvenient. +The flag will go away once this is fixed. .It Fl f Forcibly destroy .Nm ggate diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index 86a8f147c562..0b8345068150 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -53,6 +53,7 @@ #include #include #include +#include #include #include "ggate.h" @@ -62,6 +63,8 @@ static enum { UNSET, CREATE, DESTROY, LIST, RESCUE } action = UNSET; static const char *path = NULL; static const char *host = NULL; +static in_addr_t host_ip; +static const char *logfile = NULL; static int unit = G_GATE_UNIT_AUTO; static unsigned flags = 0; static int force = 0; @@ -77,6 +80,7 @@ static uint32_t token; static pthread_t sendtd, recvtd; static int reconnect; static int initialbuffersize = 131072; +static int drop_capabilities = 0; static void usage(void) @@ -389,7 +393,7 @@ handshake(int dir) */ bzero(&serv, sizeof(serv)); serv.sin_family = AF_INET; - serv.sin_addr.s_addr = g_gate_str2ip(host); + serv.sin_addr.s_addr = host_ip; if (serv.sin_addr.s_addr == INADDR_NONE) { g_gate_log(LOG_DEBUG, "Invalid IP/host name: %s.", host); return (-1); @@ -497,7 +501,7 @@ static void mydaemon(void) { - if (g_gate_verbose > 0) + if (logfile == NULL && g_gate_verbose > 0) return; if (daemon(0, 0) == 0) return; @@ -562,6 +566,10 @@ g_gatec_loop(void) signal(SIGUSR1, signop); for (;;) { g_gatec_start(); + + if (cap_sandboxed()) + g_gate_xlog("Got disconnected while being sandboxed."); + g_gate_log(LOG_NOTICE, "Disconnected [%s %s]. Connecting...", host, path); while (!g_gatec_connect()) { @@ -584,9 +592,6 @@ g_gatec_create(void) if (!g_gatec_connect()) g_gate_xlog("Cannot connect: %s.", strerror(errno)); - /* - * Ok, got both sockets, time to create provider. - */ memset(&ggioc, 0, sizeof(ggioc)); ggioc.gctl_version = G_GATE_VERSION; ggioc.gctl_mediasize = mediasize; @@ -609,6 +614,9 @@ g_gatec_create(void) } unit = ggioc.gctl_unit; + if (drop_capabilities) + g_gate_drop_capabilities(sendfd, recvfd); + mydaemon(); g_gatec_loop(); } @@ -621,6 +629,9 @@ g_gatec_rescue(void) if (!g_gatec_connect()) g_gate_xlog("Cannot connect: %s.", strerror(errno)); + if (drop_capabilities) + g_gate_drop_capabilities(sendfd, recvfd); + ggioc.gctl_version = G_GATE_VERSION; ggioc.gctl_unit = unit; ggioc.gctl_seq = 0; @@ -662,17 +673,21 @@ main(int argc, char *argv[]) int ch; char *p; - ch = getopt(argc, argv, "fl:no:p:q:R:S:s:t:T:u:v"); + ch = getopt(argc, argv, "cfl:no:p:q:R:S:s:t:T:u:v"); if (ch == -1) break; switch (ch) { + case 'c': + drop_capabilities = 1; + force = 1; + break; case 'f': if (action != DESTROY) usage(); force = 1; break; case 'l': - g_gate_open_log(optarg); + logfile = optarg; break; case 'n': if (action != CREATE && action != RESCUE) @@ -786,7 +801,11 @@ main(int argc, char *argv[]) g_gate_load_module(); g_gate_open_device(); host = argv[0]; + host_ip = g_gate_str2ip(host); path = argv[1]; + if (logfile != NULL) + g_gate_open_log(logfile); + g_gate_drop_privs("hast", host_ip); g_gatec_create(); break; case DESTROY: @@ -810,7 +829,11 @@ main(int argc, char *argv[]) } g_gate_open_device(); host = argv[0]; + host_ip = g_gate_str2ip(host); path = argv[1]; + if (logfile != NULL) + g_gate_open_log(logfile); + g_gate_drop_privs("hast", host_ip); g_gatec_rescue(); break; case UNSET: diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index fef9e831a4ff..d69d5691acb0 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -560,6 +560,11 @@ connection_launch(struct ggd_connection *conn) } g_gate_log(LOG_DEBUG, "Process created [%s].", conn->c_path); + if (getuid() == 0) + g_gate_drop_privs("hast", bindaddr); + + g_gate_drop_capabilities(conn->c_sendfd, conn->c_recvfd); + /* * Create condition variables and mutexes for in-queue and out-queue * synchronization. diff --git a/sbin/ggate/shared/ggate.c b/sbin/ggate/shared/ggate.c index 084db7304e53..4c0d3886f463 100644 --- a/sbin/ggate/shared/ggate.c +++ b/sbin/ggate/shared/ggate.c @@ -55,6 +55,11 @@ #include #include #include +#include +#include +#include +#include +#include #include #include "ggate.h" @@ -443,3 +448,135 @@ g_gate_cmd2str(int cmd) return ("unknown (invalid?) GGATE command"); } + +/* + * The functions below are based on drop_privs() from ../../hastd/subr.c + * + * Changes: + * - HAST_USER replaced with ggate_user option + * - pjdlog_* replaced with g_gate_xlog(). + * - Don't fall back to chroot if jailing fails. + */ +#define PJDLOG_VERIFY assert +void +g_gate_drop_privs(const char *ggate_user, in_addr_t jail_address) +{ + char jailhost[32]; + struct jail jailst; + struct passwd *pw; + uid_t ruid, euid, suid; + gid_t rgid, egid, sgid; + gid_t gidset[1]; + struct in_addr jail_ip; + /* + * According to getpwnam(3) we have to clear errno before calling the + * function to be able to distinguish between an error and missing + * entry (with is not treated as error by getpwnam(3)). + */ + errno = 0; + pw = getpwnam(ggate_user); + if (pw == NULL) { + if (errno != 0) { + g_gate_xlog("Unable to find info about '%s' user", + ggate_user); + } else { + g_gate_xlog("'%s' user doesn't exist.", ggate_user); + } + } + + jail_ip.s_addr = jail_address; + + bzero(&jailst, sizeof(jailst)); + jailst.version = JAIL_API_VERSION; + jailst.path = pw->pw_dir; + (void)snprintf(jailhost, sizeof(jailhost), "%s-jail", getprogname()); + jailst.hostname = jailhost; + jailst.jailname = NULL; + jailst.ip4s = 1; + jailst.ip4 = &jail_ip; + jailst.ip6s = 0; + jailst.ip6 = NULL; + if (jail(&jailst) == -1) { + g_gate_xlog("Unable to jail process in directory %s", pw->pw_dir); + } + PJDLOG_VERIFY(chdir("/") == 0); + gidset[0] = pw->pw_gid; + if (setgroups(1, gidset) == -1) { + g_gate_xlog("Unable to set groups to gid %u", + (unsigned int)pw->pw_gid); + } + if (setgid(pw->pw_gid) == -1) { + g_gate_xlog("Unable to set gid to %u", + (unsigned int)pw->pw_gid); + } + if (setuid(pw->pw_uid) == -1) { + g_gate_xlog("Unable to set uid to %u", + (unsigned int)pw->pw_uid); + } + + /* + * Better be sure that everything succeeded. + */ + PJDLOG_VERIFY(getresuid(&ruid, &euid, &suid) == 0); + PJDLOG_VERIFY(ruid == pw->pw_uid); + PJDLOG_VERIFY(euid == pw->pw_uid); + PJDLOG_VERIFY(suid == pw->pw_uid); + PJDLOG_VERIFY(getresgid(&rgid, &egid, &sgid) == 0); + PJDLOG_VERIFY(rgid == pw->pw_gid); + PJDLOG_VERIFY(egid == pw->pw_gid); + PJDLOG_VERIFY(sgid == pw->pw_gid); + PJDLOG_VERIFY(getgroups(0, NULL) == 1); + PJDLOG_VERIFY(getgroups(1, gidset) == 1); + PJDLOG_VERIFY(gidset[0] == pw->pw_gid); + + g_gate_log(LOG_DEBUG, "Privileges successfully dropped using " + "jail+setgid+setuid."); +} + +int +g_gate_drop_capabilities(int sendfd, int recvfd) +{ + cap_rights_t rights; + static const unsigned long ggatecmds[] = { + G_GATE_CMD_START, + G_GATE_CMD_DONE, + G_GATE_CMD_CANCEL, + }; + + if (cap_enter() != 0) { + g_gate_xlog("Failed to sandbox using capsicum"); + } + + cap_rights_init(&rights, CAP_PREAD, CAP_PWRITE); + if (cap_rights_limit(sendfd, &rights) == -1) { + g_gate_xlog("Unable to limit capability " + "rights on sendfd %d", sendfd); + } + if (cap_rights_limit(recvfd, &rights) == -1) { + g_gate_xlog("Unable to limit capability " + "rights on recvfd %d", recvfd); + } + + /* Only the client uses this. */ + if (g_gate_devfd != -1) { + cap_rights_init(&rights, CAP_IOCTL, CAP_PREAD, CAP_PWRITE); + if (cap_rights_limit(g_gate_devfd, &rights) == -1) { + g_gate_xlog("Unable to limit capability rights " + "to CAP_IOCTL on ggate descriptor"); + } + if (cap_ioctls_limit(g_gate_devfd, ggatecmds, + sizeof(ggatecmds) / sizeof(ggatecmds[0])) == -1) { + g_gate_xlog("Unable to limit allowed ggate ioctls"); + } + } + cap_rights_init(&rights, CAP_PWRITE); + if (g_gate_logfd != -1 && + cap_rights_limit(g_gate_logfd, &rights) == -1) { + g_gate_xlog("Unable to limit capability " + "rights on logfd %d", g_gate_logfd); + } + + g_gate_log(LOG_DEBUG, "Entered Capsicum sandbox"); + + return (0); +} diff --git a/sbin/ggate/shared/ggate.h b/sbin/ggate/shared/ggate.h index 7b72e6e192f2..45ba2c9d15e8 100644 --- a/sbin/ggate/shared/ggate.h +++ b/sbin/ggate/shared/ggate.h @@ -117,6 +117,8 @@ void g_gate_list(int unit, int verbose); #endif in_addr_t g_gate_str2ip(const char *str); const char *g_gate_cmd2str(int cmd); +void g_gate_drop_privs(const char *ggate_user, in_addr_t jail_address); +int g_gate_drop_capabilities(int sendfd, int recvfd); /* * g_gate_swap2h_* - functions swap bytes to host byte order (from big endian). -- 2.39.1 From fabd32908049bf71f29f5435a0fb897a117747ab Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 28 Apr 2015 13:02:25 +0200 Subject: [PATCH 23/29] Bump GGATE_VERSION due to FLUSH and DELETE support and various bug fixes Unpatched ggate[cd] versions may cause data corruption so we no longer want to speak to them. Obtained from: ElectroBSD --- sbin/ggate/shared/ggate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sbin/ggate/shared/ggate.h b/sbin/ggate/shared/ggate.h index 45ba2c9d15e8..2359b0af6ddf 100644 --- a/sbin/ggate/shared/ggate.h +++ b/sbin/ggate/shared/ggate.h @@ -42,7 +42,7 @@ #define G_GATE_TIMEOUT 0 #define GGATE_MAGIC "GEOM_GATE " -#define GGATE_VERSION 0 +#define GGATE_VERSION 1 #define GGATE_FLAG_RDONLY 0x0001 #define GGATE_FLAG_WRONLY 0x0002 -- 2.39.1 From cd2e9af1b42c5c1d4d58bbb329888ee16cb7601b Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Sun, 9 Aug 2015 15:20:48 +0200 Subject: [PATCH 24/29] ggate: Use dedicated users for ggatec and ggated Obtained from: ElectroBSD --- etc/group | 2 ++ etc/master.passwd | 2 ++ sbin/ggate/ggatec/ggatec.c | 4 ++-- sbin/ggate/ggated/ggated.c | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/etc/group b/etc/group index f48fef985e4d..27fc59dee2b9 100644 --- a/etc/group +++ b/etc/group @@ -35,6 +35,8 @@ ntpd:*:123: _ypldap:*:160: _tor:*:256: hast:*:845: +ggatec:*:846: +ggated:*:847: tests:*:977: nogroup:*:65533: nobody:*:65534: diff --git a/etc/master.passwd b/etc/master.passwd index 597aebe125dc..d1ad7c159d50 100644 --- a/etc/master.passwd +++ b/etc/master.passwd @@ -26,5 +26,7 @@ ntpd:*:123:123::0:0:NTP Daemon:/var/db/ntp:/usr/sbin/nologin _ypldap:*:160:160::0:0:YP LDAP unprivileged user:/var/empty:/usr/sbin/nologin _tor:*:256:256::0:0:Onion delivery agent:/nonexistent:/usr/sbin/nologin hast:*:845:845::0:0:HAST unprivileged user:/var/empty:/usr/sbin/nologin +ggatec:*:846:846::0:0:ggatec unprivileged user:/var/empty:/usr/sbin/nologin +ggated:*:847:847::0:0:ggated unprivileged user:/var/empty:/usr/sbin/nologin tests:*:977:977::0:0:Unprivileged user for tests:/nonexistent:/usr/sbin/nologin nobody:*:65534:65534::0:0:Unprivileged user:/nonexistent:/usr/sbin/nologin diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c index 0b8345068150..839239eb744a 100644 --- a/sbin/ggate/ggatec/ggatec.c +++ b/sbin/ggate/ggatec/ggatec.c @@ -805,7 +805,7 @@ main(int argc, char *argv[]) path = argv[1]; if (logfile != NULL) g_gate_open_log(logfile); - g_gate_drop_privs("hast", host_ip); + g_gate_drop_privs("ggatec", host_ip); g_gatec_create(); break; case DESTROY: @@ -833,7 +833,7 @@ main(int argc, char *argv[]) path = argv[1]; if (logfile != NULL) g_gate_open_log(logfile); - g_gate_drop_privs("hast", host_ip); + g_gate_drop_privs("ggatec", host_ip); g_gatec_rescue(); break; case UNSET: diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index d69d5691acb0..810c183c1b49 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -561,7 +561,7 @@ connection_launch(struct ggd_connection *conn) g_gate_log(LOG_DEBUG, "Process created [%s].", conn->c_path); if (getuid() == 0) - g_gate_drop_privs("hast", bindaddr); + g_gate_drop_privs("ggated", bindaddr); g_gate_drop_capabilities(conn->c_sendfd, conn->c_recvfd); -- 2.39.1 From 91a1999170e97b992ae833f1e1bcb34e5ef9b112 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Sat, 13 Nov 2021 09:56:26 +0100 Subject: [PATCH 25/29] ggated: Close the pid file and directory after forking ... so we're allowed to go to jail without having to set kern.pwd_chroot_chdir_check_open_directories=0 again. See also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=259770 Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 810c183c1b49..6db8a394303b 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -95,6 +95,7 @@ struct ggd_export { SLIST_ENTRY(ggd_export) e_next; }; +static struct pidfh *pfh; static const char *exports_file = GGATED_EXPORT_FILE; static int got_sighup = 0; static in_addr_t bindaddr; @@ -560,6 +561,9 @@ connection_launch(struct ggd_connection *conn) } g_gate_log(LOG_DEBUG, "Process created [%s].", conn->c_path); + if (pidfile_close(pfh) == -1) + g_gate_xlog("pidfile_close(): %s.", strerror(errno)); + if (getuid() == 0) g_gate_drop_privs("ggated", bindaddr); @@ -1071,7 +1075,6 @@ int main(int argc, char *argv[]) { const char *ggated_pidfile = _PATH_VARRUN "/ggated.pid"; - struct pidfh *pfh; struct sockaddr_in serv; struct sockaddr from; socklen_t fromlen; -- 2.39.1 From 03d069e8a004a5ab9e039b0afefb0a2f30fa00dc Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Sat, 13 Nov 2021 10:45:08 +0100 Subject: [PATCH 26/29] ggated: Let connection_launch() close the accepting socket after forking There's no reason why the children should have access to it. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 6db8a394303b..7870f929cc66 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -547,7 +547,7 @@ connection_ready(struct ggd_connection *conn) } static void -connection_launch(struct ggd_connection *conn) +connection_launch(struct ggd_connection *conn, int sfd) { pthread_t td; int error, pid; @@ -563,6 +563,8 @@ connection_launch(struct ggd_connection *conn) if (pidfile_close(pfh) == -1) g_gate_xlog("pidfile_close(): %s.", strerror(errno)); + if (close(sfd) == -1) + g_gate_xlog("close(sfd): %s.", strerror(errno)); if (getuid() == 0) g_gate_drop_privs("ggated", bindaddr); @@ -955,7 +957,7 @@ log_connection(struct sockaddr *from) } static int -handshake(struct sockaddr *from, int sfd) +handshake(struct sockaddr *from, int sfd, int listen_fd) { struct g_gate_version ver; struct g_gate_cinit cinit; @@ -1058,7 +1060,7 @@ handshake(struct sockaddr *from, int sfd) } if (connection_ready(conn)) { - connection_launch(conn); + connection_launch(conn, listen_fd); connection_remove(conn); } return (1); @@ -1184,7 +1186,7 @@ main(int argc, char *argv[]) exports_get(); } - if (!handshake(&from, tmpsfd)) + if (!handshake(&from, tmpsfd, sfd)) close(tmpsfd); } close(sfd); -- 2.39.1 From c4d20741d675d2259e27b80305c900d4597ee618 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 1 Nov 2021 15:06:57 +0100 Subject: [PATCH 27/29] ggated: Add undocumented -j option to test jailing Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 7870f929cc66..7f8d6bf2adef 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -1083,10 +1083,11 @@ main(int argc, char *argv[]) pid_t otherpid; int ch, sfd, tmpsfd; unsigned port; + int g_gate_jail_test = 0; bindaddr = htonl(INADDR_ANY); port = G_GATE_PORT; - while ((ch = getopt(argc, argv, "a:hnp:F:R:S:v")) != -1) { + while ((ch = getopt(argc, argv, "a:hnjp:F:R:S:v")) != -1) { switch (ch) { case 'a': bindaddr = g_gate_str2ip(optarg); @@ -1098,6 +1099,9 @@ main(int argc, char *argv[]) case 'F': ggated_pidfile = optarg; break; + case 'j': + g_gate_jail_test = 1; + break; case 'n': nagle = 0; break; @@ -1132,7 +1136,8 @@ main(int argc, char *argv[]) if (argv[0] != NULL) exports_file = argv[0]; - exports_get(); + if (!g_gate_jail_test) + exports_get(); pfh = pidfile_open(ggated_pidfile, 0600, &otherpid); if (pfh == NULL) { @@ -1173,6 +1178,13 @@ main(int argc, char *argv[]) signal(SIGHUP, huphandler); + if (g_gate_jail_test) { + pidfile_close(pfh); + g_gate_drop_privs("ggated", bindaddr); + pidfile_remove(pfh); + exit(EXIT_SUCCESS); + } + for (;;) { fromlen = sizeof(from); tmpsfd = accept(sfd, &from, &fromlen); -- 2.39.1 From a3dd72fba6050d690774a3405b5449cd03493b22 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 1 Nov 2021 16:32:40 +0100 Subject: [PATCH 28/29] ggated: Add undocumented -J flag to disable jailing Should work around: fk@r500 ~ $sudo ggated -v -j info: Reading exports file (/etc/gg.exports). debug: Added 127.0.0.1/32 /dev/zvol/r500/ggated/t520.eli RW to exports list. info: Exporting 1 object(s). info: Listen on port: 3080. error: Unable to jail process in directory /var/empty error: Exiting. Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 5 ++++- sbin/ggate/shared/ggate.c | 3 ++- sbin/ggate/shared/ggate.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 7f8d6bf2adef..9203ead8d323 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -1087,7 +1087,7 @@ main(int argc, char *argv[]) bindaddr = htonl(INADDR_ANY); port = G_GATE_PORT; - while ((ch = getopt(argc, argv, "a:hnjp:F:R:S:v")) != -1) { + while ((ch = getopt(argc, argv, "a:hnjJp:F:R:S:v")) != -1) { switch (ch) { case 'a': bindaddr = g_gate_str2ip(optarg); @@ -1099,6 +1099,9 @@ main(int argc, char *argv[]) case 'F': ggated_pidfile = optarg; break; + case 'J': + g_gate_no_jailing = 1; + break; case 'j': g_gate_jail_test = 1; break; diff --git a/sbin/ggate/shared/ggate.c b/sbin/ggate/shared/ggate.c index 4c0d3886f463..05a2cdcd1f83 100644 --- a/sbin/ggate/shared/ggate.c +++ b/sbin/ggate/shared/ggate.c @@ -66,6 +66,7 @@ int g_gate_devfd = -1; +int g_gate_no_jailing = 0; int g_gate_verbose = 0; static int g_gate_logfd = -1; @@ -496,7 +497,7 @@ g_gate_drop_privs(const char *ggate_user, in_addr_t jail_address) jailst.ip4 = &jail_ip; jailst.ip6s = 0; jailst.ip6 = NULL; - if (jail(&jailst) == -1) { + if (!g_gate_no_jailing && jail(&jailst) == -1) { g_gate_xlog("Unable to jail process in directory %s", pw->pw_dir); } PJDLOG_VERIFY(chdir("/") == 0); diff --git a/sbin/ggate/shared/ggate.h b/sbin/ggate/shared/ggate.h index 2359b0af6ddf..e3af84e437ea 100644 --- a/sbin/ggate/shared/ggate.h +++ b/sbin/ggate/shared/ggate.h @@ -61,6 +61,7 @@ #define GGATE_CMD_DELETE 4 extern int g_gate_devfd; +extern int g_gate_no_jailing; extern int g_gate_verbose; extern int nagle; -- 2.39.1 From d81f276f11fa99baae968954a828d01de42d5f4b Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Sun, 19 Apr 2015 22:58:49 +0200 Subject: [PATCH 29/29] ggated: Default to listening to 127.0.0.1 only Obtained from: ElectroBSD --- sbin/ggate/ggated/ggated.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c index 9203ead8d323..35a097954203 100644 --- a/sbin/ggate/ggated/ggated.c +++ b/sbin/ggate/ggated/ggated.c @@ -1085,7 +1085,7 @@ main(int argc, char *argv[]) unsigned port; int g_gate_jail_test = 0; - bindaddr = htonl(INADDR_ANY); + bindaddr = g_gate_str2ip("127.0.0.1"); port = G_GATE_PORT; while ((ch = getopt(argc, argv, "a:hnjJp:F:R:S:v")) != -1) { switch (ch) { -- 2.39.1