From 7f3323072295d6396e0554f2f632f8c4cba48ee5 Mon Sep 17 00:00:00 2001 From: Usman Akinyemi Date: Mon, 6 Jan 2025 16:00:49 +0530 Subject: [PATCH 1/4] version: refactor redact_non_printables() The git_user_agent_sanitized() function performs some sanitizing to avoid special characters being sent over the line and possibly messing up with the protocol or with the parsing on the other side. Let's extract this sanitizing into a new redact_non_printables() function, as we will want to reuse it in a following patch. For now the new redact_non_printables() function is still static as it's only needed locally. While at it, let's also make a few small improvements: - use 'size_t' for 'i' instead of 'int', - move the declaration of 'i' inside the 'for ( ... )', - use strbuf_detach() to explicitly detach the string contained by the 'buf' strbuf. Mentored-by: Christian Couder Signed-off-by: Usman Akinyemi Signed-off-by: Junio C Hamano --- version.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/version.c b/version.c index 4d763ab48dd76c..78f025c808ae5e 100644 --- a/version.c +++ b/version.c @@ -6,6 +6,20 @@ const char git_version_string[] = GIT_VERSION; const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT; +/* + * Trim and replace each character with ascii code below 32 or above + * 127 (included) using a dot '.' character. + * TODO: ensure consecutive non-printable characters are only replaced once +*/ +static void redact_non_printables(struct strbuf *buf) +{ + strbuf_trim(buf); + for (size_t i = 0; i < buf->len; i++) { + if (buf->buf[i] <= 32 || buf->buf[i] >= 127) + buf->buf[i] = '.'; + } +} + const char *git_user_agent(void) { static const char *agent = NULL; @@ -27,12 +41,8 @@ const char *git_user_agent_sanitized(void) struct strbuf buf = STRBUF_INIT; strbuf_addstr(&buf, git_user_agent()); - strbuf_trim(&buf); - for (size_t i = 0; i < buf.len; i++) { - if (buf.buf[i] <= 32 || buf.buf[i] >= 127) - buf.buf[i] = '.'; - } - agent = buf.buf; + redact_non_printables(&buf); + agent = strbuf_detach(&buf, NULL); } return agent; From 54db285459bb7b703eb9153035dc0d8085cd9171 Mon Sep 17 00:00:00 2001 From: Usman Akinyemi Date: Mon, 6 Jan 2025 16:00:50 +0530 Subject: [PATCH 2/4] version: refactor get_uname_info() Some code from "builtin/bugreport.c" uses uname(2) to get system information. Let's refactor this code into a new get_uname_info() function, so that we can reuse it in a following commit. We may need to refactor this function in the future if an `osVersion.format` config option is added, but for now we only need it to accept a "full" flag that makes it switch between providing full OS information and providing only the OS name. The mode providing only the OS name is needed in a following commit. Mentored-by: Christian Couder Signed-off-by: Usman Akinyemi Signed-off-by: Junio C Hamano --- builtin/bugreport.c | 13 ++----------- version.c | 23 +++++++++++++++++++++++ version.h | 7 +++++++ 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/builtin/bugreport.c b/builtin/bugreport.c index 7c2df035c9cff0..e3288a86c8eae8 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -12,10 +12,10 @@ #include "diagnose.h" #include "object-file.h" #include "setup.h" +#include "version.h" static void get_system_info(struct strbuf *sys_info) { - struct utsname uname_info; char *shell = NULL; /* get git version from native cmd */ @@ -24,16 +24,7 @@ static void get_system_info(struct strbuf *sys_info) /* system call for other version info */ strbuf_addstr(sys_info, "uname: "); - if (uname(&uname_info)) - strbuf_addf(sys_info, _("uname() failed with error '%s' (%d)\n"), - strerror(errno), - errno); - else - strbuf_addf(sys_info, "%s %s %s %s\n", - uname_info.sysname, - uname_info.release, - uname_info.version, - uname_info.machine); + get_uname_info(sys_info, 1); strbuf_addstr(sys_info, _("compiler info: ")); get_compiler_info(sys_info); diff --git a/version.c b/version.c index 78f025c808ae5e..44ffc4dd576c76 100644 --- a/version.c +++ b/version.c @@ -2,6 +2,7 @@ #include "version.h" #include "version-def.h" #include "strbuf.h" +#include "gettext.h" const char git_version_string[] = GIT_VERSION; const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT; @@ -47,3 +48,25 @@ const char *git_user_agent_sanitized(void) return agent; } + +int get_uname_info(struct strbuf *buf, unsigned int full) +{ + struct utsname uname_info; + + if (uname(&uname_info)) { + strbuf_addf(buf, _("uname() failed with error '%s' (%d)\n"), + strerror(errno), + errno); + return -1; + } + + if (full) + strbuf_addf(buf, "%s %s %s %s\n", + uname_info.sysname, + uname_info.release, + uname_info.version, + uname_info.machine); + else + strbuf_addf(buf, "%s\n", uname_info.sysname); + return 0; +} diff --git a/version.h b/version.h index 7c62e80577154d..5eb586c0bdd25c 100644 --- a/version.h +++ b/version.h @@ -7,4 +7,11 @@ extern const char git_built_from_commit_string[]; const char *git_user_agent(void); const char *git_user_agent_sanitized(void); +/* + Try to get information about the system using uname(2). + Return -1 and put an error message into 'buf' in case of uname() + error. Return 0 and put uname info into 'buf' otherwise. +*/ +int get_uname_info(struct strbuf *buf, unsigned int full); + #endif /* VERSION_H */ From c3231ae3309b43483073b983e15ddb35eeb7c03a Mon Sep 17 00:00:00 2001 From: Usman Akinyemi Date: Mon, 6 Jan 2025 16:00:51 +0530 Subject: [PATCH 3/4] connect: advertise OS version As some issues that can happen with a Git client can be operating system specific, it can be useful for a server to know which OS a client is using. In the same way it can be useful for a client to know which OS a server is using. Let's introduce a new protocol (`os-version`) allowing Git clients and servers to exchange operating system information. The protocol is controlled by the new `transfer.advertiseOSVersion` config option. Add the `transfer.advertiseOSVersion` config option to address privacy concerns issue. It defaults to `true` and can be changed to `false`. When enabled, this option makes clients and servers send each other the OS name (e.g., "Linux" or "Windows"). The information is retrieved using the 'sysname' field of the `uname(2)` system call. However, there are differences between `uname(1)` (command-line utility) and `uname(2)` (system call) outputs on Windows. These discrepancies complicate testing on Windows platforms. For example: - `uname(1)` output: MINGW64_NT-10.0-20348.3.4.10-87d57229.x86_64\ .2024-02-14.20:17.UTC.x86_64 - `uname(2)` output: Windows.10.0.20348 Until a good way to test the feature on Windows is found, the transfer.advertiseOSVersion is set to false on Windows during testing. Mentored-by: Christian Couder Signed-off-by: Usman Akinyemi Signed-off-by: Junio C Hamano --- Documentation/config/transfer.txt | 7 ++++++ Documentation/gitprotocol-v2.txt | 20 +++++++++++++++ connect.c | 3 +++ serve.c | 14 +++++++++++ t/t5555-http-smart-common.sh | 12 ++++++++- t/t5701-git-serve.sh | 12 ++++++++- t/test-lib-functions.sh | 8 ++++++ version.c | 42 +++++++++++++++++++++++++++++++ version.h | 6 +++++ 9 files changed, 122 insertions(+), 2 deletions(-) diff --git a/Documentation/config/transfer.txt b/Documentation/config/transfer.txt index f1ce50f4a6e6ba..e2d95d1ccd2804 100644 --- a/Documentation/config/transfer.txt +++ b/Documentation/config/transfer.txt @@ -125,3 +125,10 @@ transfer.bundleURI:: transfer.advertiseObjectInfo:: When `true`, the `object-info` capability is advertised by servers. Defaults to false. + +transfer.advertiseOSVersion:: + When `true`, the `os-version` capability is advertised by clients and + servers. It makes clients and servers send to each other a string + representing the operating system name, like "Linux" or "Windows". + This string is retrieved from the 'sysname' field of the struct returned + by the uname(2) system call. Defaults to true. diff --git a/Documentation/gitprotocol-v2.txt b/Documentation/gitprotocol-v2.txt index 1652fef3aeb166..c28262c60b05bc 100644 --- a/Documentation/gitprotocol-v2.txt +++ b/Documentation/gitprotocol-v2.txt @@ -190,6 +190,26 @@ printable ASCII characters except space (i.e., the byte range 32 < x < and debugging purposes, and MUST NOT be used to programmatically assume the presence or absence of particular features. +os-version +~~~~~~~~~~ + +In the same way as the `agent` capability above, the server can +advertise the `os-version` capability with a value `X` (in the form +`os-version=X`) to notify the client that the server is running an +operating system that can be identified by `X`. The client may +optionally send its own `os-version` string by including the +`os-version` capability with a value `Y` (in the form `os-version=Y`) +in its request to the server (but it MUST NOT do so if the server did +not advertise the os-version capability). The `X` and `Y` strings may +contain any printable ASCII characters except space (i.e., the byte +range 32 < x < 127), and are typically made from the result of +`uname -s`(OS name e.g Linux). The os-version capability can be disabled +entirely by setting the `transfer.advertiseOSVersion` config option +to `false`. The `os-version` strings are purely informative for +statistics and debugging purposes, and MUST NOT be used to +programmatically assume the presence or absence of particular +features. + ls-refs ~~~~~~~ diff --git a/connect.c b/connect.c index 10fad43e98652f..6d5792b63c7e63 100644 --- a/connect.c +++ b/connect.c @@ -492,6 +492,9 @@ static void send_capabilities(int fd_out, struct packet_reader *reader) if (server_supports_v2("agent")) packet_write_fmt(fd_out, "agent=%s", git_user_agent_sanitized()); + if (server_supports_v2("os-version") && advertise_os_version(the_repository)) + packet_write_fmt(fd_out, "os-version=%s", os_version_sanitized()); + if (server_feature_v2("object-format", &hash_name)) { int hash_algo = hash_algo_by_name(hash_name); if (hash_algo == GIT_HASH_UNKNOWN) diff --git a/serve.c b/serve.c index c8694e375159ca..5b0d54ae9a912c 100644 --- a/serve.c +++ b/serve.c @@ -31,6 +31,16 @@ static int agent_advertise(struct repository *r UNUSED, return 1; } +static int os_version_advertise(struct repository *r, + struct strbuf *value) +{ + if (!advertise_os_version(r)) + return 0; + if (value) + strbuf_addstr(value, os_version_sanitized()); + return 1; +} + static int object_format_advertise(struct repository *r, struct strbuf *value) { @@ -123,6 +133,10 @@ static struct protocol_capability capabilities[] = { .name = "agent", .advertise = agent_advertise, }, + { + .name = "os-version", + .advertise = os_version_advertise, + }, { .name = "ls-refs", .advertise = ls_refs_advertise, diff --git a/t/t5555-http-smart-common.sh b/t/t5555-http-smart-common.sh index e47ea1ad106048..f9e2a66cba54e8 100755 --- a/t/t5555-http-smart-common.sh +++ b/t/t5555-http-smart-common.sh @@ -123,9 +123,19 @@ test_expect_success 'git receive-pack --advertise-refs: v1' ' ' test_expect_success 'git upload-pack --advertise-refs: v2' ' + printf "agent=FAKE" >agent_and_os_name && + if test_have_prereq WINDOWS + then + # We do not use test_config here so that any tests below can reuse + # the "expect" file from this test + git config transfer.advertiseOSVersion false + else + printf "\nos-version=%s\n" $(uname -s | test_redact_non_printables) >>agent_and_os_name + fi && + cat >expect <<-EOF && version 2 - agent=FAKE + $(cat agent_and_os_name) ls-refs=unborn fetch=shallow wait-for-done server-option diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh index de904c165501c9..f4668b7acd2b25 100755 --- a/t/t5701-git-serve.sh +++ b/t/t5701-git-serve.sh @@ -8,13 +8,23 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh test_expect_success 'test capability advertisement' ' + printf "agent=git/$(git version | cut -d" " -f3)" >agent_and_os_name && + if test_have_prereq WINDOWS + then + # We do not use test_config here so that tests below will be able to reuse + # the expect.base and expect.trailer files + git config transfer.advertiseOSVersion false + else + printf "\nos-version=%s\n" $(uname -s | test_redact_non_printables) >>agent_and_os_name + fi && + test_oid_cache <<-EOF && wrong_algo sha1:sha256 wrong_algo sha256:sha1 EOF cat >expect.base <<-EOF && version 2 - agent=git/$(git version | cut -d" " -f3) + $(cat agent_and_os_name) ls-refs=unborn fetch=shallow wait-for-done server-option diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 78e054ab503a65..447c698d748336 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -2007,3 +2007,11 @@ test_trailing_hash () { test-tool hexdump | sed "s/ //g" } + +# Trim and replace each character with ascii code below 32 or above +# 127 (included) using a dot '.' character. +# Octal intervals \001-\040 and \177-\377 +# corresponds to decimal intervals 1-32 and 127-255 +test_redact_non_printables () { + tr -d "\n" | tr "[\001-\040][\177-\377]" "." +} diff --git a/version.c b/version.c index 44ffc4dd576c76..8242baf41cd380 100644 --- a/version.c +++ b/version.c @@ -3,6 +3,7 @@ #include "version-def.h" #include "strbuf.h" #include "gettext.h" +#include "config.h" const char git_version_string[] = GIT_VERSION; const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT; @@ -70,3 +71,44 @@ int get_uname_info(struct strbuf *buf, unsigned int full) strbuf_addf(buf, "%s\n", uname_info.sysname); return 0; } + +const char *os_version(void) +{ + static const char *os = NULL; + + if (!os) { + struct strbuf buf = STRBUF_INIT; + + get_uname_info(&buf, 0); + os = strbuf_detach(&buf, NULL); + } + + return os; +} + +const char *os_version_sanitized(void) +{ + static const char *os_sanitized = NULL; + + if (!os_sanitized) { + struct strbuf buf = STRBUF_INIT; + + strbuf_addstr(&buf, os_version()); + redact_non_printables(&buf); + os_sanitized = strbuf_detach(&buf, NULL); + } + + return os_sanitized; +} + +int advertise_os_version(struct repository *r) +{ + static int transfer_advertise_os_version = -1; + + if (transfer_advertise_os_version == -1) { + repo_config_get_bool(r, "transfer.advertiseosversion", &transfer_advertise_os_version); + /* enabled by default */ + transfer_advertise_os_version = !!transfer_advertise_os_version; + } + return transfer_advertise_os_version; +} diff --git a/version.h b/version.h index 5eb586c0bdd25c..8167ce956a6dfb 100644 --- a/version.h +++ b/version.h @@ -1,6 +1,8 @@ #ifndef VERSION_H #define VERSION_H +struct repository; + extern const char git_version_string[]; extern const char git_built_from_commit_string[]; @@ -14,4 +16,8 @@ const char *git_user_agent_sanitized(void); */ int get_uname_info(struct strbuf *buf, unsigned int full); +const char *os_version(void); +const char *os_version_sanitized(void); +int advertise_os_version(struct repository *r); + #endif /* VERSION_H */ From cbb94dc50f8d69f01a34b58352cd2b2f6e603ad1 Mon Sep 17 00:00:00 2001 From: Usman Akinyemi Date: Mon, 6 Jan 2025 16:00:52 +0530 Subject: [PATCH 4/4] version: introduce osversion.command config for os-version output Currently by default, the new `os-version` capability only exchange the operating system name between servers and clients i.e "Linux" or "Windows". Let's introduce a new configuration option, `osversion.command`, to handle the string exchange between servers and clients. This option allows customization of the exchanged string by leveraging the output of the specified command. If this is not set, the `os-version` capability exchange just the operating system name. Mentored-by: Christian Couder Signed-off-by: Usman Akinyemi Signed-off-by: Junio C Hamano --- Documentation/config/transfer.txt | 11 ++++++- Documentation/gitprotocol-v2.txt | 13 ++++---- t/t5555-http-smart-common.sh | 29 ++++++++++++++++++ t/t5701-git-serve.sh | 33 ++++++++++++++++++++ version.c | 51 ++++++++++++++++++++++++++++++- 5 files changed, 129 insertions(+), 8 deletions(-) diff --git a/Documentation/config/transfer.txt b/Documentation/config/transfer.txt index e2d95d1ccd2804..28a08f21fc33d6 100644 --- a/Documentation/config/transfer.txt +++ b/Documentation/config/transfer.txt @@ -131,4 +131,13 @@ transfer.advertiseOSVersion:: servers. It makes clients and servers send to each other a string representing the operating system name, like "Linux" or "Windows". This string is retrieved from the 'sysname' field of the struct returned - by the uname(2) system call. Defaults to true. + by the uname(2) system call. If the `osVersion.command` is set, the + output of the command specified will be the string exchanged by the clients + and the servers. Defaults to true. + +osVersion.command:: + If this variable is set, the specified command will be run and the output + will be used as the value `X` for `os-version` capability (in the form + `os-version=X`). `osVersion.command` is only used if `transfer.advertiseOSVersion` + is true. Refer to the linkgit:git-config[1] documentation to learn more about + `transfer.advertiseOSVersion` config option. diff --git a/Documentation/gitprotocol-v2.txt b/Documentation/gitprotocol-v2.txt index c28262c60b05bc..53621c0bce81c8 100644 --- a/Documentation/gitprotocol-v2.txt +++ b/Documentation/gitprotocol-v2.txt @@ -203,12 +203,13 @@ in its request to the server (but it MUST NOT do so if the server did not advertise the os-version capability). The `X` and `Y` strings may contain any printable ASCII characters except space (i.e., the byte range 32 < x < 127), and are typically made from the result of -`uname -s`(OS name e.g Linux). The os-version capability can be disabled -entirely by setting the `transfer.advertiseOSVersion` config option -to `false`. The `os-version` strings are purely informative for -statistics and debugging purposes, and MUST NOT be used to -programmatically assume the presence or absence of particular -features. +`uname -s`(OS name e.g Linux). If the `osVersion.command` is set, +the `X` and `Y` are made from the ouput of the command specified. +The os-version capability can be disabled entirely by setting the +`transfer.advertiseOSVersion` config option to `false`. The `os-version` +strings are purely informative for statistics and debugging purposes, and +MUST NOT be used to programmatically assume the presence or absence of +particular features. ls-refs ~~~~~~~ diff --git a/t/t5555-http-smart-common.sh b/t/t5555-http-smart-common.sh index f9e2a66cba54e8..8d5844eaf20e65 100755 --- a/t/t5555-http-smart-common.sh +++ b/t/t5555-http-smart-common.sh @@ -152,6 +152,35 @@ test_expect_success 'git upload-pack --advertise-refs: v2' ' test_cmp actual expect ' +test_expect_success 'git upload-pack --advertise-refs: v2 with osVersion.command config set' ' + # test_config is used here as we are not reusing any file output from here + test_config osVersion.command "uname -srvm" && + printf "agent=FAKE" >agent_and_long_os_name && + + if test_have_prereq !WINDOWS + then + printf "\nos-version=%s\n" $(uname -srvm | test_redact_non_printables) >>agent_and_long_os_name + fi && + + cat >expect <<-EOF && + version 2 + $(cat agent_and_long_os_name) + ls-refs=unborn + fetch=shallow wait-for-done + server-option + object-format=$(test_oid algo) + 0000 + EOF + + GIT_PROTOCOL=version=2 \ + GIT_USER_AGENT=FAKE \ + git upload-pack --advertise-refs . >out 2>err && + + test-tool pkt-line unpack actual && + test_must_be_empty err && + test_cmp actual expect +' + test_expect_success 'git receive-pack --advertise-refs: v2' ' # There is no v2 yet for receive-pack, implicit v0 cat >expect <<-EOF && diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh index f4668b7acd2b25..51d99cd62c6c6b 100755 --- a/t/t5701-git-serve.sh +++ b/t/t5701-git-serve.sh @@ -41,6 +41,39 @@ test_expect_success 'test capability advertisement' ' test_cmp expect actual ' +test_expect_success 'test capability advertisement with osVersion.command config set' ' + # test_config is used here as we are not reusing any file output from here + test_config osVersion.command "uname -srvm" && + printf "agent=git/$(git version | cut -d" " -f3)" >agent_and_long_os_name && + + if test_have_prereq !WINDOWS + then + printf "\nos-version=%s\n" $(uname -srvm | test_redact_non_printables) >>agent_and_long_os_name + fi && + + test_oid_cache <<-EOF && + wrong_algo sha1:sha256 + wrong_algo sha256:sha1 + EOF + cat >expect.base_long <<-EOF && + version 2 + $(cat agent_and_long_os_name) + ls-refs=unborn + fetch=shallow wait-for-done + server-option + object-format=$(test_oid algo) + EOF + cat >expect.trailer_long <<-EOF && + 0000 + EOF + cat expect.base_long expect.trailer_long >expect && + + GIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \ + --advertise-capabilities >out && + test-tool pkt-line unpack actual && + test_cmp expect actual +' + test_expect_success 'stateless-rpc flag does not list capabilities' ' # Empty request test-tool pkt-line pack >in <<-EOF && diff --git a/version.c b/version.c index 8242baf41cd380..b4462328987772 100644 --- a/version.c +++ b/version.c @@ -1,9 +1,13 @@ +#define USE_THE_REPOSITORY_VARIABLE + #include "git-compat-util.h" #include "version.h" #include "version-def.h" #include "strbuf.h" #include "gettext.h" #include "config.h" +#include "run-command.h" +#include "alias.h" const char git_version_string[] = GIT_VERSION; const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT; @@ -72,6 +76,50 @@ int get_uname_info(struct strbuf *buf, unsigned int full) return 0; } +/* + * Return -1 if unable to retrieve the osversion.command config or + * if the command is malformed; otherwise, return 0 if successful. + */ +static int fill_os_version_command(struct child_process *cmd) +{ + const char *os_version_command; + const char **argv; + char *os_version_copy; + int n; + + if (git_config_get_string_tmp("osversion.command", &os_version_command)) + return -1; + + os_version_copy = xstrdup(os_version_command); + n = split_cmdline(os_version_copy, &argv); + + if (n < 0) { + warning(_("malformed osVersion.command config option: %s"), + _(split_cmdline_strerror(n))); + free(os_version_copy); + return -1; + } + + for (int i = 0; i < n; i++) + strvec_push(&cmd->args, argv[i]); + free(os_version_copy); + free(argv); + + return 0; +} + +static int capture_os_version(struct strbuf *buf) +{ + struct child_process cmd = CHILD_PROCESS_INIT; + + if (fill_os_version_command(&cmd)) + return -1; + if (capture_command(&cmd, buf, 0)) + return -1; + + return 0; +} + const char *os_version(void) { static const char *os = NULL; @@ -79,7 +127,8 @@ const char *os_version(void) if (!os) { struct strbuf buf = STRBUF_INIT; - get_uname_info(&buf, 0); + if (capture_os_version(&buf)) + get_uname_info(&buf, 0); os = strbuf_detach(&buf, NULL); }