Add support to TLSSetting to not only read from file path, but from m…

…emory (#7676) * Add in memory support to TLSSetting Signed-off-by: erikbaranowski <[email protected]> * Add changelog Signed-off-by: erikbaranowski <[email protected]> * Add documentation Signed-off-by: erikbaranowski <[email protected]> * Update some message verbiage Signed-off-by: erikbaranowski <[email protected]> * Fix up test error messages Signed-off-by: erikbaranowski <[email protected]> * swap PEM properties to confiqopaque.String instead of []byte Signed-off-by: erikbaranowski <[email protected]> * make linter happy Signed-off-by: erikbaranowski <[email protected]> * Fix unit test for windows Signed-off-by: Erik Baranowski <[email protected]> --------- Signed-off-by: erikbaranowski <[email protected]> Signed-off-by: Erik Baranowski <[email protected]>
open-telemetry · Jun 7, 2023 · 07262f2 · 07262f2
1 parent db99c42
commit 07262f2
Show file tree

Hide file tree

Showing 7 changed files with 264 additions and 54 deletions.
diff --git a/.chloggen/add-tls-settings-from-memory.yaml b/.chloggen/add-tls-settings-from-memory.yaml
@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: configtls
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Allow TLS Settings to be provided in memory in addition to filepath.
+
+# One or more tracking issues or pull requests related to the change
+issues: [7313]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go
@@ -228,7 +228,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
 		host     component.Host
 	}{
 		{
-			err: "^failed to load TLS config: failed to load CA CertPool: failed to load CA /doesnt/exist:",
+			err: "^failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:",
 			settings: GRPCClientSettings{
 				Headers:     nil,
 				Endpoint:    "",
@@ -244,7 +244,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
 			},
 		},
 		{
-			err: "^failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither",
+			err: "^failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither",
 			settings: GRPCClientSettings{
 				Headers:     nil,
 				Endpoint:    "",
@@ -414,7 +414,7 @@ func TestGRPCServerSettingsError(t *testing.T) {
 		err      string
 	}{
 		{
-			err: "^failed to load TLS config: failed to load CA CertPool: failed to load CA /doesnt/exist:",
+			err: "^failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:",
 			settings: GRPCServerSettings{
 				NetAddr: confignet.NetAddr{
 					Endpoint:  "127.0.0.1:1234",
@@ -428,7 +428,7 @@ func TestGRPCServerSettingsError(t *testing.T) {
 			},
 		},
 		{
-			err: "^failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither",
+			err: "^failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither",
 			settings: GRPCServerSettings{
 				NetAddr: confignet.NetAddr{
 					Endpoint:  "127.0.0.1:1234",

diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go
@@ -212,7 +212,7 @@ func TestHTTPClientSettingsError(t *testing.T) {
 		err      string
 	}{
 		{
-			err: "^failed to load TLS config: failed to load CA CertPool: failed to load CA /doesnt/exist:",
+			err: "^failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:",
 			settings: HTTPClientSettings{
 				Endpoint: "",
 				TLSSetting: configtls.TLSClientSetting{
@@ -225,7 +225,7 @@ func TestHTTPClientSettingsError(t *testing.T) {
 			},
 		},
 		{
-			err: "^failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither",
+			err: "^failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither",
 			settings: HTTPClientSettings{
 				Endpoint: "",
 				TLSSetting: configtls.TLSClientSetting{
@@ -395,7 +395,7 @@ func TestHTTPServerSettingsError(t *testing.T) {
 		err      string
 	}{
 		{
-			err: "^failed to load TLS config: failed to load CA CertPool: failed to load CA /doesnt/exist:",
+			err: "^failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:",
 			settings: HTTPServerSettings{
 				Endpoint: "localhost:0",
 				TLSSetting: &configtls.TLSServerSetting{
@@ -406,7 +406,7 @@ func TestHTTPServerSettingsError(t *testing.T) {
 			},
 		},
 		{
-			err: "^failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither",
+			err: "^failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither",
 			settings: HTTPServerSettings{
 				Endpoint: "localhost:0",
 				TLSSetting: &configtls.TLSServerSetting{

diff --git a/config/configtls/README.md b/config/configtls/README.md
@@ -18,14 +18,18 @@ As a result, the following parameters are also required:
 
 - `cert_file`: Path to the TLS cert to use for TLS required connections. Should
   only be used if `insecure` is set to false.
+  - `cert_pem`: Alternative to `cert_file`. Provide the certificate contents as a string instead of a filepath.
+
 - `key_file`: Path to the TLS key to use for TLS required connections. Should
   only be used if `insecure` is set to false.
+  - `key_pem`: Alternative to `key_file`. Provide the key contents as a string instead of a filepath.
 
 A certificate authority may also need to be defined:
 
 - `ca_file`: Path to the CA cert. For a client this verifies the server
   certificate. For a server this verifies client certificates. If empty uses
   system root CA. Should only be used if `insecure` is set to false.
+  - `ca_pem`: Alternative to `ca_file`. Provide the CA cert contents as a string instead of a filepath.
 
 Additionally you can configure TLS to be enabled but skip verifying the server's
 certificate chain. This cannot be combined with `insecure` since `insecure`

diff --git a/config/configtls/configtls.go b/config/configtls/configtls.go
@@ -6,12 +6,13 @@ package configtls // import "go.opentelemetry.io/collector/config/configtls"
 import (
 	"crypto/tls"
 	"crypto/x509"
-	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	"sync"
 	"time"
+
+	"go.opentelemetry.io/collector/config/configopaque"
 )
 
 // We should avoid that users unknowingly use a vulnerable TLS version.
@@ -30,12 +31,21 @@ type TLSSetting struct {
 	// (optional)
 	CAFile string `mapstructure:"ca_file"`
 
+	// In memory PEM encoded cert. (optional)
+	CAPem configopaque.String `mapstructure:"ca_pem"`
+
 	// Path to the TLS cert to use for TLS required connections. (optional)
 	CertFile string `mapstructure:"cert_file"`
 
+	// In memory PEM encoded TLS cert to use for TLS required connections. (optional)
+	CertPem configopaque.String `mapstructure:"cert_pem"`
+
 	// Path to the TLS key to use for TLS required connections. (optional)
 	KeyFile string `mapstructure:"key_file"`
 
+	// In memory PEM encoded TLS key to use for TLS required connections. (optional)
+	KeyPem configopaque.String `mapstructure:"key_pem"`
+
 	// MinVersion sets the minimum TLS version that is acceptable.
 	// If not set, TLS 1.2 will be used. (optional)
 	MinVersion string `mapstructure:"min_version"`
@@ -96,29 +106,21 @@ type TLSServerSetting struct {
 // Its GetCertificate method will either return the current certificate or reload from disk
 // if the last reload happened more than ReloadInterval ago
 type certReloader struct {
-	// Path to the TLS cert
-	CertFile string
-	// Path to the TLS key
-	KeyFile string
-	// ReloadInterval specifies the duration after which the certificate will be reloaded
-	// If not set, it will never be reloaded (optional)
-	ReloadInterval time.Duration
-	nextReload     time.Time
-	cert           *tls.Certificate
-	lock           sync.RWMutex
+	nextReload time.Time
+	cert       *tls.Certificate
+	lock       sync.RWMutex
+	tls        TLSSetting
 }
 
-func newCertReloader(certFile, keyFile string, reloadInterval time.Duration) (*certReloader, error) {
-	cert, err := tls.LoadX509KeyPair(certFile, keyFile)
+func (c TLSSetting) newCertReloader() (*certReloader, error) {
+	cert, err := c.loadCertificate()
 	if err != nil {
 		return nil, err
 	}
 	return &certReloader{
-		CertFile:       certFile,
-		KeyFile:        keyFile,
-		ReloadInterval: reloadInterval,
-		nextReload:     time.Now().Add(reloadInterval),
-		cert:           &cert,
+		tls:        c,
+		nextReload: time.Now().Add(c.ReloadInterval),
+		cert:       &cert,
 	}, nil
 }
 
@@ -128,47 +130,36 @@ func (r *certReloader) GetCertificate() (*tls.Certificate, error) {
 	// If a reload is in progress this will block and we will skip reloading in the current
 	// call once we can continue
 	r.lock.RLock()
-	if r.ReloadInterval != 0 && r.nextReload.Before(now) {
+	if r.tls.ReloadInterval != 0 && r.nextReload.Before(now) && (r.tls.hasCertFile() || r.tls.hasKeyFile()) {
 		// Need to release the read lock, otherwise we deadlock
 		r.lock.RUnlock()
 		r.lock.Lock()
 		defer r.lock.Unlock()
-		cert, err := tls.LoadX509KeyPair(r.CertFile, r.KeyFile)
+		cert, err := r.tls.loadCertificate()
 		if err != nil {
 			return nil, fmt.Errorf("failed to load TLS cert and key: %w", err)
 		}
 		r.cert = &cert
-		r.nextReload = now.Add(r.ReloadInterval)
+		r.nextReload = now.Add(r.tls.ReloadInterval)
 		return r.cert, nil
 	}
 	defer r.lock.RUnlock()
 	return r.cert, nil
 }
 
-// LoadTLSConfig loads TLS certificates and returns a tls.Config.
+// loadTLSConfig loads TLS certificates and returns a tls.Config.
 // This will set the RootCAs and Certificates of a tls.Config.
 func (c TLSSetting) loadTLSConfig() (*tls.Config, error) {
-	// There is no need to load the System Certs for RootCAs because
-	// if the value is nil, it will default to checking against th System Certs.
-	var err error
-	var certPool *x509.CertPool
-	if len(c.CAFile) != 0 {
-		// Set up user specified truststore.
-		certPool, err = c.loadCert(c.CAFile)
-		if err != nil {
-			return nil, fmt.Errorf("failed to load CA CertPool: %w", err)
-		}
-	}
-
-	if (c.CertFile == "" && c.KeyFile != "") || (c.CertFile != "" && c.KeyFile == "") {
-		return nil, errors.New("for auth via TLS, either both certificate and key must be supplied, or neither")
+	certPool, err := c.loadCACertPool()
+	if err != nil {
+		return nil, err
 	}
 
 	var getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error)
 	var getClientCertificate func(*tls.CertificateRequestInfo) (*tls.Certificate, error)
-	if c.CertFile != "" && c.KeyFile != "" {
+	if c.hasCert() || c.hasKey() {
 		var certReloader *certReloader
-		certReloader, err = newCertReloader(c.CertFile, c.KeyFile, c.ReloadInterval)
+		certReloader, err = c.newCertReloader()
 		if err != nil {
 			return nil, fmt.Errorf("failed to load TLS cert and key: %w", err)
 		}
@@ -194,6 +185,89 @@ func (c TLSSetting) loadTLSConfig() (*tls.Config, error) {
 	}, nil
 }
 
+func (c TLSSetting) loadCACertPool() (*x509.CertPool, error) {
+	// There is no need to load the System Certs for RootCAs because
+	// if the value is nil, it will default to checking against th System Certs.
+	var err error
+	var certPool *x509.CertPool
+
+	switch {
+	case c.hasCAFile() && c.hasCAPem():
+		return nil, fmt.Errorf("failed to load CA CertPool: provide either a CA file or the PEM-encoded string, but not both")
+	case c.hasCAFile():
+		// Set up user specified truststore from file
+		certPool, err = c.loadCertFile(c.CAFile)
+		if err != nil {
+			return nil, fmt.Errorf("failed to load CA CertPool File: %w", err)
+		}
+	case c.hasCAPem():
+		// Set up user specified truststore from PEM
+		certPool, err = c.loadCertPem([]byte(c.CAPem))
+		if err != nil {
+			return nil, fmt.Errorf("failed to load CA CertPool PEM: %w", err)
+		}
+	}
+
+	return certPool, nil
+}
+
+func (c TLSSetting) loadCertFile(certPath string) (*x509.CertPool, error) {
+	certPem, err := os.ReadFile(filepath.Clean(certPath))
+	if err != nil {
+		return nil, fmt.Errorf("failed to load cert %s: %w", certPath, err)
+	}
+
+	return c.loadCertPem(certPem)
+}
+
+func (c TLSSetting) loadCertPem(certPem []byte) (*x509.CertPool, error) {
+	certPool := x509.NewCertPool()
+	if !certPool.AppendCertsFromPEM(certPem) {
+		return nil, fmt.Errorf("failed to parse cert")
+	}
+	return certPool, nil
+}
+
+func (c TLSSetting) loadCertificate() (tls.Certificate, error) {
+	switch {
+	case c.hasCert() != c.hasKey():
+		return tls.Certificate{}, fmt.Errorf("for auth via TLS, provide both certificate and key, or neither")
+	case !c.hasCert() && !c.hasKey():
+		return tls.Certificate{}, nil
+	case c.hasCertFile() && c.hasCertPem():
+		return tls.Certificate{}, fmt.Errorf("for auth via TLS, provide either a certificate or the PEM-encoded string, but not both")
+	case c.hasKeyFile() && c.hasKeyPem():
+		return tls.Certificate{}, fmt.Errorf("for auth via TLS, provide either a key or the PEM-encoded string, but not both")
+	}
+
+	var certPem, keyPem []byte
+	var err error
+	if c.hasCertFile() {
+		certPem, err = os.ReadFile(c.CertFile)
+		if err != nil {
+			return tls.Certificate{}, err
+		}
+	} else {
+		certPem = []byte(c.CertPem)
+	}
+
+	if c.hasKeyFile() {
+		keyPem, err = os.ReadFile(c.KeyFile)
+		if err != nil {
+			return tls.Certificate{}, err
+		}
+	} else {
+		keyPem = []byte(c.KeyPem)
+	}
+
+	certificate, err := tls.X509KeyPair(certPem, keyPem)
+	if err != nil {
+		return tls.Certificate{}, fmt.Errorf("failed to load TLS cert and key PEMs: %w", err)
+	}
+
+	return certificate, err
+}
+
 func (c TLSSetting) loadCert(caPath string) (*x509.CertPool, error) {
 	caPEM, err := os.ReadFile(filepath.Clean(caPath))
 	if err != nil {
@@ -209,7 +283,7 @@ func (c TLSSetting) loadCert(caPath string) (*x509.CertPool, error) {
 
 // LoadTLSConfig loads the TLS configuration.
 func (c TLSClientSetting) LoadTLSConfig() (*tls.Config, error) {
-	if c.Insecure && c.CAFile == "" {
+	if c.Insecure && !c.hasCA() {
 		return nil, nil
 	}
 
@@ -250,6 +324,19 @@ func (c TLSServerSetting) loadClientCAFile() (*x509.CertPool, error) {
 	return c.loadCert(c.ClientCAFile)
 }
 
+func (c TLSSetting) hasCA() bool   { return c.hasCAFile() || c.hasCAPem() }
+func (c TLSSetting) hasCert() bool { return c.hasCertFile() || c.hasCertPem() }
+func (c TLSSetting) hasKey() bool  { return c.hasKeyFile() || c.hasKeyPem() }
+
+func (c TLSSetting) hasCAFile() bool { return c.CAFile != "" }
+func (c TLSSetting) hasCAPem() bool  { return len(c.CAPem) != 0 }
+
+func (c TLSSetting) hasCertFile() bool { return c.CertFile != "" }
+func (c TLSSetting) hasCertPem() bool  { return len(c.CertPem) != 0 }
+
+func (c TLSSetting) hasKeyFile() bool { return c.KeyFile != "" }
+func (c TLSSetting) hasKeyPem() bool  { return len(c.KeyPem) != 0 }
+
 func convertVersion(v string, defaultVersion uint16) (uint16, error) {
 	// Use a default that is explicitly defined
 	if v == "" {