From 2ebe5be70a20f2c47d4c412c65b53446f7ff5b5d Mon Sep 17 00:00:00 2001 From: niksis02 Date: Sat, 3 Oct 2026 09:02:40 +0200 Subject: [PATCH] fix: reload standalone IAM and private-channel mTLS certs on SIGHUP Fixes #2464 `versitygw iam` never set `IAMConfig.SigHup`, so the reload loop in `RunIAMAPI` never ran and the IAM service kept serving the certificates it started with. It is now connected to `gwcli.SigHup`. The signal forwarder used blocking sends into the one-slot `SigHup` and `SigDone` channels. In modes that never read `SigHup` (`iam` until this fix, `admin`, `utils`), the second SIGHUP left the forwarder stuck, every later SIGINT/SIGTERM was lost, and the process could only be killed. `forwardSignals` now sends without blocking, and a notification that is already pending stands in for the new one. The private listener loaded its client CA pool (`--private-client-ca`) only once. `netutil.TLSOptions.ClientCAs` is replaced by `GetClientCAs`, which `GetConfigForClient` calls on every handshake and which is backed by the new `netutil.CAPoolStorage`, so a reloaded bundle applies to the next connection. When no pool is loaded the listener refuses the client rather than falling back to the system roots. On SIGHUP the IAM service now also reloads the client CA pool and the certificate of the WebUI it hosts, in addition to its public and private certificates. The gateway's standalone IAM client loaded its client cert/key and server CA (`--iam-standalone-client-cert[-key]`, `--iam-standalone-server-ca`) only once. With the new `auth.CertReloader` interface, the gateway's SIGHUP handler reloads them through `IAMServiceStandalone.ReloadCerts`. It builds a new `http.Client` from the files, swaps it in atomically and closes the old client's idle connections. Requests already in flight finish on the old client, and a failed load keeps the current one. The transports now set `IdleConnTimeout`, so a connection left on a replaced client cannot stay open indefinitely. The IAM wrapper in `vgwrdma` re-exports `CertReloader` so the capability is not lost there. --- auth/iam.go | 8 + auth/iam_standalone.go | 38 +++- auth/iam_standalone_test.go | 196 ++++++++++++++++++++ chart/README.md | 2 +- cmd/internal/gwcli/signal.go | 35 +++- cmd/versitygw/iam.go | 1 + cmd/vgwrdma/main.go | 1 + embedgw/embedgw.go | 10 +- embedgw/iam.go | 78 +++++--- embedgw/iam_test.go | 234 ++++++++++++++++++++++++ iamapi/private/listener.go | 2 +- internal/netutil/cert.go | 29 +++ internal/netutil/multi_listener.go | 26 ++- internal/netutil/multi_listener_test.go | 136 ++++++++++++++ 14 files changed, 748 insertions(+), 48 deletions(-) diff --git a/auth/iam.go b/auth/iam.go index 8dba2550..0296eea6 100644 --- a/auth/iam.go +++ b/auth/iam.go @@ -162,6 +162,14 @@ type IAMService interface { Shutdown() error } +// CertReloader is implemented by IAM backends that load TLS material from +// files for their own outbound connections, so a SIGHUP can pick up rotated +// certificates and CA bundles without a restart. On error the backend keeps +// using what it had. +type CertReloader interface { + ReloadCerts() error +} + var ( // ErrUserExists is returned when the user already exists ErrUserExists = errors.New("user already exists") diff --git a/auth/iam_standalone.go b/auth/iam_standalone.go index a2a8fc6c..cdadf4f8 100644 --- a/auth/iam_standalone.go +++ b/auth/iam_standalone.go @@ -26,6 +26,7 @@ import ( "net/http" "net/url" "strconv" + "sync/atomic" "time" "github.com/versity/versitygw/iamapi/private" @@ -41,6 +42,9 @@ const ( standaloneSigningService = sigv4auth.ServiceIAM standaloneRequestTimeout = 10 * time.Second + // standaloneIdleConnTimeout also bounds how long a connection stranded + // on a client retired by ReloadCerts can stay open. + standaloneIdleConnTimeout = 90 * time.Second ) // standaloneProbeWindow bounds how long NewIAMServiceStandalone waits for the @@ -116,7 +120,9 @@ type IAMServiceStandaloneConfig struct { // are unsupported here for the same reason: mutating a user requires setting a secret, which must never // flow into this process — manage users via the IAM service's own control-plane API instead. type IAMServiceStandalone struct { - client *http.Client + // client is replaced as a whole by ReloadCerts, since a transport's TLS + // config cannot change once it has dialed. + client atomic.Pointer[http.Client] baseURL string access string secret string @@ -136,6 +142,7 @@ var ( _ PolicyEvaluator = (*IAMServiceStandalone)(nil) _ FixedBucketOwner = (*IAMServiceStandalone)(nil) _ PrincipalResolver = (*IAMServiceStandalone)(nil) + _ CertReloader = (*IAMServiceStandalone)(nil) ) // NewIAMServiceStandalone constructs the standalone IAM service client. @@ -161,13 +168,13 @@ func NewIAMServiceStandalone(rootAcc Account, cfg IAMServiceStandaloneConfig) (* } svc := &IAMServiceStandalone{ - client: client, baseURL: baseURL, access: access, secret: secret, rootAcc: rootAcc, cfg: cfg, } + svc.client.Store(client) if err := svc.probeProtocol(); err != nil { return nil, err @@ -257,6 +264,7 @@ func newStandaloneHTTPClient(cfg IAMServiceStandaloneConfig) (*http.Client, stri DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { return (&net.Dialer{}).DialContext(ctx, "unix", sock) }, + IdleConnTimeout: standaloneIdleConnTimeout, } // The host in this URL is never actually resolved/dialed — the // DialContext override above always connects to the unix socket @@ -283,10 +291,32 @@ func newStandaloneHTTPClient(cfg IAMServiceStandaloneConfig) (*http.Client, stri Certificates: []tls.Certificate{cert}, RootCAs: pool, }, + IdleConnTimeout: standaloneIdleConnTimeout, } return &http.Client{Transport: transport, Timeout: standaloneRequestTimeout}, "https://" + cfg.Endpoint, nil } +// ReloadCerts implements CertReloader: it re-reads the client cert/key and +// the server CA bundle into a new client, so requests from here on connect +// with the rotated files. Requests already in flight finish on the old +// client, whose connections close once idle instead of being reused. A unix +// socket endpoint has no TLS material, so there is nothing to reload. +func (s *IAMServiceStandalone) ReloadCerts() error { + if netutil.IsUnixSocketPath(s.cfg.Endpoint) { + return nil + } + + client, _, err := newStandaloneHTTPClient(s.cfg) + if err != nil { + return err + } + s.client.Swap(client).CloseIdleConnections() + + fmt.Printf("iam standalone client cert reloaded (cert: %s, key: %s, ca: %s)\n", + s.cfg.ClientCert, s.cfg.ClientCertKey, s.cfg.ServerCA) + return nil +} + // doPrivateRequest signs reqBody as this client's own identity (s.access/ // s.secret, the one place in this file that touches a secret directly — // signing an outbound request as itself, not verifying an inbound one) and @@ -332,7 +362,7 @@ func (s *IAMServiceStandalone) doPrivateRequest(path string, reqBody, respBody a req.Header.Set("X-Amz-Date", result.AmzDate) req.Header.Set("Authorization", result.AuthorizationHeader) - resp, err := s.client.Do(req) + resp, err := s.client.Load().Do(req) if err != nil { return fmt.Errorf("iam standalone: request to %s failed: %w", path, err) } @@ -726,6 +756,6 @@ func (s *IAMServiceStandalone) ListUserAccounts() ([]Account, error) { } func (s *IAMServiceStandalone) Shutdown() error { - s.client.CloseIdleConnections() + s.client.Load().CloseIdleConnections() return nil } diff --git a/auth/iam_standalone_test.go b/auth/iam_standalone_test.go index 42b2c74a..fe0671b1 100644 --- a/auth/iam_standalone_test.go +++ b/auth/iam_standalone_test.go @@ -15,13 +15,23 @@ package auth import ( "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/json" + "encoding/pem" "errors" "fmt" + "math/big" "net" "net/http" "os" "path/filepath" "strconv" + "sync/atomic" "testing" "time" @@ -955,4 +965,190 @@ func TestIAMServiceStandaloneCapabilityInterfaces(t *testing.T) { if _, ok := iam.(PrincipalResolver); !ok { t.Error("standalone client must implement PrincipalResolver") } + if _, ok := iam.(CertReloader); !ok { + t.Error("standalone client must implement CertReloader") + } +} + +// TestIAMServiceStandaloneReloadCerts covers rotating the gateway's side of +// the private mTLS channel: after ReloadCerts, requests present the reloaded +// client certificate and verify the IAM service against the reloaded CA +// bundle, and a reload that fails leaves the working client in place. +func TestIAMServiceStandaloneReloadCerts(t *testing.T) { + oldCA := newStandaloneTestCA(t, "old-ca") + newCA := newStandaloneTestCA(t, "new-ca") + + // The fake IAM service serves a certificate from whichever CA is current, + // and reports the CN of the client certificate each request arrived + // with as its server version. + var serverCert atomic.Pointer[tls.Certificate] + serverCert.Store(oldCA.issue(t, "iam")) + ln, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{ + GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + return serverCert.Load(), nil + }, + ClientAuth: tls.RequireAnyClientCert, + }) + if err != nil { + t.Fatalf("listen: %v", err) + } + srv := &http.Server{Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(private.ProtocolHeader, strconv.Itoa(private.ProtocolVersion)) + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(private.VersionResponse{ + Protocol: private.ProtocolVersion, + MinClient: private.ProtocolVersion, + ServerVersion: r.TLS.PeerCertificates[0].Subject.CommonName, + }) + })} + go srv.Serve(ln) + t.Cleanup(func() { srv.Close() }) + + dir := t.TempDir() + certFile := filepath.Join(dir, "client.crt") + keyFile := filepath.Join(dir, "client.key") + caFile := filepath.Join(dir, "ca.pem") + oldCA.writeFiles(t, "gateway-1", certFile, keyFile, caFile) + + rootAcc := Account{Access: standaloneTestRootAccess, Secret: standaloneTestRootSecret, Role: RoleAdmin} + client, err := NewIAMServiceStandalone(rootAcc, IAMServiceStandaloneConfig{ + Endpoint: ln.Addr().String(), + ClientCert: certFile, + ClientCertKey: keyFile, + ServerCA: caFile, + }) + if err != nil { + t.Fatalf("NewIAMServiceStandalone: %v", err) + } + t.Cleanup(func() { client.Shutdown() }) + + presented := func() string { + t.Helper() + var resp private.VersionResponse + if err := client.doPrivateRequest(private.VersionPath, struct{}{}, &resp); err != nil { + t.Fatalf("private request: %v", err) + } + return resp.ServerVersion + } + + if cn := presented(); cn != "gateway-1" { + t.Fatalf("client cert CN = %q, want gateway-1", cn) + } + + // Both sides move to a new CA, as a trust-bundle rotation does: the old + // client could no longer verify the IAM service at all. + serverCert.Store(newCA.issue(t, "iam")) + newCA.writeFiles(t, "gateway-2", certFile, keyFile, caFile) + if err := client.ReloadCerts(); err != nil { + t.Fatalf("ReloadCerts: %v", err) + } + if cn := presented(); cn != "gateway-2" { + t.Fatalf("client cert CN after reload = %q, want gateway-2", cn) + } + + if err := os.WriteFile(keyFile, []byte("not a key"), 0o600); err != nil { + t.Fatal(err) + } + if err := client.ReloadCerts(); err == nil { + t.Fatal("ReloadCerts accepted an unreadable key") + } + if cn := presented(); cn != "gateway-2" { + t.Fatalf("client cert CN after failed reload = %q, want gateway-2", cn) + } +} + +// TestIAMServiceStandaloneReloadCertsUnixSocket covers an endpoint with no +// TLS material, where a SIGHUP must neither fail nor disturb the client. +func TestIAMServiceStandaloneReloadCertsUnixSocket(t *testing.T) { + _, sock := standaloneTestServer(t) + client := newStandaloneTestClient(t, sock) + + if err := client.ReloadCerts(); err != nil { + t.Fatalf("ReloadCerts: %v", err) + } + var resp private.VersionResponse + if err := client.doPrivateRequest(private.VersionPath, struct{}{}, &resp); err != nil { + t.Fatalf("private request after reload: %v", err) + } +} + +type standaloneTestCA struct { + cert *x509.Certificate + key *ecdsa.PrivateKey +} + +func newStandaloneTestCA(t *testing.T, cn string) standaloneTestCA { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate CA key: %v", err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatalf("create CA cert: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("parse CA cert: %v", err) + } + + return standaloneTestCA{cert: cert, key: key} +} + +// issue signs a certificate usable for either end of a connection to +// 127.0.0.1. +func (ca standaloneTestCA) issue(t *testing.T, cn string) *tls.Certificate { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate %s key: %v", cn, err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: cn}, + IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1)}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, + } + der, err := x509.CreateCertificate(rand.Reader, template, ca.cert, &key.PublicKey, ca.key) + if err != nil { + t.Fatalf("create %s cert: %v", cn, err) + } + + return &tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key} +} + +// writeFiles writes a fresh certificate for cn and its key, plus this CA's +// own certificate as the peer-verification bundle. +func (ca standaloneTestCA) writeFiles(t *testing.T, cn, certFile, keyFile, caFile string) { + t.Helper() + + cert := ca.issue(t, cn) + keyDER, err := x509.MarshalPKCS8PrivateKey(cert.PrivateKey) + if err != nil { + t.Fatalf("marshal %s key: %v", cn, err) + } + + for path, block := range map[string]*pem.Block{ + certFile: {Type: "CERTIFICATE", Bytes: cert.Certificate[0]}, + keyFile: {Type: "PRIVATE KEY", Bytes: keyDER}, + caFile: {Type: "CERTIFICATE", Bytes: ca.cert.Raw}, + } { + if err := os.WriteFile(path, pem.EncodeToMemory(block), 0o600); err != nil { + t.Fatalf("write %s: %v", path, err) + } + } } diff --git a/chart/README.md b/chart/README.md index 6f763d07..27c59635 100644 --- a/chart/README.md +++ b/chart/README.md @@ -167,7 +167,7 @@ Key points: - **External IAM service**: to point a gateway at a standalone IAM service deployed outside this chart (or by a separate chart release), set `iam.standalone.endpoint` to its `host:port` and provide the mTLS material via `iam.standalone.certificate.existingSecret`. - **OIDC identity providers**: `AssumeRoleWithWebIdentity` only trusts an OIDC provider reachable over verified https at a publicly routable address on the implicit `:443`. An in-cluster provider satisfies none of that, so `iamServer.oidc.allowPrivateEndpoints` permits private/loopback addresses and an explicit port (a SPIRE OIDC discovery provider on a ClusterIP `Service`), and `iamServer.oidc.allowInsecureTransport` additionally permits plaintext `http://` providers and drops TLS verification (the same provider bound to `127.0.0.1` as a sidecar). Each relaxes real protections — see the comments in `values.yaml` — so enable only the one your topology needs. `iamServer.oidc.discoveryUrls` is the narrower alternative for a provider that is public but reachable in-cluster: it moves only one provider's discovery fetch to an address you name (private addresses included, without `allowPrivateEndpoints`), while tokens keep naming their public issuer. - **WebUI access**: to manage IAM users from the WebUI, set `webui.iamGateways` to the URL a browser can reach `iamServer` on, and `iamServer.corsAllowOrigin` to the WebUI's own origin. Every WebUI call to the IAM API is cross-origin, so without `corsAllowOrigin` the browser blocks it and the WebUI's IAM navigation silently never appears. -- **Secret rotation**: the processes load mTLS material and environment-based credentials at startup. After a referenced Secret rotates, restart both Deployments or configure a Secret-reloader controller through `deploymentAnnotations` and `iamServer.deploymentAnnotations`. +- **Secret rotation**: both processes re-read their TLS certificates and CA bundles, the private mTLS material on both sides included, on SIGHUP, so a certificate rotator sidecar sharing the process namespace (`shareProcessNamespace`) can rotate them in place. Environment-based credentials are read only at startup: after a referenced Secret rotates, restart both Deployments or configure a Secret-reloader controller through `deploymentAnnotations` and `iamServer.deploymentAnnotations`. ## Scaling and Persistence diff --git a/cmd/internal/gwcli/signal.go b/cmd/internal/gwcli/signal.go index fd9f0add..9504594f 100644 --- a/cmd/internal/gwcli/signal.go +++ b/cmd/internal/gwcli/signal.go @@ -34,15 +34,30 @@ func SetupSignalHandler() { sigs := make(chan os.Signal, 1) signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) - go func() { - for sig := range sigs { - fmt.Fprintf(os.Stderr, "caught signal %v\n", sig) - switch sig { - case syscall.SIGINT, syscall.SIGTERM: - SigDone <- struct{}{} - case syscall.SIGHUP: - SigHup <- struct{}{} - } + go forwardSignals(sigs, SigDone, SigHup) +} + +// forwardSignals relays sigs into done and hup without ever blocking. Every +// subcommand installs this handler, but only some of them read hup, so a +// blocking send would park this goroutine on the second SIGHUP and every +// SIGINT/SIGTERM after it would be lost. A notification already pending +// covers the new one: a reload re-reads everything it reloads, and shutdown +// only needs to start once. +func forwardSignals(sigs <-chan os.Signal, done, hup chan<- struct{}) { + for sig := range sigs { + fmt.Fprintf(os.Stderr, "caught signal %v\n", sig) + switch sig { + case syscall.SIGINT, syscall.SIGTERM: + notify(done) + case syscall.SIGHUP: + notify(hup) } - }() + } +} + +func notify(ch chan<- struct{}) { + select { + case ch <- struct{}{}: + default: + } } diff --git a/cmd/versitygw/iam.go b/cmd/versitygw/iam.go index 610dc331..43022007 100644 --- a/cmd/versitygw/iam.go +++ b/cmd/versitygw/iam.go @@ -84,6 +84,7 @@ func runIAM(ctx *cli.Context) error { WebuiIAMGateways: webuiIAMGateways, WebuiGateways: webuiGateways, WebuiAdminGateways: webuiAdminGateways, + SigHup: gwcli.SigHup, Version: Version, Build: Build, BuildTime: BuildTime, diff --git a/cmd/vgwrdma/main.go b/cmd/vgwrdma/main.go index 0873a8b7..378e38e0 100644 --- a/cmd/vgwrdma/main.go +++ b/cmd/vgwrdma/main.go @@ -151,6 +151,7 @@ type standaloneIAMExtensions interface { auth.PolicyEvaluator auth.FixedBucketOwner auth.PrincipalResolver + auth.CertReloader } type shutdownOnceService struct { diff --git a/embedgw/embedgw.go b/embedgw/embedgw.go index dd65d738..f9df8887 100644 --- a/embedgw/embedgw.go +++ b/embedgw/embedgw.go @@ -519,8 +519,9 @@ type Config struct { WebsiteNoTLS bool // SigHup is an optional channel that signals the gateway to reload TLS - // certificates and rotate log files (equivalent to SIGHUP). When nil, - // this feature is disabled. + // certificates, including the IAM backend's own client certificates and + // CA bundle when it implements auth.CertReloader, and rotate log files + // (equivalent to SIGHUP). When nil, this feature is disabled. SigHup <-chan struct{} // S3Options are appended to the internally built s3api options before the @@ -1212,6 +1213,11 @@ Loop: fmt.Printf("wsSrv cert reloaded (cert: %s, key: %s)\n", wsTLSCert, wsTLSKey) } } + if reloader, ok := iam.(auth.CertReloader); ok { + if reloadErr := reloader.ReloadCerts(); reloadErr != nil { + debuglogger.InternalError(fmt.Errorf("iam cert reload failed: %w", reloadErr)) + } + } } } saveErr := err diff --git a/embedgw/iam.go b/embedgw/iam.go index 00006406..4f7d2a6f 100644 --- a/embedgw/iam.go +++ b/embedgw/iam.go @@ -177,8 +177,9 @@ type IAMConfig struct { // dashboard at an S3 gateway as well. Region string - // SigHup is an optional channel that signals the IAM API to reload TLS - // certificates. When nil, this feature is disabled. + // SigHup is an optional channel that signals the IAM API to reload its + // TLS certificates and the private listener's client CA bundle. When + // nil, this feature is disabled. SigHup <-chan struct{} // Version, Build, and BuildTime are displayed in the startup banner. @@ -221,12 +222,13 @@ type IAMConfig struct { // privateAPIServer is the standalone IAM service's private endpoint set // together with everything RunIAMAPI needs to serve and maintain it: the -// TLS options ServeMultiPort will enforce, and the cert storage backing -// them so a SIGHUP can swap in a rotated certificate. +// TLS options ServeMultiPort will enforce, and the cert and client CA +// storage backing them so a SIGHUP can swap in rotated files. type privateAPIServer struct { api *private.PrivateAPI tlsOpts netutil.TLSOptions certStorage *netutil.CertStorage + clientCAs *netutil.CAPoolStorage } // newPrivateAPI builds the standalone IAM service's private endpoint set @@ -241,18 +243,19 @@ func newPrivateAPI(store storage.Storer, cfg *IAMConfig) (*privateAPIServer, err var tlsOpts netutil.TLSOptions var certStorage *netutil.CertStorage + var clientCAs *netutil.CAPoolStorage if allSet { certStorage = netutil.NewCertStorage() if err := certStorage.SetCertificate(cfg.PrivateCertFile, cfg.PrivateKeyFile); err != nil { return nil, fmt.Errorf("private listener: load certs: %w", err) } - pool, err := netutil.LoadCACertPool(cfg.PrivateClientCAFile) - if err != nil { + clientCAs = netutil.NewCAPoolStorage() + if err := clientCAs.SetCAPool(cfg.PrivateClientCAFile); err != nil { return nil, fmt.Errorf("private listener: %w", err) } tlsOpts = netutil.TLSOptions{ GetCertificate: certStorage.GetCertificate, - ClientCAs: pool, + GetClientCAs: clientCAs.GetCAPool, RequireClientCert: true, } } @@ -280,7 +283,7 @@ func newPrivateAPI(store storage.Storer, cfg *IAMConfig) (*privateAPIServer, err return nil, fmt.Errorf("init private IAM API: %w", err) } - return &privateAPIServer{api: p, tlsOpts: tlsOpts, certStorage: certStorage}, nil + return &privateAPIServer{api: p, tlsOpts: tlsOpts, certStorage: certStorage, clientCAs: clientCAs}, nil } // iamWebUIGateways resolves the IAM service URLs the WebUI login page offers. @@ -303,6 +306,19 @@ func iamWebUIGateways(cfg *IAMConfig) ([]string, error) { return gateways, nil } +// iamWebUITLSFiles resolves the certificate the IAM-hosted WebUI serves: its +// own when either file is set, otherwise the IAM API's, and none under +// WebuiNoTLS. +func iamWebUITLSFiles(cfg *IAMConfig) (certFile, keyFile string) { + if cfg.WebuiNoTLS { + return "", "" + } + if cfg.WebuiCertFile == "" && cfg.WebuiKeyFile == "" { + return cfg.CertFile, cfg.KeyFile + } + return cfg.WebuiCertFile, cfg.WebuiKeyFile +} + // newIAMWebUI builds the WebUI server hosted by the IAM service process. It // returns nil when no WebuiPorts are configured. func newIAMWebUI(cfg *IAMConfig) (*webui.Server, error) { @@ -328,24 +344,19 @@ func newIAMWebUI(cfg *IAMConfig) (*webui.Server, error) { } var webOpts []webui.Option - if !cfg.WebuiNoTLS { - webTLSCert, webTLSKey := cfg.WebuiCertFile, cfg.WebuiKeyFile - if webTLSCert == "" && webTLSKey == "" { - webTLSCert, webTLSKey = cfg.CertFile, cfg.KeyFile + webTLSCert, webTLSKey := iamWebUITLSFiles(cfg) + if webTLSCert != "" || webTLSKey != "" { + if webTLSCert == "" { + return nil, fmt.Errorf("webui TLS key specified without cert file") } - if webTLSCert != "" || webTLSKey != "" { - if webTLSCert == "" { - return nil, fmt.Errorf("webui TLS key specified without cert file") - } - if webTLSKey == "" { - return nil, fmt.Errorf("webui TLS cert specified without key file") - } - cs := netutil.NewCertStorage() - if err := cs.SetCertificate(webTLSCert, webTLSKey); err != nil { - return nil, fmt.Errorf("tls: load certs: %v", err) - } - webOpts = append(webOpts, webui.WithTLS(cs)) + if webTLSKey == "" { + return nil, fmt.Errorf("webui TLS cert specified without key file") } + cs := netutil.NewCertStorage() + if err := cs.SetCertificate(webTLSCert, webTLSKey); err != nil { + return nil, fmt.Errorf("tls: load certs: %v", err) + } + webOpts = append(webOpts, webui.WithTLS(cs)) } if cfg.Quiet { webOpts = append(webOpts, webui.WithQuiet()) @@ -562,6 +573,25 @@ Loop: fmt.Printf("private iam api cert reloaded (cert: %s, key: %s)\n", cfg.PrivateCertFile, cfg.PrivateKeyFile) } } + // a rotated CA bundle has to be picked up as well, or gateways + // presenting client certs from the new CA are refused. + if privateAPI != nil && privateAPI.clientCAs != nil { + reloadErr := privateAPI.clientCAs.SetCAPool(cfg.PrivateClientCAFile) + if reloadErr != nil { + debuglogger.InternalError(fmt.Errorf("private iam api client ca reload failed: %w", reloadErr)) + } else { + fmt.Printf("private iam api client ca reloaded (ca: %s)\n", cfg.PrivateClientCAFile) + } + } + if webSrv != nil && webSrv.CertStorage != nil { + webTLSCert, webTLSKey := iamWebUITLSFiles(cfg) + reloadErr := webSrv.CertStorage.SetCertificate(webTLSCert, webTLSKey) + if reloadErr != nil { + debuglogger.InternalError(fmt.Errorf("webSrv cert reload failed: %w", reloadErr)) + } else { + fmt.Printf("webSrv cert reloaded (cert: %s, key: %s)\n", webTLSCert, webTLSKey) + } + } } } saveErr := err diff --git a/embedgw/iam_test.go b/embedgw/iam_test.go index 3d92b06b..b5a5eb47 100644 --- a/embedgw/iam_test.go +++ b/embedgw/iam_test.go @@ -16,8 +16,20 @@ package embedgw import ( "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "os" + "path/filepath" "strings" "testing" + "time" ) func TestRunIAMAPIValidatesConfig(t *testing.T) { @@ -136,3 +148,225 @@ func TestRunIAMAPIRejectsNilConfig(t *testing.T) { t.Fatalf("error = %q", err) } } + +// TestRunIAMAPIReloadsCertsOnSigHup rotates every certificate and CA file the +// IAM service reads to a new CA and sends one SIGHUP: the public API, the +// private listener and the hosted WebUI must all serve the new certificates, +// and the private listener must switch to verifying gateways against the new +// client CA. +func TestRunIAMAPIReloadsCertsOnSigHup(t *testing.T) { + oldCA := newIAMTestCA(t, "old-ca") + newCA := newIAMTestCA(t, "new-ca") + oldGateway := oldCA.issue(t, "gateway") + newGateway := newCA.issue(t, "gateway") + + dir := t.TempDir() + certFile := filepath.Join(dir, "iam.crt") + keyFile := filepath.Join(dir, "iam.key") + privCertFile := filepath.Join(dir, "private.crt") + privKeyFile := filepath.Join(dir, "private.key") + clientCAFile := filepath.Join(dir, "client-ca.pem") + writeCerts := func(ca iamTestCA) { + ca.writeCertFiles(t, "iam", certFile, keyFile) + ca.writeCertFiles(t, "iam-private", privCertFile, privKeyFile) + ca.writeCAFile(t, clientCAFile) + } + writeCerts(oldCA) + + // A unix socket path is limited to ~104 bytes on macOS, which t.TempDir() + // can exceed once it embeds the test name. + sockDir, err := os.MkdirTemp("", "vgw-iam") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(sockDir) }) + publicSock := filepath.Join(sockDir, "api.sock") + privateSock := filepath.Join(sockDir, "priv.sock") + webuiSock := filepath.Join(sockDir, "web.sock") + + sigHup := make(chan struct{}) + ctx, cancel := context.WithCancel(context.Background()) + errCh := make(chan error, 1) + go func() { + errCh <- RunIAMAPI(ctx, &IAMConfig{ + RootUserAccess: "root", + RootUserSecret: "secret", + Ports: []string{publicSock}, + MaxConnections: 16, + MaxRequests: 16, + CertFile: certFile, + KeyFile: keyFile, + Quiet: true, + PrivatePorts: []string{privateSock}, + PrivateCertFile: privCertFile, + PrivateKeyFile: privKeyFile, + PrivateClientCAFile: clientCAFile, + IAMDir: t.TempDir(), + CORSAllowOrigin: "*", + WebuiPorts: []string{webuiSock}, + SigHup: sigHup, + }) + }() + t.Cleanup(func() { + cancel() + select { + case err := <-errCh: + if err != nil { + t.Errorf("RunIAMAPI: %v", err) + } + case <-time.After(15 * time.Second): + t.Error("RunIAMAPI did not return after cancel") + } + }) + + // servedBy dials sock and returns the CN of the CA that issued the + // certificate it serves. Pinned to TLS 1.2 so a refused client + // certificate fails the handshake itself, as in netutil's mTLS tests. + servedBy := func(sock string, clientCert *tls.Certificate) (string, error) { + conf := &tls.Config{InsecureSkipVerify: true, MaxVersion: tls.VersionTLS12} + if clientCert != nil { + conf.Certificates = []tls.Certificate{*clientCert} + } + conn, err := tls.Dial("unix", sock, conf) + if err != nil { + return "", err + } + defer conn.Close() + return conn.ConnectionState().PeerCertificates[0].Issuer.CommonName, nil + } + // expect reports the first listener not in the state ca describes: + // serving certificates from ca and accepting only gateways ca issued. + expect := func(ca iamTestCA, accepted, refused *tls.Certificate) error { + for _, sock := range []string{publicSock, webuiSock} { + issuer, err := servedBy(sock, nil) + if err != nil { + return fmt.Errorf("%s: %w", filepath.Base(sock), err) + } + if issuer != ca.cert.Subject.CommonName { + return fmt.Errorf("%s serves a cert issued by %s, want %s", filepath.Base(sock), issuer, ca.cert.Subject.CommonName) + } + } + issuer, err := servedBy(privateSock, accepted) + if err != nil { + return fmt.Errorf("private listener refused a gateway cert issued by %s: %w", ca.cert.Subject.CommonName, err) + } + if issuer != ca.cert.Subject.CommonName { + return fmt.Errorf("private listener serves a cert issued by %s, want %s", issuer, ca.cert.Subject.CommonName) + } + if _, err := servedBy(privateSock, refused); err == nil { + return fmt.Errorf("private listener accepted a gateway cert from a CA other than %s", ca.cert.Subject.CommonName) + } + return nil + } + waitFor := func(ca iamTestCA, accepted, refused *tls.Certificate) { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for { + err := expect(ca, accepted, refused) + if err == nil { + return + } + select { + case runErr := <-errCh: + t.Fatalf("RunIAMAPI exited early: %v", runErr) + default: + } + if time.Now().After(deadline) { + t.Fatal(err) + } + time.Sleep(20 * time.Millisecond) + } + } + + waitFor(oldCA, oldGateway, newGateway) + + writeCerts(newCA) + select { + case sigHup <- struct{}{}: + case <-time.After(5 * time.Second): + t.Fatal("RunIAMAPI is not receiving SIGHUP notifications") + } + + waitFor(newCA, newGateway, oldGateway) +} + +type iamTestCA struct { + cert *x509.Certificate + key *ecdsa.PrivateKey +} + +func newIAMTestCA(t *testing.T, cn string) iamTestCA { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate CA key: %v", err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatalf("create CA cert: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("parse CA cert: %v", err) + } + + return iamTestCA{cert: cert, key: key} +} + +func (ca iamTestCA) issue(t *testing.T, cn string) *tls.Certificate { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate %s key: %v", cn, err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: cn}, + DNSNames: []string{cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, + } + der, err := x509.CreateCertificate(rand.Reader, template, ca.cert, &key.PublicKey, ca.key) + if err != nil { + t.Fatalf("create %s cert: %v", cn, err) + } + + return &tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key} +} + +func (ca iamTestCA) writeCertFiles(t *testing.T, cn, certFile, keyFile string) { + t.Helper() + + cert := ca.issue(t, cn) + keyDER, err := x509.MarshalPKCS8PrivateKey(cert.PrivateKey) + if err != nil { + t.Fatalf("marshal %s key: %v", cn, err) + } + writeIAMTestPEM(t, certFile, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Certificate[0]}) + writeIAMTestPEM(t, keyFile, &pem.Block{Type: "PRIVATE KEY", Bytes: keyDER}) +} + +func (ca iamTestCA) writeCAFile(t *testing.T, path string) { + t.Helper() + writeIAMTestPEM(t, path, &pem.Block{Type: "CERTIFICATE", Bytes: ca.cert.Raw}) +} + +func writeIAMTestPEM(t *testing.T, path string, block *pem.Block) { + t.Helper() + if err := os.WriteFile(path, pem.EncodeToMemory(block), 0o600); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} diff --git a/iamapi/private/listener.go b/iamapi/private/listener.go index 437dd04c..bcf27a0f 100644 --- a/iamapi/private/listener.go +++ b/iamapi/private/listener.go @@ -37,7 +37,7 @@ func (p *PrivateAPI) ServeMultiPort(addrs []string, tlsOpts netutil.TLSOptions) return fmt.Errorf("no private listener addresses specified") } - hasMTLS := tlsOpts.GetCertificate != nil && tlsOpts.ClientCAs != nil && tlsOpts.RequireClientCert + hasMTLS := tlsOpts.GetCertificate != nil && tlsOpts.GetClientCAs != nil && tlsOpts.RequireClientCert for _, addr := range addrs { if err := netutil.RequireSecureTransport(addr, hasMTLS); err != nil { return err diff --git a/internal/netutil/cert.go b/internal/netutil/cert.go index c9748c65..97b5b526 100644 --- a/internal/netutil/cert.go +++ b/internal/netutil/cert.go @@ -16,6 +16,7 @@ package netutil import ( "crypto/tls" + "crypto/x509" "fmt" "sync/atomic" ) @@ -42,3 +43,31 @@ func (cs *CertStorage) SetCertificate(certFile string, keyFile string) error { return nil } + +// CAPoolStorage is CertStorage's counterpart for the CA bundle that verifies +// a peer: it holds the pool loaded from a file, and SetCAPool swaps in a +// reloaded one without disturbing connections already verified. +type CAPoolStorage struct { + pool atomic.Pointer[x509.CertPool] +} + +func NewCAPoolStorage() *CAPoolStorage { + return &CAPoolStorage{} +} + +func (cs *CAPoolStorage) GetCAPool() *x509.CertPool { + return cs.pool.Load() +} + +// SetCAPool loads caFile and makes it the current pool. On error the +// previous pool stays in place. +func (cs *CAPoolStorage) SetCAPool(caFile string) error { + pool, err := LoadCACertPool(caFile) + if err != nil { + return err + } + + cs.pool.Store(pool) + + return nil +} diff --git a/internal/netutil/multi_listener.go b/internal/netutil/multi_listener.go index 045a6d7a..a4f8d7c7 100644 --- a/internal/netutil/multi_listener.go +++ b/internal/netutil/multi_listener.go @@ -289,12 +289,13 @@ func NewMultiAddrListener(network, address string, opts ListenerOptions) (net.Li } // TLSOptions configures the server-side tls.Config for -// NewMultiAddrTLSListenerWithOptions. A non-nil ClientCAs enables mTLS: -// inbound connections must present a certificate verified against that -// pool +// NewMultiAddrTLSListenerWithOptions. A non-nil GetClientCAs enables mTLS: +// inbound connections must present a certificate verified against the pool +// it returns, which is fetched again for every handshake so a reloaded CA +// bundle applies to the next connection. type TLSOptions struct { GetCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error) - ClientCAs *x509.CertPool + GetClientCAs func() *x509.CertPool RequireClientCert bool } @@ -313,13 +314,26 @@ func NewMultiAddrTLSListenerWithOptions(network, address string, tlsOpts TLSOpti MinVersion: tls.VersionTLS12, GetCertificate: tlsOpts.GetCertificate, } - if tlsOpts.ClientCAs != nil { - config.ClientCAs = tlsOpts.ClientCAs + if tlsOpts.GetClientCAs != nil { if tlsOpts.RequireClientCert { config.ClientAuth = tls.RequireAndVerifyClientCert } else { config.ClientAuth = tls.VerifyClientCertIfGiven } + // ClientCAs is read from the config a handshake runs with, so the + // pool is attached per handshake instead of once here. + base := config.Clone() + config.GetConfigForClient = func(*tls.ClientHelloInfo) (*tls.Config, error) { + pool := tlsOpts.GetClientCAs() + if pool == nil { + // A nil ClientCAs verifies against the system roots, so an + // empty pool must refuse the client rather than fall back. + return nil, errors.New("no client CA pool loaded") + } + hsConfig := base.Clone() + hsConfig.ClientCAs = pool + return hsConfig, nil + } } if IsUnixSocketPath(address) { diff --git a/internal/netutil/multi_listener_test.go b/internal/netutil/multi_listener_test.go index e09a8d24..b811c21b 100644 --- a/internal/netutil/multi_listener_test.go +++ b/internal/netutil/multi_listener_test.go @@ -20,8 +20,12 @@ import ( "crypto/tls" "crypto/x509" "crypto/x509/pkix" + "encoding/pem" "math/big" "net" + "os" + "path/filepath" + "strings" "testing" "time" ) @@ -135,6 +139,129 @@ func TestMTLSListenerRejectsClientWithoutCert(t *testing.T) { }) } +// TestMTLSListenerReloadsClientCAs covers rotating the CA bundle behind a +// listener that is already serving: the next handshake must verify against +// the reloaded pool, and a listener whose pool was never loaded must refuse +// every client instead of falling back to the system roots. +func TestMTLSListenerReloadsClientCAs(t *testing.T) { + oldCA := generateTestCA(t) + newCA := generateTestCA(t) + serverCert := issueTestCert(t, oldCA, "server") + oldClient := issueTestCert(t, oldCA, "client") + newClient := issueTestCert(t, newCA, "client") + + serverPool := x509.NewCertPool() + serverPool.AddCert(oldCA.cert) + + listen := func(t *testing.T, clientCAs *CAPoolStorage) (net.Listener, <-chan error) { + t.Helper() + ln, err := NewMultiAddrTLSListenerWithOptions("tcp", "127.0.0.1:0", TLSOptions{ + GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + return &serverCert, nil + }, + GetClientCAs: clientCAs.GetCAPool, + RequireClientCert: true, + }, ListenerOptions{}) + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { ln.Close() }) + + serverErrs := make(chan error, 8) + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + serverErrs <- conn.(*tls.Conn).Handshake() + conn.Close() + } + }() + return ln, serverErrs + } + + // Pinned to TLS 1.2 for the reason given in + // TestMTLSListenerRejectsClientWithoutCert. + handshake := func(ln net.Listener, clientCert tls.Certificate) error { + conn, err := tls.Dial("tcp", ln.Addr().String(), &tls.Config{ + RootCAs: serverPool, + MaxVersion: tls.VersionTLS12, + Certificates: []tls.Certificate{clientCert}, + ServerName: "server", + }) + if err != nil { + return err + } + return conn.Close() + } + + t.Run("reloaded pool applies to the next handshake", func(t *testing.T) { + caFile := filepath.Join(t.TempDir(), "ca.pem") + writeTestCAFile(t, caFile, oldCA) + clientCAs := NewCAPoolStorage() + if err := clientCAs.SetCAPool(caFile); err != nil { + t.Fatalf("SetCAPool: %v", err) + } + ln, _ := listen(t, clientCAs) + + if err := handshake(ln, oldClient); err != nil { + t.Fatalf("client from the loaded CA rejected: %v", err) + } + if err := handshake(ln, newClient); err == nil { + t.Fatal("client from a CA not yet loaded was accepted") + } + + writeTestCAFile(t, caFile, newCA) + if err := clientCAs.SetCAPool(caFile); err != nil { + t.Fatalf("SetCAPool after rotation: %v", err) + } + + if err := handshake(ln, newClient); err != nil { + t.Fatalf("client from the reloaded CA rejected: %v", err) + } + if err := handshake(ln, oldClient); err == nil { + t.Fatal("client from the rotated-out CA was still accepted") + } + }) + + t.Run("failed reload keeps the current pool", func(t *testing.T) { + caFile := filepath.Join(t.TempDir(), "ca.pem") + writeTestCAFile(t, caFile, oldCA) + clientCAs := NewCAPoolStorage() + if err := clientCAs.SetCAPool(caFile); err != nil { + t.Fatalf("SetCAPool: %v", err) + } + pool := clientCAs.GetCAPool() + + if err := os.WriteFile(caFile, []byte("not a certificate"), 0o600); err != nil { + t.Fatal(err) + } + if err := clientCAs.SetCAPool(caFile); err == nil { + t.Fatal("SetCAPool accepted a file with no certificates") + } + if clientCAs.GetCAPool() != pool { + t.Fatal("failed SetCAPool replaced the current pool") + } + }) + + t.Run("no pool loaded refuses every client", func(t *testing.T) { + ln, serverErrs := listen(t, NewCAPoolStorage()) + + if err := handshake(ln, oldClient); err == nil { + t.Fatal("handshake succeeded with no client CA pool loaded") + } + select { + case err := <-serverErrs: + if err == nil || !strings.Contains(err.Error(), "no client CA pool loaded") { + t.Fatalf("server handshake error = %v, want the missing-pool refusal", err) + } + case <-time.After(5 * time.Second): + t.Fatal("server never finished the handshake") + } + }) +} + type testCA struct { cert *x509.Certificate key *ecdsa.PrivateKey @@ -235,3 +362,12 @@ func TestMultiListenerAddrs(t *testing.T) { t.Errorf("Addr() = %s, want the first address %s", ml.Addr(), want[0]) } } + +func writeTestCAFile(t *testing.T, path string, ca testCA) { + t.Helper() + + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: ca.cert.Raw}) + if err := os.WriteFile(path, pemBytes, 0o600); err != nil { + t.Fatalf("write CA file: %v", err) + } +}