Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
50 changes: 45 additions & 5 deletions support/oidc-discovery-provider/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ The configuration file is **required** by the provider. It contains

| Key | Type | Required? | Description | Default |
| ----------------------- | ------- | ------------------ | ---------------------------------------------------------------------------------------------- | -------- |
| `acme` | section | required[1] | Provides the ACME configuration. | |
| `serving_cert_file` | section | required\[1\]\[4\] | Provides the serving certificate configuration. | |
| `serving_cert_source` | section | required\[1\] | Provides the [serving certificate source](#serving-certificate-source-section) configuration. | |
| `acme` | section | required[1] | Provides the ACME configuration. Deprecated, use `serving_cert_source "acme"` instead. | |
| `serving_cert_file` | section | required\[1\]\[4\] | Serving certificate configuration. Deprecated, use `serving_cert_source "cert_file"` instead. | |
| `allow_insecure_scheme` | bool | optional\[3\] | Serves OIDC configuration response with HTTP url. A warning is logged at startup when enabled. | `false` |
| `domains` | strings | required | One or more domains the provider is being served from. | |
| `experimental` | section | optional | The experimental options that are subject to change or removal. | |
Expand Down Expand Up @@ -80,13 +81,13 @@ the only way to bound growth of `log_path`.

#### Considerations for Unix platforms

[1]: One of `acme`, `serving_cert_file` or `listen_socket_path` must be defined.
[1]: One of `serving_cert_source`, `acme`, `serving_cert_file` or `listen_socket_path` must be defined.

[3]: The `allow_insecure_scheme` should only be enabled when the network path to the provider is trusted end-to-end (for example, when TLS is terminated at a trusted reverse proxy or load balancer on a private network). It only works in conjunction with `insecure_addr` or `listen_socket_path`.

#### Considerations for Windows platforms

[1]: One of `acme`, `serving_cert_file` or `listen_named_pipe_name` must be defined.
[1]: One of `serving_cert_source`, `acme`, `serving_cert_file` or `listen_named_pipe_name` must be defined.

[3]: The `allow_insecure_scheme` should only be enabled when the network path to the provider is trusted end-to-end (for example, when TLS is terminated at a trusted reverse proxy or load balancer on a private network). It only works in conjunction with `insecure_addr` or `listen_named_pipe_name`.

Expand All @@ -103,6 +104,31 @@ will terminate if another domain is requested.

[4]: SPIRE OIDC Discovery provider monitors and reloads the files provided in the `serving_cert_file` configuration at runtime.

#### Serving Certificate Source Section

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few things that would help in this section:

  • DNS names: browsers and most HTTP clients check that the certificate's DNS names match the host they connect to. An X509-SVID only has DNS names if the registration entry sets them (-dns), so the entry needs DNS names that match domains. Without them, clients like the kube-apiserver fetching the JWKS will fail. Could we add a note about this next to the trust bundle note?
  • What it can't be combined with: serving_cert_source "workload_api" can't be used together with insecure_addr, listen_socket_path or listen_named_pipe_name. The provider fails to start in that case, so it's worth one sentence.
  • More than one SVID: if the workload gets more than one SVID, the first one is served. A short note would avoid surprises.


The `serving_cert_source` section selects where the certificate used to serve
HTTPS comes from. The section label selects the source and the body holds its
configuration: `serving_cert_source "acme" {}` takes the options of the
[ACME Section](#acme-section), `serving_cert_source "cert_file" {}` takes the
options of the [Serving Certificate Section](#serving-certificate-section), and
`serving_cert_source "workload_api" {}` serves an X509-SVID obtained from the
SPIFFE Workload API, kept up to date as it is rotated, with the options below.
Only one `serving_cert_source` section can be configured, and it cannot be used
together with the deprecated `acme` and `serving_cert_file` sections. Note that
clients need the trust domain's X.509 bundle to authenticate an X509-SVID.

| Key | Type | Required? | Description | Default |
|----------------|---------|---------------|---------------------------------------------------------------------------|---------|
| `experimental` | section | optional | The experimental options that are subject to change or removal. | |
| `socket_path` | string | required\[5\] | Path on disk to the Workload API Unix Domain socket. Unix platforms only. | |
| `addr` | string | optional | Exposes the service on the given address. | `:443` |

| experimental | Type | Required? | Description | Default |
|:------------------|--------|---------------|---------------------------------------------------------|---------|
| `named_pipe_name` | string | required\[5\] | Pipe name of the Workload API named pipe. Windows only. | |

[5]: Optional when the `workload_api` section is configured, in which case it defaults to the value configured in that section.

#### ACME Section

| Key | Type | Required? | Description | Default |
Expand All @@ -123,7 +149,7 @@ will terminate if another domain is requested.

#### TLS Config Section

Applied to **terminating** HTTPS listeners when using `serving_cert_file` or `acme`.
Applied to **terminating** HTTPS listeners when using `serving_cert_source`, `serving_cert_file` or `acme`.
Not applied to `insecure_addr`, `listen_socket_path`, named pipe modes, or any
outbound TLS client connections. Parsed once at startup; invalid values prevent
the provider from starting.
Expand Down Expand Up @@ -245,6 +271,20 @@ workload_api {
}
```

#### Workload API and Serving Certificate from the Workload API

```hcl
log_level = "debug"
domains = ["mypublicdomain.test"]
serving_cert_source "workload_api" {
addr = ":8443"
}
workload_api {
socket_path = "/tmp/spire-agent/public/api.sock"
trust_domain = "domain.test"
}
```

#### Listening on a Unix Socket

The following configuration has the OIDC Discovery Provider listen for requests
Expand Down
79 changes: 79 additions & 0 deletions support/oidc-discovery-provider/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,24 @@ type Config struct {
// on, for when deployed behind another webserver or sidecar.
ListenSocketPath string `hcl:"listen_socket_path"`

// ServingCertSource is the configuration for the source of the certificate
// used to serve HTTPS. It is a labeled section whose label selects the
// source, e.g. `serving_cert_source "workload_api" {}`. It is required
// unless InsecureAddr or ListenSocketPath is set.
Comment on lines +67 to +70

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"required unless InsecureAddr or ListenSocketPath is set" doesn't match the code, can you update it?

ServingCertSource *ServingCertSourceConfig `hcl:"serving_cert_source"`

// ACME is the ACME configuration. It is required unless InsecureAddr or
// ListenSocketPath is set, or if ServingCertFile is used.
//
// Deprecated: use ServingCertSource with the "acme" label instead. It is
// populated from that section by ParseConfig when it is used.
ACME *ACMEConfig `hcl:"acme"`

// ServingCertFile is the configuration for using a serving certificate to serve HTTPS.
// It is required unless InsecureAddr or ListenSocketPath is set, or if ACME configuration is used.
//
// Deprecated: use ServingCertSource with the "cert_file" label instead. It
// is populated from that section by ParseConfig when it is used.
ServingCertFile *ServingCertFileConfig `hcl:"serving_cert_file"`

// ServerAPI is the configuration for using the SPIRE Server API as the
Expand Down Expand Up @@ -107,6 +119,30 @@ type Config struct {
ServerPathPrefix string `hcl:"server_path_prefix"`
}

// ServingCertSourceConfig holds the configuration of the serving certificate
// source. The label of the serving_cert_source section is decoded into the
// field of the same name. Exactly one source must be configured.
type ServingCertSourceConfig struct {
// ACME obtains the serving certificate via ACME.
ACME *ACMEConfig `hcl:"acme"`
// CertFile loads the serving certificate and key from disk.
CertFile *ServingCertFileConfig `hcl:"cert_file"`
// WorkloadAPI serves an X509-SVID obtained from the SPIFFE Workload API.
WorkloadAPI *ServingCertWorkloadAPIConfig `hcl:"workload_api"`
}

type ServingCertWorkloadAPIConfig struct {
// SocketPath is the path to the Workload API Unix Domain socket. It defaults
// to the socket path of the workload_api section when that is configured.
SocketPath string `hcl:"socket_path"`
// Addr is the address to listen on. This is optional and defaults to ":443".
Addr *net.TCPAddr `hcl:"-"`
// RawAddr holds the string version of the Addr. Consumers should use Addr instead.
RawAddr string `hcl:"addr"`
// Experimental options that are subject to change or removal.
Experimental experimentalWorkloadAPIConfig `hcl:"experimental"`
}

type ServingCertFileConfig struct {
// CertFilePath is the path to the certificate file. The provider will watch
// this file for changes and reload the certificate when it changes.
Expand Down Expand Up @@ -261,6 +297,40 @@ func ParseConfig(hclConfig string) (_ *Config, err error) {
}
c.Domains = dedupeList(c.Domains)

if c.ServingCertSource != nil {
if c.ACME != nil || c.ServingCertFile != nil {
return nil, errors.New("the acme and serving_cert_file sections cannot be used together with the serving_cert_source section")
}
var sourceCount int
for _, configured := range []bool{c.ServingCertSource.ACME != nil, c.ServingCertSource.CertFile != nil, c.ServingCertSource.WorkloadAPI != nil} {
if configured {
sourceCount++
}
}
switch sourceCount {
case 0:
return nil, errors.New(`serving_cert_source must be one of "acme", "cert_file", or "workload_api"`)
case 1:
default:
return nil, errors.New("only one serving_cert_source section can be configured")
}
// The deprecated acme and serving_cert_file sections are the legacy
// spelling of the "acme" and "cert_file" sources. Alias them so the
// rest of the provider has a single place to look.
c.ACME = c.ServingCertSource.ACME

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When the new syntax is used, these errors still point at the old section names. For example, serving_cert_source "cert_file" {} without cert_file_path fails with:

cert_file_path must be configured in the serving_cert_file configuration section

serving_cert_source "acme" {} does the same and mentions "the acme configuration section". The user never wrote those sections, so it can be confusing, especially since the old ones are now deprecated.

A simple fix would be to keep the section name in a variable and use it in these messages:

acmeSection, certFileSection := "acme", "serving_cert_file"
if c.ServingCertSource != nil {
    acmeSection, certFileSection = `serving_cert_source "acme"`, `serving_cert_source "cert_file"`
}

c.ServingCertFile = c.ServingCertSource.CertFile

if workloadAPI := c.ServingCertSource.WorkloadAPI; workloadAPI != nil {
if workloadAPI.RawAddr == "" {
workloadAPI.RawAddr = defaultAddr
}
workloadAPI.Addr, err = net.ResolveTCPAddr("tcp", workloadAPI.RawAddr)
if err != nil {
return nil, fmt.Errorf(`invalid addr in the serving_cert_source "workload_api" configuration section: %w`, err)
}
}
}

if c.ACME != nil {
c.ACME.CacheDir = defaultCacheDir
if c.ACME.RawCacheDir != nil {
Expand Down Expand Up @@ -376,6 +446,15 @@ func ParseConfig(hclConfig string) (_ *Config, err error) {
return c, nil
}

// servingCertWorkloadAPI returns the serving_cert_source "workload_api"
// configuration, or nil if it is not configured.
func (c *Config) servingCertWorkloadAPI() *ServingCertWorkloadAPIConfig {
if c.ServingCertSource == nil {
return nil
}
return c.ServingCertSource.WorkloadAPI
}

func dedupeList(items []string) []string {
keys := make(map[string]bool)
var list []string
Expand Down
90 changes: 90 additions & 0 deletions support/oidc-discovery-provider/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package main

import (
"crypto/tls"
"net"
"os"
"path/filepath"
"runtime"
"testing"
"time"

"github.com/spiffe/spire/pkg/common/tlspolicy"
"github.com/spiffe/spire/test/spiretest"
Expand Down Expand Up @@ -185,3 +187,91 @@ func TestApplyTLSPolicyWithInvalidServerTLSConfig(t *testing.T) {
require.Contains(t, err.Error(), "invalid minTLSVersion")
})
}

func TestParseConfigServingCertSource(t *testing.T) {
serverAPI := "server_api {\n address = \"unix:///some/socket/path\"\n}"
workloadAPI := "workload_api {\n socket_path = \"/some/socket/path\"\n trust_domain = \"domain.test\"\n}"
if runtime.GOOS == "windows" {
serverAPI = "server_api {\n experimental {\n named_pipe_name = \"\\\\name\\\\for\\\\server\\\\api\"\n }\n}"
workloadAPI = "workload_api {\n experimental {\n named_pipe_name = \"\\\\name\\\\for\\\\workload\\\\api\"\n }\n trust_domain = \"domain.test\"\n}"
}
acme := "serving_cert_source \"acme\" {\n email = \"admin@domain.test\"\n tos_accepted = true\n}"

for _, tt := range []struct {
name string
in string
err string
check func(t *testing.T, c *Config)
}{
{
name: "acme source is aliased to the acme section",
in: acme + serverAPI,
check: func(t *testing.T, c *Config) {
require.Equal(t, &ACMEConfig{CacheDir: defaultCacheDir, Email: "admin@domain.test", ToSAccepted: true}, c.ServingCertSource.ACME)
require.Same(t, c.ServingCertSource.ACME, c.ACME)
},
},
{
name: "cert_file source is aliased to the serving_cert_file section",
in: "serving_cert_source \"cert_file\" {\n cert_file_path = \"test.crt\"\n key_file_path = \"test.key\"\n}" + serverAPI,
check: func(t *testing.T, c *Config) {
require.Same(t, c.ServingCertSource.CertFile, c.ServingCertFile)
require.Equal(t, defaultAddr, c.ServingCertFile.RawAddr)
require.Equal(t, time.Minute, c.ServingCertFile.FileSyncInterval)
},
},
{
name: "workload_api source with defaults",
in: "serving_cert_source \"workload_api\" {}" + workloadAPI,
check: func(t *testing.T, c *Config) {
require.Equal(t, &net.TCPAddr{Port: 443}, c.ServingCertSource.WorkloadAPI.Addr)
require.Equal(t, c.WorkloadAPI.SocketPath, c.ServingCertSource.WorkloadAPI.SocketPath)
require.Equal(t, c.WorkloadAPI.Experimental, c.ServingCertSource.WorkloadAPI.Experimental)
require.Nil(t, c.ACME)
require.Nil(t, c.ServingCertFile)
},
},
{
name: "workload_api source with addr",
in: "serving_cert_source \"workload_api\" {\n addr = \"127.0.0.1:9090\"\n}" + workloadAPI,
check: func(t *testing.T, c *Config) {
require.Equal(t, &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 9090}, c.ServingCertSource.WorkloadAPI.Addr)
},
},
{
name: "workload_api source without a Workload API address",
in: "serving_cert_source \"workload_api\" {}" + serverAPI,
err: `must be configured in the serving_cert_source "workload_api" configuration section`,
},
{
name: "workload_api source with insecure_addr",
in: "insecure_addr = \":8080\"\nserving_cert_source \"workload_api\" {}" + workloadAPI,
err: `serving_cert_source "workload_api" is mutually exclusive with insecure_addr`,
},
{
name: "unknown source",
in: "serving_cert_source \"unknown\" {}" + serverAPI,
err: `serving_cert_source must be one of "acme", "cert_file", or "workload_api"`,
},
{
name: "multiple sources",
in: acme + "serving_cert_source \"workload_api\" {}" + workloadAPI,
err: "only one serving_cert_source section can be configured",
},
{
name: "mixed with the deprecated acme section",
in: "acme {\n email = \"admin@domain.test\"\n tos_accepted = true\n}\n" + acme + serverAPI,
err: "the acme and serving_cert_file sections cannot be used together with the serving_cert_source section",
},
} {
t.Run(tt.name, func(t *testing.T) {
c, err := ParseConfig("domains = [\"domain.test\"]\n" + tt.in)
if tt.err != "" {
require.ErrorContains(t, err, tt.err)
return
}
require.NoError(t, err)
tt.check(t, c)
})
}
}
Loading
Loading