On Sun, Jan 22, 2023 at 12:48:00PM +0100, Omar Polo wrote:
> gotd fails to handle the early disconnection of the clients.  this has
> the effect of causing new connections to hit the limits when they
> shouldn't.
> 
> The fix is easy, just the first hunk in the diff below, ie. don't
> ignore GOT_ERR_EOF.  Nothing in gotd_request (except for
> gotd_imsg_recv) can return it.  We may still have something left
> queued up to send, but it shouldn't be an issue since the client
> closed the connection anyway.
> 
> I've discovered this gotd issue by playing with gotsh and forgetting
> the path, i.e. `ssh gotdev@localhost git-upload-pack'.
> 
> So, I'm bundling also a tweak for gotsh to error earlier (by exporting
> parse_command call it before socket(2).)  This means that connection
> with an invalid cmdline now are _not_ counted against the user limits,
> but I don't think it's a big issue.  (before they were counted only
> 'sometimes'.)
> 
>     before:
>     % ssh gotdev@localhost git-receive-pack
>     gotsh: bad packet received
> 
>     after:
>     % ssh gotdev@localhost git-receive-pack
>     usage: gotsh -c 'git-receive-pack|git-upload-pack repository-path'

Thanks, I like this. ok.

> For last, there's a very simple test case for it.  It first try to hit
> the limits via `ssh', and then only via nc(1).  It's not committable
> as-is (and btw needs `permit nopass $yourself as gotdev' in doas.conf)
> but helps in reproducing the issue at least :)
> Will probably drop the second part.

Anyone on the local system can connect to the socket now, so doas
should actually not be needed?

It would be nice to have ways to test such things in the test suite.