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

feat: Extended Root CA for client TLS connections (#181) #744

Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
818510e
feat: Extended Root CA for upstream connections (#181)
wraix Apr 25, 2021
59307d2
Adds support for appending certificates to the Root CA
wraix Apr 30, 2021
d01b48a
Adds upstream certificate documentation
wraix Apr 30, 2021
c17465a
Remove unused code
wraix May 3, 2021
0be3ef5
proxy: Tests append cert to root ca for upstream
wraix May 3, 2021
5f98726
driver: Adds cacheing of Upstream Transport
wraix May 29, 2021
91c9d9c
docs: Adds ca_refresh_frequency param
wraix May 29, 2021
ad8f3e7
feat: use atomic counter
christian-roggia Jun 8, 2021
8b11c65
feat: allow multiple CA certs and add cache TTL
christian-roggia Jun 8, 2021
d7d9758
feat: disable cache TTL if set to 0
christian-roggia Jun 8, 2021
163401a
fix: do not use system cert pool on windows
christian-roggia Jun 8, 2021
a5397a5
refactor: set certs for all HTTP requests
christian-roggia Jun 9, 2021
b321d46
refactor: better support for TLS client connections
christian-roggia Jun 15, 2021
86c5e92
fix: update docs
christian-roggia Jun 15, 2021
89092df
fix: rename functions and OAuth 2.0 import
christian-roggia Jun 15, 2021
c1110d1
fix: set default array value to null
christian-roggia Jun 15, 2021
dfee24b
fix: do not refresh the transport every request
christian-roggia Jun 20, 2021
3db8369
fix: linting and formatting
christian-roggia Jun 20, 2021
a3645e3
styles: format code
christian-roggia Jun 21, 2021
913ebe9
feat: add tls configuration to token endpoint
christian-roggia Jul 4, 2021
a389da2
fix: set TLS context for token fetching
christian-roggia Jul 4, 2021
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
46 changes: 46 additions & 0 deletions .schema/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,52 @@
"timeout": {
"$ref": "#/definitions/serverTimeout"
},
"client_tls": {
"type": "object",
"title": "Client TLS",
"additionalProperties": false,
"properties": {
"trusted_certificates": {
"type": "array",
"items": {
"type": "string"
},
"default": null,
"examples": [
[
"./self-signed.crt"
]
],
"title": "Trusted Certificates",
"description": "The array of files containing the certificates to trust when opening new TLS connections."
},
"cache": {
"type": "object",
"title": "Certificates Cache",
"additionalProperties": false,
"properties": {
"refresh_frequency": {
"type": "integer",
"default": 1000,
"title": "Refresh Frequency",
"description": "Test if transport needs certificate refresh once every 1000 requests."
},
"ttl": {
"type": "string",
"title": "Expire After",
"description": "Sets the time-to-live of the certificate cache.",
"pattern": "^[0-9]+(ns|us|ms|s|m|h)$",
"default": "5m",
"examples": [
"1h",
"1m",
"30s"
]
}
}
}
}
},
"cors": {
"$ref": "#/definitions/cors"
},
Expand Down
262 changes: 125 additions & 137 deletions CHANGELOG.md

Large diffs are not rendered by default.

2,724 changes: 1,669 additions & 1,055 deletions docs/docs/CHANGELOG.md

Large diffs are not rendered by default.

34 changes: 34 additions & 0 deletions docs/docs/api-access-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,40 @@ authenticators:
}
```

## Client TLS Transport

christian-roggia marked this conversation as resolved.
Show resolved Hide resolved
In case you want to enable TLS connections to services protected by a
self-signed certificate you will very likely need to setup your own custom
certificates. Through the `client_tls` configuration you can set custom
certificates that will be used by oathkeeper to connect to other services such
as the upstream service, authorizers, authenticators, and mutators. The
certificates are cached in memory to reduce latency, you can set the cache
expiration conditions through the `cache` configuration or disable caching
entirely. NOTE: Caching is enabled by default.

Use the `cache.refresh_frequency` to test once every `1000` requests for a
certificate file size or modification timestamp change. Setting the value to 0
will disable the refresh on a frequency functionality, but retrain the update on
file path change functionality.

Use the `cache.ttl` to force the refresh of certificates once every `5m`
regardless of the cache status. Setting this value to 0 will disable forced
cache eviction due to an expired time-to-live.

**oathkeeper.yml**

```yaml
serve:
proxy:
client_tls:
trusted_certificates:
Comment on lines +260 to +263
Copy link
Member

Choose a reason for hiding this comment

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

If this is used for other upstream services (authenticators, ...) I think it would make sense to have client_tls as a root level config item

- /my-certs/certificate-to-append.crt
- /my-certs/another-certificate.crt
cache:
refresh_frequency: 1000
ttl: 5m
```

## Scoped Credentials

Some credentials are scoped. For example, OAuth 2.0 Access Tokens usually are
Expand Down
57 changes: 57 additions & 0 deletions docs/docs/reference/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1538,6 +1538,63 @@ serve:
#
read: 5s

## Client TLS Transport ##
#
# Control the client TLS transport.
#
client_tls:
## Trusted Root CA Certificates ##
#
# The path to a certificate files to append to the Root Certificate Authority for TLS connections. Use this to accept self-signed certificates while keeping the host system certificate authority unaltered.
#
# Default value: ""
#
# Examples:
# - self-signed.crt
#
# Set this value using environment variables on
# - Linux/macOS:
# $ export SERVE_PROXY_CLIENT_TLS_TRUSTED_CERTIFICATES=<value>
# - Windows Command Line (CMD):
# > set SERVE_PROXY_CLIENT_TLS_TRUSTED_CERTIFICATES=<value>
#
trusted_certificates:
- /my-certs/certificate-to-append.crt
- /my-certs/another-certificate.crt

## cache ##
#
cache:
## Refresh Frequency ##
#
# The interval at which to test if the upstream transport certificates changed. Use 0 to disable test.
#
# Default value: 1000
#
# Examples:
# - 1000
# - 0
#
# Set this value using environment variables on
# - Linux/macOS:
# $ export SERVE_PROXY_CLIENT_TLS_CACHE_REFRESH_FREQUENCY=<value>
# - Windows Command Line (CMD):
# > set SERVE_PROXY_CLIENT_TLS_CACHE_REFRESH_FREQUENCY=<value>
#
refresh_frequency: 1000

## Refresh Frequency ##
#
# Sets the time-to-live of the certificate cache. Use 0 to disable cache expiration.
#
# Set this value using environment variables on
# - Linux/macOS:
# $ export SERVE_PROXY_CLIENT_TLS_CACHE_TTL=<value>
# - Windows Command Line (CMD):
# > set SERVE_PROXY_CLIENT_TLS_CACHE_TTL=<value>
#
ttl: 5m

## Cross Origin Resource Sharing (CORS) ##
#
# Configure [Cross Origin Resource Sharing (CORS)](http://www.w3.org/TR/cors/) using the following options.
Expand Down
6 changes: 5 additions & 1 deletion driver/configuration/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import (
"net/url"
"time"

"github.com/gobuffalo/packr/v2"
packr "github.com/gobuffalo/packr/v2"

"github.com/ory/fosite"
"github.com/ory/x/tracing"

Expand Down Expand Up @@ -40,6 +41,9 @@ type Provider interface {
ProxyReadTimeout() time.Duration
ProxyWriteTimeout() time.Duration
ProxyIdleTimeout() time.Duration
ProxyServeClientTLSTrustedCerts() []string
ProxyServeClientTLSCacheRefreshFrequency() int
ProxyServeClientTLSCacheTimeToLive() time.Duration
Comment on lines +44 to +46
Copy link
Member

Choose a reason for hiding this comment

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

If we move the config to the root level we need to change this


APIReadTimeout() time.Duration
APIWriteTimeout() time.Duration
Expand Down
15 changes: 15 additions & 0 deletions driver/configuration/provider_viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const (
ViperKeyProxyIdleTimeout = "serve.proxy.timeout.idle"
ViperKeyProxyServeAddressHost = "serve.proxy.host"
ViperKeyProxyServeAddressPort = "serve.proxy.port"
ViperKeyProxyClientTLSTrustedCerts = "serve.proxy.client_tls.trusted_certificates"
ViperKeyProxyClientTLSCacheRefreshFrequency = "serve.proxy.client_tls.cache.refresh_frequency"
ViperKeyProxyClientTLSCacheTimeToLive = "serve.proxy.client_tls.cache.ttl"
Comment on lines +46 to +48
Copy link
Member

Choose a reason for hiding this comment

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

If we move the config to the root level we need to change this

ViperKeyAPIServeAddressHost = "serve.api.host"
ViperKeyAPIServeAddressPort = "serve.api.port"
ViperKeyAPIReadTimeout = "serve.api.timeout.read"
Expand Down Expand Up @@ -179,6 +182,18 @@ func (v *ViperProvider) ProxyServeAddress() string {
)
}

func (v *ViperProvider) ProxyServeClientTLSTrustedCerts() []string {
return viperx.GetStringSlice(v.l, ViperKeyProxyClientTLSTrustedCerts, nil)
}

func (v *ViperProvider) ProxyServeClientTLSCacheRefreshFrequency() int {
return viperx.GetInt(v.l, ViperKeyProxyClientTLSCacheRefreshFrequency, 1000)
}

func (v *ViperProvider) ProxyServeClientTLSCacheTimeToLive() time.Duration {
return viperx.GetDuration(v.l, ViperKeyProxyClientTLSCacheTimeToLive, time.Minute*5)
}

func (v *ViperProvider) APIReadTimeout() time.Duration {
return viperx.GetDuration(v.l, ViperKeyAPIReadTimeout, time.Second*5)
}
Expand Down
183 changes: 183 additions & 0 deletions internal/certs/cert_manager.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
package certs

import (
"crypto/x509"
"io/ioutil"
"os"
"runtime"
"sync"
"sync/atomic"
"time"

"github.com/pkg/errors"

"github.com/ory/oathkeeper/driver/configuration"
)

type CertManager struct {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add some tests to the certmanager? Also, what should we take care of here. For example, test different cert pools? How would this look like on Windows? :)

I haven't worked with x509 cert pools before. We should also generally ensure that:

  1. The cache is used
  2. The cache is invalidated properly
  3. No untrusted certs are being used
  4. The OS cert store is being used appropriately on the different OS'es (could be difficult for mac as we don't have a pipeline for that...)

c configuration.Provider

// isWindows is set to true if the application is running in a Windows
// environment. This information is used to decide whether the system
// certificate pool should be loaded or not as Windows does not support
// this feature and will return an error.
isWindows bool

// cachePool is used to optimize performance and to avoid reloading the cert
// pool at every request. The cache expires after its time-to-live has ended
// or the an external action has invalidated the cache itself.
cachePool *x509.CertPool

// cacheTime contains the last update of the cache and is used to check
// whether it has reached the end of its time-to-live. This value is used for
// caching purposes.
cacheTime time.Time

// cacheCerts contains the cached certificates and all file information
// associated with them such as size and last modification time.
cacheCerts sync.Map

// requests is an atomic counter that keeps track of how many requests have
// been served since the last certificate refresh. This value is used for
// caching purposes.
requests int64
}

type CertCache struct {
path string
stat *os.FileInfo
}

func NewCertManager(c configuration.Provider) *CertManager {
return &CertManager{
c: c,
isWindows: runtime.GOOS == "windows",
}
}

func (cm *CertManager) systemCertPool() (*x509.CertPool, error) {
if cm.isWindows {
return x509.NewCertPool(), nil
christian-roggia marked this conversation as resolved.
Show resolved Hide resolved
}

return x509.SystemCertPool()
}

func (cm *CertManager) additionalCerts() ([][]byte, error) {
var certs [][]byte
for _, cert := range cm.c.ProxyServeClientTLSTrustedCerts() {
data, err := ioutil.ReadFile(cert)
if err != nil {
return nil, err
}

stat, err := os.Stat(cert)
if err != nil {
return nil, err
}

cm.cacheCerts.Store(cert, &CertCache{
path: cert,
stat: &stat,
})

certs = append(certs, data)
}

return certs, nil
}

func (cm *CertManager) cacheIsExpired() bool {
if cm.c.ProxyServeClientTLSCacheTimeToLive() == 0 {
return false
}

// If the last time the transport was accessed is beyond the cache TTL then
// a refresh of the cache is forced whether the file was updated or not.
// This ensures that the cache is always refreshed at fixed intervals
// regardless of the environment. If the TTL is set to 0 skip the check.
return time.Since(cm.cacheTime) > cm.c.ProxyServeClientTLSCacheTimeToLive()
}

func (cm *CertManager) lookupCertificate(cert string) (*CertCache, bool) {
cacheItem, ok := cm.cacheCerts.Load(cert)
if !ok {
return nil, false
}

cache, ok := cacheItem.(*CertCache)
return cache, ok
}

func (cm *CertManager) isCertificateValid(cert string) (bool, error) {
cache, ok := cm.lookupCertificate(cert)
if !ok {
return false, nil
}

refreshFrequency := cm.c.ProxyServeClientTLSCacheRefreshFrequency()
if refreshFrequency <= 0 {
return true, nil
}

if cm.requests%int64(refreshFrequency) != 0 {
return true, nil
}

stat, err := os.Stat(cert)
if err != nil {
return false, err
}

if stat.Size() != (*cache.stat).Size() || stat.ModTime() != (*cache.stat).ModTime() {
return true, nil
}

return false, nil
}

func (cm *CertManager) cacheIsValid() (bool, error) {
for _, cert := range cm.c.ProxyServeClientTLSTrustedCerts() {
valid, err := cm.isCertificateValid(cert)
if err != nil {
return false, err
}

if !valid {
return false, nil
}
}

return true, nil
}

func (cm *CertManager) CertPool() (*x509.CertPool, error) {
atomic.AddInt64(&cm.requests, 1)

cacheValid, err := cm.cacheIsValid()
if err != nil {
return nil, err
}

if !cm.cacheIsExpired() && cacheValid {
return cm.cachePool, nil
}

certs, err := cm.additionalCerts()
if err != nil {
return nil, err
}

pool, err := cm.systemCertPool()
if err != nil {
return nil, err
}

for _, data := range certs {
if ok := pool.AppendCertsFromPEM(data); !ok {
return nil, errors.New("No certs appended, only system certs present, did you specify the correct cert file?")
}
}

return pool, nil
}
Loading