For debugging, statistical analysis, and security purposes, it can be valuable for Git servers to know the operating system the clients are using. For example: - A server noticing that a client is using an old Git version with security issues on one platform, like macOS, could verify if the user is indeed running macOS before sending a message to upgrade." - Similarly, a server identifying a client that could benefit from an upgrade (e.g., for performance reasons) could better customize the message it sends to nudge the client to upgrade. So let's add a new 'os-version' capability to the v2 protocol, in the same way as the existing 'agent' capability that lets clients and servers exchange the Git version they are running. By default this sends similar info as `git bugreport` is already sending, which uses uname(2). The difference is that it is sanitized in the same way as the Git version sent by the 'agent' capability is sanitized (by replacing characters having an ascii code less than 32 or more than 127 with '.'). Also, it only sends the result of `uname -s` i.e just only the operating system name (e.g "Linux"). Due to privacy issues and concerns, let's add the `transfer.advertiseOSVersion` config option. This boolean option is enabled by default, but allows users to disable this feature completely by setting it to "false". To provide flexibility and customization, let also add the `osversion.command` config option. This allows users to specify a custom command whose output will be used as the string exchanged via the "os-version" capability. If this option is not set, the default behavior exchanges only the operating system name, such as "Linux" or "Windows". This option was particularly suggested by Randall S. Becker in a previous conversation. You can find the reference here https://lore.kernel.org/git/000a01dac25c$df7b23e0$9e716ba0$@nexbridge.com/ Note that, due to differences between `uname(1)` (command-line utility) and `uname(2)` (system call) outputs on Windows, `transfer.advertiseOSVersion` is set to false on Windows during testing. See the message part of patch 5/6 for more details. My mentor, Christian Couder, sent a previous patch series about this before. You can find it here https://lore.kernel.org/git/20240619125708.3719150-1-christian.couder@xxxxxxxxx/ Changes since v1 ================ - Refactored documentation for improved clarity. - Splitted patch "refactor get_uname_info()" into two patches with first part doing refactoring and the second part doing enhancement for code clearity and cleanliness. - Made test_redact_non_printables() to trim carriage-returns. - Fixed outdated commit message. - Splitted part of the "test capability advertisement" into a setup"-type to remove side-effect dependency. - Changed the name of some created files used in testing for better clearity of what their content is. - Added comment to os_version(), os_version_sanitized() and advertise_os_version() for improved clarity of what they do. Usman Akinyemi (6): version: refactor redact_non_printables() version: refactor get_uname_info() version: extend get_uname_info() to hide system details t5701: add setup test to remove side-effect dependency connect: advertise OS version version: introduce osversion.command config for os-version output Documentation/config/transfer.txt | 16 ++++ Documentation/gitprotocol-v2.txt | 17 ++++ builtin/bugreport.c | 13 +-- connect.c | 3 + serve.c | 14 ++++ t/t5555-http-smart-common.sh | 38 ++++++++- t/t5701-git-serve.sh | 59 ++++++++++++- t/test-lib-functions.sh | 8 ++ version.c | 135 ++++++++++++++++++++++++++++-- version.h | 28 +++++++ 10 files changed, 309 insertions(+), 22 deletions(-) Range-diff versus v1: 1: d23091031c ! 1: 97bccab6d5 version: refactor redact_non_printables() @@ Commit message 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. + While at it, let's use strbuf_detach() to explicitly detach the string + contained by the 'buf' strbuf. Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> Signed-off-by: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> 2: 1336622be9 ! 2: 1f8a4024a4 version: refactor get_uname_info() @@ Commit message 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 <chriscool@xxxxxxxxxxxxx> Signed-off-by: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> @@ builtin/bugreport.c: static void get_system_info(struct strbuf *sys_info) - uname_info.release, - uname_info.version, - uname_info.machine); -+ get_uname_info(sys_info, 1); ++ get_uname_info(sys_info); strbuf_addstr(sys_info, _("compiler info: ")); get_compiler_info(sys_info); @@ version.c: const char *git_user_agent_sanitized(void) return agent; } + -+int get_uname_info(struct strbuf *buf, unsigned int full) ++int get_uname_info(struct strbuf *buf) +{ + struct utsname uname_info; + @@ version.c: const char *git_user_agent_sanitized(void) + 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); ++ strbuf_addf(buf, "%s %s %s %s\n", ++ uname_info.sysname, ++ uname_info.release, ++ uname_info.version, ++ uname_info.machine); + return 0; +} @@ version.h: extern const char git_built_from_commit_string[]; + 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); ++int get_uname_info(struct strbuf *buf); + #endif /* VERSION_H */ -: ---------- > 3: 962b42702f version: extend get_uname_info() to hide system details -: ---------- > 4: 7f0ec75a0d t5701: add setup test to remove side-effect dependency 3: b90a24813f ! 5: 499eda49cf connect: advertise OS version @@ Commit message 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 + privacy concerns. 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. @@ Commit message .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. + On Windows, uname(2) is not actually system-supplied but is instead + already faked up by Git itself. We could have overcome the test issue + on Windows by implementing a new `uname` subcommand in `test-tool` + using uname(2), but except uname(2), which would be tested against + itself, there would be nothing platform specific, so it's just simpler + to disable the tests on Windows. Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> Signed-off-by: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> @@ Documentation/config/transfer.txt: transfer.bundleURI:: + 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 ++ This string is retrieved from the `sysname` field of the struct returned + by the uname(2) system call. Defaults to true. ## Documentation/gitprotocol-v2.txt ## @@ Documentation/gitprotocol-v2.txt: printable ASCII characters except space (i.e., +~~~~~~~~~~ + +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 ++advertise the `os-version` capability to notify the client the ++kind of operating system it is running on. The client may optionally ++send its own `os-version` capability, to notify the server the kind of ++operating system it is also running on in its request to the server ++(but it MUST NOT do so if the server did not advertise the os-version ++capability). The value of this capability may consist of ASCII printable ++characters(from 33 to 126 inclusive) 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 @@ t/t5555-http-smart-common.sh: test_expect_success 'git receive-pack --advertise- ' test_expect_success 'git upload-pack --advertise-refs: v2' ' -+ printf "agent=FAKE" >agent_and_os_name && ++ printf "agent=FAKE" >agent_and_osversion && + 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 ++ printf "\nos-version=%s\n" $(uname -s | test_redact_non_printables) >>agent_and_osversion + fi && + cat >expect <<-EOF && version 2 - agent=FAKE -+ $(cat agent_and_os_name) ++ $(cat agent_and_osversion) ls-refs=unborn fetch=shallow wait-for-done server-option ## t/t5701-git-serve.sh ## -@@ t/t5701-git-serve.sh: 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 && +@@ t/t5701-git-serve.sh: test_expect_success 'setup to generate files with expected content' ' + cat >expect.trailer <<-EOF && + 0000 + EOF ++ + 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 ++ printf "\nos-version=%s\n" $(uname -s | test_redact_non_printables) >>agent_and_osversion + fi && + - test_oid_cache <<-EOF && - wrong_algo sha1:sha256 - wrong_algo sha256:sha1 ++ cat >expect_osversion.base <<-EOF ++ version 2 ++ $(cat agent_and_osversion) ++ ls-refs=unborn ++ fetch=shallow wait-for-done ++ server-option ++ object-format=$(test_oid algo) ++ EOF + ' + + test_expect_success 'test capability advertisement' ' +- cat expect.base expect.trailer >expect && ++ cat expect_osversion.base expect.trailer >expect && + + GIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \ + --advertise-capabilities >out && +@@ t/t5701-git-serve.sh: test_expect_success 'test capability advertisement with uploadpack.advertiseBund + cat >expect.extra <<-EOF && + bundle-uri 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 +- cat expect.base \ ++ cat expect_osversion.base \ + expect.extra \ + expect.trailer >expect && + ## t/test-lib-functions.sh ## @@ t/test-lib-functions.sh: test_trailing_hash () { @@ t/test-lib-functions.sh: test_trailing_hash () { +# 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]" "." ++ tr -d "\n\r" | tr "[\001-\040][\177-\377]" "." +} ## version.c ## @@ version.c const char git_version_string[] = GIT_VERSION; const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT; @@ version.c: int get_uname_info(struct strbuf *buf, unsigned int full) - strbuf_addf(buf, "%s\n", uname_info.sysname); + strbuf_addf(buf, "%s\n", uname_info.sysname); return 0; } + @@ version.h: const char *git_user_agent_sanitized(void); */ int get_uname_info(struct strbuf *buf, unsigned int full); ++/* ++ Retrieve and cache system information for subsequent calls. ++ Return a pointer to the cached system information string. ++*/ +const char *os_version(void); ++ ++/* ++ Retrieve system information string from os_version(). Then ++ sanitize and cache it. Return a pointer to the sanitized ++ system information string. ++*/ +const char *os_version_sanitized(void); ++ ++/* ++ Retrieve and cache whether os-version capability is enabled. ++ Return 1 if enabled, 0 if disabled. ++*/ +int advertise_os_version(struct repository *r); + #endif /* VERSION_H */ 4: 745e63060e ! 6: a1637dc7cf version: introduce osversion.command config for os-version output @@ Commit message 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. + specified command. This customization might be especially useful on some + quite uncommon platforms like NonStop where interesting OS information is + available from other means than uname(2). + If this new configuration option is not set, the `os-version` capability + exchanges just the operating system name. + + Helped-by: Randall S. Becker <rsbecker@xxxxxxxxxxxxx> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> Signed-off-by: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> @@ Documentation/config/transfer.txt @@ Documentation/config/transfer.txt: 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 + 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 @@ Documentation/config/transfer.txt: transfer.advertiseOSVersion:: + `transfer.advertiseOSVersion` config option. ## Documentation/gitprotocol-v2.txt ## -@@ Documentation/gitprotocol-v2.txt: 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 +@@ Documentation/gitprotocol-v2.txt: 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 to notify the client the +-kind of operating system it is running on. The client may optionally +-send its own `os-version` capability, to notify the server the kind of +-operating system it is also running on in its request to the server +-(but it MUST NOT do so if the server did not advertise the os-version +-capability). The value of this capability may consist of ASCII printable ++In the same way as the `agent` capability above, the server can advertise ++the `os-version` capability to notify the client the kind of operating system ++it is running on. The client may optionally send its own `os-version` capability, ++to notify the server the kind of operating system it is also running on in its ++request to the server (but it MUST NOT do so if the server did not advertise the ++os-version capability). The value of this capability may consist of ASCII printable + characters(from 33 to 126 inclusive) 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. ++`uname -s`(OS name e.g Linux). If the `osVersion.command` is set, the value of this ++capability 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 ~~~~~~~ @@ t/t5555-http-smart-common.sh: test_expect_success 'git upload-pack --advertise-r ' +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 && ++ printf "agent=FAKE" >agent_and_long_osversion && + + if test_have_prereq !WINDOWS + then -+ printf "\nos-version=%s\n" $(uname -srvm | test_redact_non_printables) >>agent_and_long_os_name ++ printf "\nos-version=%s\n" $(uname -srvm | test_redact_non_printables) >>agent_and_long_osversion + fi && + + cat >expect <<-EOF && + version 2 -+ $(cat agent_and_long_os_name) ++ $(cat agent_and_long_osversion) + ls-refs=unborn + fetch=shallow wait-for-done + server-option @@ t/t5701-git-serve.sh: test_expect_success 'test capability advertisement' ' ' +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 && ++ printf "agent=git/$(git version | cut -d" " -f3)" >agent_and_long_osversion && + + if test_have_prereq !WINDOWS + then -+ printf "\nos-version=%s\n" $(uname -srvm | test_redact_non_printables) >>agent_and_long_os_name ++ printf "\nos-version=%s\n" $(uname -srvm | test_redact_non_printables) >>agent_and_long_osversion + fi && + + test_oid_cache <<-EOF && + wrong_algo sha1:sha256 + wrong_algo sha256:sha1 + EOF -+ cat >expect.base_long <<-EOF && ++ cat >expect_long.base <<-EOF && + version 2 -+ $(cat agent_and_long_os_name) ++ $(cat agent_and_long_osversion) + 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 && ++ cat expect_long.base expect.trailer >expect && + + GIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \ + --advertise-capabilities >out && -- 2.48.0
