fix(hosts): make identity persistence durable and check the CA file

apps/host_agent/internal/identity/durability_test.go

@@ -276,7 +276,18 @@ func TestInitialPossessionDurabilityFailureNeverReportsSuccess(t *testing.T) {
 	for _, stage := range []string{"create", "write", "file_sync", "file_close", "link", "parent_open", "parent_sync", "parent_close"} {
 		t.Run(stage, func(t *testing.T) {
 			dir := filepath.Join(physicalTemp(t), "identity")
+			real := identityFiles
 			reset := failFiles(t, stage)
+			if faulty := identityFiles.openDir; strings.HasPrefix(stage, "parent_") {
+				// Aim the fault at the identity directory's own sync; the parent
+				// sync after Mkdir has its own tests in hardening_test.go.
+				identityFiles.openDir = func(p string) (durableFile, error) {
+					if p != dir {
+						return real.openDir(p)
+					}
+					return faulty(p)
+				}
+			}
 			s, e := Prepare(dir, "https://localhost:8443", "")
 			reset()
 			if s != nil || !errors.Is(e, ErrIdentity) {

apps/host_agent/internal/identity/durable_file.go

@@ -18,11 +18,13 @@ type durableFiles struct {
 	openDir    func(string) (durableFile, error)
 	rename     func(string, string) error
 	link       func(string, string) error
+	remove     func(string) error
 }
 
 var identityFiles = durableFiles{
 	createTemp: func(dir, pattern string) (durableFile, error) { return os.CreateTemp(dir, pattern) },
 	openDir:    func(dir string) (durableFile, error) { return os.Open(dir) },
 	rename:     os.Rename,
 	link:       os.Link,
+	remove:     os.Remove,
 }

apps/host_agent/internal/identity/hardening_test.go

@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: EUPL-1.2
+package identity
+
+import (
+	"crypto/ed25519"
+	"crypto/rand"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"math/big"
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+	"time"
+)
+
+func traceFiles(t *testing.T, log *[]string, failRemove bool) {
+	t.Helper()
+	before := identityFiles
+	ops := before
+	ops.openDir = func(d string) (durableFile, error) {
+		*log = append(*log, "opendir:"+d)
+		return before.openDir(d)
+	}
+	ops.link = func(a, b string) error { *log = append(*log, "link"); return before.link(a, b) }
+	ops.remove = func(p string) error {
+		*log = append(*log, "remove")
+		if failRemove {
+			return os.ErrPermission
+		}
+		return before.remove(p)
+	}
+	identityFiles = ops
+	t.Cleanup(func() { identityFiles = before })
+}
+
+func TestPrepareSyncsParentAfterMkdirAndRemovesTempBeforeDirectorySync(t *testing.T) {
+	root := physicalTemp(t)
+	dir := filepath.Join(root, "identity")
+	var log []string
+	traceFiles(t, &log, false)
+	if _, e := Prepare(dir, "https://localhost:8443", ""); e != nil {
+		t.Fatal(e)
+	}
+	want := []string{"opendir:" + root, "link", "remove", "opendir:" + dir}
+	if len(log) != len(want) {
+		t.Fatalf("order %v", log)
+	}
+	for i := range want {
+		if log[i] != want[i] {
+			t.Fatalf("order %v", log)
+		}
+	}
+	left, _ := filepath.Glob(filepath.Join(dir, ".identity-*"))
+	if len(left) != 0 {
+		t.Fatal("temp key copy remains")
+	}
+}
+
+func TestPrepareRefusesWhenTempRemovalFails(t *testing.T) {
+	var log []string
+	traceFiles(t, &log, true)
+	if _, e := Prepare(filepath.Join(physicalTemp(t), "identity"), "https://localhost:8443", ""); e == nil {
+		t.Fatal("reported success without durable removal")
+	}
+}
+
+func writeCA(t *testing.T, mode os.FileMode) string {
+	t.Helper()
+	pub, key, _ := ed25519.GenerateKey(rand.Reader)
+	tpl := &x509.Certificate{SerialNumber: big.NewInt(1), Subject: pkix.Name{CommonName: "ca"}, NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(time.Hour), IsCA: true, BasicConstraintsValid: true, KeyUsage: x509.KeyUsageCertSign}
+	der, e := x509.CreateCertificate(rand.Reader, tpl, tpl, pub, key)
+	if e != nil {
+		t.Fatal(e)
+	}
+	p := filepath.Join(physicalTemp(t), "ca.pem")
+	if e = os.WriteFile(p, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), 0600); e != nil {
+		t.Fatal(e)
+	}
+	if e = os.Chmod(p, mode); e != nil {
+		t.Fatal(e)
+	}
+	return p
+}
+
+func TestServerCAPathMustBeAbsoluteAndClean(t *testing.T) {
+	for _, p := range []string{"ca.pem", "./ca.pem", "/tmp/../tmp/ca.pem", "/tmp//ca.pem"} {
+		if _, e := Prepare(filepath.Join(physicalTemp(t), "identity"), "https://localhost:8443", p); e == nil {
+			t.Fatalf("Prepare accepted %q", p)
+		}
+	}
+	dir := filepath.Join(physicalTemp(t), "identity")
+	s, e := Prepare(dir, "https://localhost:8443", writeCA(t, 0600))
+	if e != nil {
+		t.Fatal(e)
+	}
+	s.ServerCAFile = "relative.pem"
+	if validate(s) == nil || Save(dir, s) == nil {
+		t.Fatal("validate accepted relative CA path")
+	}
+}
+
+func TestServerCAFileRefusesWritableAndSymlinkAndAcceptsPrivate(t *testing.T) {
+	for _, mode := range []os.FileMode{0620, 0602, 0666} {
+		s := &State{ServerCAFile: writeCA(t, mode)}
+		if _, e := TLSConfig(s, false); e == nil {
+			t.Fatalf("accepted CA mode %o", mode)
+		}
+	}
+	for _, mode := range []os.FileMode{0600, 0644} {
+		if _, e := TLSConfig(&State{ServerCAFile: writeCA(t, mode)}, false); e != nil {
+			t.Fatalf("refused CA mode %o", mode)
+		}
+	}
+	link := filepath.Join(physicalTemp(t), "link.pem")
+	if e := os.Symlink(writeCA(t, 0600), link); e != nil {
+		t.Fatal(e)
+	}
+	if _, e := TLSConfig(&State{ServerCAFile: link}, false); e == nil {
+		t.Fatal("accepted symlink CA")
+	}
+}
+
+func TestPrepareRefusesWhenParentSyncAfterMkdirFails(t *testing.T) {
+	for _, stage := range []string{"parent_open", "parent_sync", "parent_close"} {
+		dir := filepath.Join(physicalTemp(t), "identity")
+		reset := failFiles(t, stage)
+		s, e := Prepare(dir, "https://localhost:8443", "")
+		reset()
+		if s != nil || e == nil {
+			t.Fatalf("%s: success without durable directory", stage)
+		}
+		if _, err := os.Lstat(filepath.Join(dir, "identity.json")); err == nil {
+			t.Fatalf("%s: identity written before directory was durable", stage)
+		}
+	}
+}
+
+func TestPrepareRetrySyncsParentWhenDirectoryAlreadyExists(t *testing.T) {
+	dir := filepath.Join(physicalTemp(t), "identity")
+	reset := failFiles(t, "parent_sync")
+	_, e := Prepare(dir, "https://localhost:8443", "")
+	reset()
+	if e == nil {
+		t.Fatal("first attempt should fail")
+	}
+	var log []string
+	traceFiles(t, &log, false)
+	if _, e = Prepare(dir, "https://localhost:8443", ""); e != nil {
+		t.Fatal(e)
+	}
+	if len(log) == 0 || log[0] != "opendir:"+filepath.Dir(dir) {
+		t.Fatalf("retry skipped parent sync: %v", log)
+	}
+}
+
+func TestServerCAFifoRefusedWithoutBlocking(t *testing.T) {
+	p := filepath.Join(physicalTemp(t), "ca.fifo")
+	if e := syscall.Mkfifo(p, 0600); e != nil {
+		t.Fatal(e)
+	}
+	done := make(chan error, 1)
+	go func() { _, e := TLSConfig(&State{ServerCAFile: p}, false); done <- e }()
+	select {
+	case e := <-done:
+		if e == nil {
+			t.Fatal("accepted FIFO CA")
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("FIFO CA blocked TLS setup")
+	}
+}

apps/host_agent/internal/identity/identity.go

@@ -13,6 +13,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"errors"
+	"io"
 	"math/big"
 	"net/url"
 	"os"
@@ -74,9 +75,17 @@ func Prepare(dir, endpoint, serverCA string) (*State, error) {
 	} else if !os.IsNotExist(e) {
 		return nil, ErrIdentity
 	}
+	if serverCA != "" && !caPathOK(serverCA) {
+		return nil, ErrIdentity
+	}
 	if e := os.Mkdir(dir, 0700); e != nil && !os.IsExist(e) {
 		return nil, ErrIdentity
 	}
+	// Sync even when the directory already exists: a retry after a failed sync
+	// must not report success for an entry that may not be durable.
+	if syncDir(filepath.Dir(dir)) != nil {
+		return nil, ErrIdentity
+	}
 	if e := privatePath(dir, true); e != nil {
 		return nil, e
 	}
@@ -162,6 +171,16 @@ func save(dir string, state *State, exclusive bool) error {
 	if e != nil {
 		return ErrIdentity
 	}
+	if exclusive {
+		// Drop the second name before the directory fsync so one sync makes the
+		// link and the removal durable; a crash cannot leave a second key copy.
+		if identityFiles.remove(temporary) != nil {
+			return ErrIdentity
+		}
+	}
+	return syncDir(dir)
+}
+func syncDir(dir string) error {
 	parent, e := identityFiles.openDir(dir)
 	if e != nil {
 		return ErrIdentity
@@ -173,6 +192,11 @@ func save(dir string, state *State, exclusive bool) error {
 	}
 	return nil
 }
+
+// caPathOK requires an absolute, already-clean server CA path.
+func caPathOK(path string) bool {
+	return filepath.IsAbs(path) && filepath.Clean(path) == path
+}
 func privatePath(path string, directory bool) error {
 	if noSymlinkAncestors(path, false) != nil {
 		return ErrIdentity
@@ -280,6 +304,9 @@ func validate(s *State) error {
 	if s == nil || s.SchemaVersion != 1 || !endpointOK(s.Endpoint) {
 		return ErrIdentity
 	}
+	if s.ServerCAFile != "" && !caPathOK(s.ServerCAFile) {
+		return ErrIdentity
+	}
 	if s.Active == nil {
 		if s.OwnerID != "" || s.HostID != "" || s.Pending == nil || s.Pending.PredecessorIdentityID != "" || s.Pending.Certificate != "" || s.Pending.IssuerCertificate != "" || s.Pending.IdentityID != "" {
 			return ErrIdentity
@@ -435,11 +462,7 @@ func TLSConfig(s *State, pending bool) (*tls.Config, error) {
 		if noSymlinkAncestors(s.ServerCAFile, false) != nil {
 			return nil, ErrIdentity
 		}
-		info, e := os.Lstat(s.ServerCAFile)
-		if e != nil || !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 || info.Size() > 32768 {
-			return nil, ErrIdentity
-		}
-		bytes, e := os.ReadFile(s.ServerCAFile)
+		bytes, e := readCA(s.ServerCAFile)
 		if e != nil {
 			return nil, ErrIdentity
 		}
@@ -548,3 +571,26 @@ func exactKeys(m map[string]any, keys []string) bool {
 	}
 	return true
 }
+
+// readCA opens the CA without following symlinks and checks the open file, so
+// no stat-then-open race can swap it. Ancestor-directory ownership is not
+// checked (known limitation, out of scope for #86).
+func readCA(path string) ([]byte, error) {
+	f, e := os.OpenFile(path, os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC|syscall.O_NONBLOCK, 0)
+	if e != nil {
+		return nil, ErrIdentity
+	}
+	defer f.Close()
+	info, e := f.Stat()
+	if e != nil || !info.Mode().IsRegular() || info.Size() > 32768 || info.Mode().Perm()&0022 != 0 {
+		return nil, ErrIdentity
+	}
+	if unix, ok := info.Sys().(*syscall.Stat_t); !ok || (unix.Uid != 0 && unix.Uid != uint32(os.Geteuid())) {
+		return nil, ErrIdentity
+	}
+	data, e := io.ReadAll(io.LimitReader(f, 32769))
+	if e != nil || len(data) > 32768 {
+		return nil, ErrIdentity
+	}
+	return data, nil
+}

docs/development/ci-acceptance.json

@@ -572,6 +572,13 @@
       "github.com/wstein/workharbor-nxt/apps/host_agent/internal/connection/TestAcknowledgementSequenceFailsClosed",
       "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestAtomicIdentityPersistenceFailuresAndRestartTransitions",
       "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestInitialPossessionDurabilityFailureNeverReportsSuccess",
+      "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestPrepareSyncsParentAfterMkdirAndRemovesTempBeforeDirectorySync",
+      "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestPrepareRefusesWhenTempRemovalFails",
+      "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestServerCAPathMustBeAbsoluteAndClean",
+      "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestServerCAFileRefusesWritableAndSymlinkAndAcceptsPrivate",
+      "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestServerCAFifoRefusedWithoutBlocking",
+      "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestPrepareRefusesWhenParentSyncAfterMkdirFails",
+      "github.com/wstein/workharbor-nxt/apps/host_agent/internal/identity/TestPrepareRetrySyncsParentWhenDirectoryAlreadyExists",
       "github.com/wstein/workharbor-nxt/apps/host_agent/cmd/workharbor-agent/TestPrivateDockerProxyModeRejectsMalformedLifetimeAndSocket",
       "github.com/wstein/workharbor-nxt/apps/host_agent/internal/config/TestDockerfilePlanBindsSelectedStageToExternalBase",
       "github.com/wstein/workharbor-nxt/apps/host_agent/internal/config/TestDockerfilePlanCanonicalizesMixedCaseStagesAndAliases",