Re: [PATCH v6 2/5] Documentation: define protected configuration

[Glen Choo <chooglen@google.com>]

Taylor Blau <me@ttaylorr.com> writes:

> On Thu, Jun 30, 2022 at 06:13:56PM +0000, Glen Choo via GitGitGadget wrote:
>> @@ -380,6 +381,18 @@ Most configuration options are respected regardless of the scope it is
>>  defined in, but some options are only respected in certain scopes. See the
>>  option's documentation for the full details.
>>
>> +Protected configuration
>> +~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Protected configuration refers to the 'system', 'global', and 'command' scopes.
>> +For security reasons, certain options are only respected when they are
>> +specified in protected configuration, and ignored otherwise.
>> +
>> +Git treats these scopes as if they are controlled by the user or a trusted
>> +administrator. This is because an attacker who controls these scopes can do
>> +substantial harm without using Git, so it is assumed that the user's environment
>> +protects these scopes against attackers.
>> +
>
> I think this description is a good starting point, but I think I would
> have liked to see some more from the commit description make it into the
> documentation here.

Yeah, there's a bit of a tradeoff here. Glossing over some of the
details helps keep the documentation briefer and easier to understand
for the less experienced/invested, but is bound to frustrate others. I'd
appreciate any wording suggestions if you have any.

> One thing that I didn't see mentioned in either is that the list of
> protected configuration is far from exhaustive. There are dozens upon
> dozens of configuration values that Git will happily execute as a
> subprocess (core.editor, core.pager, core.alternateRefsCommand, to name
> just a few).
>
> I don't think we should try and enumerate every possible path from
> configuration to command execution. But it is worth noting in the
> documentation that the list of configuration values which are only read
> in the protected context is non-exhaustive and best-effort only.

By referencing command execution, I think you are alluding to Stolee's
Security Boundary discussion thread [1], and in particular, the "Example
Security Boundary Question: Unprotected Config and Executing Code"
section?

That section discusses the problem of arbitrary command execution based
on repository-local config, and how protected configuration might give
us a way to prevent that. That's a reasonable extension to this series,
though it seems a little premature to include allusions to command
execution, especially since I don't think we're anywhere close to a
long-term direction on what should/shouldn't be inside protected
configuration. For example, Stolee noted that, most of the command
execution options really do want per-repository customization, so if we
want to continue to support that, we'll need to use protected
configuration in a somewhat sophisticated manner (and not, e.g. only
respect command execution options in protected configuration). Perhaps
we could shelve this wording change until we've committed to such a
direction.

[1] https://lore.kernel.org/git/6af83767-576b-75c4-c778-0284344a8fe7@github.com

> Thanks,
> Taylor