fix: cert auth method watches cert file change and NewCreds() notific…

…ation (#28126)

Signed-off-by: Jason Joo <[email protected]>

changelog/28126.txt

@@ -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
+```

command/agent/config/config.go

@@ -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"`
 }
 

command/agentproxyshared/auth/cert/cert.go

@@ -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:
+			}
+		}
+	}
+}