diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index de03c2cf..ae922ff3 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -458,27 +458,15 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, repositoryURL := obj.Spec.URL // managed GIT transport only affects the libgit2 implementation if managed.Enabled() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation { - // At present only HTTP connections have the ability to define remote options. - // Although this can be easily extended by ensuring that the fake URL below uses the - // target ssh scheme, and the libgit2/managed/ssh.go pulls that information accordingly. - // - // This is due to the fact the key libgit2 remote callbacks do not take place for HTTP - // whilst most still work for SSH. + // We set the TransportAuthID of this set of authentication options here by constructing + // a unique ID that won't clash in a multi tenant environment. This unique ID is used by + // libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in + // libgit2, which is inflexible and unstable. if strings.HasPrefix(repositoryURL, "http") { - // Due to the lack of the callback feature, a fake target URL is created to allow - // for the smart sub transport be able to pick the options specific for this - // GitRepository object. - // The URL should use unique information that do not collide in a multi tenant - // deployment. - repositoryURL = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation) - managed.AddTransportOptions(repositoryURL, - managed.TransportOptions{ - TargetURL: obj.Spec.URL, - CABundle: authOpts.CAFile, - }) - - // We remove the options from memory, to avoid accumulating unused options over time. - defer managed.RemoveTransportOptions(repositoryURL) + authOpts.TransportAuthID = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation) + } + if strings.HasPrefix(repositoryURL, "ssh") { + authOpts.TransportAuthID = fmt.Sprintf("ssh://%s/%s/%d", obj.Name, obj.UID, obj.Generation) } } diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index cc6f8e48..56752951 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -69,6 +69,22 @@ type CheckoutBranch struct { func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { defer recoverPanic(&err) + if managed.Enabled() { + // We store the target url and auth options mapped to a unique ID. We overwrite the target url + // with the TransportAuthID, because managed transports don't provide a way for any kind of + // dependency injection. This lets us have a way of doing interop between application level code + // and transport level code. + // Performing all fetch operations with the TransportAuthID as the url, lets the managed + // transport action use it to fetch the registered transport options which contains the + // _actual_ target url and the correct credentials to use. + managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{ + TargetURL: url, + AuthOpts: opts, + }) + url = opts.TransportAuthID + defer managed.RemoveTransportOptions(opts.TransportAuthID) + } + remoteCallBacks := RemoteCallbacks(ctx, opts) proxyOpts := &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto} @@ -170,6 +186,15 @@ type CheckoutTag struct { func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { defer recoverPanic(&err) + if managed.Enabled() { + managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{ + TargetURL: url, + AuthOpts: opts, + }) + url = opts.TransportAuthID + defer managed.RemoveTransportOptions(opts.TransportAuthID) + } + remoteCallBacks := RemoteCallbacks(ctx, opts) proxyOpts := &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto} @@ -249,6 +274,15 @@ type CheckoutCommit struct { func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { defer recoverPanic(&err) + if managed.Enabled() { + managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{ + TargetURL: url, + AuthOpts: opts, + }) + url = opts.TransportAuthID + defer managed.RemoveTransportOptions(opts.TransportAuthID) + } + repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, @@ -278,6 +312,15 @@ type CheckoutSemVer struct { func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { defer recoverPanic(&err) + if managed.Enabled() { + managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{ + TargetURL: url, + AuthOpts: opts, + }) + url = opts.TransportAuthID + defer managed.RemoveTransportOptions(opts.TransportAuthID) + } + verConstraint, err := semver.NewConstraint(c.SemVer) if err != nil { return nil, fmt.Errorf("semver parse error: %w", err) diff --git a/pkg/git/libgit2/managed/http.go b/pkg/git/libgit2/managed/http.go index 09c0ee26..ffccb1f6 100644 --- a/pkg/git/libgit2/managed/http.go +++ b/pkg/git/libgit2/managed/http.go @@ -86,7 +86,7 @@ type httpSmartSubtransport struct { httpTransport *http.Transport } -func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) { +func (t *httpSmartSubtransport) Action(transportAuthID string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) { var proxyFn func(*http.Request) (*url.URL, error) proxyOpts, err := t.transport.SmartProxyOptions() if err != nil { @@ -109,7 +109,7 @@ func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServ t.httpTransport.Proxy = proxyFn t.httpTransport.DisableCompression = false - client, req, err := createClientRequest(targetUrl, action, t.httpTransport) + client, req, err := createClientRequest(transportAuthID, action, t.httpTransport) if err != nil { return nil, err } @@ -142,7 +142,7 @@ func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServ return stream, nil } -func createClientRequest(targetUrl string, action git2go.SmartServiceAction, t *http.Transport) (*http.Client, *http.Request, error) { +func createClientRequest(transportAuthID string, action git2go.SmartServiceAction, t *http.Transport) (*http.Client, *http.Request, error) { var req *http.Request var err error @@ -150,28 +150,14 @@ func createClientRequest(targetUrl string, action git2go.SmartServiceAction, t * return nil, nil, fmt.Errorf("failed to create client: transport cannot be nil") } - finalUrl := targetUrl - opts, found := transportOptions(targetUrl) - if found { - if opts.TargetURL != "" { - // override target URL only if options are found and a new targetURL - // is provided. - finalUrl = opts.TargetURL - } + opts, found := getTransportOptions(transportAuthID) - // Add any provided certificate to the http transport. - if len(opts.CABundle) > 0 { - cap := x509.NewCertPool() - if ok := cap.AppendCertsFromPEM(opts.CABundle); !ok { - return nil, nil, fmt.Errorf("failed to use certificate from PEM") - } - t.TLSClientConfig = &tls.Config{ - RootCAs: cap, - } - } + if !found { + return nil, nil, fmt.Errorf("failed to create client: could not find transport options for the object: %s", transportAuthID) } + targetURL := opts.TargetURL - if len(finalUrl) > URLMaxLength { + if len(targetURL) > URLMaxLength { return nil, nil, fmt.Errorf("URL exceeds the max length (%d)", URLMaxLength) } @@ -182,20 +168,20 @@ func createClientRequest(targetUrl string, action git2go.SmartServiceAction, t * switch action { case git2go.SmartServiceActionUploadpackLs: - req, err = http.NewRequest("GET", finalUrl+"/info/refs?service=git-upload-pack", nil) + req, err = http.NewRequest("GET", targetURL+"/info/refs?service=git-upload-pack", nil) case git2go.SmartServiceActionUploadpack: - req, err = http.NewRequest("POST", finalUrl+"/git-upload-pack", nil) + req, err = http.NewRequest("POST", targetURL+"/git-upload-pack", nil) if err != nil { break } req.Header.Set("Content-Type", "application/x-git-upload-pack-request") case git2go.SmartServiceActionReceivepackLs: - req, err = http.NewRequest("GET", finalUrl+"/info/refs?service=git-receive-pack", nil) + req, err = http.NewRequest("GET", targetURL+"/info/refs?service=git-receive-pack", nil) case git2go.SmartServiceActionReceivepack: - req, err = http.NewRequest("POST", finalUrl+"/git-receive-pack", nil) + req, err = http.NewRequest("POST", targetURL+"/git-receive-pack", nil) if err != nil { break } @@ -209,6 +195,20 @@ func createClientRequest(targetUrl string, action git2go.SmartServiceAction, t * return nil, nil, err } + // Add any provided certificate to the http transport. + if opts.AuthOpts != nil { + req.SetBasicAuth(opts.AuthOpts.Username, opts.AuthOpts.Password) + if len(opts.AuthOpts.CAFile) > 0 { + certPool := x509.NewCertPool() + if ok := certPool.AppendCertsFromPEM(opts.AuthOpts.CAFile); !ok { + return nil, nil, fmt.Errorf("failed to use certificate from PEM") + } + t.TLSClientConfig = &tls.Config{ + RootCAs: certPool, + } + } + } + req.Header.Set("User-Agent", "git/2.0 (flux-libgit2)") return client, req, nil } @@ -239,7 +239,6 @@ type httpSmartSubtransportStream struct { recvReply sync.WaitGroup httpError error m sync.RWMutex - targetURL string } func newManagedHttpStream(owner *httpSmartSubtransport, req *http.Request, client *http.Client) *httpSmartSubtransportStream { @@ -324,29 +323,8 @@ func (self *httpSmartSubtransportStream) sendRequest() error { var resp *http.Response var err error - var userName string - var password string - - // Obtain the credentials and use them if available. - cred, err := self.owner.transport.SmartCredentials("", git2go.CredentialTypeUserpassPlaintext) - if err != nil { - // Passthrough error indicates that no credentials were provided. - // Continue without credentials. - if err.Error() != git2go.ErrorCodePassthrough.String() { - return err - } - } - - if cred != nil { - defer cred.Free() - - userName, password, err = cred.GetUserpassPlaintext() - if err != nil { - return err - } - } - var content []byte + for { req := &http.Request{ Method: self.req.Method, @@ -365,7 +343,6 @@ func (self *httpSmartSubtransportStream) sendRequest() error { req.ContentLength = -1 } - req.SetBasicAuth(userName, password) traceLog.Info("[http]: new request", "method", req.Method, "URL", req.URL) resp, err = self.client.Do(req) if err != nil { diff --git a/pkg/git/libgit2/managed/http_test.go b/pkg/git/libgit2/managed/http_test.go new file mode 100644 index 00000000..bf54de59 --- /dev/null +++ b/pkg/git/libgit2/managed/http_test.go @@ -0,0 +1,235 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package managed + +import ( + "fmt" + "net/http" + "os" + "path/filepath" + "testing" + + "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/source-controller/pkg/git" + "github.com/go-logr/logr" + . "github.com/onsi/gomega" + + git2go "github.com/libgit2/git2go/v33" +) + +func TestHttpAction_CreateClientRequest(t *testing.T) { + opts := &TransportOptions{ + TargetURL: "https://final-target/abc", + } + + optsWithAuth := &TransportOptions{ + TargetURL: "https://final-target/abc", + AuthOpts: &git.AuthOptions{ + Username: "user", + Password: "pwd", + }, + } + id := "https://obj-id" + + tests := []struct { + name string + assertFunc func(g *WithT, req *http.Request, client *http.Client) + action git2go.SmartServiceAction + opts *TransportOptions + transport *http.Transport + wantedErr error + }{ + { + name: "Uploadpack: URL and method are correctly set", + action: git2go.SmartServiceActionUploadpack, + transport: &http.Transport{}, + assertFunc: func(g *WithT, req *http.Request, _ *http.Client) { + g.Expect(req.URL.String()).To(Equal("https://final-target/abc/git-upload-pack")) + g.Expect(req.Method).To(Equal("POST")) + }, + opts: opts, + wantedErr: nil, + }, + { + name: "UploadpackLs: URL and method are correctly set", + action: git2go.SmartServiceActionUploadpackLs, + transport: &http.Transport{}, + assertFunc: func(g *WithT, req *http.Request, _ *http.Client) { + g.Expect(req.URL.String()).To(Equal("https://final-target/abc/info/refs?service=git-upload-pack")) + g.Expect(req.Method).To(Equal("GET")) + }, + opts: opts, + wantedErr: nil, + }, + { + name: "Receivepack: URL and method are correctly set", + action: git2go.SmartServiceActionReceivepack, + transport: &http.Transport{}, + assertFunc: func(g *WithT, req *http.Request, _ *http.Client) { + g.Expect(req.URL.String()).To(Equal("https://final-target/abc/git-receive-pack")) + g.Expect(req.Method).To(Equal("POST")) + }, + opts: opts, + wantedErr: nil, + }, + { + name: "ReceivepackLs: URL and method are correctly set", + action: git2go.SmartServiceActionReceivepackLs, + transport: &http.Transport{}, + assertFunc: func(g *WithT, req *http.Request, _ *http.Client) { + g.Expect(req.URL.String()).To(Equal("https://final-target/abc/info/refs?service=git-receive-pack")) + g.Expect(req.Method).To(Equal("GET")) + }, + opts: opts, + wantedErr: nil, + }, + { + name: "credentials are correctly configured", + action: git2go.SmartServiceActionUploadpack, + transport: &http.Transport{}, + opts: optsWithAuth, + assertFunc: func(g *WithT, req *http.Request, client *http.Client) { + g.Expect(req.URL.String()).To(Equal("https://final-target/abc/git-upload-pack")) + g.Expect(req.Method).To(Equal("POST")) + + username, pwd, ok := req.BasicAuth() + if !ok { + t.Errorf("could not find Authentication header in request.") + } + g.Expect(username).To(Equal("user")) + g.Expect(pwd).To(Equal("pwd")) + }, + wantedErr: nil, + }, + { + name: "error when no http.transport provided", + action: git2go.SmartServiceActionUploadpack, + transport: nil, + opts: opts, + wantedErr: fmt.Errorf("failed to create client: transport cannot be nil"), + }, + { + name: "error when no transport options are registered", + action: git2go.SmartServiceActionUploadpack, + transport: &http.Transport{}, + opts: nil, + wantedErr: fmt.Errorf("failed to create client: could not find transport options for the object: https://obj-id"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + if tt.opts != nil { + AddTransportOptions(id, *tt.opts) + } + + client, req, err := createClientRequest(id, tt.action, tt.transport) + if err != nil { + t.Log(err) + } + if tt.wantedErr != nil { + g.Expect(err).To(Equal(tt.wantedErr)) + } else { + tt.assertFunc(g, req, client) + } + + if tt.opts != nil { + RemoveTransportOptions(id) + } + }) + } +} + +func TestHTTPManagedTransport_E2E(t *testing.T) { + g := NewWithT(t) + + server, err := gittestserver.NewTempGitServer() + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(server.Root()) + + user := "test-user" + pwd := "test-pswd" + server.Auth(user, pwd) + server.KeyDir(filepath.Join(server.Root(), "keys")) + + err = server.ListenSSH() + g.Expect(err).ToNot(HaveOccurred()) + + err = server.StartHTTP() + g.Expect(err).ToNot(HaveOccurred()) + defer server.StopHTTP() + + go func() { + server.StartSSH() + }() + defer server.StopSSH() + + // Force managed transport to be enabled + InitManagedTransport(logr.Discard()) + + repoPath := "test.git" + err = server.InitRepo("../../testdata/git/repo", git.DefaultBranch, repoPath) + g.Expect(err).ToNot(HaveOccurred()) + + tmpDir := t.TempDir() + + // Register the auth options and target url mapped to a unique id. + id := "http://obj-id" + AddTransportOptions(id, TransportOptions{ + TargetURL: server.HTTPAddress() + "/" + repoPath, + AuthOpts: &git.AuthOptions{ + Username: user, + Password: pwd, + }, + }) + + // We call Clone with id instead of the actual url, as the transport action + // will fetch the actual url and the required credentials using the id as + // a identifier. + repo, err := git2go.Clone(id, tmpDir, &git2go.CloneOptions{ + CheckoutOptions: git2go.CheckoutOptions{ + Strategy: git2go.CheckoutForce, + }, + }) + g.Expect(err).ToNot(HaveOccurred()) + repo.Free() +} + +func TestHTTPManagedTransport_HandleRedirect(t *testing.T) { + g := NewWithT(t) + + tmpDir := t.TempDir() + + // Force managed transport to be enabled + InitManagedTransport(logr.Discard()) + + id := "http://obj-id" + AddTransportOptions(id, TransportOptions{ + TargetURL: "http://github.com/stefanprodan/podinfo", + }) + + // GitHub will cause a 301 and redirect to https + repo, err := git2go.Clone(id, tmpDir, &git2go.CloneOptions{ + CheckoutOptions: git2go.CheckoutOptions{ + Strategy: git2go.CheckoutForce, + }, + }) + + g.Expect(err).ToNot(HaveOccurred()) + repo.Free() +} diff --git a/pkg/git/libgit2/managed/managed_test.go b/pkg/git/libgit2/managed/managed_test.go index 5bfd1c1e..beda7fc2 100644 --- a/pkg/git/libgit2/managed/managed_test.go +++ b/pkg/git/libgit2/managed/managed_test.go @@ -17,287 +17,32 @@ limitations under the License. package managed import ( - "fmt" - "net/http" "os" - "path/filepath" - "reflect" "testing" - - "github.com/fluxcd/pkg/gittestserver" - "github.com/fluxcd/pkg/ssh" - "github.com/fluxcd/source-controller/pkg/git" - "github.com/go-logr/logr" - - git2go "github.com/libgit2/git2go/v33" - . "github.com/onsi/gomega" - "gotest.tools/assert" ) -func TestHttpAction_CreateClientRequest(t *testing.T) { - tests := []struct { - name string - url string - expectedUrl string - expectedMethod string - action git2go.SmartServiceAction - opts *TransportOptions - transport *http.Transport - wantedErr error - }{ - { - name: "Uploadpack: no changes when no options found", - url: "https://sometarget/abc", - expectedUrl: "https://sometarget/abc/git-upload-pack", - expectedMethod: "POST", - action: git2go.SmartServiceActionUploadpack, - transport: &http.Transport{}, - opts: nil, - wantedErr: nil, - }, - { - name: "UploadpackLs: no changes when no options found", - url: "https://sometarget/abc", - expectedUrl: "https://sometarget/abc/info/refs?service=git-upload-pack", - expectedMethod: "GET", - action: git2go.SmartServiceActionUploadpackLs, - transport: &http.Transport{}, - opts: nil, - wantedErr: nil, - }, - { - name: "Receivepack: no changes when no options found", - url: "https://sometarget/abc", - expectedUrl: "https://sometarget/abc/git-receive-pack", - expectedMethod: "POST", - action: git2go.SmartServiceActionReceivepack, - transport: &http.Transport{}, - opts: nil, - wantedErr: nil, - }, - { - name: "ReceivepackLs: no changes when no options found", - url: "https://sometarget/abc", - expectedUrl: "https://sometarget/abc/info/refs?service=git-receive-pack", - expectedMethod: "GET", - action: git2go.SmartServiceActionReceivepackLs, - transport: &http.Transport{}, - opts: nil, - wantedErr: nil, - }, - { - name: "override URL via options", - url: "https://initial-target/abc", - expectedUrl: "https://final-target/git-upload-pack", - expectedMethod: "POST", - action: git2go.SmartServiceActionUploadpack, - transport: &http.Transport{}, - opts: &TransportOptions{ - TargetURL: "https://final-target", - }, - wantedErr: nil, - }, - { - name: "error when no http.transport provided", - url: "https://initial-target/abc", - expectedUrl: "", - expectedMethod: "", - action: git2go.SmartServiceActionUploadpack, - transport: nil, - opts: nil, - wantedErr: fmt.Errorf("failed to create client: transport cannot be nil"), - }, +func TestFlagStatus(t *testing.T) { + if Enabled() { + t.Errorf("experimental transport should not be enabled by default") } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.opts != nil { - AddTransportOptions(tt.url, *tt.opts) - } - - _, req, err := createClientRequest(tt.url, tt.action, tt.transport) - if tt.wantedErr != nil { - if tt.wantedErr.Error() != err.Error() { - t.Errorf("wanted: %v got: %v", tt.wantedErr, err) - } - } else { - assert.Equal(t, req.URL.String(), tt.expectedUrl) - assert.Equal(t, req.Method, tt.expectedMethod) - } - - if tt.opts != nil { - RemoveTransportOptions(tt.url) - } - }) - } -} - -func TestOptions(t *testing.T) { - tests := []struct { - name string - registerOpts bool - url string - opts TransportOptions - expectOpts bool - expectedOpts *TransportOptions - }{ - { - name: "return registered option", - registerOpts: true, - url: "https://target/?123", - opts: TransportOptions{}, - expectOpts: true, - expectedOpts: &TransportOptions{}, - }, - { - name: "match registered options", - registerOpts: true, - url: "https://target/?876", - opts: TransportOptions{ - TargetURL: "https://new-target/321", - CABundle: []byte{123, 213, 132}, - }, - expectOpts: true, - expectedOpts: &TransportOptions{ - TargetURL: "https://new-target/321", - CABundle: []byte{123, 213, 132}, - }, - }, - { - name: "ignore when options not registered", - registerOpts: false, - url: "", - opts: TransportOptions{}, - expectOpts: false, - expectedOpts: nil, - }, + os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") + if !Enabled() { + t.Errorf("experimental transport should be enabled when env EXPERIMENTAL_GIT_TRANSPORT=true") } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.registerOpts { - AddTransportOptions(tt.url, tt.opts) - } + os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "1") + if !Enabled() { + t.Errorf("experimental transport should be enabled when env EXPERIMENTAL_GIT_TRANSPORT=1") + } - opts, found := transportOptions(tt.url) - if tt.expectOpts != found { - t.Errorf("%s: wanted %v got %v", tt.name, tt.expectOpts, found) - } + os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "somethingelse") + if Enabled() { + t.Errorf("experimental transport should be enabled only when env EXPERIMENTAL_GIT_TRANSPORT is 1 or true but was enabled for 'somethingelse'") + } - if tt.expectOpts { - if reflect.DeepEqual(opts, *tt.expectedOpts) { - t.Errorf("%s: wanted %v got %v", tt.name, *tt.expectedOpts, opts) - } - } - - if tt.registerOpts { - RemoveTransportOptions(tt.url) - } - - if _, found = transportOptions(tt.url); found { - t.Errorf("%s: option for %s was not removed", tt.name, tt.url) - } - }) + os.Unsetenv("EXPERIMENTAL_GIT_TRANSPORT") + if Enabled() { + t.Errorf("experimental transport should not be enabled when env EXPERIMENTAL_GIT_TRANSPORT is not present") } } - -func TestManagedTransport_E2E(t *testing.T) { - g := NewWithT(t) - - server, err := gittestserver.NewTempGitServer() - g.Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(server.Root()) - - user := "test-user" - pasword := "test-pswd" - server.Auth(user, pasword) - server.KeyDir(filepath.Join(server.Root(), "keys")) - - err = server.ListenSSH() - g.Expect(err).ToNot(HaveOccurred()) - - err = server.StartHTTP() - g.Expect(err).ToNot(HaveOccurred()) - defer server.StopHTTP() - - go func() { - server.StartSSH() - }() - defer server.StopSSH() - - // Force managed transport to be enabled - InitManagedTransport(logr.Discard()) - - repoPath := "test.git" - err = server.InitRepo("../../testdata/git/repo", git.DefaultBranch, repoPath) - g.Expect(err).ToNot(HaveOccurred()) - - tmpDir := t.TempDir() - - // Test HTTP transport - - // Use a fake-url and force it to be overriden by the smart transport. - // This was the way found to ensure that the built-in transport was not used. - httpAddress := "http://fake-url" - AddTransportOptions(httpAddress, TransportOptions{ - TargetURL: server.HTTPAddress() + "/" + repoPath, - }) - - repo, err := git2go.Clone(httpAddress, tmpDir, &git2go.CloneOptions{ - FetchOptions: git2go.FetchOptions{ - RemoteCallbacks: git2go.RemoteCallbacks{ - CredentialsCallback: func(url, username_from_url string, allowed_types git2go.CredentialType) (*git2go.Credential, error) { - return git2go.NewCredentialUserpassPlaintext(user, pasword) - }, - }, - }, - CheckoutOptions: git2go.CheckoutOptions{ - Strategy: git2go.CheckoutForce, - }, - }) - g.Expect(err).ToNot(HaveOccurred()) - repo.Free() - - tmpDir2 := t.TempDir() - - kp, err := ssh.NewEd25519Generator().Generate() - g.Expect(err).ToNot(HaveOccurred()) - - // Test SSH transport - sshAddress := server.SSHAddress() + "/" + repoPath - repo, err = git2go.Clone(sshAddress, tmpDir2, &git2go.CloneOptions{ - FetchOptions: git2go.FetchOptions{ - RemoteCallbacks: git2go.RemoteCallbacks{ - CredentialsCallback: func(url, username_from_url string, allowed_types git2go.CredentialType) (*git2go.Credential, error) { - return git2go.NewCredentialSSHKeyFromMemory("git", "", string(kp.PrivateKey), "") - }, - }, - }, - CheckoutOptions: git2go.CheckoutOptions{ - Strategy: git2go.CheckoutForce, - }, - }) - - g.Expect(err).ToNot(HaveOccurred()) - repo.Free() -} - -func TestManagedTransport_HandleRedirect(t *testing.T) { - g := NewWithT(t) - - tmpDir := t.TempDir() - - // Force managed transport to be enabled - InitManagedTransport(logr.Discard()) - - // GitHub will cause a 301 and redirect to https - repo, err := git2go.Clone("http://github.com/stefanprodan/podinfo", tmpDir, &git2go.CloneOptions{ - FetchOptions: git2go.FetchOptions{}, - CheckoutOptions: git2go.CheckoutOptions{ - Strategy: git2go.CheckoutForce, - }, - }) - - g.Expect(err).ToNot(HaveOccurred()) - repo.Free() -} diff --git a/pkg/git/libgit2/managed/options.go b/pkg/git/libgit2/managed/options.go index d4d346ad..58a04da7 100644 --- a/pkg/git/libgit2/managed/options.go +++ b/pkg/git/libgit2/managed/options.go @@ -18,35 +18,38 @@ package managed import ( "sync" + + "github.com/fluxcd/source-controller/pkg/git" ) // TransportOptions represents options to be applied at transport-level // at request time. type TransportOptions struct { TargetURL string - CABundle []byte + AuthOpts *git.AuthOptions } var ( + // transportOpts maps a unique id to a set of transport options. transportOpts = make(map[string]TransportOptions, 0) m sync.RWMutex ) -func AddTransportOptions(targetUrl string, opts TransportOptions) { +func AddTransportOptions(id string, opts TransportOptions) { m.Lock() - transportOpts[targetUrl] = opts + transportOpts[id] = opts m.Unlock() } -func RemoveTransportOptions(targetUrl string) { +func RemoveTransportOptions(id string) { m.Lock() - delete(transportOpts, targetUrl) + delete(transportOpts, id) m.Unlock() } -func transportOptions(targetUrl string) (*TransportOptions, bool) { +func getTransportOptions(id string) (*TransportOptions, bool) { m.RLock() - opts, found := transportOpts[targetUrl] + opts, found := transportOpts[id] m.RUnlock() if found { @@ -60,16 +63,16 @@ func transportOptions(targetUrl string) (*TransportOptions, bool) { // Given that TransportOptions can allow for the target URL to be overriden // this returns the same input if Managed Transport is disabled or if no TargetURL // is set on TransportOptions. -func EffectiveURL(targetUrl string) string { +func EffectiveURL(id string) string { if !Enabled() { - return targetUrl + return id } - if opts, found := transportOptions(targetUrl); found { + if opts, found := getTransportOptions(id); found { if opts.TargetURL != "" { return opts.TargetURL } } - return targetUrl + return id } diff --git a/pkg/git/libgit2/managed/options_test.go b/pkg/git/libgit2/managed/options_test.go new file mode 100644 index 00000000..4f35a0fc --- /dev/null +++ b/pkg/git/libgit2/managed/options_test.go @@ -0,0 +1,94 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package managed + +import ( + "testing" + + "github.com/fluxcd/source-controller/pkg/git" + . "github.com/onsi/gomega" +) + +func TestTransportOptions(t *testing.T) { + tests := []struct { + name string + registerOpts bool + url string + opts TransportOptions + expectOpts bool + expectedOpts *TransportOptions + }{ + { + name: "return registered option", + registerOpts: true, + url: "https://target/?123", + opts: TransportOptions{}, + expectOpts: true, + expectedOpts: &TransportOptions{}, + }, + { + name: "match registered options", + registerOpts: true, + url: "https://target/?876", + opts: TransportOptions{ + TargetURL: "https://new-target/321", + AuthOpts: &git.AuthOptions{ + CAFile: []byte{123, 213, 132}, + }, + }, + expectOpts: true, + expectedOpts: &TransportOptions{ + TargetURL: "https://new-target/321", + AuthOpts: &git.AuthOptions{ + CAFile: []byte{123, 213, 132}, + }, + }, + }, + { + name: "ignore when options not registered", + registerOpts: false, + url: "", + opts: TransportOptions{}, + expectOpts: false, + expectedOpts: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + if tt.registerOpts { + AddTransportOptions(tt.url, tt.opts) + } + + opts, found := getTransportOptions(tt.url) + g.Expect(found).To(Equal(found)) + + if tt.expectOpts { + g.Expect(tt.expectedOpts).To(Equal(opts)) + } + + if tt.registerOpts { + RemoveTransportOptions(tt.url) + } + + _, found = getTransportOptions(tt.url) + g.Expect(found).To(BeFalse()) + }) + } +} diff --git a/pkg/git/libgit2/managed/ssh.go b/pkg/git/libgit2/managed/ssh.go index ea7bd491..3895fbe4 100644 --- a/pkg/git/libgit2/managed/ssh.go +++ b/pkg/git/libgit2/managed/ssh.go @@ -96,11 +96,16 @@ type sshSmartSubtransport struct { connected bool } -func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) { +func (t *sshSmartSubtransport) Action(credentialsID string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) { runtime.LockOSThread() defer runtime.UnlockOSThread() - u, err := url.Parse(urlString) + opts, found := getTransportOptions(credentialsID) + if !found { + return nil, fmt.Errorf("could not find transport options for object: %s", credentialsID) + } + + u, err := url.Parse(opts.TargetURL) if err != nil { return nil, err } @@ -146,19 +151,13 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi _ = t.Close() } - cred, err := t.transport.SmartCredentials("", git2go.CredentialTypeSSHMemory) - if err != nil { - return nil, err - } - defer cred.Free() - port := "22" if u.Port() != "" { port = u.Port() } t.addr = net.JoinHostPort(u.Hostname(), port) - sshConfig, err := clientConfig(t.addr, cred) + sshConfig, err := createClientConfig(opts.AuthOpts) if err != nil { return nil, err } @@ -307,39 +306,28 @@ func (stream *sshSmartSubtransportStream) Free() { traceLog.Info("[ssh]: sshSmartSubtransportStream.Free()") } -func clientConfig(remoteAddress string, cred *git2go.Credential) (*ssh.ClientConfig, error) { - if cred == nil { - return nil, fmt.Errorf("cannot create ssh client config from a nil credential") +func createClientConfig(authOpts *git.AuthOptions) (*ssh.ClientConfig, error) { + if authOpts == nil { + return nil, fmt.Errorf("cannot create ssh client config from nil ssh auth options") } - username, _, privatekey, passphrase, err := cred.GetSSHKey() - if err != nil { - return nil, err - } - - var pemBytes []byte - if cred.Type() == git2go.CredentialTypeSSHMemory { - pemBytes = []byte(privatekey) + var signer ssh.Signer + var err error + if authOpts.Password != "" { + signer, err = ssh.ParsePrivateKeyWithPassphrase(authOpts.Identity, []byte(authOpts.Password)) } else { - return nil, fmt.Errorf("file based SSH credential is not supported") + signer, err = ssh.ParsePrivateKey(authOpts.Identity) } - - var key ssh.Signer - if passphrase != "" { - key, err = ssh.ParsePrivateKeyWithPassphrase(pemBytes, []byte(passphrase)) - } else { - key, err = ssh.ParsePrivateKey(pemBytes) - } - if err != nil { return nil, err } cfg := &ssh.ClientConfig{ - User: username, - Auth: []ssh.AuthMethod{ssh.PublicKeys(key)}, + User: authOpts.Username, + Auth: []ssh.AuthMethod{ssh.PublicKeys(signer)}, Timeout: sshConnectionTimeOut, } + if len(git.KexAlgos) > 0 { cfg.Config.KeyExchanges = git.KexAlgos } diff --git a/pkg/git/libgit2/managed/ssh_test.go b/pkg/git/libgit2/managed/ssh_test.go new file mode 100644 index 00000000..4d5a7b37 --- /dev/null +++ b/pkg/git/libgit2/managed/ssh_test.go @@ -0,0 +1,124 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package managed + +import ( + "os" + "path/filepath" + "testing" + + "github.com/fluxcd/pkg/ssh" + "github.com/fluxcd/source-controller/pkg/git" + "github.com/go-logr/logr" + . "github.com/onsi/gomega" + + "github.com/fluxcd/pkg/gittestserver" + git2go "github.com/libgit2/git2go/v33" +) + +func TestSSHAction_clientConfig(t *testing.T) { + kp, err := ssh.GenerateKeyPair(ssh.RSA_4096) + if err != nil { + t.Fatalf("could not generate keypair: %s", err) + } + tests := []struct { + name string + authOpts *git.AuthOptions + expectedUsername string + expectedAuthLen int + expectErr string + }{ + { + name: "nil SSHTransportOptions returns an error", + authOpts: nil, + expectErr: "cannot create ssh client config from nil ssh auth options", + }, + { + name: "valid SSHTransportOptions returns a valid SSHClientConfig", + authOpts: &git.AuthOptions{ + Identity: kp.PrivateKey, + Username: "user", + }, + expectedUsername: "user", + expectedAuthLen: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + cfg, err := createClientConfig(tt.authOpts) + if tt.expectErr != "" { + g.Expect(tt.expectErr).To(Equal(err.Error())) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cfg.User).To(Equal(tt.expectedUsername)) + g.Expect(len(cfg.Auth)).To(Equal(tt.expectedAuthLen)) + }) + } +} + +func TestSSHManagedTransport_E2E(t *testing.T) { + g := NewWithT(t) + + server, err := gittestserver.NewTempGitServer() + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(server.Root()) + + server.KeyDir(filepath.Join(server.Root(), "keys")) + + err = server.ListenSSH() + g.Expect(err).ToNot(HaveOccurred()) + + go func() { + server.StartSSH() + }() + defer server.StopSSH() + InitManagedTransport(logr.Discard()) + + kp, err := ssh.NewEd25519Generator().Generate() + g.Expect(err).ToNot(HaveOccurred()) + + repoPath := "test.git" + err = server.InitRepo("../../testdata/git/repo", git.DefaultBranch, repoPath) + g.Expect(err).ToNot(HaveOccurred()) + + transportID := "ssh://git@fake-url" + sshAddress := server.SSHAddress() + "/" + repoPath + AddTransportOptions(transportID, TransportOptions{ + TargetURL: sshAddress, + AuthOpts: &git.AuthOptions{ + Username: "user", + Identity: kp.PrivateKey, + }, + }) + + tmpDir := t.TempDir() + + // We call git2go.Clone with transportID, so that the managed ssh transport can + // fetch the correct set of credentials and the actual target url as well. + repo, err := git2go.Clone(transportID, tmpDir, &git2go.CloneOptions{ + FetchOptions: git2go.FetchOptions{}, + CheckoutOptions: git2go.CheckoutOptions{ + Strategy: git2go.CheckoutForce, + }, + }) + + g.Expect(err).ToNot(HaveOccurred()) + repo.Free() +} diff --git a/pkg/git/libgit2/managed_test.go b/pkg/git/libgit2/managed_test.go index 0d812a23..728c61fe 100644 --- a/pkg/git/libgit2/managed_test.go +++ b/pkg/git/libgit2/managed_test.go @@ -19,6 +19,7 @@ package libgit2 import ( "context" "fmt" + "math/rand" "net/url" "os" "path/filepath" @@ -36,7 +37,6 @@ import ( . "github.com/onsi/gomega" cryptossh "golang.org/x/crypto/ssh" - corev1 "k8s.io/api/core/v1" ) const testRepositoryPath = "../testdata/git/repo" @@ -50,12 +50,36 @@ func Test_ManagedSSH_KeyTypes(t *testing.T) { authorized bool wantErr string }{ - {name: "RSA 4096", keyType: ssh.RSA_4096, authorized: true}, - {name: "ECDSA P256", keyType: ssh.ECDSA_P256, authorized: true}, - {name: "ECDSA P384", keyType: ssh.ECDSA_P384, authorized: true}, - {name: "ECDSA P521", keyType: ssh.ECDSA_P521, authorized: true}, - {name: "ED25519", keyType: ssh.ED25519, authorized: true}, - {name: "unauthorized key", keyType: ssh.RSA_4096, wantErr: "Failed to retrieve list of SSH authentication methods"}, + { + name: "RSA 4096", + keyType: ssh.RSA_4096, + authorized: true, + }, + { + name: "ECDSA P256", + keyType: ssh.ECDSA_P256, + authorized: true, + }, + { + name: "ECDSA P384", + keyType: ssh.ECDSA_P384, + authorized: true, + }, + { + name: "ECDSA P521", + keyType: ssh.ECDSA_P521, + authorized: true, + }, + { + name: "ED25519", + keyType: ssh.ED25519, + authorized: true, + }, + { + name: "unauthorized key", + keyType: ssh.RSA_4096, + wantErr: "unable to authenticate, attempted methods [none publickey], no supported methods remain", + }, } serverRootDir := t.TempDir() @@ -99,6 +123,9 @@ func Test_ManagedSSH_KeyTypes(t *testing.T) { knownHosts, err := ssh.ScanHostKey(u.Host, timeout, git.HostKeyAlgos, false) g.Expect(err).ToNot(HaveOccurred()) + os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") + managed.InitManagedTransport(logr.Discard()) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) @@ -112,15 +139,21 @@ func Test_ManagedSSH_KeyTypes(t *testing.T) { authorizedPublicKey = string(kp.PublicKey) } - secret := corev1.Secret{ - Data: map[string][]byte{ - "identity": kp.PrivateKey, - "known_hosts": knownHosts, - }, - } + // secret := corev1.Secret{ + // Data: map[string][]byte{ + // "identity": kp.PrivateKey, + // "known_hosts": knownHosts, + // }, + // } + // + // authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) + // g.Expect(err).ToNot(HaveOccurred()) - authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) - g.Expect(err).ToNot(HaveOccurred()) + authOpts := &git.AuthOptions{ + Identity: kp.PrivateKey, + KnownHosts: knownHosts, + } + authOpts.TransportAuthID = "ssh://" + getTransportAuthID() // Prepare for checkout. branchCheckoutStrat := &CheckoutBranch{Branch: git.DefaultBranch} @@ -200,6 +233,9 @@ func Test_ManagedSSH_KeyExchangeAlgos(t *testing.T) { }, } + os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") + managed.InitManagedTransport(logr.Discard()) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) @@ -223,8 +259,6 @@ func Test_ManagedSSH_KeyExchangeAlgos(t *testing.T) { }() defer server.StopSSH() - os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") - managed.InitManagedTransport(logr.Discard()) repoPath := "test.git" err := server.InitRepo(testRepositoryPath, git.DefaultBranch, repoPath) @@ -246,15 +280,20 @@ func Test_ManagedSSH_KeyExchangeAlgos(t *testing.T) { kp, err := ssh.GenerateKeyPair(ssh.ED25519) g.Expect(err).ToNot(HaveOccurred()) - secret := corev1.Secret{ - Data: map[string][]byte{ - "identity": kp.PrivateKey, - "known_hosts": knownHosts, - }, + // secret := corev1.Secret{ + // Data: map[string][]byte{ + // "identity": kp.PrivateKey, + // "known_hosts": knownHosts, + // }, + // } + // + // authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) + // g.Expect(err).ToNot(HaveOccurred()) + authOpts := &git.AuthOptions{ + Identity: kp.PrivateKey, + KnownHosts: knownHosts, } - - authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) - g.Expect(err).ToNot(HaveOccurred()) + authOpts.TransportAuthID = "ssh://" + getTransportAuthID() // Prepare for checkout. branchCheckoutStrat := &CheckoutBranch{Branch: git.DefaultBranch} @@ -363,6 +402,9 @@ func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { }, } + os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") + managed.InitManagedTransport(logr.Discard()) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) @@ -396,8 +438,6 @@ func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { }() defer server.StopSSH() - os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") - managed.InitManagedTransport(logr.Discard()) repoPath := "test.git" err = server.InitRepo(testRepositoryPath, git.DefaultBranch, repoPath) @@ -419,15 +459,20 @@ func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { kp, err := ssh.GenerateKeyPair(ssh.ED25519) g.Expect(err).ToNot(HaveOccurred()) - secret := corev1.Secret{ - Data: map[string][]byte{ - "identity": kp.PrivateKey, - "known_hosts": knownHosts, - }, + // secret := corev1.Secret{ + // Data: map[string][]byte{ + // "identity": kp.PrivateKey, + // "known_hosts": knownHosts, + // }, + // } + // + // authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) + // g.Expect(err).ToNot(HaveOccurred()) + authOpts := &git.AuthOptions{ + Identity: kp.PrivateKey, + KnownHosts: knownHosts, } - - authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) - g.Expect(err).ToNot(HaveOccurred()) + authOpts.TransportAuthID = "ssh://" + getTransportAuthID() // Prepare for checkout. branchCheckoutStrat := &CheckoutBranch{Branch: git.DefaultBranch} @@ -442,3 +487,12 @@ func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { }) } } + +func getTransportAuthID() string { + letterRunes := []rune("abcdefghijklmnopqrstuvwxyz1234567890") + b := make([]rune, 10) + for i := range b { + b[i] = letterRunes[rand.Intn(len(letterRunes))] + } + return string(b) +} diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 592c5301..e7c9671c 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -38,7 +38,6 @@ import ( "golang.org/x/crypto/ssh/knownhosts" "github.com/fluxcd/source-controller/pkg/git" - "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" ) var ( @@ -115,18 +114,6 @@ func pushTransferProgressCallback(ctx context.Context) git2go.PushTransferProgre func credentialsCallback(opts *git.AuthOptions) git2go.CredentialsCallback { return func(url string, username string, allowedTypes git2go.CredentialType) (*git2go.Credential, error) { if allowedTypes&(git2go.CredentialTypeSSHKey|git2go.CredentialTypeSSHCustom|git2go.CredentialTypeSSHMemory) != 0 { - if managed.Enabled() { - // CredentialTypeSSHMemory requires libgit2 to be built using libssh2. - // When using managed transport (handled in go instead of libgit2), - // there may be ways to remove such requirement, thefore decreasing the - // need of libz, libssh2 and OpenSSL but further investigation is required - // once Managed Transport is no longer experimental. - // - // CredentialSSHKeyFromMemory is currently required for SSH key access - // when managed transport is enabled. - return git2go.NewCredentialSSHKeyFromMemory(opts.Username, "", string(opts.Identity), opts.Password) - } - var ( signer ssh.Signer err error diff --git a/pkg/git/options.go b/pkg/git/options.go index ff1bccac..81bbd6ce 100644 --- a/pkg/git/options.go +++ b/pkg/git/options.go @@ -72,6 +72,11 @@ type AuthOptions struct { Identity []byte KnownHosts []byte CAFile []byte + // TransportAuthID is a unique identifier for this set of authentication + // options. It's used by managed libgit2 transports to uniquely identify + // which credentials to use for a particular git operation, and avoid misuse + // of credentials in a multi tenant environment. + TransportAuthID string } // KexAlgos hosts the key exchange algorithms to be used for SSH connections.