From: Omar Polo Subject: Re: ignore config files with non-exclusive ownership To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Thu, 27 Oct 2022 14:11:12 +0200 On 2022/10/27 10:10:36 +0200, Stefan Sperling wrote: > On Tue, Oct 25, 2022 at 12:02:29PM +0200, Omar Polo wrote: > > I dislike piling up env variables. In some case it's the easiest way > > or mostly needed for the regress so it's kind of ok, but for potential > > security issues like this I prefer a line in the sand approach: we > > either don't care or straight up refuse to work. > > > > If Alice and Bob share a git repository then Alice is likely to set > > GOT_IGNORE_CONFIG_FILE_PERMISSIONS to work with Bob and this opens up > > the doors for Eve to to trick Alice. woops. > > That's quite right. And the nature of this override being a global flag > would lead to people who would benefit from the extra check the most > effectively disabling the extra check for all repositories by setting > this variable in their environment. > > I guess you are right that there is nothing much we can do to make > this 100% water tight, short of refusing to work on shared repositories > altogether. But I don't want to forbid this use case. There is maybe another thing we can do: ignore most (all?) the options in shared repositories instead of failing, and requiring users to fill that data in their worktree' got.conf. At least options like `author' and `signer_id' don't really make sense to be in a shared got.conf. `allowed_signers' and `revoked_signers' are potentially harmful if under the control of another user. Ignoring the configuration only in some circumstances however can be confusing. > There are other > problems with this use case, such as not respecting umask on file creation. > Such problems should be fixed. > > I guess our stance will then be that users shouldn't run 'got fetch' > or 'got send' on repositories that are shared without paying some > attention to where the remote repo configuration ends up taking them. > > I also forgot that it is possible to override per-repository settings > in the work-tree's .got/got.conf file (see cmd_fetch() and cmd_send() > in got/got.c). Our recommendation should be that this file should be > used to configure remote repositories in case of a shared repo setup. > I'll update the docs accordingly. > > > Both `got send' and `got fetch' are clear enough to communicate what > > they're doing: > > > > % got send > > Connecting to "origin" git@foobar.com > > ... > > > > However once the user reads that line it's very likely too late and > > the connection to the remote server has been made. > > If the user is using ssh:// rather than git:// then SSH host key > fingerprint checking can still prevent an unwanted connection to > a new host at this point. I forgot about this, right, ssh should at least ask the user if it's a host they never connected to. Tangential to the subject, but still marginally applicable, should we make `got fetch' and `got send' print the full address? i.e. % got send Connecting to "origin" ssh://git@foobar.com/path/to/repo.git ... > > So, if we consider insecure connecting to unknown servers, why don't > > just fail? Are shared bare repositories common? > > I don't know if they are very common, but apart from the concern about > unexpected remote repository config there is no reason to require > everyone on a shared system to make a fresh clone of a repository they > need. It would be pointless waste of network, disk, and CPU resources. Agreed, it's quite wasteful to require N copies of a repository for N users to work on in the same machine.