Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d7513e0
Add disk cert manager
guilhermocc May 29, 2023
9a8c675
Add serving_cert_file config to oidc discovery provider in unix
guilhermocc May 29, 2023
cbef8cf
Add serving cert file config for windows
guilhermocc May 30, 2023
125d274
Fix update tests on linux
guilhermocc May 30, 2023
e5e56a9
Fix rebase conflicts
guilhermocc May 30, 2023
67b20fb
Update documentation
guilhermocc May 30, 2023
eddf53c
Fix race condition
guilhermocc May 30, 2023
91a7695
Fix lint in windows
guilhermocc May 30, 2023
c02a6da
Fix timed assertions
guilhermocc May 30, 2023
915b914
Fix windows test
guilhermocc May 30, 2023
09acd52
Print cert manager logs on failed test
guilhermocc May 31, 2023
fcba21a
Remove fsnotify usage
guilhermocc Jun 1, 2023
fb9bd71
Refactor tests
guilhermocc Jun 1, 2023
61d8e65
Fix tests on windows
guilhermocc Jun 1, 2023
e43b173
Fix file sync
guilhermocc Jun 1, 2023
af12dbe
Make file sync interval configurable
guilhermocc Jun 1, 2023
75123b3
Refactor some comments
guilhermocc Jun 1, 2023
e4f2e0c
Address comments for unix platform
guilhermocc Jun 7, 2023
85abb9a
Address comments for windows platform
guilhermocc Jun 7, 2023
50eb322
Address comments to use context for ending goroutines
guilhermocc Jun 9, 2023
9f5da60
Fix tests on windows
guilhermocc Jun 12, 2023
3d36f1f
Address PR comments
guilhermocc Jun 13, 2023
d47e082
Enable user to select the address for serving the https service
guilhermocc Jun 22, 2023
8cacc0a
Fix lint
guilhermocc Jun 23, 2023
996e427
Fix windows tests
guilhermocc Jun 23, 2023
dceafe0
Update default HTTPS port
guilhermocc Jun 30, 2023
c18537e
Fix flaky tests
guilhermocc Jun 30, 2023
c9c3612
Remove duplicated documentation
guilhermocc Jul 3, 2023
4b2b816
Merge branch 'main' into serving-cert-file-oidc-provider
rturner3 Jul 3, 2023
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
6 changes: 6 additions & 0 deletions pkg/common/telemetry/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,12 @@ const (
// to add clarity
CallerPath = "caller_path"

// CertFilePath tags a certificate file path used for TLS connections.
CertFilePath = "cert_file_path"

// KeyFilePath tags a key file path used for TLS connections.
KeyFilePath = "key_file_path"

// CGroupPath tags a linux CGroup path, most likely for use in attestation
CGroupPath = "cgroup_path"

Expand Down
87 changes: 81 additions & 6 deletions support/oidc-discovery-provider/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ 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. | |
| `allow_insecure_scheme` | string | optional[3] | Serves OIDC configuration response with HTTP url. | `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 All @@ -56,13 +57,13 @@ The configuration file is **required** by the provider. It contains

#### Considerations for Unix platforms

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

[3]: The `allow_insecure_scheme` should only be used in a local development environment for testing purposes. It only works in conjunction with `insecure_addr` or `listen_socket_path`.

#### Considerations for Windows platforms

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

Choose a reason for hiding this comment

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

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


[3]: The `allow_insecure_scheme` should only be used in a local development environment for testing purposes. It only works in conjunction with `insecure_addr` or `listen_named_pipe_name`.

Expand All @@ -77,6 +78,8 @@ will be rejected. Likewise, when ACME is used, the `domains` list contains the
allowed domains for which certificates will be obtained. The TLS handshake
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.

#### ACME Section

| Key | Type | Required? | Description | Default |
Expand All @@ -86,6 +89,16 @@ will terminate if another domain is requested.
| `email` | string | required | The email address used to register with the ACME service | |
| `tos_accepted` | bool | required | Indicates explicit acceptance of the ACME service Terms of Service. Must be true. | |

#### Serving Certificate Section

| Key | Type | Required? | Description | Default |
|----------------------|----------|-----------|--------------------------------------------------------------------|------------|
| `cert_file_path` | string | required | The certificate file path, the file must contain PEM encoded data. | |
| `key_file_path` | string | required | The private key file path, the file must contain PEM encoded data. | |
| `file_sync_interval` | duration | optional | Controls how frequently the service polls the files for changes. | 1 minute |
| `file_sync_interval` | duration | optional | Controls how frequently the service polls the files for changes. | 1 minute |
| `addr` | string | optional | Exposes the service on the given address. | :433 |
Copy link
Member

Choose a reason for hiding this comment

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

443 should be the default (multiple places to fix)


#### Server API Section

| Key | Type | Required? | Description | Default |
Expand Down Expand Up @@ -130,7 +143,7 @@ Both states respond with a 200 OK status code for success or 500 Internal Server

### Examples (Unix platforms)

#### Server API
#### Server API and ACME

```hcl
log_level = "debug"
Expand All @@ -145,7 +158,7 @@ server_api {
}
```

#### Workload API
#### Workload API and ACME

```hcl
log_level = "debug"
Expand All @@ -161,6 +174,35 @@ workload_api {
}
```

#### Server API and Serving Certificate

```hcl
log_level = "debug"
domains = ["mypublicdomain.test"]
serving_cert_file {
cert_file_path = "/some/path/on/disk/to/cert.pem"
key_file_path = "/some/path/on/disk/to/key.pem"
}
server_api {
address = "unix:///tmp/spire-server/private/api.sock"
}
```

#### Workload API and Serving Certificate

```hcl
log_level = "debug"
domains = ["mypublicdomain.test"]
serving_cert_file {
cert_file_path = "/some/path/on/disk/to/cert.pem"
key_file_path = "/some/path/on/disk/to/key.pem"
}
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 Expand Up @@ -200,7 +242,7 @@ daemon off;

### Examples (Windows)

#### Server API
#### Server API and ACME

```hcl
log_level = "debug"
Expand All @@ -217,7 +259,7 @@ server_api {
}
```

#### Workload API
#### Workload API and ACME

```hcl
log_level = "debug"
Expand All @@ -235,6 +277,39 @@ workload_api {
}
```

#### Server API and Serving Certificate

```hcl
log_level = "debug"
domains = ["mypublicdomain.test"]
serving_cert_file {
cert_file_path = "c:\\some\\path\\on\\disk\\to\\cert.pem"
key_file_path = "c:\\some\\path\\on\\disk\\to\\key.pem"
}
server_api {
experimental {
named_pipe_name = "\\spire-server\\private\\api"
}
}
```

#### Workload API and Serving Certificate

```hcl
log_level = "debug"
domains = ["mypublicdomain.test"]
serving_cert_file {
cert_file_path = "c:\\some\\path\\on\\disk\\to\\cert.pem"
key_file_path = "c:\\some\\path\\on\\disk\\to\\key.pem"
}
workload_api {
experimental {
named_pipe_name = "\\spire-agent\\public\\api"
}
trust_domain = "domain.test"
}
```

#### Listening on a Named Pipe

The following configuration has the OIDC Discovery Provider listen for requests
Expand Down
161 changes: 161 additions & 0 deletions support/oidc-discovery-provider/cert_manager.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package main

import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io/fs"
"os"
"sync"
"time"

"github.com/andres-erbsen/clock"
"github.com/sirupsen/logrus"
)

// DiskCertManager is a certificate manager that loads certificates from disk, and watches for changes.
type DiskCertManager struct {
certFilePath string
keyFilePath string
certLastModified time.Time
keyLastModified time.Time
fileSyncInterval time.Duration
certMtx sync.RWMutex
cert *tls.Certificate
clk clock.Clock
log logrus.FieldLogger
}

func NewDiskCertManager(config *ServingCertFileConfig, clk clock.Clock, log logrus.FieldLogger) (*DiskCertManager, error) {
if config == nil {
return nil, errors.New("missing serving cert file configuration")
}

if clk == nil {
clk = clock.New()
}

dm := &DiskCertManager{
certFilePath: config.CertFilePath,
keyFilePath: config.KeyFilePath,
fileSyncInterval: config.FileSyncInterval,
log: log,
clk: clk,
}

if err := dm.loadCert(); err != nil {
return nil, fmt.Errorf("failed to load certificate: %w", err)
}

return dm, nil
}

// TLSConfig returns a TLS configuration that uses the provided certificate stored on disk.
func (m *DiskCertManager) TLSConfig() *tls.Config {
return &tls.Config{
GetCertificate: m.getCertificate,
NextProtos: []string{
"h2", "http/1.1", // enable HTTP/2
},
MinVersion: tls.VersionTLS12,
}
}

// getCertificate is called by the TLS stack when a new TLS connection is established.
func (m *DiskCertManager) getCertificate(chInfo *tls.ClientHelloInfo) (*tls.Certificate, error) {
m.certMtx.RLock()
defer m.certMtx.RUnlock()
cert := m.cert

return cert, nil
}

// WatchFileChanges starts a file watcher to watch for changes to the cert and key files.
func (m *DiskCertManager) WatchFileChanges(ctx context.Context) {
m.log.WithField("interval", m.fileSyncInterval).Info("Started watching certificate files")
ticker := m.clk.Ticker(m.fileSyncInterval)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
m.log.Info("Stopping file watcher")
return
case <-ticker.C:
m.syncCertificateFiles()
}
}
}

// syncCertificateFiles checks if the cert and key files have been modified, and reloads the certificate if necessary.
func (m *DiskCertManager) syncCertificateFiles() {
certFileInfo, keyFileInfo, err := m.getFilesInfo()
if err != nil {
return
}

if certFileInfo.ModTime() != m.certLastModified || keyFileInfo.ModTime() != m.keyLastModified {
m.log.Info("File change detected, reloading certificate and key...")

if err := m.loadCert(); err != nil {
m.log.Errorf("Failed to load certificate: %v", err)
} else {
m.certLastModified = certFileInfo.ModTime()
m.keyLastModified = keyFileInfo.ModTime()
m.log.Info("Loaded provided certificate with success")
}
}
}

// loadCert read the certificate and key files, and load the x509 certificate to memory.
func (m *DiskCertManager) loadCert() error {
cert, err := tls.LoadX509KeyPair(m.certFilePath, m.keyFilePath)
if err != nil {
return err
}

cert.Leaf, err = x509.ParseCertificate(cert.Certificate[0])
if err != nil {
return err
}

m.certMtx.Lock()
defer m.certMtx.Unlock()

m.cert = &cert

return nil
}

// getFilesInfo returns the file info of the cert and key files, or error if the files are unreadable or do not exist.
func (m *DiskCertManager) getFilesInfo() (os.FileInfo, os.FileInfo, error) {
certFileInfo, err := m.getFileInfo(m.certFilePath)
if err != nil {
return nil, nil, err
}

keyFileInfo, err := m.getFileInfo(m.keyFilePath)
if err != nil {
return nil, nil, err
}

return certFileInfo, keyFileInfo, nil
}

// getFileInfo returns the file info of the given path, or error if the file is unreadable or does not exist.
func (m *DiskCertManager) getFileInfo(path string) (os.FileInfo, error) {
fileInfo, err := os.Stat(path)
if err != nil {
errFs := new(fs.PathError)
switch {
case errors.Is(err, fs.ErrNotExist) && errors.As(err, &errFs):
m.log.Errorf("Failed to get file info, file path %q does not exist anymore; please check if the path is correct", errFs.Path)
default:
m.log.Errorf("Failed to get file info: %v", err)
}
return nil, err
}

return fileInfo, nil
}
Loading