From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotd flush packets
To: gameoftrees@openbsd.org
Date: Sat, 21 Jan 2023 09:15:00 +0100

On Fri, Jan 20, 2023 at 07:57:59PM +0100, Stefan Sperling wrote:
> Someone did a silly thing against our public git mirror:
> 
> Jan 20 19:20:23 florabel gotd[74429]: received flush-pkt from uid 1002
> Jan 20 19:20:43 florabel last message repeated 251236 times
> Jan 20 19:20:43 florabel gotd[70339]: list-refs request from uid 1002
> Jan 20 19:20:43 florabel gotd[74429]: received flush-pkt from uid 1002
> Jan 20 19:21:14 florabel last message repeated 386788 times
> Jan 20 19:21:43 florabel last message repeated 367291 times

Looking closer in less of a hurry, I found several instances of this
in the server logs. This is not being triggered on purpose, but happens
because gotd can keep reading from a pipe after EOF in serve_read().
The patch below should fix this by returning an EOF error code when
read(2) indicates EOF.

Also, make gotsh only forward flush packets to the session process
if the client is currrently expected to send want or have lines.

Passes regress tests, and my manual tests are still working.

OK?

(Regardless, I still want to apply my other patch which detects
mulitple flush packets, since it a local user might be connecting
to the socket directly without gotsh.)

diff /home/stsp/src/got
commit - 7713cc5e4f5544e81909670d592e89526ed86c9b
path + /home/stsp/src/got
blob - 4330e9e71a7dcd41735ee2f4222e4f39e824d121
file + lib/pkt.c
--- lib/pkt.c
+++ lib/pkt.c
@@ -36,7 +36,7 @@ got_pkt_readn(ssize_t *off, int fd, void *buf, size_t 
 		if (r == -1)
 			return got_error_from_errno("read");
 		if (r == 0)
-			return NULL;
+			return got_error(GOT_ERR_EOF);
 		*off += r;
 	}
 	return NULL;
blob - ff9cd1e8a402bfed028e0aa1db7d6cf94c159eb4
file + lib/serve.c
--- lib/serve.c
+++ lib/serve.c
@@ -928,9 +928,13 @@ serve_read(int infd, int outfd, int gotd_sock, const c
 				goto done;
 			}
 
-			err = forward_flushpkt(&ibuf);
-			if (err)
-				goto done;
+			if (curstate == STATE_EXPECT_WANT ||
+			    curstate == STATE_EXPECT_MORE_WANT ||
+			    curstate == STATE_EXPECT_HAVE) {
+				err = forward_flushpkt(&ibuf);
+				if (err)
+					goto done;
+			}
 			if (curstate == STATE_EXPECT_HAVE && !have_ack) {
 				err = send_nak(outfd, chattygot);
 				if (err)