All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Calvin Wan <calvinwan@google.com>
Cc: git@vger.kernel.org, gitster@pobox.com, jonathantanmy@google.com,
	philipoakley@iee.email, johncai86@gmail.com
Subject: Re: [PATCH v5 6/6] cat-file: add remote-object-info to batch-command
Date: Thu, 11 Aug 2022 12:58:52 +0200	[thread overview]
Message-ID: <220811.86a68bqbwb.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <CAFySSZBPoJC8je2jr4_nrrzdFG3=HuRcou6qjrJTNOzue1pvgQ@mail.gmail.com>


On Mon, Aug 08 2022, Calvin Wan wrote:

>> > Since the `info` command in cat-file --batch-command prints object info
>> > for a given object, it is natural to add another command in cat-file
>> > --batch-command to print object info for a given object from a remote.
>>
>> Is it ?:)
>
> Haha yes this could use a little rewording
>
>> > Add `remote-object-info` to cat-file --batch-command.
>>
>> I realize this bit of implementation changed in v4, i.e. it used to be
>> in "fetch", and I'm happy to have it moved out of there, we don't need
>> to overload it more.
>>
>> But I remember thinking (and perhaps commenting on-list, I can't
>> remember) that the "object-info" server verb was a bit odd at the time
>> that it was implemented. I understand the motivation, but surely it was
>> stumbling its way towards being something more generic, i.e. being able
>> to just expose cmd_cat_file() in some form.
>>
>> Which is one of the goals I've had in mind with working on fixing memory
>> leaks in various places, i.e. once you get common commands to clean up
>> after themselves it usually becomes to have a "command server".
>>
>> So (and I don't mind if this is longer term, just asking), is there a
>> reason for why we wouldn't want to do away with object-info and this
>> "cat-file talks to a remote", in favor of just having support for
>> invoking arbitrary commands from a remote.
>>
>> Of course that set of allowed RCE commands would be zero by default, but
>> if we had some way to define tha "cat-file" was allowed to be called,
>> and only if you invoked:
>>
>>         cat-file --batch="%(objectsize)"
>>
>> Or whatever, but over the v2 protocol, wouldn't we basically have
>> object-info in a more roundabout way?
>
> While I do think that if we did have a set of allowed RCE commands, this
> would be a good candidate to be one of those commands. I am worried
> about security, maintainability, and server performance risks this change
> would also carry. Figuring out which commands are secure and would
> not overload the server, and then maintaining that set seems like a much
> more worrisome design than having a secure git server.

I'm only suggesting that the interface be extendable enough to allow for
that, not that we do it right now, i.e. that the protocol command could
be:

	# With appropriate \0 delimiting etc.
	cmd cat-file -e <SHA1>

As opposed to something like:

	object-exists <SHA1>

But that we would *not* for now expose some mechanism where the server
operator could configure which arbitrary commands to allow, we'd just
intercept that specifically, and dispatch it to the appropriate code in
builtin/cat-file.c (or libify them enough to expose them, and then call
that).

Of course one of the points of doing so would be to eventually expose
the ability to run a larger set of safe commands remotely, if the server
operator agrees, but we'd leave that for the future.

Right now we'd get the benefit of not duplicating various parts of
existing code, and having a plain one-to-one mapping to existing
commands.


  reply	other threads:[~2022-08-11 11:04 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-08 23:19 [PATCH] fetch —object-info-format: client option for object-info Calvin Wan
2022-02-08 23:56 ` [PATCH v2] " Calvin Wan
2022-02-09 12:48   ` Philip Oakley
2022-02-10 22:32     ` Calvin Wan
2022-02-09 20:41   ` [PATCH v2] fetch object-info-format: " Jonathan Tan
2022-02-10 22:58     ` Calvin Wan
2022-03-28 19:11   ` [PATCH v3 0/3] object-info: add option for retrieving object info Calvin Wan
2022-03-28 19:11     ` [PATCH v3 1/3] fetch-pack: refactor packet writing and fetch options Calvin Wan
2022-03-29 22:54       ` Junio C Hamano
2022-03-29 23:01       ` Taylor Blau
2022-03-30 21:55       ` Jonathan Tan
2022-03-28 19:11     ` [PATCH v3 2/3] transfer.advertiseObjectInfo: add object-info config Calvin Wan
2022-03-29 22:34       ` Junio C Hamano
2022-03-29 22:48         ` Calvin Wan
2022-03-29 23:09           ` Taylor Blau
2022-03-28 19:11     ` [PATCH v3 3/3] object-info: add option for retrieving object info Calvin Wan
2022-03-29 19:57       ` Junio C Hamano
2022-03-29 22:54       ` Junio C Hamano
2022-03-30  0:49         ` Junio C Hamano
2022-03-30 22:31           ` Calvin Wan
2022-03-30 22:43         ` Jonathan Tan
2022-03-30 23:42           ` Junio C Hamano
2022-03-29 23:19       ` Taylor Blau
2022-03-30 22:47         ` Calvin Wan
2022-03-30 22:06       ` John Cai
2022-03-31 19:56         ` Calvin Wan
2022-04-01 16:16           ` Junio C Hamano
2022-03-30 22:07       ` Jonathan Tan
2022-03-30 22:12       ` Josh Steadmon
2022-03-30 22:46         ` Calvin Wan
2022-03-29 20:35     ` [PATCH v3 0/3] " Junio C Hamano
2022-03-29 22:40       ` Calvin Wan
2022-03-31  1:50     ` Junio C Hamano
2022-05-02 17:08     ` [PATCH v4 0/8] cat-file: add --batch-command remote-object-info command Calvin Wan
2022-05-02 17:08       ` [PATCH v4 1/8] fetch-pack: refactor packet writing Calvin Wan
2022-05-02 17:08       ` [PATCH v4 2/8] fetch-pack: move fetch default settings Calvin Wan
2022-05-02 22:58         ` Junio C Hamano
2022-05-03 23:06           ` Jonathan Tan
2022-05-05 18:13             ` Calvin Wan
2022-05-02 17:08       ` [PATCH v4 3/8] object-store: add function to free object_info contents Calvin Wan
2022-05-02 23:23         ` Junio C Hamano
2022-05-04 19:09           ` Junio C Hamano
2022-05-05  0:15           ` Junio C Hamano
2022-05-05 16:47             ` Calvin Wan
2022-05-05 17:01               ` Junio C Hamano
2022-05-02 17:09       ` [PATCH v4 4/8] object-info: send attribute packet regardless of object ids Calvin Wan
2022-05-03  0:05         ` Junio C Hamano
2022-05-03 19:21           ` Calvin Wan
2022-05-03 23:11         ` Jonathan Tan
2022-05-02 17:09       ` [PATCH v4 5/8] transport: add client side capability to request object-info Calvin Wan
2022-05-03  0:54         ` Junio C Hamano
2022-05-03 18:58           ` Calvin Wan
2022-05-04 15:42             ` Junio C Hamano
2022-05-03 23:15         ` Jonathan Tan
2022-05-04 15:50           ` Junio C Hamano
2022-05-02 17:09       ` [PATCH v4 6/8] transport: add object-info fallback to fetch Calvin Wan
2022-05-03 23:27         ` Jonathan Tan
2022-05-02 17:09       ` [PATCH v4 7/8] cat-file: move parse_cmd and DEFAULT_FORMAT up Calvin Wan
2022-05-02 17:09       ` [PATCH v4 8/8] cat-file: add --batch-command remote-object-info command Calvin Wan
2022-05-04 21:27         ` Jonathan Tan
2022-05-05 18:13           ` Calvin Wan
2022-05-05 18:44             ` Junio C Hamano
2022-05-05 19:09               ` Junio C Hamano
2022-05-05 19:19                 ` Calvin Wan
2022-07-31 15:24         ` ZheNing Hu
2022-08-08 17:43           ` Calvin Wan
2022-07-28 23:02       ` [PATCH v5 0/6] " Calvin Wan
2022-07-28 23:56         ` Junio C Hamano
2022-07-29  0:02           ` Junio C Hamano
2022-07-31  8:41         ` Phillip Wood
2022-08-04 22:57           ` Calvin Wan
2022-09-30 23:23         ` Junio C Hamano
2022-07-28 23:02       ` [PATCH v5 1/6] fetch-pack: refactor packet writing Calvin Wan
2022-07-28 23:02       ` [PATCH v5 2/6] fetch-pack: move fetch initialization Calvin Wan
2022-07-28 23:02       ` [PATCH v5 3/6] protocol-caps: initialization bug fix Calvin Wan
2022-07-29 17:51         ` Junio C Hamano
2022-07-28 23:02       ` [PATCH v5 4/6] serve: advertise object-info feature Calvin Wan
2022-07-29 17:57         ` Junio C Hamano
2022-08-01 18:28           ` Calvin Wan
2022-08-01 18:44             ` Ævar Arnfjörð Bjarmason
2022-08-01 18:47             ` Junio C Hamano
2022-08-01 18:58               ` Calvin Wan
2022-07-28 23:02       ` [PATCH v5 5/6] transport: add client support for object-info Calvin Wan
2022-07-29 18:06         ` Junio C Hamano
2022-08-04 20:28           ` Calvin Wan
2022-08-01 13:30         ` Phillip Wood
2022-08-04 22:20           ` Calvin Wan
2022-08-08 10:07             ` Phillip Wood
2022-08-01 14:26         ` Phillip Wood
2022-08-08  9:16         ` Phillip Wood
2022-07-28 23:02       ` [PATCH v5 6/6] cat-file: add remote-object-info to batch-command Calvin Wan
2022-07-29  6:25         ` Ævar Arnfjörð Bjarmason
2022-08-07  5:50           ` ZheNing Hu
2022-08-08 18:07           ` Calvin Wan
2022-08-11 10:58             ` Ævar Arnfjörð Bjarmason [this message]
2022-07-29 18:21         ` Junio C Hamano
2022-08-08 18:37           ` Calvin Wan
2022-09-28 13:12         ` Ævar Arnfjörð Bjarmason
2022-07-31 15:02       ` [PATCH v4 0/8] cat-file: add --batch-command remote-object-info command ZheNing Hu
2022-08-08 17:32         ` Calvin Wan
2022-08-13 22:17       ` Junio C Hamano
2022-02-09 19:09 ` [PATCH] fetch —object-info-format: client option for object-info John Cai
2022-02-10 22:49   ` Calvin Wan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=220811.86a68bqbwb.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=calvinwan@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johncai86@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=philipoakley@iee.email \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.