scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-10-29 17:52:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: cert auth method watches cert file change and NewCreds() notification (#28126)

Browse Source 
Signed-off-by: Jason Joo <hblzxsj@gmail.com>
This commit is contained in:
Jason Joo
2024-10-03 01:41:55 +08:00
committed by GitHub
parent 159e780d74
commit a5caf4e1cb
9 changed files with 360 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
changelog/28126.txt Normal file
View File

@@ -0,0 +1,6 @@
```release-note:improvement
auto-auth/cert: support watching changes on certificate/key files and notifying the auth handler when `enable_reauth_on_new_credentials` is enabled.
```
```release-note:improvement
auto-auth: support new config option `enable_reauth_on_new_credentials`, supporting re-authentication when receiving new credential on certain auto-auth types
```

2
command/agent/config/config.go
View File

@@ -129,8 +129,6 @@ type AutoAuth struct {
Method *Method `hcl:"-"`
Sinks []*Sink `hcl:"sinks"`
// NOTE: This is unsupported outside of testing and may disappear at any
// time.
EnableReauthOnNewCredentials bool `hcl:"enable_reauth_on_new_credentials"`
}

159
command/agentproxyshared/auth/cert/cert.go
View File

@@ -5,14 +5,21 @@ package cert
import (
"context"
"crypto/tls"
"encoding/hex"
"errors"
"fmt"
"net/http"
"os"
"sync"
"time"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/vault/api"
"github.com/hashicorp/vault/command/agentproxyshared/auth"
"github.com/hashicorp/vault/sdk/helper/consts"
"github.com/hashicorp/vault/sdk/helper/parseutil"
"golang.org/x/crypto/blake2b"
)
type certMethod struct {
@@ -27,6 +34,14 @@ type certMethod struct {
// Client is the cached client to use if cert info was provided.
client *api.Client
stopCh chan struct{}
doneCh chan struct{}
credSuccessGate chan struct{}
ticker *time.Ticker
once *sync.Once
credsFound chan struct{}
latestHash *string
}
var _ auth.AuthMethodWithClient = &certMethod{}
@@ -38,10 +53,17 @@ func NewCertAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) {
// Not concerned if the conf.Config is empty as the 'name'
// parameter is optional when using TLS Auth
lastHash := ""
c := &certMethod{
logger: conf.Logger,
mountPath: conf.MountPath,
stopCh: make(chan struct{}),
doneCh: make(chan struct{}),
credSuccessGate: make(chan struct{}),
once: new(sync.Once),
credsFound: make(chan struct{}),
latestHash: &lastHash,
}
if conf.Config != nil {
@@ -87,6 +109,20 @@ func NewCertAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) {
}
}
if c.isCertConfigured() && c.reload {
reloadPeriod := time.Minute
if reloadPeriodRaw, ok := conf.Config["reload_period"]; ok {
period, err := parseutil.ParseDurationSecond(reloadPeriodRaw)
if err != nil {
return nil, fmt.Errorf("error parsing 'reload_period' value: %w", err)
}
reloadPeriod = period
}
c.ticker = time.NewTicker(reloadPeriod)
go c.runWatcher()
}
return c, nil
}
@@ -103,12 +139,26 @@ func (c *certMethod) Authenticate(_ context.Context, client *api.Client) (string
}
func (c *certMethod) NewCreds() chan struct{} {
return nil
return c.credsFound
}
func (c *certMethod) CredSuccess() {}
func (c *certMethod) CredSuccess() {
c.once.Do(func() {
close(c.credSuccessGate)
})
}
func (c *certMethod) Shutdown() {}
func (c *certMethod) Shutdown() {
if c.isCertConfigured() && c.reload {
c.ticker.Stop()
close(c.stopCh)
<-c.doneCh
}
}
func (c *certMethod) isCertConfigured() bool {
return c.caCert != "" || (c.clientKey != "" && c.clientCert != "")
}
// AuthClient uses the existing client's address and returns a new client with
// the auto-auth method's certificate information if that's provided in its
@@ -118,7 +168,7 @@ func (c *certMethod) AuthClient(client *api.Client) (*api.Client, error) {
clientToAuth := client
if c.caCert != "" || (c.clientKey != "" && c.clientCert != "") {
if c.isCertConfigured() {
// Return cached client if present
if c.client != nil && !c.reload {
return c.client, nil
@@ -141,6 +191,13 @@ func (c *certMethod) AuthClient(client *api.Client) (*api.Client, error) {
return nil, err
}
// set last hash if load it successfully
if hash, err := c.hashCert(c.clientCert, c.clientKey, c.caCert); err != nil {
return nil, err
} else {
c.latestHash = &hash
}
var err error
clientToAuth, err = api.NewClient(config)
if err != nil {
@@ -156,3 +213,95 @@ func (c *certMethod) AuthClient(client *api.Client) (*api.Client, error) {
return clientToAuth, nil
}
// hashCert returns reads and verifies the given cert/key pair and return the hashing result
// in string representation. Otherwise, returns an error.
// As the pair of cert/key and ca cert are optional because they may be configured externally
// or use system default ca bundle, empty paths are simply skipped.
// A valid hashing result means:
// 1. All presented files are readable.
// 2. The client cert/key pair is valid if presented.
// 3. Any presented file in this bundle changed, the hash changes.
func (c *certMethod) hashCert(certFile, keyFile, caFile string) (string, error) {
var buf []byte
if certFile != "" && keyFile != "" {
certPEMBlock, err := os.ReadFile(certFile)
if err != nil {
return "", err
}
c.logger.Debug("Loaded cert file", "file", certFile, "length", len(certPEMBlock))
keyPEMBlock, err := os.ReadFile(keyFile)
if err != nil {
return "", err
}
c.logger.Debug("Loaded key file", "file", keyFile, "length", len(keyPEMBlock))
// verify
_, err = tls.X509KeyPair(certPEMBlock, keyPEMBlock)
if err != nil {
return "", err
}
c.logger.Debug("The cert/key are valid")
buf = append(certPEMBlock, keyPEMBlock...)
}
if caFile != "" {
data, err := os.ReadFile(caFile)
if err != nil {
return "", err
}
c.logger.Debug("Loaded ca file", "file", caFile, "length", len(data))
buf = append(buf, data...)
}
sum := blake2b.Sum256(buf)
return hex.EncodeToString(sum[:]), nil
}
// runWatcher uses polling instead of inotify to sense the changes on the cert/key/ca files.
// The reason not to use inotify:
// 1. To not miss any changes, we need to watch the directory instead of files when using inotify.
// 2. These files are not frequently changed/renewed, and they don't need to be reloaded immediately after renewal.
// 3. Some network based filesystem and FUSE don't support inotify.
func (c *certMethod) runWatcher() {
defer close(c.doneCh)
select {
case <-c.stopCh:
return
case <-c.credSuccessGate:
// We only start the next loop once we're initially successful,
// since at startup Authenticate will be called, and we don't want
// to end up immediately re-authenticating by having found a new
// value
}
for {
changed := false
select {
case <-c.stopCh:
return
case <-c.ticker.C:
c.logger.Debug("Checking if files changed", "cert", c.clientCert, "key", c.clientKey)
hash, err := c.hashCert(c.clientCert, c.clientKey, c.caCert)
// ignore errors in watcher
if err == nil {
c.logger.Debug("hash before/after", "new", hash, "old", *c.latestHash)
changed = *c.latestHash != hash
} else {
c.logger.Warn("hash failed for cert/key files", "err", err)
}
}
if changed {
c.logger.Info("The cert/key files changed")
select {
case c.credsFound <- struct{}{}:
case <-c.stopCh:
}
}
}
}

155
command/agentproxyshared/auth/cert/cert_test.go
View File

@@ -5,10 +5,13 @@ package cert
import (
"context"
"fmt"
"os"
"path"
"path/filepath"
"reflect"
"testing"
"time"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/vault/api"
@@ -28,6 +31,7 @@ func TestCertAuthMethod_Authenticate(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer method.Shutdown()
client, err := api.NewClient(nil)
if err != nil {
@@ -65,6 +69,7 @@ func TestCertAuthMethod_AuthClient_withoutCerts(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer method.Shutdown()
client, err := api.NewClient(api.DefaultConfig())
if err != nil {
@@ -108,6 +113,7 @@ func TestCertAuthMethod_AuthClient_withCerts(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer method.Shutdown()
client, err := api.NewClient(nil)
if err != nil {
@@ -134,29 +140,38 @@ func TestCertAuthMethod_AuthClient_withCerts(t *testing.T) {
}
}
func copyFile(from, to string) error {
data, err := os.ReadFile(from)
if err != nil {
return err
}
return os.WriteFile(to, data, 0o600)
}
// TestCertAuthMethod_AuthClient_withCertsReload makes the file change and ensures the cert auth method deliver the event.
func TestCertAuthMethod_AuthClient_withCertsReload(t *testing.T) {
clientCert, err := os.Open("./test-fixtures/keys/cert.pem")
if err != nil {
t.Fatal(err)
// Initial the cert/key pair to temp path
certPath := filepath.Join(os.TempDir(), "app.crt")
keyPath := filepath.Join(os.TempDir(), "app.key")
if err := copyFile("./test-fixtures/keys/cert.pem", certPath); err != nil {
t.Fatal("copy cert file failed", err)
}
defer clientCert.Close()
clientKey, err := os.Open("./test-fixtures/keys/key.pem")
if err != nil {
t.Fatal(err)
defer os.Remove(certPath)
if err := copyFile("./test-fixtures/keys/key.pem", keyPath); err != nil {
t.Fatal("copy key file failed", err)
}
defer clientKey.Close()
defer os.Remove(keyPath)
config := &auth.AuthConfig{
Logger: hclog.NewNullLogger(),
MountPath: "cert-test",
Config: map[string]interface{}{
"name": "with-certs-reloaded",
"client_cert": clientCert.Name(),
"client_key": clientKey.Name(),
"reload": true,
"name": "with-certs-reloaded",
"client_cert": certPath,
"client_key": keyPath,
"reload": true,
"reload_period": 1,
},
}
@@ -164,6 +179,7 @@ func TestCertAuthMethod_AuthClient_withCertsReload(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer method.Shutdown()
client, err := api.NewClient(nil)
if err != nil {
@@ -188,4 +204,113 @@ func TestCertAuthMethod_AuthClient_withCertsReload(t *testing.T) {
if reloadedClient == clientToUse {
t.Fatal("expected client from AuthClient to return back a new client")
}
method.CredSuccess()
// Only make a change to the cert file, it doesn't match the key file so the client won't pick and load them.
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
if err = copyFile("./test-fixtures/keys/cert1.pem", certPath); err != nil {
t.Fatal("update cert file failed", err)
}
select {
case <-ctx.Done():
case <-method.NewCreds():
cancel()
t.Fatal("malformed cert should not be observed as a change")
}
// Make a change to the key file and now they are good to be picked.
if err = copyFile("./test-fixtures/keys/key1.pem", keyPath); err != nil {
t.Fatal("update key file failed", err)
}
ctx, cancel = context.WithTimeout(context.Background(), 3*time.Second)
select {
case <-ctx.Done():
t.Fatal("failed to watch the cert change: timeout")
case <-method.NewCreds():
cancel()
}
}
// TestCertAuthMethod_hashCert_withEmptyPaths tests hashCert() if it works well with optional options.
func TestCertAuthMethod_hashCert_withEmptyPaths(t *testing.T) {
c := &certMethod{
logger: hclog.NewNullLogger(),
}
// It skips empty file paths
sum, err := c.hashCert("", "", "")
if sum == "" || err != nil {
t.Fatal("hashCert() should skip empty file paths and succeed.")
}
emptySum := sum
// Only present ca cert
sum, err = c.hashCert("", "", "./test-fixtures/root/rootcacert.pem")
if sum == "" || err != nil {
t.Fatal("hashCert() should succeed when only present ca cert.")
}
// Only present client cert/key
sum, err = c.hashCert("./test-fixtures/keys/cert.pem", "./test-fixtures/keys/key.pem", "")
if sum == "" || err != nil {
fmt.Println(sum, err)
t.Fatal("hashCert() should succeed when only present client cert/key.")
}
// The client cert/key should be presented together or will be skipped
sum, err = c.hashCert("./test-fixtures/keys/cert.pem", "", "")
if sum == "" || err != nil {
t.Fatal("hashCert() should succeed when only present client cert.")
} else if sum != emptySum {
t.Fatal("hashCert() should skip the client cert/key when only present client cert.")
}
}
// TestCertAuthMethod_hashCert_withInvalidClientCert adds test cases for invalid input for hashCert().
func TestCertAuthMethod_hashCert_withInvalidClientCert(t *testing.T) {
c := &certMethod{
logger: hclog.NewNullLogger(),
}
// With mismatched cert/key pair
sum, err := c.hashCert("./test-fixtures/keys/cert1.pem", "./test-fixtures/keys/key.pem", "")
if sum != "" || err == nil {
t.Fatal("hashCert() should fail with invalid client cert.")
}
// With non-existed paths
sum, err = c.hashCert("./test-fixtures/keys/cert2.pem", "./test-fixtures/keys/key.pem", "")
if sum != "" || err == nil {
t.Fatal("hashCert() should fail with non-existed client cert path.")
}
}
// TestCertAuthMethod_hashCert_withChange tests hashCert() if it detects changes from both client cert/key and ca cert.
func TestCertAuthMethod_hashCert_withChange(t *testing.T) {
c := &certMethod{
logger: hclog.NewNullLogger(),
}
// A good first case.
sum, err := c.hashCert("./test-fixtures/keys/cert.pem", "./test-fixtures/keys/key.pem", "./test-fixtures/root/rootcacert.pem")
if sum == "" || err != nil {
t.Fatal("hashCert() shouldn't fail with a valid pair of cert/key.")
}
// Only change the ca cert from the first case.
sum1, err := c.hashCert("./test-fixtures/keys/cert.pem", "./test-fixtures/keys/key.pem", "./test-fixtures/keys/cert.pem")
if sum1 == "" || err != nil {
t.Fatal("hashCert() shouldn't fail with valid pair of cert/key.")
} else if sum == sum1 {
t.Fatal("The hash should be different with a different ca cert.")
}
// Only change the cert/key pair from the first case.
sum2, err := c.hashCert("./test-fixtures/keys/cert1.pem", "./test-fixtures/keys/key1.pem", "./test-fixtures/root/rootcacert.pem")
if sum2 == "" || err != nil {
t.Fatal("hashCert() shouldn't fail with a valid cert/key pair")
} else if sum == sum2 || sum1 == sum2 {
t.Fatal("The hash should be different with a different pair of cert/key.")
}
}

17
command/agentproxyshared/auth/cert/test-fixtures/keys/cert1.pem Normal file
View File

@@ -0,0 +1,17 @@
-----BEGIN CERTIFICATE-----
MIICrTCCAZUCCQDDXho7UXdaIjANBgkqhkiG9w0BAQsFADAWMRQwEgYDVQQDEwtl
eGFtcGxlLmNvbTAeFw0yNDA4MTcwOTE0MDRaFw0zNDA4MTUwOTE0MDRaMBsxGTAX
BgNVBAMMEGNlcnQuZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw
ggEKAoIBAQDDSMAi8aL1XLCRrPl8KjJcH/pJe9QJtzUIU3T9tfj+Eq8yMUbFu+so
ec+knsxTi5zN7wq1/t9B9tIvDVG0C9T7BbhX2dYPNC1oY7DtdI3KqA76Z78v533Y
p/WFMHn9X1v0g7qOHm9Y7V6oHg7m+ICq84fORbmfgNW/tPNqTJRU4wyzlIPw1Toi
9awHMZHZmbjUwFgSQ8TOXgZfWo1ZmbOFY2epBIRCapsYpJgwKXy1UjIfQIQ6e6xm
KbKQ/IIeuufo5U8vYV91nGNOVkieeGQ8vmVa1f/oyFfChCRR+aLCqbUGfJWzdicm
eqyQVmPqJxTFuh7WMq+cOX5A068sYj0FAgMBAAEwDQYJKoZIhvcNAQELBQADggEB
AFtUgRS+OZXmDmhIiaw4OrMruz3N2PCjWo/y+rK5gECuApGv7may3k9E65yRUvBb
Ch68y1TMr+7J0MDl1CIbJUnLJkmcID+IvLVS3hVJ9H0raP6epDRvfkM3Xc/RwNgS
PS1H1K8oxDPoo4an1yc6UoKng5KCAUYN+8dR9iVpCIPzRm0LSDIqMyamxoeNLfrO
Nta+sKu1iS/MHy/MVLqyRDwTP2DnfYJTvhQDK5Y5bi7Chkv7g3ug/o2RZ38rRiRd
Os90dDmTCgnYBSJtfKWF5gSnzP+OTs6Yb6KOIY7gLY/r1PBPabSuAnRMS/iTi6tq
l91Cs+vnv6HNcZsGphoQJq8=
-----END CERTIFICATE-----

28
command/agentproxyshared/auth/cert/test-fixtures/keys/key1.pem Normal file
View File

@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDDSMAi8aL1XLCR
rPl8KjJcH/pJe9QJtzUIU3T9tfj+Eq8yMUbFu+soec+knsxTi5zN7wq1/t9B9tIv
DVG0C9T7BbhX2dYPNC1oY7DtdI3KqA76Z78v533Yp/WFMHn9X1v0g7qOHm9Y7V6o
Hg7m+ICq84fORbmfgNW/tPNqTJRU4wyzlIPw1Toi9awHMZHZmbjUwFgSQ8TOXgZf
Wo1ZmbOFY2epBIRCapsYpJgwKXy1UjIfQIQ6e6xmKbKQ/IIeuufo5U8vYV91nGNO
VkieeGQ8vmVa1f/oyFfChCRR+aLCqbUGfJWzdicmeqyQVmPqJxTFuh7WMq+cOX5A
068sYj0FAgMBAAECggEAP1vIMs4xL+g9xVXYsAdExYz+eH77gZd2Vlg1eedjfJN1
UhSYwKjCmCRFUUTQSD7gxhPLZtblepJpCSkKHB9Gn5bwg1hC0jX8kYTer3wEUP8L
tQSaDCHQO83qo6bhvWoF/KQMj/Wh7Lk+3864yQlRPaW7pxoKKozzTLqZyyBDc/KR
YaUco+9NFqClHd/TRehoykYa7OvNVJjBDxTnnxijE0d5w83rP+wDJczhe/Xn/0f1
Q7JFa4NpKmLEXj93GZiteloE80AbVnMiIemGB8ZZGHcySiib3wzuLk32dLS8zguU
gp3E1FhL5xI7gsS7ClA/S6+tK1c46FzQYuIA105tAQKBgQDhnoxCW0N8xV5I1tje
Q1AW+tMVyO6yaREFuoz6tE6AZ1drywyHdqia/QNbXkdllYVQDMlszQ3tmZ8RT/+5
NdJ+LnNag8T6PaN3AtXAf0uveCL1et5ffWuRicesJCCJ10ESFQaVccZEqhJhtnQk
giqICNHV0dWIEVsZGi5R4sA0wQKBgQDdlICpZud2SLrSx00Fb6TfZumxWjDwDb9D
avoQJb376pg1qpAh53hUJbHWPlspeG/k24J0oRrnb3aln8QS21qVov90YllEWwnO
xebYgdjvfOIZ1b8vJ2/UkfLX9Xa9KuzvGpv4BSNOZ8UNHI6Dj/eFmWP+q/a3vzJT
rEgoC1xFRQKBgQCGkZtUxLxnAg1vYn3ta7asTiRyvOrqDNKzWQZXTg34dirlRzGM
5pBACSLkb0Ika98c1NObCl8BVXxTxiRfoqOO0UPKPAfTvcnu5Qj7DLHm0cAALK3P
xK3RG521pcKmlHXiRBouLrM0J0BZeYqib+TQSHpnjwVOaBOu0DfKbXV4wQKBgAaU
VEWzcogGnNWJaXYR3Jltmt7TSMS4A8fis04rcLq8OozNZb47+0y0WdV8wIQ4uUnY
YsVHy1635pQAbHgK32O2FVPFX9UxxtbG9ZXUNTbXRHdz61thFmb/dnCHL2FqluJ6
rcrtjCDV3/oFsQ2jBryG03tKa+cE3F+zq+jUfYbpAoGAauV0h6kqS5mF+sa3F+I1
zIZ7k81r5csZXkgQ6HphIAvo5NSv7H1jeSkGbZmg29irReggZLsy6nU4B4ZVt1p9
GIsLgJfkCkHT+Vf0ipygAwFnbEUKqs6A/D0EUtAF2Oc7nVl0NIX+9LmEx7Dwl34i
bTTPVgw5bid08eiN46NN9J4=
-----END PRIVATE KEY-----

2
command/proxy/config/config.go
View File

@@ -117,8 +117,6 @@ type AutoAuth struct {
Method *Method `hcl:"-"`
Sinks []*Sink `hcl:"sinks"`
// NOTE: This is unsupported outside of testing and may disappear at any
// time.
EnableReauthOnNewCredentials bool `hcl:"enable_reauth_on_new_credentials"`
}

4
website/content/docs/agent-and-proxy/autoauth/index.mdx
View File

@@ -105,6 +105,10 @@ The top level `auto_auth` block has two configuration entries:
- `sinks` `(array of objects: optional)` - Configuration for the sinks
- `enable_reauth_on_new_credentials` `(bool: false)` - If enabled, Auto-auth will
handle new credential events from supported auth methods (AliCloud/AWS/Cert/JWT/LDAP/OCI)
and re-authenticate with the new credential.
### Configuration (Method)
~> Auto-auth does not support using tokens with a limited number of uses. Auto-auth

13
website/content/docs/agent-and-proxy/autoauth/methods/cert.mdx
View File

@@ -31,5 +31,14 @@ config stanza, Agent and Proxy will fall back to using TLS settings from their r
- `client_key` `(string: optional)` - Path on the local disk to a single
PEM-encoded private key matching the client certificate from client_cert.
- `reload` `(bool: optional, default: false)` - If true, causes the local x509 key-pair to be reloaded from disk on each authentication attempt.
This is useful in situations where client certificates are short-lived and automatically renewed.
- `reload` `(bool: optional, default: false)` - If true, causes the local x509
key-pair to be reloaded from disk on each authentication attempt. This is useful
in situations where client certificates are short-lived and automatically renewed.
Note that `enable_reauth_on_new_credentials` for auto-auth will need to be additionally
enabled for immediate re-auth on a new certificate.
See [Auto-Auth Configuration](/vault/docs/agent-and-proxy/autoauth#configuration).
- `reload_period` `(duration: "1m", optional)` - The duration after which auto-auth
will check if there are any changes for the files that are configured through
`ca_cert`/`client_cert`/`client_key`. Defaults to `1m`.
Uses [duration format strings](/vault/docs/concepts/duration-format).