Fix managed http/https transport failures

[darkowlzz]

NOTE: Reimplemented in #870

For http transport, perviously, an initial request without any credentials was being sent and upon unauthorized response, credentials were fetched and the request was retried with the obtained credentials. This appeared to have resulted in the remote server closing the connection, resulting in clone failure error:

unable to clone: Post "http://test-user:***@127.0.0.1:40463/bar/test-reponame/git-upload-pack": io: read/write on closed pipe

Querying the credentials at the very beginning and not failing fixes the closed pipe issue. It also works when there are no credentials.

NOTE: There may be good reason for the previous structure, but in all of my tests, which covers cloning without any credentials and cloning with basic auth, it works well without the previous method of requesting without credentials and failing first. This change continues to work with following changes for https certificate.

For https transport, since the go transport doesn't have access to the certificate, it results in the following failure:

unable to clone: Get "https://127.0.0.1:44185/bar/test-reponame/info/refs?service=git-upload-pack": x509: certificate signed by unknown authority

Since the go smart transport is private, there seems to be no way to pass the certificates to it. Unlike the credentials, there seems to be no method to fetch the certificate from libgit2.

Some past discussions in libgit2 talks about keeping the data outside of libgit2 when using an external transport. With the current structure of the code, it's hard to pass the certificate to the go transport.

This change introduces a global CA certs pool that can be populated by the users of git2go and the smart transport can lookup for the presence of any certificate associated with the target URL in the global pool before making any https requests. This solves the cloning issue due to cert signing.

A user would populate the global cert pool with:

if err := git2go.RegisterCACerts(url, caBundle); err != nil {
	return err
}

The transport will automatically pick the certificate if available for a URL and use it.

NOTE: This implementation came out of various attempts to debug and fix the issue. I would like to have some guidance in finding proper ways to implement this or fix this issue. With these changes, all the aforementioned fluxcd tests work with libgit2 v1.3 and the smart transport. We've had trouble with the libgit2 library transport and this could help us not depend on those and resolve the issues.

[darkowlzz]

I'd work on fixing the test failures once I get some feedback and clarity about the solution.

[hiddeco]

Side note that for us to be able to properly make use of the Go transport, having a non-global (custom) cert pool (or other way of configuration) is a pretty hard requirement to ensure proper isolation.

[hiddeco]

Having thought some more about it, this issue isn't unique to git2go but also present in e.g. the Go native go-git project due to it using global protocol based configurations: https://github.com/go-git/go-git/blob/641ee1dd69d3b8616127623e4b9341f4f4196d12/plumbing/transport/client/client.go#L16.

Which - while constructed a bit different - results in the same pattern of it being hard to overwrite defaults on a per-repository/operation basis.

[jasperem]

Hi @darkowlzz, to fix Push you need to change req, err = http.NewRequest("POST", url+"/info/refs?service=git-upload-pack", nil) in https://github.com/darkowlzz/git2go/blob/7f64702bd0195300f75d0e5ff24706dd03776363/http.go#L94 to req, err = http.NewRequest("POST", url+"/info/refs?service="git-receive-pack", nil). After these changes, your implementation works for me.

[darkowlzz]

@jasperem Thanks for pointing that out. All of the testing I did were related to cloning. Didn't try pushing. Very helpful. I'll update that. I'm also about to attempt to reimplement the same without the global cert pool. I think that'll be a better approach in general.

[darkowlzz]

Update: A rewrite of this is in #870 . I took a different approach this time. Since we had a need to not share the secrets globally, I added the ability to setup the smart transport and configure it accordingly per operation. This way, any secrets used in the transport can be deleted and not shared. Also, figured out the reason for the test failures above. When there's no credentials provided and no remote callbacks set, it resulted in passthrough error. Handled that accordingly and now the tests pass.

Hi @jasperem, I've added your suggestion as part of #870. It'd be nice if you could try that and see if it works for you. Any feedback would be very helpful.

[jasperem]

@darkowlzz thx for your work! I will test it and report how it works for me