public_git/kaniko
1
0
Fork 0
mirror of https://github.com/GoogleContainerTools/kaniko synced 2026-10-09 11:56:55 +02:00
Code Issues Releases Wiki Activity

Remove use of DefaultTransport

Browse Source 
Using DefaultTransport and manipulating its tls configuration may lead to unexpected behaviour
This commit is contained in:
Ben Einaudi
2020-05-20 11:43:13 +02:00
parent 31feaa4c12
commit 2f6090dcd7
10 changed files with 227 additions and 247 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/executor/build.go
+4 -3
View File
@@ -44,6 +44,7 @@ import (
"github.com/GoogleContainerTools/kaniko/pkg/config"
"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
image_util "github.com/GoogleContainerTools/kaniko/pkg/image"
"github.com/GoogleContainerTools/kaniko/pkg/snapshot"
"github.com/GoogleContainerTools/kaniko/pkg/timing"
"github.com/GoogleContainerTools/kaniko/pkg/util"
@@ -84,7 +85,7 @@ type stageBuilder struct {
// newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage
func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, crossStageDeps map[int][]string, dcm map[string]string, sid map[string]string, stageNameToIdx map[string]string) (*stageBuilder, error) {
sourceImage, err := util.RetrieveSourceImage(stage, opts)
sourceImage, err := image_util.RetrieveSourceImage(stage, opts)
if err != nil {
return nil, err
}
@@ -517,7 +518,7 @@ func CalculateDependencies(stages []config.KanikoStage, opts *config.KanikoOptio
} else if s.Name == constants.NoBaseImage {
image = empty.Image
} else {
image, err = util.RetrieveSourceImage(s, opts)
image, err = image_util.RetrieveSourceImage(s, opts)
if err != nil {
return nil, err
}
@@ -751,7 +752,7 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e
// This must be an image name, fetch it.
logrus.Debugf("Found extra base image stage %s", c.From)
sourceImage, err := util.RetrieveRemoteImage(c.From, opts)
sourceImage, err := image_util.RetrieveRemoteImage(c.From, opts)
if err != nil {
return err
}
pkg/executor/push.go
+7 -63
View File
@@ -17,8 +17,6 @@ limitations under the License.
package executor
import (
"crypto/tls"
"crypto/x509"
"encoding/json"
"fmt"
"io/ioutil"
@@ -34,6 +32,7 @@ import (
"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/creds"
"github.com/GoogleContainerTools/kaniko/pkg/timing"
"github.com/GoogleContainerTools/kaniko/pkg/util"
"github.com/GoogleContainerTools/kaniko/pkg/version"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
@@ -48,15 +47,15 @@ import (
"github.com/spf13/afero"
)
type withUserAgent struct {
t http.RoundTripper
}
// for testing
var (
newRetry = transport.NewRetry
)
type withUserAgent struct {
t http.RoundTripper
}
const (
UpstreamClientUaKey = "UPSTREAM_CLIENT_TYPE"
)
@@ -82,41 +81,6 @@ func (w *withUserAgent) RoundTrip(r *http.Request) (*http.Response, error) {
return w.t.RoundTrip(r)
}
type CertPool interface {
value() *x509.CertPool
append(path string) error
}
type X509CertPool struct {
inner x509.CertPool
}
func (p *X509CertPool) value() *x509.CertPool {
return &p.inner
}
func (p *X509CertPool) append(path string) error {
pem, err := ioutil.ReadFile(path)
if err != nil {
return err
}
p.inner.AppendCertsFromPEM(pem)
return nil
}
type systemCertLoader func() CertPool
var defaultX509Handler systemCertLoader = func() CertPool {
systemCertPool, err := x509.SystemCertPool()
if err != nil {
logrus.Warn("Failed to load system cert pool. Loading empty one instead.")
systemCertPool = x509.NewCertPool()
}
return &X509CertPool{
inner: *systemCertPool,
}
}
// for testing
var (
fs = afero.NewOsFs()
@@ -161,7 +125,7 @@ func CheckPushPermissions(opts *config.KanikoOptions) error {
}
destRef.Repository.Registry = newReg
}
tr := makeTransport(opts, registryName, defaultX509Handler)
tr := newRetry(util.MakeTransport(opts, registryName))
if err := checkRemotePushPermission(destRef, creds.GetKeychain(), tr); err != nil {
return errors.Wrapf(err, "checking push permission for %q", destRef)
}
@@ -258,7 +222,7 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error {
return errors.Wrap(err, "resolving pushAuth")
}
tr := makeTransport(opts, registryName, defaultX509Handler)
tr := newRetry(util.MakeTransport(opts, registryName))
rt := &withUserAgent{t: tr}
if err := remote.Write(destRef, image, remote.WithAuth(pushAuth), remote.WithTransport(rt)); err != nil {
@@ -300,26 +264,6 @@ func writeImageOutputs(image v1.Image, destRefs []name.Tag) error {
return nil
}
func makeTransport(opts *config.KanikoOptions, registryName string, loader systemCertLoader) http.RoundTripper {
// Create a transport to set our user-agent.
var tr http.RoundTripper = http.DefaultTransport.(*http.Transport).Clone()
if opts.SkipTLSVerify || opts.SkipTLSVerifyRegistries.Contains(registryName) {
tr.(*http.Transport).TLSClientConfig = &tls.Config{
InsecureSkipVerify: true,
}
} else if certificatePath := opts.RegistriesCertificates[registryName]; certificatePath != "" {
systemCertPool := loader()
if err := systemCertPool.append(certificatePath); err != nil {
logrus.WithError(err).Warnf("Failed to load certificate %s for %s\n", certificatePath, registryName)
} else {
tr.(*http.Transport).TLSClientConfig = &tls.Config{
RootCAs: systemCertPool.value(),
}
}
}
return newRetry(tr)
}
// pushLayerToCache pushes layer (tagged with cacheKey) to opts.Cache
// if opts.Cache doesn't exist, infer the cache from the given destination
func pushLayerToCache(opts *config.KanikoOptions, cacheKey string, tarPath string, createdBy string) error {
pkg/executor/push_test.go
-100
View File
@@ -18,8 +18,6 @@ package executor
import (
"bytes"
"crypto/tls"
"crypto/x509"
"fmt"
"io/ioutil"
"net/http"
@@ -34,7 +32,6 @@ import (
"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/v1/layout"
"github.com/google/go-containerregistry/pkg/v1/random"
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
"github.com/google/go-containerregistry/pkg/v1/validate"
"github.com/spf13/afero"
@@ -272,103 +269,6 @@ func TestImageNameDigestFile(t *testing.T) {
}
type mockedCertPool struct {
certificatesPath []string
}
func (m *mockedCertPool) value() *x509.CertPool {
return &x509.CertPool{}
}
func (m *mockedCertPool) append(path string) error {
m.certificatesPath = append(m.certificatesPath, path)
return nil
}
type retryTransport struct {
inner http.RoundTripper
}
func (t *retryTransport) RoundTrip(in *http.Request) (out *http.Response, err error) {
return nil, nil
}
func Test_makeTransport(t *testing.T) {
registryName := "my.registry.name"
tests := []struct {
name string
opts *config.KanikoOptions
check func(*tls.Config, *mockedCertPool)
}{
{
name: "SkipTLSVerify set",
opts: &config.KanikoOptions{SkipTLSVerify: true},
check: func(config *tls.Config, pool *mockedCertPool) {
if !config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify not set while SkipTLSVerify set")
}
},
},
{
name: "SkipTLSVerifyRegistries set with expected registry",
opts: &config.KanikoOptions{SkipTLSVerifyRegistries: []string{registryName}},
check: func(config *tls.Config, pool *mockedCertPool) {
if !config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify not set while SkipTLSVerifyRegistries set with registry name")
}
},
},
{
name: "SkipTLSVerifyRegistries set with other registry",
opts: &config.KanikoOptions{SkipTLSVerifyRegistries: []string{fmt.Sprintf("other.%s", registryName)}},
check: func(config *tls.Config, pool *mockedCertPool) {
if config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify set while SkipTLSVerifyRegistries not set with registry name")
}
},
},
{
name: "RegistriesCertificates set for registry",
opts: &config.KanikoOptions{RegistriesCertificates: map[string]string{registryName: "/path/to/the/certificate.cert"}},
check: func(config *tls.Config, pool *mockedCertPool) {
if len(pool.certificatesPath) != 1 || pool.certificatesPath[0] != "/path/to/the/certificate.cert" {
t.Errorf("makeTransport().RegistriesCertificates certificate not appended to system certificates")
}
},
},
{
name: "RegistriesCertificates set for another registry",
opts: &config.KanikoOptions{RegistriesCertificates: map[string]string{fmt.Sprintf("other.%s=", registryName): "/path/to/the/certificate.cert"}},
check: func(config *tls.Config, pool *mockedCertPool) {
if len(pool.certificatesPath) != 0 {
t.Errorf("makeTransport().RegistriesCertificates certificate appended to system certificates while added for other registry")
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
originalRetry := newRetry
newRetry = func(inner http.RoundTripper, _ ...transport.Option) http.RoundTripper {
return &retryTransport{
inner: inner,
}
}
defer func() { newRetry = originalRetry }()
var certificatesPath []string
certPool := mockedCertPool{
certificatesPath: certificatesPath,
}
var mockedSystemCertLoader systemCertLoader = func() CertPool {
return &certPool
}
actual := makeTransport(tt.opts, registryName, mockedSystemCertLoader)
tt.check(actual.(*retryTransport).inner.(*http.Transport).TLSClientConfig, &certPool)
})
}
}
var calledExecCommand = false
var calledCheckPushPermission = false