Re: [PATCH v2 07/11] transport: log received server trace2 SID

From: Junio C Hamano <gitster@pobox.com>
To: Josh Steadmon <steadmon@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 07/11] transport: log received server trace2 SID
Date: Wed, 04 Nov 2020 13:14:31 -0800	[thread overview]
Message-ID: <xmqqpn4slt8o.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <21bdbf23f39c800b1722c54b666df7a91b5879b5.1604355792.git.steadmon@google.com> (Josh Steadmon's message of "Mon, 2 Nov 2020 14:31:05 -0800")

Josh Steadmon <steadmon@google.com> writes:

> When a client receives a trace2-sid capability from a protocol v0, v1,
> or v2 server, log the received session ID via a trace2 data event.

Would this pose a new security threat surface?  Just wondering if we
want to ignore the capability if it is not enabled on our end with
the configuration.

Thanks.

> diff --git a/transport.c b/transport.c
> index 47da955e4f..d16be597bd 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -286,6 +286,8 @@ static struct ref *handshake(struct transport *transport, int for_push,
>  	struct git_transport_data *data = transport->data;
>  	struct ref *refs = NULL;
>  	struct packet_reader reader;
> +	int sid_len;
> +	const char *server_trace2_sid;
>  
>  	connect_setup(transport, for_push);
>  
> @@ -297,6 +299,8 @@ static struct ref *handshake(struct transport *transport, int for_push,
>  	data->version = discover_version(&reader);
>  	switch (data->version) {
>  	case protocol_v2:
> +		if (server_feature_v2("trace2-sid", &server_trace2_sid))
> +			trace2_data_string("trace2", NULL, "server-sid", server_trace2_sid);
>  		if (must_list_refs)
>  			get_remote_refs(data->fd[1], &reader, &refs, for_push,
>  					ref_prefixes,
> @@ -310,6 +314,12 @@ static struct ref *handshake(struct transport *transport, int for_push,
>  				 for_push ? REF_NORMAL : 0,
>  				 &data->extra_have,
>  				 &data->shallow);
> +		server_trace2_sid = server_feature_value("trace2-sid", &sid_len);
> +		if (server_trace2_sid) {
> +			char *server_sid = xstrndup(server_trace2_sid, sid_len);
> +			trace2_data_string("trace2", NULL, "server-sid", server_sid);
> +			free(server_sid);
> +		}
>  		break;
>  	case protocol_unknown_version:
>  		BUG("unknown protocol version");