Merge branch 'main' into rego-v1/docs

cmd/run.go

@@ -282,6 +282,7 @@ func initRuntime(ctx context.Context, params runCmdParams, args []string, addrSe
 	params.rt.CertificateFile = params.tlsCertFile
 	params.rt.CertificateKeyFile = params.tlsPrivateKeyFile
 	params.rt.CertificateRefresh = params.tlsCertRefresh
+	params.rt.CertPoolFile = params.tlsCACertFile
 
 	if params.tlsCACertFile != "" {
 		pool, err := loadCertPool(params.tlsCACertFile)

go.mod

@@ -17,7 +17,7 @@ require (
 	github.com/gobwas/glob v0.2.3
 	github.com/golang/protobuf v1.5.3
 	github.com/google/go-cmp v0.6.0
-	github.com/google/uuid v1.4.0
+	github.com/google/uuid v1.5.0
 	github.com/gorilla/mux v1.8.1
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/opencontainers/go-digest v1.0.0

go.sum

@@ -116,8 +116,8 @@ github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
-github.com/google/uuid v1.4.0 h1:MtMxsa51/r9yyhkyLsVeVt0B+BGQZzpQiTQ4eHZ8bc4=
-github.com/google/uuid v1.4.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/google/uuid v1.5.0 h1:1p67kYwdtXjb0gL0BPiP1Av9wiZPo5A8z2cWkTZ+eyU=
+github.com/google/uuid v1.5.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
 github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 h1:YBftPWNWd4WwGqtY2yeZL2ef8rHAxPBD8KFhJpmcqms=

runtime/runtime.go

@@ -23,20 +23,20 @@ import (
 
 	"github.com/fsnotify/fsnotify"
 	"github.com/gorilla/mux"
-	"github.com/open-policy-agent/opa/internal/compiler"
-	"github.com/open-policy-agent/opa/internal/pathwatcher"
-	"github.com/open-policy-agent/opa/internal/ref"
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace"
 	"go.opentelemetry.io/otel/propagation"
 	"go.uber.org/automaxprocs/maxprocs"
 
 	"github.com/open-policy-agent/opa/bundle"
 	opa_config "github.com/open-policy-agent/opa/config"
+	"github.com/open-policy-agent/opa/internal/compiler"
 	"github.com/open-policy-agent/opa/internal/config"
 	internal_tracing "github.com/open-policy-agent/opa/internal/distributedtracing"
 	internal_logging "github.com/open-policy-agent/opa/internal/logging"
+	"github.com/open-policy-agent/opa/internal/pathwatcher"
 	"github.com/open-policy-agent/opa/internal/prometheus"
+	"github.com/open-policy-agent/opa/internal/ref"
 	"github.com/open-policy-agent/opa/internal/report"
 	"github.com/open-policy-agent/opa/internal/runtime"
 	initload "github.com/open-policy-agent/opa/internal/runtime/init"
@@ -115,6 +115,8 @@ type Params struct {
 
 	// CertPool holds the CA certs trusted by the OPA server.
 	CertPool *x509.CertPool
+	// CertPoolFile, if set permits the reloading of the CA cert pool from disk
+	CertPoolFile string
 
 	// MinVersion contains the minimum TLS version that is acceptable.
 	// If zero, TLS 1.2 is currently taken as the minimum.
@@ -537,8 +539,8 @@ func (rt *Runtime) Serve(ctx context.Context) error {
 		WithPprofEnabled(rt.Params.PprofEnabled).
 		WithAddresses(*rt.Params.Addrs).
 		WithH2CEnabled(rt.Params.H2CEnabled).
+		// always use the initial values for the certificate and ca pool, reloading behavior is configured below
 		WithCertificate(rt.Params.Certificate).
-		WithCertificatePaths(rt.Params.CertificateFile, rt.Params.CertificateKeyFile, rt.Params.CertificateRefresh).
 		WithCertPool(rt.Params.CertPool).
 		WithAuthentication(rt.Params.Authentication).
 		WithAuthorization(rt.Params.Authorization).
@@ -562,6 +564,22 @@ func (rt *Runtime) Serve(ctx context.Context) error {
 		rt.server = rt.server.WithUnixSocketPermission(rt.Params.UnixSocketPerm)
 	}
 
+	// If a refresh period is set, then we will periodically reload the certificate and ca pool. Otherwise, we will only
+	// reload cert, key and ca pool files when they change on disk.
+	if rt.Params.CertificateRefresh > 0 {
+		rt.server = rt.server.WithCertRefresh(rt.Params.CertificateRefresh)
+	}
+
+	// if either the cert or the ca pool file is set then these fields will be set on the server and reloaded when they
+	// change on disk.
+	if rt.Params.CertificateFile != "" || rt.Params.CertPoolFile != "" {
+		rt.server = rt.server.WithTLSConfig(&server.TLSConfig{
+			CertFile:     rt.Params.CertificateFile,
+			KeyFile:      rt.Params.CertificateKeyFile,
+			CertPoolFile: rt.Params.CertPoolFile,
+		})
+	}
+
 	rt.server, err = rt.server.Init(ctx)
 	if err != nil {
 		rt.logger.WithFields(map[string]interface{}{"err": err}).Error("Unable to initialize server.")

server/certs.go

@@ -8,52 +8,175 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"crypto/tls"
+	"crypto/x509"
+	"fmt"
 	"io"
 	"os"
 	"time"
 
+	"github.com/fsnotify/fsnotify"
+
+	"github.com/open-policy-agent/opa/internal/errors"
+	"github.com/open-policy-agent/opa/internal/pathwatcher"
 	"github.com/open-policy-agent/opa/logging"
 )
 
 func (s *Server) getCertificate(h *tls.ClientHelloInfo) (*tls.Certificate, error) {
-	s.certMtx.RLock()
-	defer s.certMtx.RUnlock()
+	s.tlsConfigMtx.RLock()
+	defer s.tlsConfigMtx.RUnlock()
 	return s.cert, nil
 }
 
-func (s *Server) certLoop(logger logging.Logger) Loop {
+// reloadTLSConfig reloads the TLS config if the cert, key files or cert pool contents have changed.
+func (s *Server) reloadTLSConfig(logger logging.Logger) error {
+	s.tlsConfigMtx.Lock()
+	defer s.tlsConfigMtx.Unlock()
+
+	// reloading of the certificate key pair and the CA pool are independent operations,
+	// though errors from either operation are aggregated.
+	var errs error
+
+	// if the server has a cert configured, then we need to check the cert and key for changes.
+	if s.certFile != "" {
+		newCert, certFileHash, certKeyFileHash, updated, err := reloadCertificateKeyPair(
+			s.certFile,
+			s.certKeyFile,
+			s.certFileHash,
+			s.certKeyFileHash,
+			logger,
+		)
+		if err != nil {
+			errs = errors.Join(errs, err)
+		} else if updated {
+			s.cert = newCert
+			s.certFileHash = certFileHash
+			s.certKeyFileHash = certKeyFileHash
+
+			logger.Debug("Refreshed server certificate.")
+		}
+	}
+
+	// if the server has a cert pool configured, also attempt to reload this
+	if s.certPoolFile != "" {
+		pool, certPoolFileHash, updated, err := reloadCertificatePool(s.certPoolFile, s.certPoolFileHash, logger)
+		if err != nil {
+			errs = errors.Join(errs, err)
+		} else if updated {
+			s.certPool = pool
+			s.certPoolFileHash = certPoolFileHash
+			logger.Debug("Refreshed server CA certificate pool.")
+		}
+	}
+
+	return errs
+}
+
+// reloadCertificatePool loads the CA cert pool from the given file and returns a new pool if the file has changed.
+func reloadCertificatePool(certPoolFile string, certPoolFileHash []byte, logger logging.Logger) (*x509.CertPool, []byte, bool, error) {
+	certPoolHash, err := hash(certPoolFile)
+	if err != nil {
+		return nil, nil, false, fmt.Errorf("failed to hash CA cert pool file: %w", err)
+	}
+
+	if bytes.Equal(certPoolFileHash, certPoolHash) {
+		return nil, nil, false, nil
+	}
+	caCertPEM, err := os.ReadFile(certPoolFile)
+	if err != nil {
+		return nil, nil, false, fmt.Errorf("failed to read CA cert pool file %q: %w", certPoolFile, err)
+	}
+
+	pool := x509.NewCertPool()
+	if ok := pool.AppendCertsFromPEM(caCertPEM); !ok {
+		return nil, nil, false, fmt.Errorf("failed to load CA cert pool file %q", certPoolFile)
+	}
+
+	return pool, certPoolHash, true, nil
+}
+
+// reloadCertificateKeyPair loads the certificate and key from the given files and returns a new certificate if either
+// file has changed.
+func reloadCertificateKeyPair(
+	certFile, certKeyFile string,
+	certFileHash, certKeyFileHash []byte,
+	logger logging.Logger,
+) (*tls.Certificate, []byte, []byte, bool, error) {
+	certHash, err := hash(certFile)
+	if err != nil {
+		return nil, nil, nil, false, fmt.Errorf("failed to hash server certificate file: %w", err)
+	}
+
+	certKeyHash, err := hash(certKeyFile)
+	if err != nil {
+		return nil, nil, nil, false, fmt.Errorf("failed to hash server key file: %w", err)
+	}
+
+	differentCert := !bytes.Equal(certFileHash, certHash)
+	differentKey := !bytes.Equal(certKeyFileHash, certKeyHash)
+
+	if differentCert && !differentKey {
+		logger.Warn("Server certificate file changed but server key file did not change.")
+	}
+	if !differentCert && differentKey {
+		logger.Warn("Server key file changed but server certificate file did not change.")
+	}
+
+	if !differentCert && !differentKey {
+		return nil, nil, nil, false, nil
+	}
+
+	newCert, err := tls.LoadX509KeyPair(certFile, certKeyFile)
+	if err != nil {
+		return nil, nil, nil, false, fmt.Errorf("server certificate key pair was not updated, update failed: %w", err)
+	}
+
+	return &newCert, certHash, certKeyHash, true, nil
+}
+
+func (s *Server) certLoopPolling(logger logging.Logger) Loop {
 	return func() error {
 		for range time.NewTicker(s.certRefresh).C {
-			certHash, err := hash(s.certFile)
-			if err != nil {
-				logger.Info("Failed to refresh server certificate: %s.", err.Error())
-				continue
-			}
-			certKeyHash, err := hash(s.certKeyFile)
+			err := s.reloadTLSConfig(logger)
 			if err != nil {
-				logger.Info("Failed to refresh server certificate: %s.", err.Error())
-				continue
+				logger.Error(fmt.Sprintf("Failed to reload TLS config: %s", err))
 			}
+		}
 
-			s.certMtx.Lock()
+		return nil
+	}
+}
 
-			different := !bytes.Equal(s.certFileHash, certHash) ||
-				!bytes.Equal(s.certKeyFileHash, certKeyHash)
+func (s *Server) certLoopNotify(logger logging.Logger) Loop {
+	return func() error {
+
+		var paths []string
 
-			if different { // load and store
-				newCert, err := tls.LoadX509KeyPair(s.certFile, s.certKeyFile)
+		// if a cert file is set, then we want to watch the cert and key
+		if s.certFile != "" {
+			paths = append(paths, s.certFile, s.certKeyFile)
+		}
+
+		// if a cert pool file is set, then we want to watch the cert pool. This might be set without the cert and key
+		// being set too.
+		if s.certPoolFile != "" {
+			paths = append(paths, s.certPoolFile)
+		}
+
+		watcher, err := pathwatcher.CreatePathWatcher(paths)
+		if err != nil {
+			return fmt.Errorf("failed to create tls path watcher: %w", err)
+		}
+
+		for evt := range watcher.Events {
+			removalMask := fsnotify.Remove | fsnotify.Rename
+			mask := fsnotify.Create | fsnotify.Write | removalMask
+			if (evt.Op & mask) != 0 {
+				err = s.reloadTLSConfig(s.manager.Logger())
 				if err != nil {
-					logger.Info("Failed to refresh server certificate: %s.", err.Error())
-					s.certMtx.Unlock()
-					continue
+					logger.Error("failed to reload TLS config: %s", err)
 				}
-				s.cert = &newCert
-				s.certFileHash = certHash
-				s.certKeyFileHash = certKeyHash
-				logger.Debug("Refreshed server certificate.")
+				logger.Info("TLS config reloaded")
 			}
-
-			s.certMtx.Unlock()
 		}
 
 		return nil