Re: git version statistics

8 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: git version statistics

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:57

Jeff King [off-list ref] writes:

I don't know if anybody cares about the security or privacy implications
of advertising your client version. Maybe it should be configurable?

I do not think it is worth it.

My initial reaction to the patch was a bit of trouble with the word
"agent", as we do not call Git acting on behalf of the end user "an
agent" in general. But it could be used as an excuse for not giving
an extra knob to tweak, as you generally do not muck with User-Agent
strings, either ;-).

quoted

Do we want a similar identifier string on the other side of the
connection?

We could. I don't see much point, unless you were going to conduct a
similar survey by hitting random IPs looking for git ports (but even
then, you're not likely to turn up much, because you have to know a repo
name before you can convince git to show a capability string). I suppose
it could also help with debugging if your client is having trouble
talking to a server that is not under your control.

The latter use case was exactly what I had in mind.

Some traditional security advice I have heard is that servers should not
advertise their versions, as it makes it more obvious what holes they
have. Personally, I find that argument to be mostly security through
obscurity.

I do, too, but shipping with a configuration knob to optionally turn
it off would be sufficient.

Re: git version statistics

From: Jeff King <hidden>
Date: 2016-06-15 22:53:58

On Fri, Jun 01, 2012 at 07:49:17AM -0700, Junio C Hamano wrote:

My initial reaction to the patch was a bit of trouble with the word
"agent", as we do not call Git acting on behalf of the end user "an
agent" in general.

Yeah, I don't especially like the term "agent". I had initially called
it "version", but rejected that for two reasons:

  1. It is not just a version, but also telling what software is in use
     (so I would expect git to write git/v1.7.10, and other
     implementations to write write dulwich/1.2.3 or whatever).

  2. I didn't want it to be confused as a protocol version.

But maybe those are non-issues. It should be fairly obvious what it is
when you see even one example of the value.

quoted

Some traditional security advice I have heard is that servers should not
advertise their versions, as it makes it more obvious what holes they
have. Personally, I find that argument to be mostly security through
obscurity.

I do, too, but shipping with a configuration knob to optionally turn
it off would be sufficient.

I think the most sensible thing is to just add a Makefile variable
that defaults to $(GIT_VERSION), and let people override it if they want
privacy. The http user-agent variable actually respects an environment
variable, but I don't see much point in going that far.

I'll cook up a new version of the patch.

-Peff

Re: git version statistics

From: Tomas Carnecky <hidden>
Date: 2016-06-15 22:53:58

On Sat, 02 Jun 2012 12:32:48 -0400, Jeff King [off-list ref] wrote:

On Fri, Jun 01, 2012 at 07:49:17AM -0700, Junio C Hamano wrote:

quoted

My initial reaction to the patch was a bit of trouble with the word
"agent", as we do not call Git acting on behalf of the end user "an
agent" in general.

Wikipedia defines User agent as 'a software [...] that is acting on behalf of
a user'. That fits this situation quite well, don't you think?

Re: git version statistics

From: Jeff King <hidden>
Date: 2016-06-15 22:53:58

On Sat, Jun 02, 2012 at 12:32:48PM -0400, Jeff King wrote:

I'll cook up a new version of the patch.

The refactoring ended up expanding this into a few patches:

  [1/4]: move git_version_string into version.c
  [2/4]: version: add git_user_agent function
  [3/4]: http: get default user-agent from git_user_agent
  [4/4]: include agent identifier in capability string

-Peff

[PATCH 1/4] move git_version_string into version.c

From: Jeff King <hidden>
Date: 2016-06-15 22:53:58

The global git_version_string currently lives in git.c, but
doesn't have anything to do with the git wrapper. Let's move
it into its own file, where it will be more appropriate to
build more version-related functions.

Signed-off-by: Jeff King <redacted>
---
 Makefile      | 8 ++++++--
 builtin.h     | 1 -
 builtin/log.c | 1 +
 git.c         | 2 --
 help.c        | 1 +
 version.c     | 4 ++++
 version.h     | 6 ++++++
 7 files changed, 18 insertions(+), 5 deletions(-)
 create mode 100644 version.c
 create mode 100644 version.h

diff --git a/Makefile b/Makefile
index 4592f1f..b394f85 100644
--- a/Makefile
+++ b/Makefile

@@ -799,6 +799,7 @@ LIB_OBJS += usage.o
 LIB_OBJS += userdiff.o
 LIB_OBJS += utf8.o
 LIB_OBJS += varint.o
+LIB_OBJS += version.o
 LIB_OBJS += walker.o
 LIB_OBJS += wrapper.o
 LIB_OBJS += write_or_die.o

@@ -1962,7 +1963,7 @@ strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
 git.o: common-cmds.h
-git.sp git.s git.o: EXTRA_CPPFLAGS = -DGIT_VERSION='"$(GIT_VERSION)"' \
+git.sp git.s git.o: EXTRA_CPPFLAGS = \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
 	'-DGIT_INFO_PATH="$(infodir_SQ)"'

@@ -1979,6 +1980,9 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
 	'-DGIT_INFO_PATH="$(infodir_SQ)"'
 
+version.sp version.s version.o: EXTRA_CPPFLAGS = \
+	'-DGIT_VERSION="$(GIT_VERSION)"'
+
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
 	ln git$X $@ 2>/dev/null || \

@@ -2089,7 +2093,7 @@ configure: configure.ac
 	$(RM) $<+
 
 # These can record GIT_VERSION
-git.o git.spec http.o \
+version.o git.spec http.o \
 	$(patsubst %.sh,%,$(SCRIPT_SH)) \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE

diff --git a/builtin.h b/builtin.h
index 338f540..dea1643 100644
--- a/builtin.h
+++ b/builtin.h

@@ -9,7 +9,6 @@
 
 #define DEFAULT_MERGE_LOG_LEN 20
 
-extern const char git_version_string[];
 extern const char git_usage_string[];
 extern const char git_more_info_string[];

diff --git a/builtin/log.c b/builtin/log.c
index 906dca4..4f1b42a 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -21,6 +21,7 @@
 #include "parse-options.h"
 #include "branch.h"
 #include "streaming.h"
+#include "version.h"
 
 /* Set a default date-time format for git log ("log.date" config variable) */
 static const char *default_date_mode = NULL;

diff --git a/git.c b/git.c
index d232de9..4da3db5 100644
--- a/git.c
+++ b/git.c

@@ -256,8 +256,6 @@ static int handle_alias(int *argcp, const char ***argv)
 	return ret;
 }
 
-const char git_version_string[] = GIT_VERSION;
-
 #define RUN_SETUP		(1<<0)
 #define RUN_SETUP_GENTLY	(1<<1)
 #define USE_PAGER		(1<<2)

diff --git a/help.c b/help.c
index 6012c07..662349d 100644
--- a/help.c
+++ b/help.c

@@ -6,6 +6,7 @@
 #include "common-cmds.h"
 #include "string-list.h"
 #include "column.h"
+#include "version.h"
 
 void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {

diff --git a/version.c b/version.c
new file mode 100644
index 0000000..ca68653
--- /dev/null
+++ b/version.c

@@ -0,0 +1,4 @@
+#include "git-compat-util.h"
+#include "version.h"
+
+const char git_version_string[] = GIT_VERSION;

diff --git a/version.h b/version.h
new file mode 100644
index 0000000..8d6c413
--- /dev/null
+++ b/version.h

@@ -0,0 +1,6 @@
+#ifndef VERSION_H
+#define VERSION_H
+
+extern const char git_version_string[];
+
+#endif /* VERSION_H */

-- 
1.7.7.7.32.g4b73117

[PATCH 2/4] version: add git_user_agent function

From: Jeff King <hidden>
Date: 2016-06-15 22:53:58

This is basically a fancy way of saying "git/$GIT_VERSION",
except that it is overridable at build-time and through the
environment. Which means that people who don't want to
advertise their git version (for privacy or security
reasons) can tweak it.

Signed-off-by: Jeff King <redacted>
---
The next patch adapts http.c to use this, and of course the one after
that adds support for the git protocol itself. There are a few other
places where the git version leaks publicly, including at least:

  1. in the x-mailer header of send-email

  2. at the bottom of format-patch emails

  3. in the mime boundaries (!) of format-patch emails

Since the default here is functionally identical, and the only people
who would care are those interested in masking their version, and nobody
has actually come forward and said they want to do that, I am inclined
not to worry about it. I'd consider the masking to be more important (if
it is important at all) on the server side.  Perhaps this series will
catch the attention of people who do care, and they can decide if they
would like to take it farther.

 Makefile  | 11 +++++++++++
 version.c | 13 +++++++++++++
 version.h |  2 ++
 3 files changed, 26 insertions(+)

diff --git a/Makefile b/Makefile
index b394f85..e6e65ca 100644
--- a/Makefile
+++ b/Makefile

@@ -296,6 +296,9 @@ all::
 # the diff algorithm.  It gives a nice speedup if your processor has
 # fast unaligned word loads.  Does NOT work on big-endian systems!
 # Enabled by default on x86_64.
+#
+# Define GIT_USER_AGENT if you want to change how git identifies itself during
+# network interactions.  The default is "git/$(GIT_VERSION)".
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN

@@ -905,6 +908,8 @@ BUILTIN_OBJS += builtin/write-tree.o
 GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
 EXTLIBS =
 
+GIT_USER_AGENT = git/$(GIT_VERSION)
+
 #
 # Platform specific tweaks
 #

@@ -1916,6 +1921,11 @@ SHELL_PATH_CQ_SQ = $(subst ','\'',$(SHELL_PATH_CQ))
 BASIC_CFLAGS += -DSHELL_PATH='$(SHELL_PATH_CQ_SQ)'
 endif
 
+GIT_USER_AGENT_SQ = $(subst ','\'',$(GIT_USER_AGENT))
+GIT_USER_AGENT_CQ = "$(subst ",\",$(subst \,\\,$(GIT_USER_AGENT)))"
+GIT_USER_AGENT_CQ_SQ = $(subst ','\'',$(GIT_USER_AGENT_CQ))
+BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)'
+
 ALL_CFLAGS += $(BASIC_CFLAGS)
 ALL_LDFLAGS += $(BASIC_LDFLAGS)

@@ -2000,6 +2010,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
     -e 's|@@DIFF@@|$(DIFF_SQ)|' \
     -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+    -e 's|@@GIT_USER_AGENT@@|$(GIT_USER_AGENT_SQ)|g' \
     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
     -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
     -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \

diff --git a/version.c b/version.c
index ca68653..f98d5a6 100644
--- a/version.c
+++ b/version.c

@@ -2,3 +2,16 @@
 #include "version.h"
 
 const char git_version_string[] = GIT_VERSION;
+
+const char *git_user_agent(void)
+{
+	static const char *agent = NULL;
+
+	if (!agent) {
+		agent = getenv("GIT_USER_AGENT");
+		if (!agent)
+			agent = GIT_USER_AGENT;
+	}
+
+	return agent;
+}

diff --git a/version.h b/version.h
index 8d6c413..fd9cdd6 100644
--- a/version.h
+++ b/version.h

@@ -3,4 +3,6 @@
 
 extern const char git_version_string[];
 
+const char *git_user_agent(void);
+
 #endif /* VERSION_H */

-- 
1.7.7.7.32.g4b73117

[PATCH 3/4] http: get default user-agent from git_user_agent

From: Jeff King <hidden>
Date: 2016-06-15 22:53:58

This means we will respect the GIT_USER_AGENT build-time
configuration and run-time environment variable.

Signed-off-by: Jeff King <redacted>
---
Note that this needs to be applied on the recent "http.o depends
on GIT-VERSION-FILE" fix by Erik, which just went into master. It
actually undoes that commit, but that is because the dependency no
longer exists.

 Makefile | 5 +----
 http.c   | 3 ++-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index e6e65ca..62de0b4 100644
--- a/Makefile
+++ b/Makefile

@@ -2104,7 +2104,7 @@ configure: configure.ac
 	$(RM) $<+
 
 # These can record GIT_VERSION
-version.o git.spec http.o \
+version.o git.spec \
 	$(patsubst %.sh,%,$(SCRIPT_SH)) \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE

@@ -2274,9 +2274,6 @@ attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \
 gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \
 	-DGIT_LOCALE_PATH='"$(localedir_SQ)"'
 
-http.sp http.s http.o: EXTRA_CPPFLAGS = \
-	-DGIT_HTTP_USER_AGENT='"git/$(GIT_VERSION)"'
-
 ifdef NO_EXPAT
 http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT
 endif

diff --git a/http.c b/http.c
index 5cb87f1..b61ac85 100644
--- a/http.c
+++ b/http.c

@@ -4,6 +4,7 @@
 #include "run-command.h"
 #include "url.h"
 #include "credential.h"
+#include "version.h"
 
 int active_requests;
 int http_is_verbose;

@@ -299,7 +300,7 @@ static CURL *get_curl_handle(void)
 		curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
 
 	curl_easy_setopt(result, CURLOPT_USERAGENT,
-		user_agent ? user_agent : GIT_HTTP_USER_AGENT);
+		user_agent ? user_agent : git_user_agent());
 
 	if (curl_ftp_no_epsv)
 		curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);

-- 
1.7.7.7.32.g4b73117

[PATCH 4/4] include agent identifier in capability string

From: Jeff King <hidden>
Date: 2016-06-15 22:53:58

Instead of having the client advertise a particular version
number in the git protocol, we have managed extensions and
backwards compatibility by having clients and servers
advertise capabilities that they support. This is far more
robust than having each side consult a table of
known versions, and provides sufficient information for the
protocol interaction to complete.

However, it does not allow servers to keep statistics on
which client versions are being used. This information is
not necessary to complete the network request (the
capabilities provide enough information for that), but it
may be helpful to conduct a general survey of client
versions in use.

We already send the client version in the user-agent header
for http requests; adding it here allows us to gather
similar statistics for non-http requests.

Signed-off-by: Jeff King <redacted>
---
Two important changes from the previous round:

  1. We sanitize the agent string to remove non-printable characters and
     whitespace (they are replaced with '.'). This generally shouldn't
     happen, but is a defensive measure against breaking the protocol.

  2. The server half of the connection will advertise their versions in
     the capability strings, too.

 builtin/fetch-pack.c   |  2 ++
 builtin/receive-pack.c |  6 ++++--
 builtin/send-pack.c    |  7 +++++--
 upload-pack.c          |  7 +++++--
 version.c              | 21 +++++++++++++++++++++
 version.h              |  1 +
 6 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 149db88..fe56596 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c

@@ -10,6 +10,7 @@
 #include "remote.h"
 #include "run-command.h"
 #include "transport.h"
+#include "version.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;

@@ -327,6 +328,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 			if (args.no_progress)   strbuf_addstr(&c, " no-progress");
 			if (args.include_tag)   strbuf_addstr(&c, " include-tag");
 			if (prefer_ofs_delta)   strbuf_addstr(&c, " ofs-delta");
+			strbuf_addf(&c, " agent=%s", git_user_agent_sanitized());
 			packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
 			strbuf_release(&c);
 		} else

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0afb8b2..fbfa128 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c

@@ -12,6 +12,7 @@
 #include "string-list.h"
 #include "sha1-array.h"
 #include "connected.h"
+#include "version.h"
 
 static const char receive_pack_usage[] = "git receive-pack <git-dir>";

@@ -121,10 +122,11 @@ static void show_ref(const char *path, const unsigned char *sha1)
 	if (sent_capabilities)
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
 	else
-		packet_write(1, "%s %s%c%s%s\n",
+		packet_write(1, "%s %s%c%s%s agent=%s\n",
 			     sha1_to_hex(sha1), path, 0,
 			     " report-status delete-refs side-band-64k quiet",
-			     prefer_ofs_delta ? " ofs-delta" : "");
+			     prefer_ofs_delta ? " ofs-delta" : "",
+			     git_user_agent_sanitized());
 	sent_capabilities = 1;
 }

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index d5d7105..c4d4211 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c

@@ -8,6 +8,7 @@
 #include "send-pack.h"
 #include "quote.h"
 #include "transport.h"
+#include "version.h"
 
 static const char send_pack_usage[] =
 "git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"

@@ -306,11 +307,13 @@ int send_pack(struct send_pack_args *args,
 			int quiet = quiet_supported && (args->quiet || !args->progress);
 
 			if (!cmds_sent && (status_report || use_sideband || args->quiet)) {
-				packet_buf_write(&req_buf, "%s %s %s%c%s%s%s",
+				packet_buf_write(&req_buf,
+						 "%s %s %s%c%s%s%s agent=%s",
 						 old_hex, new_hex, ref->name, 0,
 						 status_report ? " report-status" : "",
 						 use_sideband ? " side-band-64k" : "",
-						 quiet ? " quiet" : "");
+						 quiet ? " quiet" : "",
+						 git_user_agent_sanitized());
 			}
 			else
 				packet_buf_write(&req_buf, "%s %s %s",

diff --git a/upload-pack.c b/upload-pack.c
index bb08e2e..2e90ccb 100644
--- a/upload-pack.c
+++ b/upload-pack.c

@@ -11,6 +11,7 @@
 #include "list-objects.h"
 #include "run-command.h"
 #include "sigchain.h"
+#include "version.h"
 
 static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=<n>] <dir>";

@@ -734,9 +735,11 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 	}
 
 	if (capabilities)
-		packet_write(1, "%s %s%c%s%s\n", sha1_to_hex(sha1), refname_nons,
+		packet_write(1, "%s %s%c%s%s agent=%s\n",
+			     sha1_to_hex(sha1), refname_nons,
 			     0, capabilities,
-			     stateless_rpc ? " no-done" : "");
+			     stateless_rpc ? " no-done" : "",
+			     git_user_agent_sanitized());
 	else
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), refname_nons);
 	capabilities = NULL;

diff --git a/version.c b/version.c
index f98d5a6..6106a80 100644
--- a/version.c
+++ b/version.c

@@ -1,5 +1,6 @@
 #include "git-compat-util.h"
 #include "version.h"
+#include "strbuf.h"
 
 const char git_version_string[] = GIT_VERSION;

@@ -15,3 +16,23 @@ const char *git_user_agent(void)
 
 	return agent;
 }
+
+const char *git_user_agent_sanitized(void)
+{
+	static const char *agent = NULL;
+
+	if (!agent) {
+		struct strbuf buf = STRBUF_INIT;
+		int i;
+
+		strbuf_addstr(&buf, git_user_agent());
+		strbuf_trim(&buf);
+		for (i = 0; i < buf.len; i++) {
+			if (buf.buf[i] <= 32 || buf.buf[i] >= 127)
+				buf.buf[i] = '.';
+		}
+		agent = buf.buf;
+	}
+
+	return agent;
+}

diff --git a/version.h b/version.h
index fd9cdd6..6911a4f 100644
--- a/version.h
+++ b/version.h

@@ -4,5 +4,6 @@
 extern const char git_version_string[];
 
 const char *git_user_agent(void);
+const char *git_user_agent_sanitized(void);
 
 #endif /* VERSION_H */

-- 
1.7.7.7.32.g4b73117

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help