Skip to content

Commit

Permalink
Allow specifying a cert and a key manually for federation endpoint. (#…
Browse files Browse the repository at this point in the history
…5163)

Allow specifying a cert and a key manually for federation endpoint.

* Move DiskCertManager to pkg/common
* Rename TLSConfig to GetTLSConfig
* spire-server: allow directly specifying a certificate and key for the bundle endpoint
* Update documentation
* Start file watcher as a task
* Update conf/server/server_full.conf

Signed-off-by: Sorin Dumitru <sdumitru@bloomberg.net>
Signed-off-by: Sorin Dumitru <sorin@returnze.ro>
  • Loading branch information
sorindumitru committed Jun 18, 2024
1 parent cdb7955 commit 60e8844
Show file tree
Hide file tree
Showing 14 changed files with 220 additions and 44 deletions.
70 changes: 54 additions & 16 deletions cmd/spire-server/cli/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/spiffe/spire/pkg/common/bundleutil"
"github.com/spiffe/spire/pkg/common/catalog"
common_cli "github.com/spiffe/spire/pkg/common/cli"
"github.com/spiffe/spire/pkg/common/diskcertmanager"
"github.com/spiffe/spire/pkg/common/fflag"
"github.com/spiffe/spire/pkg/common/health"
"github.com/spiffe/spire/pkg/common/log"
Expand Down Expand Up @@ -142,11 +143,19 @@ type bundleEndpointConfigProfile struct {
}

type bundleEndpointProfileHTTPSWebConfig struct {
ACME *bundleEndpointACMEConfig `hcl:"acme"`
ACME *bundleEndpointACMEConfig `hcl:"acme"`
ServingCertFile *bundleEndpointServingCertFile `hcl:"serving_cert_file"`
}

type bundleEndpointProfileHTTPSSPIFFEConfig struct{}

type bundleEndpointServingCertFile struct {
CertFilePath string `hcl:"cert_file_path"`
KeyFilePath string `hcl:"key_file_path"`
FileSyncInterval time.Duration `hcl:"-"`
RawFileSyncInterval string `hcl:"file_sync_interval"`
}

type bundleEndpointACMEConfig struct {
DirectoryURL string `hcl:"directory_url"`
DomainName string `hcl:"domain_name"`
Expand Down Expand Up @@ -460,11 +469,10 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool
}

if c.Server.Federation.BundleEndpoint != nil {
acmeConfig, err := parseBundleEndpointConfigProfile(c.Server.Federation.BundleEndpoint, sc.DataDir, sc.Log)
err := setBundleEndpointConfigProfile(c.Server.Federation.BundleEndpoint, sc.DataDir, sc.Log, &sc.Federation)
if err != nil {
return nil, err
}
sc.Federation.BundleEndpoint.ACME = acmeConfig
}
}

Expand Down Expand Up @@ -692,46 +700,54 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool
return sc, nil
}

func parseBundleEndpointConfigProfile(config *bundleEndpointConfig, dataDir string, log logrus.FieldLogger) (*bundle.ACMEConfig, error) {
func setBundleEndpointConfigProfile(config *bundleEndpointConfig, dataDir string, log logrus.FieldLogger, federationConfig *server.FederationConfig) error {
switch {
case config.ACME != nil && config.Profile != nil:
return nil, errors.New("either bundle endpoint 'acme' or 'profile' can be set, but not both")
return errors.New("either bundle endpoint 'acme' or 'profile' can be set, but not both")

case config.ACME != nil:
log.Warn("ACME configuration within the bundle_endpoint is deprecated. Please use ACME configuration as part of the https_web profile instead.")
return configToACMEConfig(config.ACME, dataDir), nil
federationConfig.BundleEndpoint.ACME = configToACMEConfig(config.ACME, dataDir)
return nil

case config.Profile == nil:
log.Warn("Bundle endpoint is configured but has no profile set, using https_spiffe as default; please configure a profile explicitly. This will be fatal in a future release.")
return nil, nil
return nil
}

// Profile is set, parse it
configString, err := parseBundleEndpointProfileASTNode(config.Profile)
if err != nil {
return nil, err
return err
}

profileConfig := new(bundleEndpointConfigProfile)
if err := hcl.Decode(profileConfig, configString); err != nil {
return nil, fmt.Errorf("failed to decode configuration: %w", err)
return fmt.Errorf("failed to decode configuration: %w", err)
}

switch {
case profileConfig.HTTPSWeb != nil:
acme := profileConfig.HTTPSWeb.ACME
if acme == nil {
return nil, fmt.Errorf("malformed https_web profile configuration: ACME is required")
switch {
case profileConfig.HTTPSWeb.ACME != nil:
federationConfig.BundleEndpoint.ACME = configToACMEConfig(profileConfig.HTTPSWeb.ACME, dataDir)
return nil
case profileConfig.HTTPSWeb.ServingCertFile != nil:
federationConfig.BundleEndpoint.DiskCertManager, err = configToDiskCertManager(profileConfig.HTTPSWeb.ServingCertFile, log)
if err != nil {
return err
}
return nil
default:
return errors.New("malformed https_web profile configuration: 'acme' or 'serving_cert_file' is required")
}
acmeConfig := configToACMEConfig(acme, dataDir)
return acmeConfig, nil

// For now ignore SPIFFE configuration
case profileConfig.HTTPSSPIFFE != nil:
return nil, nil
return nil

default:
return nil, errors.New(`unknown bundle endpoint profile configured; current supported profiles are "https_spiffe" and 'https_web"`)
return errors.New(`unknown bundle endpoint profile configured; current supported profiles are "https_spiffe" and 'https_web"`)
}
}

Expand All @@ -745,6 +761,28 @@ func configToACMEConfig(acme *bundleEndpointACMEConfig, dataDir string) *bundle.
}
}

func configToDiskCertManager(serviceCertFile *bundleEndpointServingCertFile, log logrus.FieldLogger) (*diskcertmanager.DiskCertManager, error) {
fileSyncInterval, err := time.ParseDuration(serviceCertFile.RawFileSyncInterval)
if err != nil {
return nil, err
}

serviceCertFile.FileSyncInterval = fileSyncInterval
if serviceCertFile.FileSyncInterval == time.Duration(0) {
serviceCertFile.FileSyncInterval = time.Hour
}

return diskcertmanager.New(
&diskcertmanager.Config{
CertFilePath: serviceCertFile.CertFilePath,
KeyFilePath: serviceCertFile.KeyFilePath,
FileSyncInterval: serviceCertFile.FileSyncInterval,
},
nil,
log,
)
}

func parseBundleEndpointProfile(config federatesWithConfig) (trustDomainConfig *bundleClient.TrustDomainConfig, err error) {
configString, err := parseBundleEndpointProfileASTNode(config.BundleEndpointProfile)
if err != nil {
Expand Down
38 changes: 36 additions & 2 deletions cmd/spire-server/cli/run/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ func TestNewServerConfig(t *testing.T) {
msg: "bundle endpoint has web profile",
input: func(c *Config) {
c.Server.Federation = &federationConfig{
BundleEndpoint: bundleEndpointProfileHTTPSWebTest(t),
BundleEndpoint: bundleEndpointProfileHTTPSWebACMETest(t),
}
},
test: func(t *testing.T, c *server.Config) {
Expand All @@ -694,6 +694,23 @@ func TestNewServerConfig(t *testing.T) {
CacheDir: "bundle-acme",
}
require.Equal(t, expectACME, c.Federation.BundleEndpoint.ACME)
require.Nil(t, c.Federation.BundleEndpoint.DiskCertManager)
},
},
{
msg: "bundle endpoint has web profile with certs on disk",
input: func(c *Config) {
c.Server.Federation = &federationConfig{
BundleEndpoint: bundleEndpointProfileHTTPSWebServingCertFileTest(t),
}
},
test: func(t *testing.T, c *server.Config) {
require.Equal(t, "0.0.0.0", c.Federation.BundleEndpoint.Address.IP.String())
require.Equal(t, 8443, c.Federation.BundleEndpoint.Address.Port)
require.Equal(t, 10*time.Minute, c.Federation.BundleEndpoint.RefreshHint)

require.Nil(t, c.Federation.BundleEndpoint.ACME)
require.NotNil(t, c.Federation.BundleEndpoint.DiskCertManager)
},
},
{
Expand Down Expand Up @@ -1899,7 +1916,24 @@ func bundleEndpointProfileACMEAndProfileTest(t *testing.T) *bundleEndpointConfig
return config
}

func bundleEndpointProfileHTTPSWebTest(t *testing.T) *bundleEndpointConfig {
func bundleEndpointProfileHTTPSWebServingCertFileTest(t *testing.T) *bundleEndpointConfig {
configString := `address = "0.0.0.0"
port = 8443
refresh_hint = "10m"
profile "https_web" {
serving_cert_file {
cert_file_path = "../../../../test/fixture/certs/svid.pem"
key_file_path = "../../../../test/fixture/certs/svid_key.pem"
file_sync_interval = "5m"
}
}`
config := new(bundleEndpointConfig)
require.NoError(t, hcl.Decode(config, configString))

return config
}

func bundleEndpointProfileHTTPSWebACMETest(t *testing.T) *bundleEndpointConfig {
configString := `address = "0.0.0.0"
port = 8443
refresh_hint = "10m"
Expand Down
7 changes: 7 additions & 0 deletions conf/server/server_full.conf
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ server {
# Default: false.
# tos_accepted = false
}

# Use certificate and key from disk
# serving_cert_file = {
# cert_file_path= "conf/server/bundleendpoint.crt"
# key_file_path = "conf/server/bundleendpoint.key"
# file_sync_interval = "1h"
# }
}

# profile "https_spiffe": Configuration for the https_spiffe profile.
Expand Down
10 changes: 9 additions & 1 deletion doc/spire_server.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ When setting a `bundle_endpoint`, it is `required` to specify the bundle profile

Allowed profiles:

- `https_web` allow to configure the [Automated Certificate Management Environment](#Configuration options for `federation.bundle_endpoint.profile "https_web".acme`) section.
- `https_web` allow to configure either the [Automated Certificate Management Environment](#Configuration options for `federation.bundle_endpoint.profile "https_web".acme`) or the [serving cert file](#Configure options for 'federation.bundle_endpoint.porfile "https_web".serving_cert_file') section.
- `https_spiffe`

### Configuration options for `federation.bundle_endpoint.profile "https_web".acme`
Expand All @@ -269,6 +269,14 @@ Allowed profiles:
| email | Contact email address. This is used by CAs, such as Let's Encrypt, to notify about problems with issued certificates | |
| tos_accepted | ACME Terms of Service acceptance. If not true, and the provider requires acceptance, then certificate retrieval will fail | false |

### Configuration options for `federation.bundle_endpoint.profile "https_web".serving_cert_file`

| Configuration | Description | Default |
|--------------------|-------------------------------------------------|---------|
| cert_file_path | Path to the certificate file, in PEM format | |
| key_file_path | Path to the key file, in PEM format | |
| file_sync_interval | Interval on which to reload the files from disk | 1h |

### Configuration options for `federation.bundle_endpoint.profile "https_spiffe"`

Default bundle profile configuration.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package main
package diskcertmanager

import (
"context"
Expand Down Expand Up @@ -28,7 +28,13 @@ type DiskCertManager struct {
log logrus.FieldLogger
}

func NewDiskCertManager(config *ServingCertFileConfig, clk clock.Clock, log logrus.FieldLogger) (*DiskCertManager, error) {
type Config struct {
CertFilePath string
KeyFilePath string
FileSyncInterval time.Duration
}

func New(config *Config, clk clock.Clock, log logrus.FieldLogger) (*DiskCertManager, error) {
if config == nil {
return nil, errors.New("missing serving cert file configuration")
}
Expand All @@ -53,7 +59,7 @@ func NewDiskCertManager(config *ServingCertFileConfig, clk clock.Clock, log logr
}

// TLSConfig returns a TLS configuration that uses the provided certificate stored on disk.
func (m *DiskCertManager) TLSConfig() *tls.Config {
func (m *DiskCertManager) GetTLSConfig() *tls.Config {
return &tls.Config{
GetCertificate: m.getCertificate,
NextProtos: []string{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package main
package diskcertmanager

import (
"context"
Expand Down Expand Up @@ -27,6 +27,8 @@ import (
var (
oidcServerKey = testkey.MustEC256()
oidcServerKeyNew = testkey.MustEC256()

fileDontExistMessage = "no such file or directory"
)

func TestTLSConfig(t *testing.T) {
Expand Down Expand Up @@ -92,7 +94,7 @@ func TestTLSConfig(t *testing.T) {
}

ctx, cancelFn := context.WithCancel(context.Background())
certManager, err := NewDiskCertManager(&ServingCertFileConfig{
certManager, err := New(&Config{
CertFilePath: certFilePath,
KeyFilePath: keyFilePath,
FileSyncInterval: 10 * time.Millisecond,
Expand All @@ -103,15 +105,15 @@ func TestTLSConfig(t *testing.T) {
certManager.WatchFileChanges(ctx)
}()

tlsConfig := certManager.TLSConfig()
tlsConfig := certManager.GetTLSConfig()

t.Run("error when configuration does not contain serving cert file settings", func(t *testing.T) {
_, err := NewDiskCertManager(nil, nil, logger)
_, err := New(nil, nil, logger)
require.EqualError(t, err, "missing serving cert file configuration")
})

t.Run("error when provided cert path do not exist", func(t *testing.T) {
_, err := NewDiskCertManager(&ServingCertFileConfig{
_, err := New(&Config{
CertFilePath: filepath.Join(tmpDir, "nonexistent_cert.pem"),
KeyFilePath: keyFilePath,
}, clk, logger)
Expand All @@ -120,7 +122,7 @@ func TestTLSConfig(t *testing.T) {
})

t.Run("error when provided key path do not exist", func(t *testing.T) {
_, err := NewDiskCertManager(&ServingCertFileConfig{
_, err := New(&Config{
CertFilePath: certFilePath,
KeyFilePath: filepath.Join(tmpDir, "nonexistent_key.pem"),
}, clk, logger)
Expand All @@ -129,7 +131,7 @@ func TestTLSConfig(t *testing.T) {
})

t.Run("error when provided cert is invalid", func(t *testing.T) {
_, err := NewDiskCertManager(&ServingCertFileConfig{
_, err := New(&Config{
CertFilePath: invalidCertFilePath,
KeyFilePath: keyFilePath,
}, clk, logger)
Expand All @@ -138,7 +140,7 @@ func TestTLSConfig(t *testing.T) {
})

t.Run("error when provided key is invalid", func(t *testing.T) {
_, err := NewDiskCertManager(&ServingCertFileConfig{
_, err := New(&Config{
CertFilePath: certFilePath,
KeyFilePath: invalidKeyFilePath,
}, clk, logger)
Expand Down
4 changes: 4 additions & 0 deletions pkg/server/endpoints/bundle/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package bundle
import (
"net"
"time"

"github.com/spiffe/spire/pkg/common/diskcertmanager"
)

type EndpointConfig struct {
Expand All @@ -13,5 +15,7 @@ type EndpointConfig struct {
// If unset, the bundle endpoint will use SPIFFE auth.
ACME *ACMEConfig

DiskCertManager *diskcertmanager.DiskCertManager

RefreshHint time.Duration
}
Loading

0 comments on commit 60e8844

Please sign in to comment.