- 10 Jun, 2019 27 commits
-
-
We stop the read loop when we have read all the data. We should also consider the server's feelings. If the server hangs up on us, we need to stop our read loop. Otherwise, we'll try to read from the server - and fail - ad infinitum.
Edward Thomson committed -
Edward Thomson committed
-
Instead of using `is_complete` to decide whether we have connection or request affinity for authentication mechanisms, set a boolean on the mechanism definition itself.
Edward Thomson committed -
For request-based authentication mechanisms (Basic, Digest) we should keep the authentication context alive across socket connections, since the authentication headers must be transmitted with every request. However, we should continue to remove authentication contexts for mechanisms with connection affinity (NTLM, Negotiate) since we need to reauthenticate for every socket connection.
Edward Thomson committed -
Hold an individual authentication context instead of trying to maintain all the contexts; we can select the preferred context during the initial negotiation. Subsequent authentication steps will re-use the chosen authentication (until such time as it's rejected) instead of trying to manage multiple contexts when all but one will never be used (since we can only authenticate with a single mechanism at a time.) Also, when we're given a 401 or 407 in the middle of challenge/response handling, short-circuit immediately without incrementing the retry count. The multi-step authentication is expected, and not a "retry" and should not be penalized as such. This means that we don't need to keep the contexts around and ensures that we do not unnecessarily fail for too many retries when we have challenge/response auth on a proxy and a server and potentially redirects in play as well.
Edward Thomson committed -
Edward Thomson committed
-
A "connection" to a server is transient, and we may reconnect to a server in the midst of authentication failures (if the remote indicates that we should, via `Connection: close`) or in a redirect.
Edward Thomson committed -
Edward Thomson committed
-
Include https://github.com/ethomson/ntlmclient as a dependency.
Edward Thomson committed -
Ensure that the server supports the particular credential type that we're specifying. Previously we considered credential types as an input to an auth mechanism - since the HTTP transport only supported default credentials (via negotiate) and username/password credentials (via basic), this worked. However, if we are to add another mechanism that uses username/password credentials, we'll need to be careful to identify the types that are accepted.
Edward Thomson committed -
We must always consume the full parser body if we're going to keep-alive. So in the authentication failure case, continue advancing the http message parser until it's complete, then we can retry the connection. Not doing so would mean that we have to tear the connection down and start over. Advancing through fully (even though we don't use the data) will ensure that we can retry a connection with keep-alive.
Edward Thomson committed -
When we get an authentication failure, we must consume the entire body of the response. If we only read half of the body (on the assumption that we can ignore the rest) then we will never complete the parsing of the message. This means that we will never set the complete flag, and our replay must actually tear down the connection and try again. This is particularly problematic for stateful authentication mechanisms (SPNEGO, NTLM) that require that we keep the connection alive. Note that the prior code is only a problem when the 401 that we are parsing is too large to be read in a single chunked read from the http parser. But now we will continue to invoke the http parser until we've got a complete message in the authentication failed scenario. Note that we need not do anything with the message, so when we get an authentication failed, we'll stop adding data to our buffer, we'll simply loop in the parser and let it advance its internal state.
Edward Thomson committed -
Edward Thomson committed
-
Some authentication mechanisms (like HTTP Basic and Digest) have a one-step mechanism to create credentials, but there are more complex mechanisms like NTLM and Negotiate that require challenge/response after negotiation, requiring several round-trips. Add an `is_complete` function to know when they have round-tripped enough to be a single authentication and should now either have succeeded or failed to authenticate.
Edward Thomson committed -
We cannot examine the keep-alive status of the http parser in `http_connect`; it's too late and the critical information about whether keep-alive is supported has been destroyed. Per the documentation for `http_should_keep_alive`: > If http_should_keep_alive() in the on_headers_complete or > on_message_complete callback returns 0, then this should be > the last message on the connection. Query then and set the state.
Edward Thomson committed -
Increase the permissible replay count; with multiple-step authentication schemes (NTLM, Negotiate), proxy authentication and redirects, we need to be mindful of the number of steps it takes to get connected. 7 seems high but can be exhausted quickly with just a single authentication failure over a redirected multi-state authentication pipeline.
Edward Thomson committed -
Edward Thomson committed
-
Edward Thomson committed
-
Update our CI tests to start a proxy that requires NTLM authentication; ensure that our WIndows HTTP client can speak NTLM.
Edward Thomson committed -
Edward Thomson committed
-
We did not properly support default credentials for proxies, only for destination servers. Refactor the credential handling to support sending either username/password _or_ default credentials to either the proxy or the destination server. This actually shares the authentication logic between proxy servers and destination servers. Due to copy/pasta drift over time, they had diverged. Now they share a common logic which is: first, use credentials specified in the URL (if there were any), treating empty username and password (ie, "http://:@foo.com/") as default credentials, for compatibility with git. Next, call the credential callbacks. Finally, fallback to WinHTTP compatibility layers using built-in authentication like we always have. Allowing default credentials for proxies requires moving the security level downgrade into the credential setting routines themselves. We will update our security level to "high" by default which means that we will never send default credentials without prompting. (A lower setting, like the WinHTTP default of "medium" would allow WinHTTP to handle credentials for us, despite what a user may have requested with their structures.) Now we start with "high" and downgrade to "low" only after a user has explicitly requested default credentials.
Edward Thomson committed -
There's no reason a git repository couldn't be at the root of a server, and URLs should have an implicit path of '/' when one is not specified.
Edward Thomson committed -
"Connection data" is an imprecise and largely incorrect name; these structures are actually parsed URLs. Provide a parser that takes a URL string and produces a URL structure (if it is valid). Separate the HTTP redirect handling logic from URL parsing, keeping a `gitno_connection_data_handle_redirect` whose only job is redirect handling logic and does not parse URLs itself.
Edward Thomson committed -
Callback type names should be suffixed with `_cb`
Edward Thomson committed -
tests: checkout: fix symlink.git being created outside of sandbox
Edward Thomson committed -
The trace logging callbacks should match the other callback naming conventions, using the `_cb` suffix instead of a `_callback` suffix.
Edward Thomson committed -
The credential callbacks should match the other callback naming conventions, using the `_cb` suffix instead of a `_callback` suffix.
Edward Thomson committed
-
- 07 Jun, 2019 1 commit
-
-
The function `populate_symlink_workdir` creates a new "symlink.git" repository with a relative path "../symlink.git". As the current working directory is the sandbox, the new repository will be created just outside of the sandbox. Fix this by using `clar_sandbox_path`.
Patrick Steinhardt committed
-
- 06 Jun, 2019 4 commits
-
-
ignore: handle escaped trailing whitespace
Edward Thomson committed -
Ignore: only treat one leading slash as a root identifier
Edward Thomson committed -
The gitignore's pattern format specifies that "Trailing spaces are ignored unless they are quoted with backslash ("\")". We do not honor this currently and will treat a pattern "foo\ " as if it was "foo\" only and a pattern "foo\ \ " as "foo\ \". Fix our code to handle those special cases and add tests to avoid regressions.
Patrick Steinhardt committed -
The stripping of trailing spaces currently happens as part of `git_attr_fnmatch__parse`. As we aren't currently parsing trailing whitespaces correct in case they're escaped, we'll have to change that code, though. To make actual behavioural change easier to review, refactor the code up-front by pulling it out into its own function that is expected to retain the exact same functionality as before. Like this, the fix will be trivial to apply.
Patrick Steinhardt committed
-
- 05 Jun, 2019 3 commits
-
-
online tests: use gitlab for auth failures
Edward Thomson committed -
GitHub recently changed their behavior from returning 401s for private or nonexistent repositories on a clone to returning 404s. For our tests that require an auth failure (and 401), move to GitLab to request a missing repository. This lets us continue to test our auth failure case, at least until they decide to mimic that decision.
Edward Thomson committed -
Ignore files: don't ignore whitespace
Edward Thomson committed
-
- 24 May, 2019 5 commits
-
-
Unlike ignore files, gitattribute files can have flexible whitespace at the beginning of the line. Ensure that by adding new ignore rules that we have not impeded correct parsing of attribute files.
Edward Thomson committed -
When `allow_space` is unset, ensure that leading whitespace is not skipped.
Edward Thomson committed -
Comments must have a '#' at the beginning of the line. For compatibility with git, '#' after a whitespace is a literal part of the filename.
Edward Thomson committed -
Ensure that leading whitespace is treated as being part of the filename, eg ` foo` in an ignore file indicates that a file literally named ` foo` is ignored.
Edward Thomson committed -
cache: fix cache eviction using deallocated key
Edward Thomson committed
-