public_git
/
kaniko
mirror of https://github.com/GoogleContainerTools/kaniko
1
0
Fork 0
Code Issues Releases Wiki Activity

Add mTLS (client cert) registry authentication (#2180) 

* Add mTLS (client cert) support

Add support for Mutual TLS (mTLS) client certificate authentication.
The expected format of the new --registry-client-cert flag is the same
as the existing --registry-certificate flag, which will allow
different client certificates for different registries:

--registry-client-cert my.registry.url=/path/to/cert.crt,/path/to/key.key

* tidy: Rename mTLS (Client Cert) flag to be in line with others

This flag didn't describe that it was for the client certs uses with
the registry. Although this should be reasonably obvious, I like the
consistency with the other registry flag.

* test: Added unit tests for mTLS (Client Cert) loading

* test: Add 2 more tests for comma split formatting

since the comma splitting is a new portion of code let's make sure
that that format works well too in other cases

* tidy: Fix formatting of flag help text

* tidy: Made invalid cert format error consistent

I was running the tests and saw the message:

Failed to load client certificate/key '/path/to/client/certificate.cert' for my.registry.name, format is my.registry.name=/path/to/cert,/path/to/key

I then realized that it'd be a lot nicer if this showed the user what
they input, and how they should change it (rather than decomposing it:

Failed to load client certificate/key 'my.registry.name=/path/to/client/certificate.cert', expected format: my.registry.name=/path/to/cert,/path/to/key

* test: Fixed incorrect test argument

This didn't fail the test before because it's only attempting to show
that certs only get loaded and used for their associated registry but
it's important to keep this correct.

This case is covered by the test below, "RegistriesClientCertificates
incorrect cert format"

* doc: Add new flag to README.md

* mod: Fail to push if there was a problem loading client certs

Rather than warning that there was an issue, we should fail if the
requested client certificates were not found or failed to load.

This feels a lot better than waiting for the build to finish then
failing later.

* mod: Return an error if the certificate authority fails to load, just like client certs

The MakeTransport function was changed in the previous commit to
allow returning errors if there was a problem loading certificates,
rather than just print warnings.

This feels a lot better as you get the error immediately that there's
a problem to fix, rather than getting a warning, then later an error
that the server's certificate could not be verified.

* tidy: fix golint issues
This commit is contained in:
Eric 2023-05-18 22:20:51 +01:00 committed by GitHub
parent 73c50bfff4
commit 7525828ef9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 154 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
README.md
View File

@ -955,6 +955,15 @@ registry.
Expected format is `my.registry.url=/path/to/the/certificate.cert`
#### Flag `--registry-client-cert`
Set this flag to provide a certificate/key pair for mutual TLS (mTLS)
communication with a given
[registry that requires mTLS](https://docs.docker.com/engine/security/certificates/)
for authentication.
Expected format is `my.registry.url=/path/to/client/cert.crt,/path/to/client/key.key`
#### Flag `--registry-mirror`
Set this flag if you want to use a registry mirror instead of the default

2
cmd/executor/cmd/root.go
View File

@ -235,6 +235,8 @@ func addKanikoOptionsFlags() {
RootCmd.PersistentFlags().VarP(&opts.SkipTLSVerifyRegistries, "skip-tls-verify-registry", "", "Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.")
opts.RegistriesCertificates = make(map[string]string)
RootCmd.PersistentFlags().VarP(&opts.RegistriesCertificates, "registry-certificate", "", "Use the provided certificate for TLS communication with the given registry. Expected format is 'my.registry.url=/path/to/the/server/certificate'.")
opts.RegistriesClientCertificates = make(map[string]string)
RootCmd.PersistentFlags().VarP(&opts.RegistriesClientCertificates, "registry-client-cert", "", "Use the provided client certificate for mutual TLS (mTLS) communication with the given registry. Expected format is 'my.registry.url=/path/to/client/cert,/path/to/client/key'.")
RootCmd.PersistentFlags().VarP(&opts.RegistryMirrors, "registry-mirror", "", "Registry mirror to use as pull-through cache instead of docker.io. Set it repeatedly for multiple mirrors.")
RootCmd.PersistentFlags().BoolVarP(&opts.IgnoreVarRun, "ignore-var-run", "", true, "Ignore /var/run directory when taking image snapshot. Set it to false to preserve /var/run/ in destination image.")
RootCmd.PersistentFlags().VarP(&opts.Labels, "label", "", "Set metadata for an image. Set it repeatedly for multiple labels.")

2
cmd/warmer/cmd/root.go
View File

@ -85,6 +85,8 @@ func addKanikoOptionsFlags() {
RootCmd.PersistentFlags().VarP(&opts.SkipTLSVerifyRegistries, "skip-tls-verify-registry", "", "Insecure registry ignoring TLS verify to pull. Set it repeatedly for multiple registries.")
opts.RegistriesCertificates = make(map[string]string)
RootCmd.PersistentFlags().VarP(&opts.RegistriesCertificates, "registry-certificate", "", "Use the provided certificate for TLS communication with the given registry. Expected format is 'my.registry.url=/path/to/the/server/certificate'.")
opts.RegistriesClientCertificates = make(map[string]string)
RootCmd.PersistentFlags().VarP(&opts.RegistriesClientCertificates, "registry-client-cert", "", "Use the provided client certificate for mutual TLS (mTLS) communication with the given registry. Expected format is 'my.registry.url=/path/to/client/cert,/path/to/client/key'.")
RootCmd.PersistentFlags().VarP(&opts.RegistryMirrors, "registry-mirror", "", "Registry mirror to use as pull-through cache instead of docker.io. Set it repeatedly for multiple mirrors.")
RootCmd.PersistentFlags().StringVarP(&opts.CustomPlatform, "customPlatform", "", "", "Specify the build platform if different from the current host")

5
pkg/cache/cache.go vendored
View File

@ -68,7 +68,10 @@ func (rc *RegistryCache) RetrieveLayer(ck string) (v1.Image, error) {
cacheRef.Repository.Registry = newReg
}
tr := util.MakeTransport(rc.Opts.RegistryOptions, registryName)
tr, err := util.MakeTransport(rc.Opts.RegistryOptions, registryName)
if err != nil {
return nil, errors.Wrapf(err, "making transport for registry %q", registryName)
}
img, err := remote.Image(cacheRef, remote.WithTransport(tr), remote.WithAuthFromKeychain(creds.GetKeychain()))
if err != nil {

19
pkg/config/options.go
View File

@ -32,15 +32,16 @@ type CacheOptions struct {
// RegistryOptions are all the options related to the registries, set by command line arguments.
type RegistryOptions struct {
RegistryMirrors multiArg
InsecureRegistries multiArg
SkipTLSVerifyRegistries multiArg
RegistriesCertificates keyValueArg
Insecure bool
SkipTLSVerify bool
InsecurePull bool
SkipTLSVerifyPull bool
PushRetry int
RegistryMirrors multiArg
InsecureRegistries multiArg
SkipTLSVerifyRegistries multiArg
RegistriesCertificates keyValueArg
RegistriesClientCertificates keyValueArg
Insecure bool
SkipTLSVerify bool
InsecurePull bool
SkipTLSVerifyPull bool
PushRetry int
}
// KanikoOptions are options that are set by command line arguments

12
pkg/executor/push.go
View File

@ -110,7 +110,11 @@ func CheckPushPermissions(opts *config.KanikoOptions) error {
}
destRef.Repository.Registry = newReg
}
tr := newRetry(util.MakeTransport(opts.RegistryOptions, registryName))
rt, err := util.MakeTransport(opts.RegistryOptions, registryName)
if err != nil {
return errors.Wrapf(err, "making transport for registry %q", registryName)
}
tr := newRetry(rt)
if err := checkRemotePushPermission(destRef, creds.GetKeychain(), tr); err != nil {
return errors.Wrapf(err, "checking push permission for %q", destRef)
}
@ -238,7 +242,11 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error {
return errors.Wrap(err, "resolving pushAuth")
}
tr := newRetry(util.MakeTransport(opts.RegistryOptions, registryName))
localRt, err := util.MakeTransport(opts.RegistryOptions, registryName)
if err != nil {
return errors.Wrapf(err, "making transport for registry %q", registryName)
}
tr := newRetry(localRt)
rt := &withUserAgent{t: tr}
logrus.Infof("Pushing image to %s", destRef.String())

8
pkg/image/remote/remote.go
View File

@ -126,7 +126,13 @@ func setNewRegistry(ref name.Reference, newReg name.Registry) name.Reference {
}
func remoteOptions(registryName string, opts config.RegistryOptions, customPlatform string) []remote.Option {
tr := util.MakeTransport(opts, registryName)
tr, err := util.MakeTransport(opts, registryName)
// The MakeTransport function will only return errors if there was a problem
// with registry certificates (Verification or mTLS)
if err != nil {
logrus.Fatalf("Unable to setup transport for registry %q: %v", customPlatform, err)
}
// The platform value has previously been validated.
platform, err := v1.ParsePlatform(customPlatform)

43
pkg/util/transport_util.go
View File

@ -19,6 +19,8 @@ package util
import (
"crypto/tls"
"crypto/x509"
"fmt"
"strings"
"io/ioutil"
"net/http"
@ -51,6 +53,19 @@ func (p *X509CertPool) append(path string) error {
var systemCertLoader CertPool
type KeyPairLoader interface {
load(string, string) (tls.Certificate, error)
}
type X509KeyPairLoader struct {
}
func (p *X509KeyPairLoader) load(certFile, keyFile string) (tls.Certificate, error) {
return tls.LoadX509KeyPair(certFile, keyFile)
}
var systemKeyPairLoader KeyPairLoader
func init() {
systemCertPool, err := x509.SystemCertPool()
if err != nil {
@ -60,9 +75,11 @@ func init() {
systemCertLoader = &X509CertPool{
inner: *systemCertPool,
}
systemKeyPairLoader = &X509KeyPairLoader{}
}
func MakeTransport(opts config.RegistryOptions, registryName string) http.RoundTripper {
func MakeTransport(opts config.RegistryOptions, registryName string) (http.RoundTripper, error) {
// Create a transport to set our user-agent.
var tr http.RoundTripper = http.DefaultTransport.(*http.Transport).Clone()
if opts.SkipTLSVerify || opts.SkipTLSVerifyRegistries.Contains(registryName) {
@ -71,12 +88,24 @@ func MakeTransport(opts config.RegistryOptions, registryName string) http.RoundT
}
} else if certificatePath := opts.RegistriesCertificates[registryName]; certificatePath != "" {
if err := systemCertLoader.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: systemCertLoader.value(),
}
return nil, fmt.Errorf("failed to load certificate %s for %s: %w", certificatePath, registryName, err)
}
tr.(*http.Transport).TLSClientConfig = &tls.Config{
RootCAs: systemCertLoader.value(),
}
}
return tr
if clientCertificatePath := opts.RegistriesClientCertificates[registryName]; clientCertificatePath != "" {
certFiles := strings.Split(clientCertificatePath, ",")
if len(certFiles) != 2 {
return nil, fmt.Errorf("failed to load client certificate/key '%s=%s', expected format: %s=/path/to/cert,/path/to/key", registryName, clientCertificatePath, registryName)
}
cert, err := systemKeyPairLoader.load(certFiles[0], certFiles[1])
if err != nil {
return nil, fmt.Errorf("failed to load client certificate/key '%s' for %s: %w", clientCertificatePath, registryName, err)
}
tr.(*http.Transport).TLSClientConfig.Certificates = []tls.Certificate{cert}
}
return tr, nil
}

83
pkg/util/transport_util_test.go
View File

@ -39,18 +39,26 @@ func (m *mockedCertPool) append(path string) error {
return nil
}
type mockedKeyPairLoader struct {
}
func (p *mockedKeyPairLoader) load(certFile, keyFile string) (tls.Certificate, error) {
foo := tls.Certificate{}
return foo, nil
}
func Test_makeTransport(t *testing.T) {
registryName := "my.registry.name"
tests := []struct {
name string
opts config.RegistryOptions
check func(*tls.Config, *mockedCertPool)
check func(*tls.Config, *mockedCertPool, error)
}{
{
name: "SkipTLSVerify set",
opts: config.RegistryOptions{SkipTLSVerify: true},
check: func(config *tls.Config, pool *mockedCertPool) {
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if !config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify not set while SkipTLSVerify set")
}
@ -59,7 +67,7 @@ func Test_makeTransport(t *testing.T) {
{
name: "SkipTLSVerifyRegistries set with expected registry",
opts: config.RegistryOptions{SkipTLSVerifyRegistries: []string{registryName}},
check: func(config *tls.Config, pool *mockedCertPool) {
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if !config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify not set while SkipTLSVerifyRegistries set with registry name")
}
@ -68,7 +76,7 @@ func Test_makeTransport(t *testing.T) {
{
name: "SkipTLSVerifyRegistries set with other registry",
opts: config.RegistryOptions{SkipTLSVerifyRegistries: []string{fmt.Sprintf("other.%s", registryName)}},
check: func(config *tls.Config, pool *mockedCertPool) {
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify set while SkipTLSVerifyRegistries not set with registry name")
}
@ -77,7 +85,7 @@ func Test_makeTransport(t *testing.T) {
{
name: "RegistriesCertificates set for registry",
opts: config.RegistryOptions{RegistriesCertificates: map[string]string{registryName: "/path/to/the/certificate.cert"}},
check: func(config *tls.Config, pool *mockedCertPool) {
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if len(pool.certificatesPath) != 1 || pool.certificatesPath[0] != "/path/to/the/certificate.cert" {
t.Errorf("makeTransport().RegistriesCertificates certificate not appended to system certificates")
}
@ -86,24 +94,81 @@ func Test_makeTransport(t *testing.T) {
{
name: "RegistriesCertificates set for another registry",
opts: config.RegistryOptions{RegistriesCertificates: map[string]string{fmt.Sprintf("other.%s=", registryName): "/path/to/the/certificate.cert"}},
check: func(config *tls.Config, pool *mockedCertPool) {
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if len(pool.certificatesPath) != 0 {
t.Errorf("makeTransport().RegistriesCertificates certificate appended to system certificates while added for other registry")
}
},
},
{
name: "RegistriesClientCertificates set for registry",
opts: config.RegistryOptions{RegistriesClientCertificates: map[string]string{registryName: "/path/to/client/certificate.cert,/path/to/client/key.key"}},
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if len(config.Certificates) != 1 {
t.Errorf("makeTransport().RegistriesClientCertificates not loaded for desired registry")
}
},
},
{
name: "RegistriesClientCertificates set for another registry",
opts: config.RegistryOptions{RegistriesClientCertificates: map[string]string{fmt.Sprintf("other.%s", registryName): "/path/to/client/certificate.cert,/path/to/key.key,/path/to/extra.crt"}},
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if len(config.Certificates) != 0 {
t.Errorf("makeTransport().RegistriesClientCertificates certificate loaded for other registry")
}
},
},
{
name: "RegistriesClientCertificates incorrect cert format",
opts: config.RegistryOptions{RegistriesClientCertificates: map[string]string{registryName: "/path/to/client/certificate.cert"}},
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if config != nil {
t.Errorf("makeTransport().RegistriesClientCertificates was incorrectly loaded without both client/key (config was not nil)")
}
expectedError := "failed to load client certificate/key 'my.registry.name=/path/to/client/certificate.cert', expected format: my.registry.name=/path/to/cert,/path/to/key"
if err == nil {
t.Errorf("makeTransport().RegistriesClientCertificates was incorrectly loaded without both client/key (expected error, got nil)")
} else if err.Error() != expectedError {
t.Errorf("makeTransport().RegistriesClientCertificates was incorrectly loaded without both client/key (expected: %s, got: %s)", expectedError, err.Error())
}
},
},
{
name: "RegistriesClientCertificates incorrect cert format extra",
opts: config.RegistryOptions{RegistriesClientCertificates: map[string]string{registryName: "/path/to/client/certificate.cert,/path/to/key.key,/path/to/extra.crt"}},
check: func(config *tls.Config, pool *mockedCertPool, err error) {
if config != nil {
t.Errorf("makeTransport().RegistriesClientCertificates was incorrectly loaded with extra paths in comma split (config was not nil)")
}
expectedError := "failed to load client certificate/key 'my.registry.name=/path/to/client/certificate.cert,/path/to/key.key,/path/to/extra.crt', expected format: my.registry.name=/path/to/cert,/path/to/key"
if err == nil {
t.Errorf("makeTransport().RegistriesClientCertificates was incorrectly loaded loaded with extra paths in comma split (expected error, got nil)")
} else if err.Error() != expectedError {
t.Errorf("makeTransport().RegistriesClientCertificates was incorrectly loaded loaded with extra paths in comma split (expected: %s, got: %s)", expectedError, err.Error())
}
},
},
}
savedSystemCertLoader := systemCertLoader
defer func() { systemCertLoader = savedSystemCertLoader }()
savedSystemKeyPairLoader := systemKeyPairLoader
defer func() {
systemCertLoader = savedSystemCertLoader
systemKeyPairLoader = savedSystemKeyPairLoader
}()
for _, tt := range tests {
var certificatesPath []string
certPool := &mockedCertPool{
certificatesPath: certificatesPath,
}
systemCertLoader = certPool
systemKeyPairLoader = &mockedKeyPairLoader{}
t.Run(tt.name, func(t *testing.T) {
tr := MakeTransport(tt.opts, registryName)
tt.check(tr.(*http.Transport).TLSClientConfig, certPool)
tr, err := MakeTransport(tt.opts, registryName)
var tlsConfig *tls.Config
if err == nil {
tlsConfig = tr.(*http.Transport).TLSClientConfig
}
tt.check(tlsConfig, certPool, err)
})
}