Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add in memory PEM support for TLSSetting #2

Merged
merged 7 commits into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions config/configgrpc/configgrpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,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: "",
Expand All @@ -256,7 +256,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, either both certificate and key must be supplied, or neither",
settings: GRPCClientSettings{
Headers: nil,
Endpoint: "",
Expand Down Expand Up @@ -376,7 +376,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",
Expand All @@ -390,7 +390,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, either both certificate and key must be supplied, or neither",
settings: GRPCServerSettings{
NetAddr: confignet.NetAddr{
Endpoint: "127.0.0.1:1234",
Expand All @@ -404,7 +404,7 @@ func TestGRPCServerSettingsError(t *testing.T) {
},
},
{
err: "^failed to load TLS config: failed to load client CA CertPool: failed to load CA /doesnt/exist:",
err: "^failed to load TLS config: failed to load client CA CertPool: failed to load cert /doesnt/exist:",
settings: GRPCServerSettings{
NetAddr: confignet.NetAddr{
Endpoint: "127.0.0.1:1234",
Expand Down
10 changes: 5 additions & 5 deletions config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,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{
Expand All @@ -230,7 +230,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, either both certificate and key must be supplied, or neither",
settings: HTTPClientSettings{
Endpoint: "",
TLSSetting: configtls.TLSClientSetting{
Expand Down Expand Up @@ -353,7 +353,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{
Expand All @@ -364,7 +364,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, either both certificate and key must be supplied, or neither",
settings: HTTPServerSettings{
Endpoint: "localhost:0",
TLSSetting: &configtls.TLSServerSetting{
Expand All @@ -375,7 +375,7 @@ func TestHTTPServerSettingsError(t *testing.T) {
},
},
{
err: "^failed to load TLS config: failed to load client CA CertPool: failed to load CA /doesnt/exist:",
err: "^failed to load TLS config: failed to load client CA CertPool: failed to load cert /doesnt/exist:",
settings: HTTPServerSettings{
Endpoint: "localhost:0",
TLSSetting: &configtls.TLSServerSetting{
Expand Down
175 changes: 126 additions & 49 deletions config/configtls/configtls.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package configtls // import "go.opentelemetry.io/collector/config/configtls"
import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -41,12 +40,21 @@ type TLSSetting struct {
// (optional)
CAFile string `mapstructure:"ca_file"`

// In memory PEM encoded cert. (optional)
CAPem []byte `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 []byte `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 []byte `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"`
Expand Down Expand Up @@ -103,29 +111,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(reloadInterval time.Duration) (*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(reloadInterval),
cert: &cert,
}, nil
}

Expand All @@ -135,53 +135,46 @@ 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.hasCAFile() || 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 != "" {
var certReloader *certReloader
certReloader, err = newCertReloader(c.CertFile, c.KeyFile, c.ReloadInterval)
if err != nil {
return nil, fmt.Errorf("failed to load TLS cert and key: %w", err)
}

var certReloader *certReloader
certReloader, err = c.newCertReloader(c.ReloadInterval)
if err != nil {
return nil, fmt.Errorf("failed to load TLS cert and key: %w", err)
}

certReloader.lock.RLock()
if certReloader.cert != nil {
getCertificate = func(chi *tls.ClientHelloInfo) (*tls.Certificate, error) { return certReloader.GetCertificate() }
getClientCertificate = func(cri *tls.CertificateRequestInfo) (*tls.Certificate, error) { return certReloader.GetCertificate() }
}
certReloader.lock.RUnlock()

minTLS, err := convertVersion(c.MinVersion, defaultMinTLSVersion)
if err != nil {
Expand All @@ -201,22 +194,106 @@ func (c TLSSetting) loadTLSConfig() (*tls.Config, error) {
}, nil
}

func (c TLSSetting) loadCert(caPath string) (*x509.CertPool, error) {
caPEM, err := os.ReadFile(filepath.Clean(caPath))
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: CA File and PEM cannot both be provided")
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(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 CA %s: %w", caPath, err)
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(caPEM) {
return nil, fmt.Errorf("failed to parse CA %s", caPath)
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, either both certificate and key must be supplied, or neither")
case !c.hasCert():
return tls.Certificate{}, nil
case c.hasCertFile() && c.hasCertPem():
return tls.Certificate{}, fmt.Errorf("for auth via TLS, certificate file and PEM cannot both be provided")
case c.hasKeyFile() && c.hasKeyPem():
return tls.Certificate{}, fmt.Errorf("for auth via TLS, key file and PEM cannot both be provided")
}

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 = c.CertPem
}

if c.hasKeyFile() {
keyPem, err = os.ReadFile(c.KeyFile)
if err != nil {
return tls.Certificate{}, err
}
} else {
keyPem = 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) 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 }

// 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
}

Expand All @@ -236,7 +313,7 @@ func (c TLSServerSetting) LoadTLSConfig() (*tls.Config, error) {
return nil, fmt.Errorf("failed to load TLS config: %w", err)
}
if c.ClientCAFile != "" {
certPool, err := c.loadCert(c.ClientCAFile)
certPool, err := c.loadCertFile(c.ClientCAFile)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just for a comment) I think in the future we'd probably want to support inline client certificate chains for authentication, but it's not something we need right now so I think it's OK to descope it.

if err != nil {
return nil, fmt.Errorf("failed to load TLS config: failed to load client CA CertPool: %w", err)
}
Expand Down
Loading