If `git rev-parse --git-dir` fails, Git doesn't think that the
destination is a valid repository for whatever reason. One of the most
common reasons for that is that the repository is owned by another user.
In such a case, we want to show the error message from Git, since we
really have no other indication that this is the case. It could be just
that the destination isn't a repository at all. Regardless, the error
message would be useful to the user to determine the problem, so let's
return it if standard error isn't empty.
When the handler setup fails, we just exit right now, instead of sending
an error message. That leads to less than helpful behaviour if the
destination repository isn't a valid Git repository for whatever reason.
To improve this, let's make sure that we return an error message at
setup if something fails so that the user in turn gets an error message.
Right now, we try to spawn multiple SSH connections using ControlMaster
when making multiple requests. However, if multiplexing is not enabled,
we can spawn multiple actual connections, which can be expensive and
require lots of authentication requests.
Instead, let's make sure we don't enable multiple connections if
multiplexing is not enabled, since this may cause the user to be
prompted for multiple SSH key connection attempts (say, if they're
using a security key) and is substantially more heavyweight than simply
spawning a new process over the same connection.
Since the code has different behaviour if `XDG_RUNTIME_DIR` is set, make
sure to unset it for the tests so that we always try to create a
temporary directory and don't otherwise fail because that environment
variable points somewhere unexpected.
There are various reasons why Negotiate might fail. Let's provide the
relevant error in the trace output so that users who have a problem can
troubleshoot the problem effectively.
Previously we were using the same value for both the HTTP request
Content-Type header (which includes a charset), and the Accept header,
where charset is not part of the valid syntax. Split them out into
separate values.
On Windows, when spawning a process, Go first looks for an
executable file with the correct name in the current directory, and
only if it fails to find one there does it look in the directories
listed in the PATH environment variable. We would prefer not to
replicate this behaviour and instead search only in the directories
in PATH. Therefore, starting with the mitigation of CVE-2020-27955
in commit 74d5f2397f9abe4834bf1fe1fa02fd6c141b77ce, we resolve paths
to executables ourselves rather than rely on Go to do so.
The subprocess.LookPath() function we introduced in that change
is adapted from Go's os/exec package. When it cannot find an
executable in PATH, it returns an empty path string and an
exec.ErrNotFound error. When that happens, we do not detect the
condition and simply set the command path to the empty string.
This can lead to undesirable behaviour in which a bug in the Go
os/exec library causes it to run another executable other than
the one we intended.
When we set the command path to the empty string and then ask to
execute the command, the native Go version of the LookPath() function
for Windows is run with an empty path because filepath.Base() returns
"." when passed the empty string and because we have left the current
working directory also set to an empty string:
1724077b78/src/os/exec/exec.go (L348-L353)
Since the path string does not contain any path separator
characters the current working directory is searched to try to
find a file with a matching name and executable file extension
(e.g., ".exe" or ".com"). To search the current working directory,
the "." directory name is prepended to the given path with
filepath.Join():
1724077b78/src/os/exec/lp_windows.go (L84)
The filepath.Join() function ignores empty arguments, so this
results in an incorrect filename of ".". All potential executable
file extensions from the PATHEXT environment variable (or from a
fixed list if that variable is not defined) are then appended to
this filename, including their leading dot separator characters,
thus producing filenames like "..com" and "..exe":
1724077b78/src/os/exec/lp_windows.go (L46-L50)
Should a file with one of these names exist in the current working
directory, its name will be returned, which means that it will be
executed instead of the command we expected to run. This is
obviously undesirable and presents a risk to users.
(Note that batch files named "..bat" and "..cmd" will also be found
in the same manner, but they will not actually be executed due to an
undocumented feature of the Windows API's family of CreateProcess*()
functions, which are used by Go to spawn processes. When passed an
lpApplicationName argument ending with a ".bat" or ".cmd" extension,
the CreateProcess*() functions appear to instead execute "cmd.exe"
and construct a "/c" command string from the lpCommandLine argument.
The value of that argument is set using the full command line we are
attempting to execute, and as such its first element is the actual
name of the executable we intended to run; therefore, the command
interpreter attempts to run the executable as a batch script and
fails, and the "..bat" or "..cmd" file is effectively ignored.)
To recap, when Git LFS attempts to execute a program on Windows,
if the executable is not found anywhere in PATH but does exist in
the current working directory, then when we call exec.Command()
Go's internal LookPath() function will find the executable and not
set the internal lookPathErr flag:
1724077b78/src/os/exec/exec.go (L174-L179)
Since we do not want to run executables in the current working
directory, we subsequently run our own LookPath() function, which
we use to reset the cmd.Path field. However, when our LookPath()
returns an empty string, we do not detect that and instead set
cmd.Path to that value. Then when we ask Go to run the command,
because lookPathErr is nil, it proceeds, and the empty path causes
it to find and run the first matching file in the working directory
named "..com" or "..exe" (or any similar name with an executable
file extension except for "..bat" and "..cmd").
We can prevent this behaviour by detecting when our LookPath()
function returns an error and propagating it upwards to all
callers of our subprocess.ExecCommand() function. We also add
checks for this error at appropriate points and log or report
the error as necessary.
One particular circumstance of note occurs when a user has a
Cygwin-installed "uname" in their PATH but not "cygpath",
but "cygpath.exe" does exist in their current working directory.
Then we will attempt to execute "cygpath" because we use the
presence of "uname" as an indication that Cygwin is fully installed.
Should a "..exe" or similar file also be present in the working
directory, then it will be executed instead of "cygpath.exe".
As with all other instances where we call subprocess.ExecCommand(),
in tools.translateCygwinPath() we will now check for a returned
error before trying to actually execute "cygpath". Unlike many
of the other cases, though, we do not need to report the error in
this one; instead, we simply return the current path from
translateCygwinPath() instead of canonicalizing it, just as we do
already if the "cygpath" executable fails for some reason.
Finally, we add a new test to t/t-path.sh which checks for the
incorrect execution of a "..exe" binary on Windows when "git.exe"
is not found in PATH but does exist in the current working
directory. This test passes when run with a Git LFS binary that
includes the remediations from this commit, and fails otherwise.
For our "malicious" binary named "..exe" we make use of the
lfstest-badpathcheck test helper we added in a previous commit.
We only run this test on Windows because the underlying bug in
Go is Windows-specific as it depends on path extensions from
PATHEXT being appended to the file name ".".
Co-authored-by: brian m. carlson <bk2204@github.com>
Following on from the changes in PR #4781, we can make
additional message strings translatable using the
tr.Tr.Get() method.
Because this method performs printf(3)-style format string
parsing and interpolation, we can simplify some of the
surrounding calls, e.g., from fmt.Errorf() to errors.New(),
and from fmt.Fprintf() to fmt.Fprintln(). This ensures
that if either the translated text or any interpolated
arguments happen to contain character sequences that would
be interpreted as Go format specifiers (e.g., "%s" or "%d"),
these will not result in warnings such as "%!s(MISSING)"
in the output text.
Note also that we try to remove newlines from the message
strings were possible and change the surrounding calls to
append them instead, e.g., with fmt.Fprintln().
A number of message strings contain embedded Git or Git LFS
commands, and while in many cases these are already delimited
with backticks, in other cases they are not.
We therefore rework the formatting of these messages to accord
with the general practice of using backticks to delimit "git"
and "git lfs" commands.
In one case for the "git lfs clone" command this requires us
to split a multi-line message into several parts, but that
also has the advantage that we can move some of the fixed
formatting and newlines out of the translatable message strings.
Note that some of these messages are not yet passed as translation
strings, but we will address that issue in a subsequent commit.
Because the tr.Tr.Get() family of methods insert arguments
into printf(3)-style format strings after translating the
format string, we can in a few cases drop a surrounding call
to fmt.Sprintf() or a similar method, as those now take no
arguments and so are redundant.
Moreover, this will help us avoid situations where either
the translated string or the argument values interpolated
by tr.Tr.Get() produce an output string which itself happens
to contain character sequences that resemble Go format
specifiers (e.g., "%s", "%d", etc.) In such cases passing the
string at runtime to a method such as fmt.Fprintf() will result
in the output containing a warning such as "%!s(MISSING)", which
is not ideal.
Note that in one case, in lfs/attribute.go, we can now also
simplify the format string to use standard format specifiers
instead of double-escaped ones (e.g., "%%q") since we can just
allow tr.Tr.Get() to do the interpolation.
We also take the opportunity to remove explicit leading or
trailing newlines from translation messages wherever it is
possible to convert the surrounding call to fmt.Print(),
fmt.Fprint(), fmt.Println(), or fmt.Fprintln().
Finally, in the commands/run.go file, we can replace two calls
to fmt.Fprintf() with fmt.Println() because they are just
printing output to os.Stdout, not os.Stderr, and in the
lfs/extension.go file, we can make better use of fmt.Errorf().
Note that at least one of these messages is not yet actually
passed as a translation string, but we will address that issue
in a subsequent commit.
We convert several log messages to use the uppercase acronym
"HTTP" instead of the lowercase variant.
We also edit one message in lfshttp/client.go to remove an
idiosyncratic prefix of the full source code file name, and
edit the message slightly for greater clarity.
We rephrase and repunctuate a few error and informational
message strings so they are clearer and more consistent with
our other messages.
In particular, in the git/gitattr package we replace one
message which consisted solely of the package's name with
a more explanatory error message.
The Negotiate authentication scheme can support multiple different types
of authentication. The two most popular are NTLM and Kerberos. When we
supported NTLM, we'd first try Kerberos, and if it failed, fall back to
NTLM.
However, we no longer support NTLM, but some people still have server
configuration that uses NTLM via Negotiate. For these people,
authentication may be broken. Let's fall back to Basic in such a case
by keeping track of which authentication mechanisms we've tried,
keeping only the supported mechanisms if we got a response, and
stripping out failing mechanisms, falling back to Basic.
To help with servers that support both Negotiate and Basic, we
specifically consider SPNEGO (Negotiate) errors as authentication
errors. This is because if the server supports Kerberos but the client
does not have a ticket, then we'll get an error trying to read the
client's tickets, which will manifest in this way.
Move several top-level variables into functions so that they can be
translated properly. Top-level variables can't be translated because
the locale is not set until after startup.
Since we're about to do a v3.0.0 release, let's bump the version to v3.
Make this change automatically with the following command to avoid any
missed items:
git grep -l github.com/git-lfs/git-lfs/v2 | \
xargs sed -i -e 's!github.com/git-lfs/git-lfs/v2!github.com/git-lfs/git-lfs/v3!g'
When our go.mod file was introduced in commit
114e85c2002091eb415040923d872f8e4a4bc636 in PR #3208, the module
path chosen did not include a trailing /v2 component. However,
the Go modules specification now advises that module paths must
have a "major version suffix" which matches the release version.
We therefore add a /v2 suffix to our module path and all its
instances in import paths.
See also https://golang.org/ref/mod#major-version-suffixes for
details regarding the Go module system's major version suffix rule.
When we process an SSH URL, we intentionally strip off the slash at the
beginning of the URL. While that was convenient for
git-lfs-authenticate, it also prevents us from handling an absolute path
in git-lfs-transfer, since the path will have its leading slash stripped
off and will therefore be relative.
Instead, let's adopt Git's behavior, which is to not remove the leading
slash. This is an incompatible change, but we're about to do a major
release, so it's a good time to make it. This will affect both
git-lfs-transfer and git-lfs-authenticate commands, but at least GitHub
already supports the proper syntax.
Note that since we process the non-URL form of SSH remotes by converting
them to a URL and then parsing, let's strip off the leading slash when
we process that form, since there we do have the ability to distinguish
between absolute and relative paths.
Update the lfs-ssh-echo binary to handle this new format.
When using the pure SSH-based protocol, we can get much higher speeds by
multiplexing multiple connections on the same SSH connection. If we're
using OpenSSH, let's enable the ControlMaster option unless
lfs.ssh.automultiplex is set to false, and multiplex these shell
operations over one connection.
We prefer XDG_RUNTIME_DIR because it's guaranteed to be private and we
can share many connections over one socket, but if that's not set, let's
default to creating a new temporary directory for the socket. On
Windows, where the native SSH client doesn't support ControlMaster,
we should fall back to using multiple connections since we use
ControlMaster=auto.
Note that the option exists because users may already be using SSH
multiplexing and we would want to provide a way for them to disable
this, in addition to the case where users have an old or broken OpenSSH
which cannot support this option.
We pass the connection object into each worker and adjust our transfer
code to pass it into each function we invoke. We also make sure to
properly terminate each connection at the end by reducing our connection
count to 0, which closes the extra (i.e., all) connections.
Co-authored-by: Chris Darroch <chrisd8088@github.com>
We'll want to access the context that was originally passed to us later
on in a future commit, so let's preserve it so we can extract it from
our API client.
In the future, we'll want to call into the SSH code from multiple
packages, so let's move it out of the lfshttp package into its own
package to avoid package import loops. While we're at it, rename the
function names to remove the "ssh" prefix, since it's implied by the
fact that they're in a package called "ssh".
Move the tests to their own package to prevent an import loop and expose
the private functions so we can test them there.
Right now, all of the SSH metadata for an endpoint is in the Endpoint
struct, but in the future we'd like to move the SSH code to its own
package. At that point, we'll want to avoid a dependency on the
Endpoint struct, so let's move the SSH metadata out into its own struct,
which we'll include in Endpoint.
While we're at it, let's adjust most of the SSH code to use this new
struct instead so we can easily move it in the future.
When we invoke the file helper to download files from a remote system,
we use the repository's temporary directory in order to make sure we can
quickly and easily rename files into the object store without the need
for a copy, which might be necessary if we used the system temporary
directory. In many cases, these files we generate are actually hard
links to the remote repository, which means we can cheaply and easily
copy files at maximum speed. Note, however, that these files don't look
like a normal object ID; instead, they have a name generated by Go's
temporary file code.
However, our current code causes a problem if, during the pull, a file
is checked out and Git causes it to be uselessly cleaned, which happens
in some cases. That's because our temporary file cleanup code will
remove all files in the temporary directory that don't look like normal
object ID components, and as a result, the clean operation can remove
files that are in use by the fetch.
Instead of immediately purging anything that looks like it's not a valid
object ID, let's wait an hour before pruning any temporary file unless
it's an object that we've already downloaded and verified is in our
object store.
We also need to do one more thing here, which is that we need to ignore
any file that lives in a directory younger than an hour old and adjust
our file helper to create a new directory for its temporary files. This
is important because if we create a hard link to an object in a remote
repository, it may be older than one hour, and we don't want to prune
those if we're not done yet.
Note that we use a sync.Map to make this as efficient as possible and
avoid the need for many additional stat calls. Ideally, we won't
actually see an increase in stat calls at all.
Currently, we have a large number of identical cert files for various
platforms, all of which are non-macOS Unix systems. These files all
simply provide the default system configuration and nothing else.
We recently noticed by looking at the NetBSD pkgsrc repository, which,
despite its name, can be used on many systems, that there is a patch to
support Solaris. Unsurprisingly, it duplicates this same file.
Since using the default system behavior is the prudent thing to do if we
don't have anything else to do, let's make our project more portable by
simply using this same file for all systems other than macOS or Windows.
That way, we'll work on Solaris and NetBSD, among other systems, and any
other Unix systems which support Go in the future will also magically
work.
Now that we have a function which does path canonicalization correctly
on a variety of systems, let's use it in favor of filepath.EvalSymlinks.
We use this function to handle any path we know doesn't come from
outside Git LFS, since we know we'll never need to handle Cygwin paths
in these cases and it's more efficient not to invoke a subprocess if
it's not necessary.
If a user has a proxy using HTTP but a URL using HTTPS, we fail to honor
the http.proxy configuration setting because we check the scheme of the
proxy URL, not the scheme of the request URL. Let's fix this by
deciding which proxy variable to set based on the request URL, as was
probably intended, rather than the proxy URL.
Add some tests for this case, as well as the case that we correctly
ignore the HTTPS_PROXY environment variable if the scheme of the request
URL is HTTP, which we were previously lacking.
The fix for CVE-2020-27955 was incomplete because we did not consider
places outside of the subprocess code that invoke binaries. As a
result, there are still some places where an attacker can execute
arbitrary code by placing a malicious binary in the repository.
To make sure we've covered all the bases, let's just use the subprocess
code for executing all programs, which means that they'll be secure. As
of this commit, all users of exec.Command are in test code or the
subprocess code itself.
Since we're uploading and downloading a large amount of data, we may
prefer to use transport compression. Currently, Go will do this for us
automatically unless we ask it not to. Let's detect this case and add a
trace output so people can find it and we can add a test that it
continues to work.
Note that it's not possible to detect this using the headers since Go
adds the headers on the request and strips the headers on the response
as part of making the request, so it's not readily apparent to users
without this trace output.
Go currently doesn't support the socks5h protocol, but it does know how
to handle it if given a URL starting with socks5:// instead. Let's
rewrite the URLs internally to make things just magically work.
Currently, we allow literal paths as named remotes, but not specified as
literal remotes on the command line, since they fail to validate as
URLs. To make this experience better, let's rewrite the local paths as
file URLs using the existing endpoint code if the validation fails and
then try validation again. We check specifically that users can use "."
as a remote since that's a common idiom to update branches.
As an addendum to commit b5ecbd9ca4ef, we now allow local path
endpoints to refer to .git files (i.e., gitlinks) as well as to
directories, per PR advice from bk2204.
When generating a local path endpoint, we now test whether the
path corresponds to a bare repo lacking a .git directory, and if so,
we ensure the endpoint path has any trailing .git segment trimmed.
Conversely, we ensure the endpoint path has a final .git segment if
a .git directory is found.
The test suite is updated to check the four relevant cases.
The Go test suite adds remotes in the form of temporary bare Git
repositories, which lack a ".git" directory. These are then configured
with their local paths as the value of the "remote.origin.url" key,
which the EndpointFinder returns, but with "/.git" appended by the
EndpointFromLocalPath() function.
When the standalone transfer adapter attempts to chdir(2) to one of
these file paths in order to run the "git rev-parse --git-dir" command
when setting up a new handler, it obviously fails.
We can address this by simply removing any trailing "/.git" path
segment in the gitDirAtPath() function in the standalone adapter.
The "git rev-parse --git-dir" command will succeed in either case,
whether in a bare repository or a regular one.
This should permit any users who have Git LFS configurations with
custom adapters that depend on the EndpointFinder's current logic
to continue to work, while allowing our Go test suite to succeed
with its bare remote repositories.
There are some TLS servers that want to perform a renegotiation to
request users use a client certificate. Allow us to use renegotiation
as a client when making a TLS connection. The DoS potential for this is
low, since the server must not only perform another key exchange, but
must re-sign the request, leading to a larger performance impact on the
server than the client.
Currently, our proxy support has some limitations. Notably, we don't
handle wildcards in the no_proxy environment variable.
Unfortunately for us, there's no standard as to how these environment
variables are supposed to be handled, so any attempt we made to handle
this ourselves would likely be incomplete. However, fortunately for us,
our needs are simple: we need standard behavior except that a user can
use a proxy for localhost, and we need to be able to read from a source
other than the environment.
The solution is to use the Go extension httpproxy module along with a
little bit of custom configuration and URL rewriting, which means we
don't have to worry about maintaining the complexity of parsing proxy
support. This also means we can drop a large amount of complex (and
subtly wrong) code.
Add support for Kerberos authentication using SPNEGO (Negotiate).
Because NTLM is also supported by GSSAPI and can also use Negotiate, try
Kerberos (which is far more secure) first, and only then fall back to
NTLM. Similar to NTLM, no credentials are required, because the user
has a credential storage mechanism that provides them automatically.
In the future, we'll adjust the transport based on the access type, so
cache clients based on both the host name and the access type. If it's
not clear what type of access we're using, default to NoneAccess, since
this will be correct for most types.
Currently, we only support local remotes using file URLs, but not local
paths. This is a highly requested feature, however, so implement
support for local paths as remotes.
First, fix the handling of local paths we already have: the handling of
absolute Unix-style paths. Instead of checking for the remote to
contain a file URL to determine whether to append "info/lfs" to the URL,
look to see if the URL we're using is a file URL, which will catch local
paths which will have been rewritten as such by this point. The
"info/lfs" part of the URL is not handled by the transfer adapter, so we
don't want to add it on.
If we get something that looks like it's not a URL, check if it's a
file on the file system, and if so, don't attempt to interpret it as an
SSH URL. This fixes the confusion with Windows paths, which resemble
SSH-style locations with a single-letter alias as the host name.
Finally, when turning a local path into a file URL, turn the path into
an absolute one if possible and rewrite it using slashes, which is
required for file URLs. Add several tests for the various cases: one
for Unix-style paths, one for native paths, and one for relative paths.
There are some servers that cannot speak HTTP/2 in all cases and demand
to fall back to HTTP/1.1 with a HTTP_1_1_REQUIRED. Notably, this
happens with IIS 10 when using NTLM. Go's HTTP library doesn't seem to
like this response and aborts the transfer, leading to a failure.
Fortunately, Git has an option (http.version) to control the protocol
used when speaking HTTP to a remote server. Implement this option to
allow users to set the protocol to use when speaking HTTP and work
around these broken servers.
In a future commit, we'll want the HTTP client to take a URL so we can
extract the scheme. Additionally, in some cases, specifically when a
user forces the use of HTTP/2 when it's not available, we'll need to
return an error, so return an additional error value.
The original parser that was used in #3825 brings in a lot of
dependencies that complicate packaging git-lfs. This replaces it with a
small parser I wrote with almost no dependencies. I've tested this as
extensively as i can and it seems to work correctly.
