diff --git a/pkg/fftls/config.go b/pkg/fftls/config.go index 426382b..8b81d05 100644 --- a/pkg/fftls/config.go +++ b/pkg/fftls/config.go @@ -44,7 +44,13 @@ const ( // HTTPConfTLSRequiredDNAttributes provides a set of regular expressions, to match against the DN of the client. Requires HTTPConfTLSClientAuth HTTPConfTLSRequiredDNAttributes = "requiredDNAttributes" - defaultHTTPTLSEnabled = false + // HTTPConfTLSIncludeSystemCAs when true and a custom CA (caFile/ca) is configured, merges that CA + // into the system certificate pool instead of replacing RootCAs. Useful for outbound HTTP clients + // that talk to a private CA endpoint and may follow redirects to publicly trusted hosts (e.g. S3). + HTTPConfTLSIncludeSystemCAs = "includeSystemCAs" + + defaultHTTPTLSEnabled = false + defaultHTTPTLSIncludeSystemCAs = false ) type Config struct { @@ -57,6 +63,7 @@ type Config struct { KeyFile string `ffstruct:"tlsconfig" json:"keyFile,omitempty"` Key string `ffstruct:"tlsconfig" json:"key,omitempty"` InsecureSkipHostVerify bool `ffstruct:"tlsconfig" json:"insecureSkipHostVerify"` + IncludeSystemCAs bool `ffstruct:"tlsconfig" json:"includeSystemCAs,omitempty"` RequiredDNAttributes map[string]interface{} `ffstruct:"tlsconfig" json:"requiredDNAttributes,omitempty"` } @@ -71,6 +78,7 @@ func InitTLSConfig(conf config.Section) { conf.AddKnownKey(HTTPConfTLSKey) conf.AddKnownKey(HTTPConfTLSRequiredDNAttributes) conf.AddKnownKey(HTTPConfTLSInsecureSkipHostVerify) + conf.AddKnownKey(HTTPConfTLSIncludeSystemCAs, defaultHTTPTLSIncludeSystemCAs) } func GenerateConfig(conf config.Section) *Config { @@ -84,6 +92,7 @@ func GenerateConfig(conf config.Section) *Config { KeyFile: conf.GetString(HTTPConfTLSKeyFile), Key: conf.GetString(HTTPConfTLSKey), InsecureSkipHostVerify: conf.GetBool(HTTPConfTLSInsecureSkipHostVerify), + IncludeSystemCAs: conf.GetBool(HTTPConfTLSIncludeSystemCAs), RequiredDNAttributes: conf.GetObject(HTTPConfTLSRequiredDNAttributes), } } diff --git a/pkg/fftls/fftls.go b/pkg/fftls/fftls.go index 520ffe2..33550d1 100644 --- a/pkg/fftls/fftls.go +++ b/pkg/fftls/fftls.go @@ -101,12 +101,14 @@ func NewTLSConfig(ctx context.Context, config *Config, tlsType TLSType) (*tls.Co } var err error - // Support custom CA file + // Support custom CA file. By default a configured CA replaces RootCAs. When IncludeSystemCAs + // is set, the custom CA is merged into the system pool (e.g. private CA + public CAs for + // HTTP clients that follow redirects to S3). var rootCAs *x509.CertPool switch { case config.CAFile != "": - log.L(ctx).Tracef("Loading CA file at %s", config.CAFile) - rootCAs = x509.NewCertPool() + log.L(ctx).Tracef("Loading CA file at %s (includeSystemCAs=%t)", config.CAFile, config.IncludeSystemCAs) + rootCAs = newRootCAPool(ctx, config.IncludeSystemCAs) var caBytes []byte caBytes, err = os.ReadFile(config.CAFile) if err == nil { @@ -119,7 +121,8 @@ func NewTLSConfig(ctx context.Context, config *Config, tlsType TLSType) (*tls.Co } } case config.CA != "": - rootCAs = x509.NewCertPool() + log.L(ctx).Tracef("Loading inline CA PEM (includeSystemCAs=%t)", config.IncludeSystemCAs) + rootCAs = newRootCAPool(ctx, config.IncludeSystemCAs) ok := rootCAs.AppendCertsFromPEM([]byte(config.CA)) if !ok { err = i18n.NewError(ctx, i18n.MsgInvalidCAFile) @@ -197,6 +200,19 @@ func NewTLSConfig(ctx context.Context, config *Config, tlsType TLSType) (*tls.Co } +// newRootCAPool returns an empty pool, or the system pool when includeSystemCAs is true. +func newRootCAPool(ctx context.Context, includeSystemCAs bool) *x509.CertPool { + if !includeSystemCAs { + return x509.NewCertPool() + } + rootCAs, err := x509.SystemCertPool() + if err != nil { + log.L(ctx).Warnf("Unable to load system cert pool; starting with empty pool: %s", err) + return x509.NewCertPool() + } + return rootCAs +} + // recordCACertExpiryMetrics decodes a PEM bundle (which may contain one or more certificates) and // records a CA certificate expiry gauge for each. It is best-effort and never fails the TLS config // construction: it is a no-op if metrics are not enabled, and certificates that cannot be parsed are diff --git a/pkg/fftls/fftls_test.go b/pkg/fftls/fftls_test.go index 98f46a9..d12523f 100644 --- a/pkg/fftls/fftls_test.go +++ b/pkg/fftls/fftls_test.go @@ -25,13 +25,13 @@ import ( "crypto/x509/pkix" "encoding/pem" "github.com/stretchr/testify/require" + "io" "math/big" "net" "os" "strings" "testing" "time" - "io" "github.com/hyperledger-firefly/common/pkg/config" "github.com/stretchr/testify/assert" @@ -542,3 +542,140 @@ func TestConnectSkipVerification(t *testing.T) { _ = conn.Close() } + +func TestIncludeSystemCAs_NilIfNotEnabled(t *testing.T) { + config.RootConfigReset() + conf := config.RootSection("fftls_client_merge") + InitTLSConfig(conf) + + tlsConfig, err := ConstructTLSConfig(context.Background(), conf, ClientType) + assert.NoError(t, err) + assert.Nil(t, tlsConfig) +} + +func TestIncludeSystemCAs_DefaultsWithoutCustomCA(t *testing.T) { + config.RootConfigReset() + conf := config.RootSection("fftls_client_merge") + InitTLSConfig(conf) + conf.Set(HTTPConfTLSEnabled, true) + conf.Set(HTTPConfTLSIncludeSystemCAs, true) + + tlsConfig, err := ConstructTLSConfig(context.Background(), conf, ClientType) + assert.NoError(t, err) + require.NotNil(t, tlsConfig) + assert.False(t, tlsConfig.InsecureSkipVerify) + assert.Equal(t, uint16(tls.VersionTLS12), tlsConfig.MinVersion) + assert.Nil(t, tlsConfig.GetClientCertificate) + + systemPool, err := x509.SystemCertPool() + if err == nil && systemPool != nil { + assert.True(t, tlsConfig.RootCAs.Equal(systemPool)) + } +} + +func TestIncludeSystemCAs_CAFileMergesSystemPool(t *testing.T) { + config.RootConfigReset() + caFile, _ := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{CommonName: "test-ca.example.com"}) + + conf := config.RootSection("fftls_client_merge") + InitTLSConfig(conf) + conf.Set(HTTPConfTLSEnabled, true) + conf.Set(HTTPConfTLSCAFile, caFile) + conf.Set(HTTPConfTLSIncludeSystemCAs, true) + + merged, err := ConstructTLSConfig(context.Background(), conf, ClientType) + assert.NoError(t, err) + require.NotNil(t, merged) + require.NotNil(t, merged.RootCAs) + + conf.Set(HTTPConfTLSIncludeSystemCAs, false) + replaced, err := ConstructTLSConfig(context.Background(), conf, ClientType) + assert.NoError(t, err) + require.NotNil(t, replaced) + require.NotNil(t, replaced.RootCAs) + + systemPool, sysErr := x509.SystemCertPool() + if sysErr != nil || systemPool == nil { + t.Skip("system cert pool unavailable") + } + expected := systemPool.Clone() + caBytes, err := os.ReadFile(caFile) + require.NoError(t, err) + require.True(t, expected.AppendCertsFromPEM(caBytes)) + + assert.True(t, merged.RootCAs.Equal(expected), "custom CA should be merged into the system pool") + assert.False(t, replaced.RootCAs.Equal(expected), "without includeSystemCAs, RootCAs should be only the custom CA") +} + +func TestIncludeSystemCAs_InlineCAMergesSystemPool(t *testing.T) { + config.RootConfigReset() + caPEM, _ := buildSelfSignedTLSKeyPair(t, pkix.Name{CommonName: "inline-ca.example.com"}) + + conf := config.RootSection("fftls_client_merge") + InitTLSConfig(conf) + conf.Set(HTTPConfTLSEnabled, true) + conf.Set(HTTPConfTLSCA, caPEM) + conf.Set(HTTPConfTLSIncludeSystemCAs, true) + + merged, err := ConstructTLSConfig(context.Background(), conf, ClientType) + assert.NoError(t, err) + require.NotNil(t, merged.RootCAs) + + systemPool, sysErr := x509.SystemCertPool() + if sysErr != nil || systemPool == nil { + t.Skip("system cert pool unavailable") + } + expected := systemPool.Clone() + require.True(t, expected.AppendCertsFromPEM([]byte(caPEM))) + assert.True(t, merged.RootCAs.Equal(expected)) +} + +func TestIncludeSystemCAs_WithClientCert(t *testing.T) { + config.RootConfigReset() + certFile, keyFile := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{CommonName: "client.example.com"}) + caFile, _ := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{CommonName: "ca.example.com"}) + + conf := config.RootSection("fftls_client_merge") + InitTLSConfig(conf) + conf.Set(HTTPConfTLSEnabled, true) + conf.Set(HTTPConfTLSCertFile, certFile) + conf.Set(HTTPConfTLSKeyFile, keyFile) + conf.Set(HTTPConfTLSCAFile, caFile) + conf.Set(HTTPConfTLSIncludeSystemCAs, true) + conf.Set(HTTPConfTLSInsecureSkipHostVerify, true) + + tlsConfig, err := ConstructTLSConfig(context.Background(), conf, ClientType) + assert.NoError(t, err) + require.NotNil(t, tlsConfig) + assert.True(t, tlsConfig.InsecureSkipVerify) + require.NotNil(t, tlsConfig.GetClientCertificate) + cert, err := tlsConfig.GetClientCertificate(nil) + assert.NoError(t, err) + assert.NotNil(t, cert) +} + +func TestIncludeSystemCAs_InvalidCAFile(t *testing.T) { + config.RootConfigReset() + _, notTheCAFileTheKey := buildSelfSignedTLSKeyPairFiles(t, pkix.Name{CommonName: "server.example.com"}) + + conf := config.RootSection("fftls_client_merge") + InitTLSConfig(conf) + conf.Set(HTTPConfTLSEnabled, true) + conf.Set(HTTPConfTLSCAFile, notTheCAFileTheKey) + conf.Set(HTTPConfTLSIncludeSystemCAs, true) + + _, err := ConstructTLSConfig(context.Background(), conf, ClientType) + assert.Regexp(t, "FF00152", err) +} + +func TestIncludeSystemCAs_CAFileNotFound(t *testing.T) { + config.RootConfigReset() + conf := config.RootSection("fftls_client_merge") + InitTLSConfig(conf) + conf.Set(HTTPConfTLSEnabled, true) + conf.Set(HTTPConfTLSCAFile, "/nonexistent/ca.pem") + conf.Set(HTTPConfTLSIncludeSystemCAs, true) + + _, err := ConstructTLSConfig(context.Background(), conf, ClientType) + assert.Error(t, err) +} diff --git a/pkg/i18n/en_base_config_descriptions.go b/pkg/i18n/en_base_config_descriptions.go index 4210f3b..3f6f8df 100644 --- a/pkg/i18n/en_base_config_descriptions.go +++ b/pkg/i18n/en_base_config_descriptions.go @@ -78,6 +78,7 @@ var ( ConfigGlobalTLSKeyFile = ffc("config.global.tls.keyFile", "The path to the private key file for TLS on this API", StringType) ConfigGlobalTLSRequiredDNAttributes = ffc("config.global.tls.requiredDNAttributes", "A set of required subject DN attributes. Each entry is a regular expression, and the subject certificate must have a matching attribute of the specified type (CN, C, O, OU, ST, L, STREET, POSTALCODE, SERIALNUMBER are valid attributes)", MapStringStringType) ConfigGlobalTLSInsecureSkipHostVerify = ffc("config.global.tls.insecureSkipHostVerify", "When to true in unit test development environments to disable TLS verification. Use with extreme caution", BooleanType) + ConfigGlobalTLSIncludeSystemCAs = ffc("config.global.tls.includeSystemCAs", "When true and a custom CA (caFile/ca) is configured, merge that CA into the system certificate pool instead of replacing it. Useful for outbound HTTP clients that may follow redirects to publicly trusted hosts", BooleanType) ConfigGlobalTLSHandshakeTimeout = ffc("config.global.tlsHandshakeTimeout", "The maximum amount of time to wait for a successful TLS handshake", TimeDurationType) ConfigGlobalBodyTemplate = ffc("config.global.bodyTemplate", "The body go template string to use when making HTTP requests", GoTemplateType)