Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom CA validation for Git over HTTPS #283

Merged
merged 1 commit into from
Feb 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 78 additions & 1 deletion controllers/gitrepository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ package controllers

import (
"context"
"crypto/tls"
"fmt"
"net/http"
"net/url"
"os"
"path"
Expand All @@ -30,6 +32,8 @@ import (
"github.com/go-git/go-git/v5/config"
"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/object"
"github.com/go-git/go-git/v5/plumbing/transport/client"
httptransport "github.com/go-git/go-git/v5/plumbing/transport/http"
"github.com/go-git/go-git/v5/storage/memory"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
Expand All @@ -40,6 +44,7 @@ import (

"github.com/fluxcd/pkg/gittestserver"

"github.com/fluxcd/pkg/apis/meta"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta1"
)

Expand All @@ -65,6 +70,18 @@ var _ = Describe("GitRepositoryReconciler", func() {
err = k8sClient.Create(context.Background(), namespace)
Expect(err).NotTo(HaveOccurred(), "failed to create test namespace")

cert := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cert",
Namespace: namespace.Name,
},
Data: map[string][]byte{
"caFile": exampleCA,
},
}
err = k8sClient.Create(context.Background(), &cert)
Expect(err).NotTo(HaveOccurred())

gitServer, err = gittestserver.NewTempGitServer()
Expect(err).NotTo(HaveOccurred())
gitServer.AutoCreate()
Expand All @@ -87,6 +104,7 @@ var _ = Describe("GitRepositoryReconciler", func() {
expectMessage string
expectRevision string

secretRef *meta.LocalObjectReference
gitImplementation string
}

Expand Down Expand Up @@ -274,6 +292,55 @@ var _ = Describe("GitRepositoryReconciler", func() {
Expect(err).NotTo(HaveOccurred())
u.Path = path.Join(u.Path, fmt.Sprintf("repository-%s.git", randStringRunes(5)))

var transport = httptransport.NewClient(&http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
},
})
client.InstallProtocol("https", transport)

fs := memfs.New()
gitrepo, err := git.Init(memory.NewStorage(), fs)
Expect(err).NotTo(HaveOccurred())

wt, err := gitrepo.Worktree()
Expect(err).NotTo(HaveOccurred())

ff, _ := fs.Create("fixture")
_ = ff.Close()
_, err = wt.Add(fs.Join("fixture"))
Expect(err).NotTo(HaveOccurred())

commit, err := wt.Commit("Sample", &git.CommitOptions{Author: &object.Signature{
Name: "John Doe",
Email: "john@example.com",
When: time.Now(),
}})
Expect(err).NotTo(HaveOccurred())

gitrepo.Worktree()

for _, ref := range t.createRefs {
hRef := plumbing.NewHashReference(plumbing.ReferenceName(ref), commit)
err = gitrepo.Storer.SetReference(hRef)
Expect(err).NotTo(HaveOccurred())
}

remote, err := gitrepo.CreateRemote(&config.RemoteConfig{
Name: "origin",
URLs: []string{u.String()},
})
Expect(err).NotTo(HaveOccurred())

err = remote.Push(&git.PushOptions{
RefSpecs: []config.RefSpec{"refs/heads/*:refs/heads/*", "refs/tags/*:refs/tags/*"},
})
Expect(err).NotTo(HaveOccurred())

t.reference.Commit = strings.Replace(t.reference.Commit, "<commit>", commit.String(), 1)

client.InstallProtocol("https", httptransport.DefaultClient)

key := types.NamespacedName{
Name: fmt.Sprintf("git-ref-test-%s", randStringRunes(5)),
Namespace: namespace.Name,
Expand All @@ -288,6 +355,7 @@ var _ = Describe("GitRepositoryReconciler", func() {
Interval: metav1.Duration{Duration: indexInterval},
Reference: t.reference,
GitImplementation: t.gitImplementation,
SecretRef: t.secretRef,
},
}
Expect(k8sClient.Create(context.Background(), created)).Should(Succeed())
Expand Down Expand Up @@ -316,13 +384,22 @@ var _ = Describe("GitRepositoryReconciler", func() {
expectStatus: metav1.ConditionFalse,
expectMessage: "x509: certificate signed by unknown authority",
}),
Entry("self signed v2", refTestCase{
Entry("self signed v2 without CA", refTestCase{
reference: &sourcev1.GitRepositoryRef{Branch: "main"},
waitForReason: sourcev1.GitOperationFailedReason,
expectStatus: metav1.ConditionFalse,
expectMessage: "error: user rejected certificate",
gitImplementation: sourcev1.LibGit2Implementation,
}),
Entry("self signed v2 with CA", refTestCase{
reference: &sourcev1.GitRepositoryRef{Branch: "some-branch"},
createRefs: []string{"refs/heads/some-branch"},
waitForReason: sourcev1.GitOperationSucceedReason,
expectStatus: metav1.ConditionTrue,
expectRevision: "some-branch",
secretRef: &meta.LocalObjectReference{Name: "cert"},
gitImplementation: sourcev1.LibGit2Implementation,
}),
)
})
})
33 changes: 33 additions & 0 deletions docs/spec/v1beta1/gitrepositories.md
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,39 @@ kubectl create secret generic pgp-public-keys \
--from-file=author2.asc
```

## Self-signed certificates

Cloning over HTTPS from a Git repository with a self-signed certificate:

```yaml
apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: GitRepository
metadata:
name: podinfo
namespace: default
spec:
interval: 1m
url: https://customdomain.com/stefanprodan/podinfo
secretRef:
name: https-credentials
gitImplementation: libgit2
---
apiVersion: v1
kind: Secret
metadata:
name: https-credentials
namespace: default
type: Opaque
data:
username: <BASE64>
password: <BASE64>
caFile: <BASE64>
```

Note that the Git implementation has to be `libgit2` as `go-git` does not support custom CA verification.
It is also possible to specify a `caFile` for public repositories, in that case the username and password
can be omitted.

## Status examples

Successful sync:
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ require (
github.com/go-git/go-billy/v5 v5.0.0
github.com/go-git/go-git/v5 v5.2.0
github.com/go-logr/logr v0.3.0
github.com/libgit2/git2go/v31 v31.3.0
github.com/libgit2/git2go/v31 v31.4.7
github.com/minio/minio-go/v7 v7.0.5
github.com/onsi/ginkgo v1.14.1
github.com/onsi/gomega v1.10.2
github.com/spf13/pflag v1.0.5
golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0
golang.org/x/crypto v0.0.0-20201203163018-be400aefbc4c
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
helm.sh/helm/v3 v3.5.0
k8s.io/api v0.20.2
Expand Down
12 changes: 10 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ github.com/google/pprof v0.0.0-20191218002539-d4f498aebedc/go.mod h1:ZgVRPoUq/hf
github.com/google/pprof v0.0.0-20200212024743-f11f1df84d12/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM=
github.com/google/pprof v0.0.0-20200229191704-1ebb73c60ed3/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM=
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y=
Expand Down Expand Up @@ -553,8 +555,8 @@ github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
github.com/lib/pq v1.8.0/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/lib/pq v1.9.0 h1:L8nSXQQzAYByakOFMTwpjRoHsMJklur4Gi59b6VivR8=
github.com/lib/pq v1.9.0/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/libgit2/git2go/v31 v31.3.0 h1:d8ciyYVKir+gKwra3KuNxTyVvbgGKn4admdt1PNNAOg=
github.com/libgit2/git2go/v31 v31.3.0/go.mod h1:mnc0hPGPs0nDi9INrurTpioeRzje9DvSXqON/+JEhwY=
github.com/libgit2/git2go/v31 v31.4.7 h1:P85qB5at5un4qPqUcvOZbAom7P0G4KAG/OLVyD29kQ0=
github.com/libgit2/git2go/v31 v31.4.7/go.mod h1:c/rkJcBcUFx6wHaT++UwNpKvIsmPNqCeQ/vzO4DrEec=
github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhnIaL+V+BEER86oLrvS+kWobKpbJuye0=
github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9B/r2mtpb6U+EI2rYA5OAXxsYw6wTamcNW+zcE=
github.com/lightstep/lightstep-tracer-common/golang/gogo v0.0.0-20190605223551-bc2310a04743/go.mod h1:qklhhLq1aX+mtWk9cPHPzaBjWImj5ULL6C7HFJtXQMM=
Expand Down Expand Up @@ -903,6 +905,8 @@ golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899/go.mod h1:LzIPMQfyMNhhGPh
golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0 h1:hb9wdF1z5waM+dSIICn1l0DkLVDT3hqhhQsDNUmHPRE=
golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20201203163018-be400aefbc4c h1:9HhBz5L/UjnK9XLtiZhYAdue5BVKep3PMmS2LuPDt8k=
golang.org/x/crypto v0.0.0-20201203163018-be400aefbc4c/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
Expand Down Expand Up @@ -1032,6 +1036,10 @@ golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201112073958-5cba982894dd h1:5CtCZbICpIOFdgO940moixOPjc0178IU44m4EjOO5IY=
golang.org/x/sys v0.0.0-20201112073958-5cba982894dd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201204225414-ed752295db88 h1:KmZPnMocC93w341XZp26yTJg8Za7lhb2KhkYmixoeso=
golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221 h1:/ZHdbVpdR/jk3g30/d4yUL0JU9kksj8+F/bnQUVLGDM=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
1 change: 1 addition & 0 deletions pkg/git/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
DefaultOrigin = "origin"
DefaultBranch = "master"
DefaultPublicKeyAuthUser = "git"
CAFile = "caFile"
hiddeco marked this conversation as resolved.
Show resolved Hide resolved
)

type Commit interface {
Expand Down
8 changes: 8 additions & 0 deletions pkg/git/v1/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ func AuthSecretStrategyForURL(URL string) (common.AuthSecretStrategy, error) {
type BasicAuth struct{}

func (s *BasicAuth) Method(secret corev1.Secret) (*common.Auth, error) {
if _, ok := secret.Data[common.CAFile]; ok {
return nil, fmt.Errorf("found caFile key in secret '%s' but go-git HTTP transport does not support custom certificates", secret.Name)
}

auth := &http.BasicAuth{}
if username, ok := secret.Data["username"]; ok {
auth.Username = string(username)
Expand All @@ -65,6 +69,10 @@ type PublicKeyAuth struct {
}

func (s *PublicKeyAuth) Method(secret corev1.Secret) (*common.Auth, error) {
if _, ok := secret.Data[common.CAFile]; ok {
return nil, fmt.Errorf("found caFile key in secret '%s' but go-git SSH transport does not support custom certificates", secret.Name)
}

identity := secret.Data["identity"]
knownHosts := secret.Data["known_hosts"]
if len(identity) == 0 || len(knownHosts) == 0 {
Expand Down
44 changes: 35 additions & 9 deletions pkg/git/v2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ import (
"bufio"
"bytes"
"crypto/sha1"
"crypto/x509"
"fmt"
"golang.org/x/crypto/ssh"
"net/url"
"strings"

"golang.org/x/crypto/ssh"

"github.com/fluxcd/source-controller/pkg/git/common"
git2go "github.com/libgit2/git2go/v31"
corev1 "k8s.io/api/core/v1"
Expand All @@ -49,6 +51,7 @@ func AuthSecretStrategyForURL(URL string) (common.AuthSecretStrategy, error) {
type BasicAuth struct{}

func (s *BasicAuth) Method(secret corev1.Secret) (*common.Auth, error) {
var credCallback git2go.CredentialsCallback
var username string
if d, ok := secret.Data["username"]; ok {
username = string(d)
Expand All @@ -57,26 +60,49 @@ func (s *BasicAuth) Method(secret corev1.Secret) (*common.Auth, error) {
if d, ok := secret.Data["password"]; ok {
password = string(d)
}
if username == "" || password == "" {
return nil, fmt.Errorf("invalid '%s' secret data: required fields 'username' and 'password'", secret.Name)
if username != "" && password != "" {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior change is needed as it should be possible to have cert validation on repositories with no authentication. This will change libgit2 behavior to not throw an error if username and password are missing if a secret is specified.

credCallback = func(url string, username_from_url string, allowed_types git2go.CredType) (*git2go.Cred, error) {
cred, err := git2go.NewCredUserpassPlaintext(username, password)
if err != nil {
return nil, err
}
return cred, nil
}
}

credCallback := func(url string, username_from_url string, allowed_types git2go.CredType) (*git2go.Cred, error) {
cred, err := git2go.NewCredUserpassPlaintext(username, password)
if err != nil {
return nil, err
var certCallback git2go.CertificateCheckCallback
if caFile, ok := secret.Data[common.CAFile]; ok {
certCallback = func(cert *git2go.Certificate, valid bool, hostname string) git2go.ErrorCode {
phillebaba marked this conversation as resolved.
Show resolved Hide resolved
roots := x509.NewCertPool()
ok := roots.AppendCertsFromPEM(caFile)
if !ok {
return git2go.ErrCertificate
}

opts := x509.VerifyOptions{
Roots: roots,
DNSName: hostname,
}
_, err := cert.X509.Verify(opts)
if err != nil {
return git2go.ErrCertificate
}
return git2go.ErrOk
}
return cred, nil
}

return &common.Auth{CredCallback: credCallback, CertCallback: nil}, nil
return &common.Auth{CredCallback: credCallback, CertCallback: certCallback}, nil
}

type PublicKeyAuth struct {
user string
}

func (s *PublicKeyAuth) Method(secret corev1.Secret) (*common.Auth, error) {
if _, ok := secret.Data[common.CAFile]; ok {
return nil, fmt.Errorf("found caFile key in secret '%s' but libgit2 SSH transport does not support custom certificates", secret.Name)
}

identity := secret.Data["identity"]
knownHosts := secret.Data["known_hosts"]
if len(identity) == 0 || len(knownHosts) == 0 {
Expand Down
10 changes: 2 additions & 8 deletions pkg/git/v2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,9 @@ func TestBasicAuthStrategy_Method(t *testing.T) {
name string
secret corev1.Secret
modify func(secret *corev1.Secret)
want *common.Auth
wantErr bool
}{
{"without username", basicAuthSecretFixture, func(s *corev1.Secret) { delete(s.Data, "username") }, nil, true},
{"without password", basicAuthSecretFixture, func(s *corev1.Secret) { delete(s.Data, "password") }, nil, true},
{"empty", corev1.Secret{}, nil, nil, true},
{"with username and password", basicAuthSecretFixture, nil, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -110,14 +107,11 @@ func TestBasicAuthStrategy_Method(t *testing.T) {
tt.modify(secret)
}
s := &BasicAuth{}
got, err := s.Method(*secret)
_, err := s.Method(*secret)
if (err != nil) != tt.wantErr {
t.Errorf("Method() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Method() got = %v, want %v", got, tt.want)
}
})
}
}
Expand Down