From 42a84252bbcfd2e2e096078032dc4b0c8a03393b Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Thu, 2 Jun 2022 22:24:32 +0000 Subject: [PATCH] Add a flag to enable external data client auth Signed-off-by: GitHub --- constraint/pkg/client/drivers/local/args.go | 8 ++++++ .../pkg/client/drivers/local/builtin.go | 15 ++-------- constraint/pkg/client/drivers/local/driver.go | 28 +++++++++++++++++++ .../client/drivers/local/driver_unit_test.go | 5 +++- constraint/pkg/externaldata/request.go | 10 +++++-- 5 files changed, 49 insertions(+), 17 deletions(-) diff --git a/constraint/pkg/client/drivers/local/args.go b/constraint/pkg/client/drivers/local/args.go index 54da81aa2..072687ba1 100644 --- a/constraint/pkg/client/drivers/local/args.go +++ b/constraint/pkg/client/drivers/local/args.go @@ -129,6 +129,14 @@ func AddClientTLSKeyPair(certFile, keyFile string) Arg { } } +func EnableExternalDataClientAuth() Arg { + return func(d *Driver) error { + d.enableExternalDataClientAuth = true + + return nil + } +} + // Externs sets the fields under `data` that Rego in ConstraintTemplates // can access. If unset, all fields can be accessed. Only fields recognized by // the system can be enabled. diff --git a/constraint/pkg/client/drivers/local/builtin.go b/constraint/pkg/client/drivers/local/builtin.go index e40dfa949..ee27249f3 100644 --- a/constraint/pkg/client/drivers/local/builtin.go +++ b/constraint/pkg/client/drivers/local/builtin.go @@ -1,7 +1,6 @@ package local import ( - "crypto/tls" "net/http" "github.com/open-policy-agent/frameworks/constraint/pkg/externaldata" @@ -21,22 +20,12 @@ func externalDataBuiltin(d *Driver) func(bctx rego.BuiltinContext, regorequest * return externaldata.HandleError(http.StatusBadRequest, err) } - certPEM, err := d.readFile(d.clientCertFile) + clientCert, err := d.getTLSCertificate() if err != nil { return externaldata.HandleError(http.StatusBadRequest, err) } - keyPEM, err := d.readFile(d.clientKeyFile) - if err != nil { - return externaldata.HandleError(http.StatusBadRequest, err) - } - - clientCert, err := tls.X509KeyPair(certPEM, keyPEM) - if err != nil { - return externaldata.HandleError(http.StatusBadRequest, err) - } - - externaldataResponse, statusCode, err := d.sendRequestToProvider(bctx.Context, &provider, regoReq.Keys, &clientCert) + externaldataResponse, statusCode, err := d.sendRequestToProvider(bctx.Context, &provider, regoReq.Keys, clientCert) if err != nil { return externaldata.HandleError(statusCode, err) } diff --git a/constraint/pkg/client/drivers/local/driver.go b/constraint/pkg/client/drivers/local/driver.go index 13b8a67a1..0b215de67 100644 --- a/constraint/pkg/client/drivers/local/driver.go +++ b/constraint/pkg/client/drivers/local/driver.go @@ -3,6 +3,7 @@ package local import ( "bytes" "context" + "crypto/tls" "encoding/json" "fmt" "io/fs" @@ -64,6 +65,10 @@ type Driver struct { // sendRequestToProvider allows Rego to send requests to the provider specified in external_data. sendRequestToProvider externaldata.SendRequestToProvider + // enableExternalDataClientAuth enables the injection of a TLS certificate into an HTTP client + // that is used to communicate with providers. + enableExternalDataClientAuth bool + // fs is the filesystem to use for reading files. fs fs.FS @@ -343,6 +348,29 @@ func (d *Driver) readFile(name string) ([]byte, error) { return ioutil.ReadAll(file) } +func (d *Driver) getTLSCertificate() (*tls.Certificate, error) { + if !d.enableExternalDataClientAuth { + return nil, nil + } + + certPEM, err := d.readFile(d.clientCertFile) + if err != nil { + return nil, err + } + + keyPEM, err := d.readFile(d.clientKeyFile) + if err != nil { + return nil, err + } + + clientCert, err := tls.X509KeyPair(certPEM, keyPEM) + if err != nil { + return nil, err + } + + return &clientCert, nil +} + // rewriteModulePackage rewrites the module's package path to path. func rewriteModulePackage(module *ast.Module) error { pathParts := ast.Ref([]*ast.Term{ast.VarTerm(templatePath)}) diff --git a/constraint/pkg/client/drivers/local/driver_unit_test.go b/constraint/pkg/client/drivers/local/driver_unit_test.go index 371c17afa..b46335ff2 100644 --- a/constraint/pkg/client/drivers/local/driver_unit_test.go +++ b/constraint/pkg/client/drivers/local/driver_unit_test.go @@ -308,7 +308,10 @@ func TestDriver_ExternalData(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - d, err := New(AddExternalDataProviderCache(externaldata.NewCache())) + d, err := New( + AddExternalDataProviderCache(externaldata.NewCache()), + EnableExternalDataClientAuth(), + ) if err != nil { t.Fatal(err) } diff --git a/constraint/pkg/externaldata/request.go b/constraint/pkg/externaldata/request.go index 491b6a0fb..50e9359be 100644 --- a/constraint/pkg/externaldata/request.go +++ b/constraint/pkg/externaldata/request.go @@ -110,14 +110,18 @@ func getClient(provider *v1alpha1.Provider, clientCert *tls.Certificate) (*http. client := &http.Client{ Timeout: time.Duration(provider.Spec.Timeout) * time.Second, } + tlsConfig := &tls.Config{ - // present our client cert to the server - // in case provider wants to verify it - Certificates: []tls.Certificate{*clientCert}, //nolint:gosec InsecureSkipVerify: provider.Spec.InsecureTLSSkipVerify, } + // present our client cert to the server + // in case provider wants to verify it + if clientCert != nil { + tlsConfig.Certificates = []tls.Certificate{*clientCert} + } + if u.Scheme == HTTPSScheme && !provider.Spec.InsecureTLSSkipVerify { // if the provider presents its own CA bundle, // we will use it to verify the server's certificate