From e82c8e8b70fe8637589508a8e4cc59036a787327 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Mon, 12 Apr 2021 15:52:51 +0100 Subject: [PATCH 1/2] Use password in git method for secret Signed-off-by: Somtochi Onyekwere --- pkg/git/gogit/transport.go | 4 +++- pkg/git/gogit/transport_test.go | 25 +++++++++++++++++++++++++ pkg/git/libgit2/transport.go | 10 ++++++++-- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/pkg/git/gogit/transport.go b/pkg/git/gogit/transport.go index 2d51c0308..6e3bc1cf3 100644 --- a/pkg/git/gogit/transport.go +++ b/pkg/git/gogit/transport.go @@ -83,12 +83,14 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { return nil, fmt.Errorf("invalid '%s' secret data: required fields 'identity' and 'known_hosts'", secret.Name) } + password := secret.Data["password"] + user := s.user if user == "" { user = git.DefaultPublicKeyAuthUser } - pk, err := ssh.NewPublicKeys(user, identity, "") + pk, err := ssh.NewPublicKeys(user, identity, string(password)) if err != nil { return nil, err } diff --git a/pkg/git/gogit/transport_test.go b/pkg/git/gogit/transport_test.go index 675ce66cb..2213dbcef 100644 --- a/pkg/git/gogit/transport_test.go +++ b/pkg/git/gogit/transport_test.go @@ -43,6 +43,21 @@ v2MYnxRjc9INpi/Dyzz2MMvOnOW+aDuOh/If2AtVCmeJUx1pf4CFk3viQwJBAKyC t824+evjv+NQBlme3AOF6PgxtV4D4wWoJ5Uk/dTejER0j/Hbl6sqPxuiILRRV9qJ Ngkgu4mLjc3RfenEhJECQAx8zjWUE6kHHPGAd9DfiAIQ4bChqnyS0Nwb9+Gd4hSE P0Ah10mHiK/M0o3T8Eanwum0gbQHPnOwqZgsPkwXRqQ= +-----END RSA PRIVATE KEY-----` + + // secretKeyFixture is a randomly generated + // 512bit RSA private key with password foobar. + secretPassphraseFixture = `-----BEGIN RSA PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-256-CBC,0B016973B2A761D31E6B388D0F327C35 + +X9GET/qAyZkAJBl/RK+1XX75NxONgdUfZDw7PIYi/g+Efh3Z5zH5kh/dx9lxH5ZG +HGCqPAeMO/ofGDGtDULWW6iqDUFRu5gPgEVSCnnbqoHNU325WHhXdhejVAItwObC +IpL/zYfs2+gDHXct/n9FJ/9D/EGXZihwPqYaK8GQSfZAxz0QjLuh0wU1qpbm3y3N +q+o9FLv3b2Ys/tCJOUsYVQOYLSrZEI77y1ii3nWgQ8lXiTJbBUKzuq4f1YWeO8Ah +RZbdhTa57AF5lUaRtL7Nrm3HJUrK1alBbU7HHyjeW4Q4n/D3fiRDC1Mh2Bi4EOOn +wGctSx4kHsZGhJv5qwKqqPEFPhUzph8D2tm2TABk8HJa5KJFDbGrcfvk2uODAoZr +MbcpIxCfl8oB09bWfY6tDQjyvwSYYo2Phdwm7kT92xc= -----END RSA PRIVATE KEY-----` // knownHostsFixture is known_hosts fixture in the expected @@ -63,6 +78,13 @@ var ( "known_hosts": []byte(knownHostsFixture), }, } + privateKeySecretWithPassphraseFixture = corev1.Secret{ + Data: map[string][]byte{ + "identity": []byte(secretPassphraseFixture), + "known_hosts": []byte(knownHostsFixture), + "password": []byte("foobar"), + }, + } ) func TestAuthSecretStrategyForURL(t *testing.T) { @@ -131,10 +153,13 @@ func TestPublicKeyStrategy_Method(t *testing.T) { wantErr bool }{ {"private key and known_hosts", privateKeySecretFixture, nil, false}, + {"private key with passphrase and known_hosts", privateKeySecretWithPassphraseFixture, nil, false}, {"missing private key", privateKeySecretFixture, func(s *corev1.Secret) { delete(s.Data, "identity") }, true}, {"invalid private key", privateKeySecretFixture, func(s *corev1.Secret) { s.Data["identity"] = []byte(`-----BEGIN RSA PRIVATE KEY-----`) }, true}, {"missing known_hosts", privateKeySecretFixture, func(s *corev1.Secret) { delete(s.Data, "known_hosts") }, true}, {"invalid known_hosts", privateKeySecretFixture, func(s *corev1.Secret) { s.Data["known_hosts"] = []byte(`invalid`) }, true}, + {"missing password", privateKeySecretWithPassphraseFixture, func(s *corev1.Secret) { delete(s.Data, "password") }, true}, + {"wrong password", privateKeySecretWithPassphraseFixture, func(s *corev1.Secret) { s.Data["password"] = []byte("pass") }, true}, {"empty", corev1.Secret{}, nil, true}, } for _, tt := range tests { diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index f53273567..7b1a9847f 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -119,9 +119,15 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { return nil, err } + password := secret.Data["password"] // Need to validate private key as it is not // done by git2go when loading the key - _, err = ssh.ParsePrivateKey(identity) + if len(password) == 0 { + _, err = ssh.ParsePrivateKey(identity) + } else { + _, err = ssh.ParsePrivateKeyWithPassphrase(identity, password) + } + if err != nil { return nil, err } @@ -132,7 +138,7 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { } credCallback := func(url string, usernameFromURL string, allowedTypes git2go.CredType) (*git2go.Cred, error) { - cred, err := git2go.NewCredSshKeyFromMemory(user, "", string(identity), "") + cred, err := git2go.NewCredSshKeyFromMemory(user, "", string(identity), string(password)) if err != nil { return nil, err } From d3d1917e5e58adb8cfa1fb7e128fbe13a0d5e336 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Mon, 12 Apr 2021 16:31:42 +0100 Subject: [PATCH 2/2] Add tests for libgit2 Signed-off-by: Somtochi Onyekwere --- pkg/git/gogit/transport.go | 3 +-- pkg/git/libgit2/transport.go | 8 ++++---- pkg/git/libgit2/transport_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/pkg/git/gogit/transport.go b/pkg/git/gogit/transport.go index 6e3bc1cf3..f07e10f5b 100644 --- a/pkg/git/gogit/transport.go +++ b/pkg/git/gogit/transport.go @@ -83,13 +83,12 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { return nil, fmt.Errorf("invalid '%s' secret data: required fields 'identity' and 'known_hosts'", secret.Name) } - password := secret.Data["password"] - user := s.user if user == "" { user = git.DefaultPublicKeyAuthUser } + password := secret.Data["password"] pk, err := ssh.NewPublicKeys(user, identity, string(password)) if err != nil { return nil, err diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 7b1a9847f..da3d04e92 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -119,13 +119,13 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { return nil, err } - password := secret.Data["password"] // Need to validate private key as it is not // done by git2go when loading the key - if len(password) == 0 { - _, err = ssh.ParsePrivateKey(identity) - } else { + password, ok := secret.Data["password"] + if ok { _, err = ssh.ParsePrivateKeyWithPassphrase(identity, password) + } else { + _, err = ssh.ParsePrivateKey(identity) } if err != nil { diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index 2a1387c1d..733fa0c96 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -44,6 +44,21 @@ v2MYnxRjc9INpi/Dyzz2MMvOnOW+aDuOh/If2AtVCmeJUx1pf4CFk3viQwJBAKyC t824+evjv+NQBlme3AOF6PgxtV4D4wWoJ5Uk/dTejER0j/Hbl6sqPxuiILRRV9qJ Ngkgu4mLjc3RfenEhJECQAx8zjWUE6kHHPGAd9DfiAIQ4bChqnyS0Nwb9+Gd4hSE P0Ah10mHiK/M0o3T8Eanwum0gbQHPnOwqZgsPkwXRqQ= +-----END RSA PRIVATE KEY-----` + + // secretKeyFixture is a randomly generated + // 512bit RSA private key with password foobar. + secretPassphraseFixture = `-----BEGIN RSA PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-256-CBC,0B016973B2A761D31E6B388D0F327C35 + +X9GET/qAyZkAJBl/RK+1XX75NxONgdUfZDw7PIYi/g+Efh3Z5zH5kh/dx9lxH5ZG +HGCqPAeMO/ofGDGtDULWW6iqDUFRu5gPgEVSCnnbqoHNU325WHhXdhejVAItwObC +IpL/zYfs2+gDHXct/n9FJ/9D/EGXZihwPqYaK8GQSfZAxz0QjLuh0wU1qpbm3y3N +q+o9FLv3b2Ys/tCJOUsYVQOYLSrZEI77y1ii3nWgQ8lXiTJbBUKzuq4f1YWeO8Ah +RZbdhTa57AF5lUaRtL7Nrm3HJUrK1alBbU7HHyjeW4Q4n/D3fiRDC1Mh2Bi4EOOn +wGctSx4kHsZGhJv5qwKqqPEFPhUzph8D2tm2TABk8HJa5KJFDbGrcfvk2uODAoZr +MbcpIxCfl8oB09bWfY6tDQjyvwSYYo2Phdwm7kT92xc= -----END RSA PRIVATE KEY-----` // knownHostsFixture is known_hosts fixture in the expected @@ -64,6 +79,13 @@ var ( "known_hosts": []byte(knownHostsFixture), }, } + privateKeySecretWithPassphraseFixture = corev1.Secret{ + Data: map[string][]byte{ + "identity": []byte(secretPassphraseFixture), + "known_hosts": []byte(knownHostsFixture), + "password": []byte("foobar"), + }, + } ) func TestAuthSecretStrategyForURL(t *testing.T) { @@ -126,10 +148,13 @@ func TestPublicKeyStrategy_Method(t *testing.T) { wantErr bool }{ {"private key and known_hosts", privateKeySecretFixture, nil, false}, + {"private key with passphrase and known_hosts", privateKeySecretWithPassphraseFixture, nil, false}, {"missing private key", privateKeySecretFixture, func(s *corev1.Secret) { delete(s.Data, "identity") }, true}, {"invalid private key", privateKeySecretFixture, func(s *corev1.Secret) { s.Data["identity"] = []byte(`-----BEGIN RSA PRIVATE KEY-----`) }, true}, {"missing known_hosts", privateKeySecretFixture, func(s *corev1.Secret) { delete(s.Data, "known_hosts") }, true}, {"invalid known_hosts", privateKeySecretFixture, func(s *corev1.Secret) { s.Data["known_hosts"] = []byte(`invalid`) }, true}, + {"missing password", privateKeySecretWithPassphraseFixture, func(s *corev1.Secret) { delete(s.Data, "password") }, true}, + {"invalid password", privateKeySecretWithPassphraseFixture, func(s *corev1.Secret) { s.Data["password"] = []byte("foo") }, true}, {"empty", corev1.Secret{}, nil, true}, } for _, tt := range tests {