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

Do not store local service account token and CA to config. #122

Merged
merged 11 commits into from
Jan 7, 2022
60 changes: 58 additions & 2 deletions backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"strings"
"sync"
"time"

"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/logical"
Expand All @@ -27,6 +28,18 @@ var (
// when adding new alias name sources make sure to update the corresponding FieldSchema description in path_role.go
aliasNameSources = []string{aliasNameSourceSAUid, aliasNameSourceSAName}
errInvalidAliasNameSource = fmt.Errorf(`invalid alias_name_source, must be one of: %s`, strings.Join(aliasNameSources, ", "))

// jwtReloadPeriod is the time period how often the in-memory copy of local
// service account token can be used, before reading it again from disk.
//
// The value is selected according to recommendation in Kubernetes 1.21 changelog:
// "Clients should reload the token from disk periodically (once per minute
// is recommended) to ensure they continue to use a valid token."
jwtReloadPeriod = 1 * time.Minute
tomhjp marked this conversation as resolved.
Show resolved Hide resolved

// caReloadPeriod is the time period how often the in-memory copy of local
// CA cert can be used, before reading it again from disk.
caReloadPeriod = 1 * time.Hour
)

// kubeAuthBackend implements logical.Backend
Expand All @@ -38,6 +51,19 @@ type kubeAuthBackend struct {
// review. Mocks should only be used in tests.
reviewFactory tokenReviewFactory

// localSATokenReader caches the service account token in memory.
// It periodically reloads the token to support token rotation/renewal.
// Local token is used when running in a pod with following configuration
// - token_reviewer_jwt is not set
// - disable_local_ca_jwt is false
localSATokenReader *cachingFileReader

// localCACertReader contains the local CA certificate. Local CA certificate is
// used when running in a pod with following configuration
// - kubernetes_ca_cert is not set
// - disable_local_ca_jwt is false
localCACertReader *cachingFileReader

l sync.RWMutex
}

Expand All @@ -51,7 +77,10 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend,
}

func Backend() *kubeAuthBackend {
b := &kubeAuthBackend{}
b := &kubeAuthBackend{
localSATokenReader: newCachingFileReader(localJWTPath, jwtReloadPeriod, time.Now),
localCACertReader: newCachingFileReader(localCACertPath, caReloadPeriod, time.Now),
}

b.Backend = &framework.Backend{
AuthRenew: b.pathLoginRenew(),
Expand Down Expand Up @@ -80,7 +109,8 @@ func Backend() *kubeAuthBackend {
return b
}

// config takes a storage object and returns a kubeConfig object
// config takes a storage object and returns a kubeConfig object.
tsaarni marked this conversation as resolved.
Show resolved Hide resolved
// It does not return local token and CA file which are specific to the pod we run in.
func (b *kubeAuthBackend) config(ctx context.Context, s logical.Storage) (*kubeConfig, error) {
raw, err := s.Get(ctx, configPath)
if err != nil {
Expand All @@ -107,6 +137,8 @@ func (b *kubeAuthBackend) config(ctx context.Context, s logical.Storage) (*kubeC
return conf, nil
}

// loadConfig fetches the kubeConfig from storage and optionally decorates it with
// local token and CA certificate.
func (b *kubeAuthBackend) loadConfig(ctx context.Context, s logical.Storage) (*kubeConfig, error) {
config, err := b.config(ctx, s)
if err != nil {
Expand All @@ -115,6 +147,30 @@ func (b *kubeAuthBackend) loadConfig(ctx context.Context, s logical.Storage) (*k
if config == nil {
return nil, errors.New("could not load backend configuration")
}

// Nothing more to do if loading local CA cert and JWT token is disabled.
if config.DisableLocalCAJwt {
return config, nil
}

// Read local JWT token unless it was not stored in config.
if config.TokenReviewerJWT == "" {
config.TokenReviewerJWT, err = b.localSATokenReader.ReadFile()
if err != nil {
// Ignore error: make best effort trying to load local JWT,
// otherwise the JWT submitted in login payload will be used.
b.Logger().Debug("failed to read local service account token, will use client token", "error", err)
}
}

// Read local CA cert unless it was stored in config.
if config.CACert == "" {
config.CACert, err = b.localCACertReader.ReadFile()
if err != nil {
return nil, err
}
}

return config, nil
}

Expand Down
68 changes: 68 additions & 0 deletions caching_file_reader.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package kubeauth

import (
"io/ioutil"
"sync"
"time"
)

// cachingFileReader reads a file and keeps an in-memory copy of it, until the
// copy is considered stale. Next ReadFile() after expiry will re-read the file from disk.
type cachingFileReader struct {
// path is the file path to the cached file.
path string

// ttl is the time-to-live duration when cached file is considered stale
ttl time.Duration

// cache is the buffer holding the in-memory copy of the file.
cache cachedFile

l sync.RWMutex

// currentTime is a function that returns the current local time.
// Normally set to time.Now but it can be overwritten by test cases to manipulate time.
currentTime func() time.Time
}

type cachedFile struct {
// buf is the buffer holding the in-memory copy of the file.
buf string

// expiry is the time when the cached copy is considered stale and must be re-read.
expiry time.Time
}

func newCachingFileReader(path string, ttl time.Duration, currentTime func() time.Time) *cachingFileReader {
return &cachingFileReader{
path: path,
ttl: ttl,
currentTime: currentTime,
}
}

func (r *cachingFileReader) ReadFile() (string, error) {
// Fast path requiring read lock only: file is already in memory and not stale.
r.l.RLock()
now := r.currentTime()
cache := r.cache
tsaarni marked this conversation as resolved.
Show resolved Hide resolved
r.l.RUnlock()
if now.Before(cache.expiry) {
return cache.buf, nil
}

// Slow path: read the file from disk.
r.l.Lock()
defer r.l.Unlock()

buf, err := ioutil.ReadFile(r.path)
if err != nil {
return "", err
}
r.cache = cachedFile{
buf: string(buf),
expiry: now.Add(r.ttl),
}

return r.cache.buf, nil
}
65 changes: 65 additions & 0 deletions caching_file_reader_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package kubeauth

import (
"io/ioutil"
"os"
"testing"
"time"
)

func TestCachingFileReader(t *testing.T) {
content1 := "before"
content2 := "after"

// Create temporary file.
f, err := ioutil.TempFile("", "testfile")
if err != nil {
t.Error(err)
}
f.Close()
defer os.Remove(f.Name())

currentTime := time.Now()

r := newCachingFileReader(f.Name(), 1*time.Minute,
func() time.Time {
return currentTime
})

// Write initial content to file and check that we can read it.
ioutil.WriteFile(f.Name(), []byte(content1), 0644)
got, err := r.ReadFile()
if err != nil {
t.Error(err)
}
if got != content1 {
t.Errorf("got '%s', expected '%s'", got, content1)
}

// Write new content to the file.
ioutil.WriteFile(f.Name(), []byte(content2), 0644)

// Advance simulated time, but not enough for cache to expire.
currentTime = currentTime.Add(30 * time.Second)

// Read again and check we still got the old cached content.
got, err = r.ReadFile()
if err != nil {
t.Error(err)
}
if got != content1 {
t.Errorf("got '%s', expected '%s'", got, content1)
}

// Advance simulated time for cache to expire.
currentTime = currentTime.Add(30 * time.Second)

// Read again and check that we got the new content.
got, err = r.ReadFile()
if err != nil {
t.Error(err)
}
if got != content2 {
t.Errorf("got '%s', expected '%s'", got, content2)
}
}
26 changes: 6 additions & 20 deletions path_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ import (
"crypto/x509"
"encoding/pem"
"errors"
"io/ioutil"

"github.com/briankassouf/jose/jws"
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/logical"
)

var (
const (
localCACertPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
localJWTPath = "/var/run/secrets/kubernetes.io/serviceaccount/token"
)
Expand Down Expand Up @@ -122,37 +121,24 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ
}

disableLocalJWT := data.Get("disable_local_ca_jwt").(bool)
localCACert := []byte{}
localTokenReviewer := []byte{}
if !disableLocalJWT {
localCACert, _ = ioutil.ReadFile(localCACertPath)
localTokenReviewer, _ = ioutil.ReadFile(localJWTPath)
}
pemList := data.Get("pem_keys").([]string)
caCert := data.Get("kubernetes_ca_cert").(string)
issuer := data.Get("issuer").(string)
disableIssValidation := data.Get("disable_iss_validation").(bool)
if len(pemList) == 0 && len(caCert) == 0 {
tomhjp marked this conversation as resolved.
Show resolved Hide resolved
if len(localCACert) > 0 {
caCert = string(localCACert)
} else {
return logical.ErrorResponse("one of pem_keys or kubernetes_ca_cert must be set"), nil
}
}

tokenReviewer := data.Get("token_reviewer_jwt").(string)
if !disableLocalJWT && len(tokenReviewer) == 0 && len(localTokenReviewer) > 0 {
tokenReviewer = string(localTokenReviewer)
}

if len(tokenReviewer) > 0 {
if tokenReviewer != "" {
// Validate it's a JWT
_, err := jws.ParseJWT([]byte(tokenReviewer))
if err != nil {
return nil, err
}
}

if disableLocalJWT && caCert == "" {
return logical.ErrorResponse("kubernetes_ca_cert must be given when disable_local_ca_jwt is true"), nil
}

config := &kubeConfig{
PublicKeys: make([]interface{}, len(pemList)),
PEMKeys: pemList,
Expand Down
Loading