Harden public TLS certificate persistence

commit: 8c0f7e846ffeb04d17ec085716f1dcd91447dd8d
author: brice · 5mo ago · 2026-04-29 18:03 UTC
parent: 32d0f87
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

6 files · +303 -45

M internal/adapters/out/acmestore/store.go +157 -23


@@ -9,6 +9,7 @@ import (
9	9		"os"
10	10		"path/filepath"
11	11		"strings"
	12	+	"syscall"
12	13		"time"
13	14
14	15		"github.com/bnema/gordon/internal/boundaries/out"

@@ -28,6 +29,8 @@ const (
28	29		lockFile = ".lock"
29	30		accountFile = "account.json"
30	31		certDir = "certs"
	32	+	backupSuffix = ".old"
	33	+	tempInfix = ".tmp-"
31	34		certFile = "cert.pem"
32	35		chainFile = "chain.pem"
33	36		fullchainFile = "fullchain.pem"

@@ -42,6 +45,8 @@ const (
42	45		pemMode os.FileMode = 0644
43	46		)
44	47
	48	+	var ErrLockHeld = errors.New("acmestore: lock already held")
	49	+
45	50		// Store implements out.CertificateStore using the local filesystem.
46	51		type Store struct {
47	52		root string

@@ -56,8 +61,8 @@ func New(root string) (*Store, error) {
56	61		if err := os.MkdirAll(root, dirMode); err != nil {
57	62		return nil, fmt.Errorf("acmestore: mkdir root: %w", err)
58	63		}
59		-	if err := os.MkdirAll(filepath.Join(root, certDir), dirMode); err != nil {
60		-	return nil, fmt.Errorf("acmestore: mkdir certs: %w", err)
	64	+	if _, err := ensureCertsDir(root); err != nil {
	65	+	return nil, err
61	66		}
62	67		return &Store{root: root}, nil
63	68		}

@@ -114,7 +119,7 @@ func (s *Store) LoadAll(_ context.Context) ([]out.StoredCertificate, error) {
114	119		continue
115	120		}
116	121		// Skip temporary and backup directories created by Save.
117		-	if strings.Contains(id, ".tmp-") \|\| strings.HasSuffix(id, ".old") {
	122	+	if isStoreInternalCertDir(id) {
118	123		continue
119	124		}
120	125

@@ -186,12 +191,15 @@ func (s Store) loadOne(id string) (out.StoredCertificate, error) {
186	191		// All PEM files and metadata are written atomically.
187	192		func (s *Store) Save(_ context.Context, cert out.StoredCertificate) error {
188	193		if !safeID(cert.ID) {
189		-	return fmt.Errorf("acmestore: unsafe certificate id %q", cert.ID)
	194	+	return fmt.Errorf("acmestore: unsafe certificate id %q: %w", cert.ID, domain.ErrPathTraversal)
190	195		}
191	196
192		-	parent := filepath.Join(s.root, certDir)
193		-	if err := os.MkdirAll(parent, dirMode); err != nil {
194		-	return fmt.Errorf("acmestore: mkdir certs: %w", err)
	197	+	parent, err := ensureCertsDir(s.root)
	198	+	if err != nil {
	199	+	return err
	200	+	}
	201	+	if err := recoverCertificateDir(parent, cert.ID); err != nil {
	202	+	return fmt.Errorf("acmestore: recover cert %s: %w", cert.ID, err)
195	203		}
196	204
197	205		dir := filepath.Join(parent, cert.ID)

@@ -210,7 +218,7 @@ func (s *Store) Save(_ context.Context, cert out.StoredCertificate) error {
210	218		return err
211	219		}
212	220
213		-	backupDir := dir + ".old"
	221	+	backupDir := dir + backupSuffix
214	222		_ = os.RemoveAll(backupDir)
215	223		hadExisting := false
216	224		if err := os.Rename(dir, backupDir); err != nil {

@@ -266,11 +274,11 @@ func writeCertificateFiles(dir string, cert out.StoredCertificate) error {
266	274		return nil
267	275		}
268	276
269		-	// Lock acquires an exclusive lock using a lock file at <root>/.lock.
270		-	// It is fail-fast and non-blocking: if the lock file already exists, Lock
271		-	// returns an error immediately without waiting. Context cancellation is
272		-	// respected before attempting the lock acquisition. The returned unlock
273		-	// function removes the lock file to clean up.
	277	+	// Lock acquires an exclusive advisory lock using a lock file at <root>/.lock.
	278	+	// It is fail-fast and non-blocking: if another live process holds the lock,
	279	+	// Lock returns an error immediately without waiting. Context cancellation is
	280	+	// respected before attempting acquisition. The returned unlock function releases
	281	+	// the OS advisory lock, closes the file descriptor, and removes the lock file.
274	282		func (s *Store) Lock(ctx context.Context) (func() error, error) {
275	283		select {
276	284		case <-ctx.Done():

@@ -279,26 +287,152 @@ func (s *Store) Lock(ctx context.Context) (func() error, error) {
279	287		}
280	288
281	289		path := filepath.Join(s.root, lockFile)
282		-	f, err := os.OpenFile(path, os.O_CREATE\|os.O_EXCL\|os.O_WRONLY, lockMode)
	290	+	f, err := os.OpenFile(path, os.O_CREATE\|os.O_RDWR, lockMode)
283	291		if err != nil {
284		-	if os.IsExist(err) {
285		-	return nil, fmt.Errorf("acmestore: lock already held")
	292	+	return nil, fmt.Errorf("acmestore: open lock file: %w", err)
	293	+	}
	294	+
	295	+	fd, err := fileDescriptor(f)
	296	+	if err != nil {
	297	+	_ = f.Close()
	298	+	return nil, err
	299	+	}
	300	+	if err := syscall.Flock(fd, syscall.LOCK_EX\|syscall.LOCK_NB); err != nil {
	301	+	_ = f.Close()
	302	+	if errors.Is(err, syscall.EWOULDBLOCK) \|\| errors.Is(err, syscall.EAGAIN) {
	303	+	return nil, ErrLockHeld
286	304		}
287	305		return nil, fmt.Errorf("acmestore: acquire lock: %w", err)
288	306		}
289		-	if err := f.Close(); err != nil {
290		-	os.Remove(path)
291		-	return nil, fmt.Errorf("acmestore: close lock file: %w", err)
	307	+
	308	+	if err := writeLockMetadata(f); err != nil {
	309	+	_ = syscall.Flock(fd, syscall.LOCK_UN)
	310	+	_ = f.Close()
	311	+	return nil, err
292	312		}
293	313
294	314		return func() error {
295		-	if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
296		-	return fmt.Errorf("acmestore: release lock: %w", err)
297		-	}
298		-	return nil
	315	+	return releaseLockFile(path, f, fd)
299	316		}, nil
300	317		}
301	318
	319	+	func writeLockMetadata(f *os.File) error {
	320	+	if err := f.Truncate(0); err != nil {
	321	+	return fmt.Errorf("acmestore: truncate lock file: %w", err)
	322	+	}
	323	+	if _, err := f.Seek(0, 0); err != nil {
	324	+	return fmt.Errorf("acmestore: seek lock file: %w", err)
	325	+	}
	326	+	metadata := fmt.Sprintf("pid=%d\nhost=%s\ncreated_at=%s\n", os.Getpid(), hostname(), time.Now().UTC().Format(time.RFC3339Nano))
	327	+	if _, err := f.WriteString(metadata); err != nil {
	328	+	return fmt.Errorf("acmestore: write lock metadata: %w", err)
	329	+	}
	330	+	return nil
	331	+	}
	332	+
	333	+	func releaseLockFile(path string, f *os.File, fd int) error {
	334	+	var errs []error
	335	+	if err := syscall.Flock(fd, syscall.LOCK_UN); err != nil {
	336	+	errs = append(errs, fmt.Errorf("unlock: %w", err))
	337	+	}
	338	+	if err := f.Close(); err != nil {
	339	+	errs = append(errs, fmt.Errorf("close: %w", err))
	340	+	}
	341	+	if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
	342	+	errs = append(errs, fmt.Errorf("remove: %w", err))
	343	+	}
	344	+	if err := errors.Join(errs...); err != nil {
	345	+	return fmt.Errorf("acmestore: release lock: %w", err)
	346	+	}
	347	+	return nil
	348	+	}
	349	+
	350	+	func fileDescriptor(f *os.File) (int, error) {
	351	+	fd := f.Fd()
	352	+	if fd > uintptr(^uint(0)>>1) {
	353	+	return 0, fmt.Errorf("acmestore: lock file descriptor overflows int")
	354	+	}
	355	+	return int(fd), nil
	356	+	}
	357	+
	358	+	func hostname() string {
	359	+	host, err := os.Hostname()
	360	+	if err != nil {
	361	+	return "unknown"
	362	+	}
	363	+	return host
	364	+	}
	365	+
	366	+	func ensureCertsDir(root string) (string, error) {
	367	+	certsPath := filepath.Join(root, certDir)
	368	+	if err := os.MkdirAll(certsPath, dirMode); err != nil {
	369	+	return "", fmt.Errorf("acmestore: mkdir certs: %w", err)
	370	+	}
	371	+	if err := recoverCertificateDirs(certsPath); err != nil {
	372	+	return "", fmt.Errorf("acmestore: recover certs: %w", err)
	373	+	}
	374	+	return certsPath, nil
	375	+	}
	376	+
	377	+	func isStoreInternalTempDir(id string) bool {
	378	+	return strings.Contains(id, tempInfix)
	379	+	}
	380	+
	381	+	func isStoreInternalCertDir(id string) bool {
	382	+	return isStoreInternalTempDir(id) \|\| strings.HasSuffix(id, backupSuffix)
	383	+	}
	384	+
	385	+	func recoverCertificateDir(certsPath, id string) error {
	386	+	if !safeID(id) {
	387	+	return nil
	388	+	}
	389	+	backupDir := filepath.Join(certsPath, id+backupSuffix)
	390	+	dir := filepath.Join(certsPath, id)
	391	+	if _, err := os.Stat(dir); err == nil {
	392	+	return nil
	393	+	} else if !os.IsNotExist(err) {
	394	+	return err
	395	+	}
	396	+	if _, err := os.Stat(backupDir); err != nil {
	397	+	if os.IsNotExist(err) {
	398	+	return nil
	399	+	}
	400	+	return err
	401	+	}
	402	+	if err := os.Rename(backupDir, dir); err != nil {
	403	+	return fmt.Errorf("restore backup %s: %w", filepath.Base(backupDir), err)
	404	+	}
	405	+	return nil
	406	+	}
	407	+
	408	+	func recoverCertificateDirs(certsPath string) error {
	409	+	entries, err := os.ReadDir(certsPath)
	410	+	if err != nil {
	411	+	if os.IsNotExist(err) {
	412	+	return nil
	413	+	}
	414	+	return err
	415	+	}
	416	+	for _, entry := range entries {
	417	+	if !entry.IsDir() {
	418	+	continue
	419	+	}
	420	+	name := entry.Name()
	421	+	if isStoreInternalTempDir(name) {
	422	+	if err := os.RemoveAll(filepath.Join(certsPath, name)); err != nil {
	423	+	return fmt.Errorf("remove temp %s: %w", name, err)
	424	+	}
	425	+	continue
	426	+	}
	427	+	if base, ok := strings.CutSuffix(name, backupSuffix); ok {
	428	+	if err := recoverCertificateDir(certsPath, base); err != nil {
	429	+	return err
	430	+	}
	431	+	}
	432	+	}
	433	+	return nil
	434	+	}
	435	+
302	436		// safeID rejects IDs that could cause path traversal.
303	437		func safeID(id string) bool {
304	438		return id != "" && !strings.HasPrefix(id, ".") && !strings.Contains(id, "/") && !strings.Contains(id, "\\") && !strings.Contains(id, "..") && !strings.Contains(id, "\x00")


@@ -9,6 +9,7 @@ import (
9		"os"	9		"os"
10		"path/filepath"	10		"path/filepath"
11		"strings"	11		"strings"
			12	+	"syscall"
12		"time"	13		"time"
13			14
14		"github.com/bnema/gordon/internal/boundaries/out"	15		"github.com/bnema/gordon/internal/boundaries/out"

@@ -28,6 +29,8 @@ const (
28		lockFile = ".lock"	29		lockFile = ".lock"
29		accountFile = "account.json"	30		accountFile = "account.json"
30		certDir = "certs"	31		certDir = "certs"
			32	+	backupSuffix = ".old"
			33	+	tempInfix = ".tmp-"
31		certFile = "cert.pem"	34		certFile = "cert.pem"
32		chainFile = "chain.pem"	35		chainFile = "chain.pem"
33		fullchainFile = "fullchain.pem"	36		fullchainFile = "fullchain.pem"

@@ -42,6 +45,8 @@ const (
42		pemMode os.FileMode = 0644	45		pemMode os.FileMode = 0644
43		)	46		)
44			47
			48	+	var ErrLockHeld = errors.New("acmestore: lock already held")
			49	+
45		// Store implements out.CertificateStore using the local filesystem.	50		// Store implements out.CertificateStore using the local filesystem.
46		type Store struct {	51		type Store struct {
47		root string	52		root string

@@ -56,8 +61,8 @@ func New(root string) (*Store, error) {
56		if err := os.MkdirAll(root, dirMode); err != nil {	61		if err := os.MkdirAll(root, dirMode); err != nil {
57		return nil, fmt.Errorf("acmestore: mkdir root: %w", err)	62		return nil, fmt.Errorf("acmestore: mkdir root: %w", err)
58		}	63		}
59	-	if err := os.MkdirAll(filepath.Join(root, certDir), dirMode); err != nil {	64	+	if _, err := ensureCertsDir(root); err != nil {
60	-	return nil, fmt.Errorf("acmestore: mkdir certs: %w", err)	65	+	return nil, err
61		}	66		}
62		return &Store{root: root}, nil	67		return &Store{root: root}, nil
63		}	68		}

@@ -114,7 +119,7 @@ func (s *Store) LoadAll(_ context.Context) ([]out.StoredCertificate, error) {
114		continue	119		continue
115		}	120		}
116		// Skip temporary and backup directories created by Save.	121		// Skip temporary and backup directories created by Save.
117	-	if strings.Contains(id, ".tmp-") \|\| strings.HasSuffix(id, ".old") {	122	+	if isStoreInternalCertDir(id) {
118		continue	123		continue
119		}	124		}
120			125

@@ -186,12 +191,15 @@ func (s Store) loadOne(id string) (out.StoredCertificate, error) {
186		// All PEM files and metadata are written atomically.	191		// All PEM files and metadata are written atomically.
187		func (s *Store) Save(_ context.Context, cert out.StoredCertificate) error {	192		func (s *Store) Save(_ context.Context, cert out.StoredCertificate) error {
188		if !safeID(cert.ID) {	193		if !safeID(cert.ID) {
189	-	return fmt.Errorf("acmestore: unsafe certificate id %q", cert.ID)	194	+	return fmt.Errorf("acmestore: unsafe certificate id %q: %w", cert.ID, domain.ErrPathTraversal)
190		}	195		}
191			196
192	-	parent := filepath.Join(s.root, certDir)	197	+	parent, err := ensureCertsDir(s.root)
193	-	if err := os.MkdirAll(parent, dirMode); err != nil {	198	+	if err != nil {
194	-	return fmt.Errorf("acmestore: mkdir certs: %w", err)	199	+	return err
			200	+	}
			201	+	if err := recoverCertificateDir(parent, cert.ID); err != nil {
			202	+	return fmt.Errorf("acmestore: recover cert %s: %w", cert.ID, err)
195		}	203		}
196			204
197		dir := filepath.Join(parent, cert.ID)	205		dir := filepath.Join(parent, cert.ID)

@@ -210,7 +218,7 @@ func (s *Store) Save(_ context.Context, cert out.StoredCertificate) error {
210		return err	218		return err
211		}	219		}
212			220
213	-	backupDir := dir + ".old"	221	+	backupDir := dir + backupSuffix
214		_ = os.RemoveAll(backupDir)	222		_ = os.RemoveAll(backupDir)
215		hadExisting := false	223		hadExisting := false
216		if err := os.Rename(dir, backupDir); err != nil {	224		if err := os.Rename(dir, backupDir); err != nil {

@@ -266,11 +274,11 @@ func writeCertificateFiles(dir string, cert out.StoredCertificate) error {
266		return nil	274		return nil
267		}	275		}
268			276
269	-	// Lock acquires an exclusive lock using a lock file at <root>/.lock.	277	+	// Lock acquires an exclusive advisory lock using a lock file at <root>/.lock.
270	-	// It is fail-fast and non-blocking: if the lock file already exists, Lock	278	+	// It is fail-fast and non-blocking: if another live process holds the lock,
271	-	// returns an error immediately without waiting. Context cancellation is	279	+	// Lock returns an error immediately without waiting. Context cancellation is
272	-	// respected before attempting the lock acquisition. The returned unlock	280	+	// respected before attempting acquisition. The returned unlock function releases
273	-	// function removes the lock file to clean up.	281	+	// the OS advisory lock, closes the file descriptor, and removes the lock file.
274		func (s *Store) Lock(ctx context.Context) (func() error, error) {	282		func (s *Store) Lock(ctx context.Context) (func() error, error) {
275		select {	283		select {
276		case <-ctx.Done():	284		case <-ctx.Done():

@@ -279,26 +287,152 @@ func (s *Store) Lock(ctx context.Context) (func() error, error) {
279		}	287		}
280			288
281		path := filepath.Join(s.root, lockFile)	289		path := filepath.Join(s.root, lockFile)
282	-	f, err := os.OpenFile(path, os.O_CREATE\|os.O_EXCL\|os.O_WRONLY, lockMode)	290	+	f, err := os.OpenFile(path, os.O_CREATE\|os.O_RDWR, lockMode)
283		if err != nil {	291		if err != nil {
284	-	if os.IsExist(err) {	292	+	return nil, fmt.Errorf("acmestore: open lock file: %w", err)
285	-	return nil, fmt.Errorf("acmestore: lock already held")	293	+	}
			294	+
			295	+	fd, err := fileDescriptor(f)
			296	+	if err != nil {
			297	+	_ = f.Close()
			298	+	return nil, err
			299	+	}
			300	+	if err := syscall.Flock(fd, syscall.LOCK_EX\|syscall.LOCK_NB); err != nil {
			301	+	_ = f.Close()
			302	+	if errors.Is(err, syscall.EWOULDBLOCK) \|\| errors.Is(err, syscall.EAGAIN) {
			303	+	return nil, ErrLockHeld
286		}	304		}
287		return nil, fmt.Errorf("acmestore: acquire lock: %w", err)	305		return nil, fmt.Errorf("acmestore: acquire lock: %w", err)
288		}	306		}
289	-	if err := f.Close(); err != nil {	307	+
290	-	os.Remove(path)	308	+	if err := writeLockMetadata(f); err != nil {
291	-	return nil, fmt.Errorf("acmestore: close lock file: %w", err)	309	+	_ = syscall.Flock(fd, syscall.LOCK_UN)
			310	+	_ = f.Close()
			311	+	return nil, err
292		}	312		}
293			313
294		return func() error {	314		return func() error {
295	-	if err := os.Remove(path); err != nil && !os.IsNotExist(err) {	315	+	return releaseLockFile(path, f, fd)
296	-	return fmt.Errorf("acmestore: release lock: %w", err)
297	-	}
298	-	return nil
299		}, nil	316		}, nil
300		}	317		}
301			318
			319	+	func writeLockMetadata(f *os.File) error {
			320	+	if err := f.Truncate(0); err != nil {
			321	+	return fmt.Errorf("acmestore: truncate lock file: %w", err)
			322	+	}
			323	+	if _, err := f.Seek(0, 0); err != nil {
			324	+	return fmt.Errorf("acmestore: seek lock file: %w", err)
			325	+	}
			326	+	metadata := fmt.Sprintf("pid=%d\nhost=%s\ncreated_at=%s\n", os.Getpid(), hostname(), time.Now().UTC().Format(time.RFC3339Nano))
			327	+	if _, err := f.WriteString(metadata); err != nil {
			328	+	return fmt.Errorf("acmestore: write lock metadata: %w", err)
			329	+	}
			330	+	return nil
			331	+	}
			332	+
			333	+	func releaseLockFile(path string, f *os.File, fd int) error {
			334	+	var errs []error
			335	+	if err := syscall.Flock(fd, syscall.LOCK_UN); err != nil {
			336	+	errs = append(errs, fmt.Errorf("unlock: %w", err))
			337	+	}
			338	+	if err := f.Close(); err != nil {
			339	+	errs = append(errs, fmt.Errorf("close: %w", err))
			340	+	}
			341	+	if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
			342	+	errs = append(errs, fmt.Errorf("remove: %w", err))
			343	+	}
			344	+	if err := errors.Join(errs...); err != nil {
			345	+	return fmt.Errorf("acmestore: release lock: %w", err)
			346	+	}
			347	+	return nil
			348	+	}
			349	+
			350	+	func fileDescriptor(f *os.File) (int, error) {
			351	+	fd := f.Fd()
			352	+	if fd > uintptr(^uint(0)>>1) {
			353	+	return 0, fmt.Errorf("acmestore: lock file descriptor overflows int")
			354	+	}
			355	+	return int(fd), nil
			356	+	}
			357	+
			358	+	func hostname() string {
			359	+	host, err := os.Hostname()
			360	+	if err != nil {
			361	+	return "unknown"
			362	+	}
			363	+	return host
			364	+	}
			365	+
			366	+	func ensureCertsDir(root string) (string, error) {
			367	+	certsPath := filepath.Join(root, certDir)
			368	+	if err := os.MkdirAll(certsPath, dirMode); err != nil {
			369	+	return "", fmt.Errorf("acmestore: mkdir certs: %w", err)
			370	+	}
			371	+	if err := recoverCertificateDirs(certsPath); err != nil {
			372	+	return "", fmt.Errorf("acmestore: recover certs: %w", err)
			373	+	}
			374	+	return certsPath, nil
			375	+	}
			376	+
			377	+	func isStoreInternalTempDir(id string) bool {
			378	+	return strings.Contains(id, tempInfix)
			379	+	}
			380	+
			381	+	func isStoreInternalCertDir(id string) bool {
			382	+	return isStoreInternalTempDir(id) \|\| strings.HasSuffix(id, backupSuffix)
			383	+	}
			384	+
			385	+	func recoverCertificateDir(certsPath, id string) error {
			386	+	if !safeID(id) {
			387	+	return nil
			388	+	}
			389	+	backupDir := filepath.Join(certsPath, id+backupSuffix)
			390	+	dir := filepath.Join(certsPath, id)
			391	+	if _, err := os.Stat(dir); err == nil {
			392	+	return nil
			393	+	} else if !os.IsNotExist(err) {
			394	+	return err
			395	+	}
			396	+	if _, err := os.Stat(backupDir); err != nil {
			397	+	if os.IsNotExist(err) {
			398	+	return nil
			399	+	}
			400	+	return err
			401	+	}
			402	+	if err := os.Rename(backupDir, dir); err != nil {
			403	+	return fmt.Errorf("restore backup %s: %w", filepath.Base(backupDir), err)
			404	+	}
			405	+	return nil
			406	+	}
			407	+
			408	+	func recoverCertificateDirs(certsPath string) error {
			409	+	entries, err := os.ReadDir(certsPath)
			410	+	if err != nil {
			411	+	if os.IsNotExist(err) {
			412	+	return nil
			413	+	}
			414	+	return err
			415	+	}
			416	+	for _, entry := range entries {
			417	+	if !entry.IsDir() {
			418	+	continue
			419	+	}
			420	+	name := entry.Name()
			421	+	if isStoreInternalTempDir(name) {
			422	+	if err := os.RemoveAll(filepath.Join(certsPath, name)); err != nil {
			423	+	return fmt.Errorf("remove temp %s: %w", name, err)
			424	+	}
			425	+	continue
			426	+	}
			427	+	if base, ok := strings.CutSuffix(name, backupSuffix); ok {
			428	+	if err := recoverCertificateDir(certsPath, base); err != nil {
			429	+	return err
			430	+	}
			431	+	}
			432	+	}
			433	+	return nil
			434	+	}
			435	+
302		// safeID rejects IDs that could cause path traversal.	436		// safeID rejects IDs that could cause path traversal.
303		func safeID(id string) bool {	437		func safeID(id string) bool {
304		return id != "" && !strings.HasPrefix(id, ".") && !strings.Contains(id, "/") && !strings.Contains(id, "\\") && !strings.Contains(id, "..") && !strings.Contains(id, "\x00")	438		return id != "" && !strings.HasPrefix(id, ".") && !strings.Contains(id, "/") && !strings.Contains(id, "\\") && !strings.Contains(id, "..") && !strings.Contains(id, "\x00")

M internal/adapters/out/acmestore/store_test.go +70 -0


@@ -138,10 +138,80 @@ func TestStoreRejectsUnsafeID(t *testing.T) {
138	138		err = store.Save(ctx, cert)
139	139		require.Error(t, err)
140	140		assert.Contains(t, err.Error(), "unsafe certificate id")
	141	+	assert.ErrorIs(t, err, domain.ErrPathTraversal)
141	142		})
142	143		}
143	144		}
144	145
	146	+	func TestStoreLockAcquireRelease(t *testing.T) {
	147	+	root := t.TempDir()
	148	+	ctx := context.Background()
	149	+
	150	+	store, err := New(root)
	151	+	require.NoError(t, err)
	152	+
	153	+	unlock, err := store.Lock(ctx)
	154	+	require.NoError(t, err)
	155	+	assert.FileExists(t, filepath.Join(root, lockFile))
	156	+
	157	+	_, err = store.Lock(ctx)
	158	+	require.Error(t, err)
	159	+	assert.Contains(t, err.Error(), "lock already held")
	160	+
	161	+	require.NoError(t, unlock())
	162	+	assert.NoFileExists(t, filepath.Join(root, lockFile))
	163	+
	164	+	unlock, err = store.Lock(ctx)
	165	+	require.NoError(t, err)
	166	+	require.NoError(t, unlock())
	167	+	}
	168	+
	169	+	func TestStoreLockContextCanceled(t *testing.T) {
	170	+	root := t.TempDir()
	171	+	store, err := New(root)
	172	+	require.NoError(t, err)
	173	+
	174	+	ctx, cancel := context.WithCancel(context.Background())
	175	+	cancel()
	176	+
	177	+	_, err = store.Lock(ctx)
	178	+	require.ErrorIs(t, err, context.Canceled)
	179	+	assert.NoFileExists(t, filepath.Join(root, lockFile))
	180	+	}
	181	+
	182	+	func TestStoreNewRestoresBackupDir(t *testing.T) {
	183	+	root := t.TempDir()
	184	+	ctx := context.Background()
	185	+
	186	+	store, err := New(root)
	187	+	require.NoError(t, err)
	188	+
	189	+	fullchainPEM, privKeyPEM := generateTestCertPEM(t)
	190	+	cert := out.StoredCertificate{
	191	+	ID: "restore.example.com",
	192	+	Names: []string{"restore.example.com"},
	193	+	Challenge: domain.ACMEChallengeHTTP01,
	194	+	CertPEM: fullchainPEM,
	195	+	FullchainPEM: fullchainPEM,
	196	+	PrivateKeyPEM: privKeyPEM,
	197	+	}
	198	+	require.NoError(t, store.Save(ctx, cert))
	199	+
	200	+	certPath := filepath.Join(root, certDir, cert.ID)
	201	+	backupPath := certPath + ".old"
	202	+	require.NoError(t, os.Rename(certPath, backupPath))
	203	+	require.NoDirExists(t, certPath)
	204	+
	205	+	store, err = New(root)
	206	+	require.NoError(t, err)
	207	+	assert.DirExists(t, certPath)
	208	+
	209	+	certs, err := store.LoadAll(ctx)
	210	+	require.NoError(t, err)
	211	+	require.Len(t, certs, 1)
	212	+	assert.Equal(t, cert.ID, certs[0].ID)
	213	+	}
	214	+
145	215		func TestStoreSaveLoadAccount(t *testing.T) {
146	216		root := t.TempDir()
147	217		ctx := context.Background()


@@ -138,10 +138,80 @@ func TestStoreRejectsUnsafeID(t *testing.T) {
138	err = store.Save(ctx, cert)	138		err = store.Save(ctx, cert)
139	require.Error(t, err)	139		require.Error(t, err)
140	assert.Contains(t, err.Error(), "unsafe certificate id")	140		assert.Contains(t, err.Error(), "unsafe certificate id")
		141	+	assert.ErrorIs(t, err, domain.ErrPathTraversal)
141	})	142		})
142	}	143		}
143	}	144		}
144		145
		146	+	func TestStoreLockAcquireRelease(t *testing.T) {
		147	+	root := t.TempDir()
		148	+	ctx := context.Background()
		149	+
		150	+	store, err := New(root)
		151	+	require.NoError(t, err)
		152	+
		153	+	unlock, err := store.Lock(ctx)
		154	+	require.NoError(t, err)
		155	+	assert.FileExists(t, filepath.Join(root, lockFile))
		156	+
		157	+	_, err = store.Lock(ctx)
		158	+	require.Error(t, err)
		159	+	assert.Contains(t, err.Error(), "lock already held")
		160	+
		161	+	require.NoError(t, unlock())
		162	+	assert.NoFileExists(t, filepath.Join(root, lockFile))
		163	+
		164	+	unlock, err = store.Lock(ctx)
		165	+	require.NoError(t, err)
		166	+	require.NoError(t, unlock())
		167	+	}
		168	+
		169	+	func TestStoreLockContextCanceled(t *testing.T) {
		170	+	root := t.TempDir()
		171	+	store, err := New(root)
		172	+	require.NoError(t, err)
		173	+
		174	+	ctx, cancel := context.WithCancel(context.Background())
		175	+	cancel()
		176	+
		177	+	_, err = store.Lock(ctx)
		178	+	require.ErrorIs(t, err, context.Canceled)
		179	+	assert.NoFileExists(t, filepath.Join(root, lockFile))
		180	+	}
		181	+
		182	+	func TestStoreNewRestoresBackupDir(t *testing.T) {
		183	+	root := t.TempDir()
		184	+	ctx := context.Background()
		185	+
		186	+	store, err := New(root)
		187	+	require.NoError(t, err)
		188	+
		189	+	fullchainPEM, privKeyPEM := generateTestCertPEM(t)
		190	+	cert := out.StoredCertificate{
		191	+	ID: "restore.example.com",
		192	+	Names: []string{"restore.example.com"},
		193	+	Challenge: domain.ACMEChallengeHTTP01,
		194	+	CertPEM: fullchainPEM,
		195	+	FullchainPEM: fullchainPEM,
		196	+	PrivateKeyPEM: privKeyPEM,
		197	+	}
		198	+	require.NoError(t, store.Save(ctx, cert))
		199	+
		200	+	certPath := filepath.Join(root, certDir, cert.ID)
		201	+	backupPath := certPath + ".old"
		202	+	require.NoError(t, os.Rename(certPath, backupPath))
		203	+	require.NoDirExists(t, certPath)
		204	+
		205	+	store, err = New(root)
		206	+	require.NoError(t, err)
		207	+	assert.DirExists(t, certPath)
		208	+
		209	+	certs, err := store.LoadAll(ctx)
		210	+	require.NoError(t, err)
		211	+	require.Len(t, certs, 1)
		212	+	assert.Equal(t, cert.ID, certs[0].ID)
		213	+	}
		214	+
145	func TestStoreSaveLoadAccount(t *testing.T) {	215		func TestStoreSaveLoadAccount(t *testing.T) {
146	root := t.TempDir()	216		root := t.TempDir()
147	ctx := context.Background()	217		ctx := context.Background()

M internal/usecase/publictls/renewal.go +3 -2


@@ -6,6 +6,7 @@ import (
6	6		"time"
7	7
8	8		"github.com/bnema/gordon/internal/boundaries/out"
	9	+	"github.com/bnema/gordon/internal/domain"
9	10		)
10	11
11	12		// renewalWindow is the window before certificate expiry when renewal should begin.

@@ -89,10 +90,10 @@ func (s *Service) renewDueCertificates(ctx context.Context, now time.Time) error
89	90		return nil
90	91		}
91	92		if s.deps.Store == nil {
92		-	return fmt.Errorf("certificate store is nil")
	93	+	return fmt.Errorf("%w: certificate store is nil", domain.ErrCertificateStoreRequired)
93	94		}
94	95		if s.deps.Issuer == nil {
95		-	return fmt.Errorf("certificate issuer is nil")
	96	+	return fmt.Errorf("%w: certificate issuer is nil", domain.ErrCertificateIssuerRequired)
96	97		}
97	98
98	99		// Collect due certs from in-memory cache under s.mu (no store lock needed).

 	"time"
 	"github.com/bnema/gordon/internal/boundaries/out"
+	"github.com/bnema/gordon/internal/domain"
 )
 // renewalWindow is the window before certificate expiry when renewal should begin.
 		return nil
 	}
 	if s.deps.Store == nil {
-		return fmt.Errorf("certificate store is nil")
+		return fmt.Errorf("%w: certificate store is nil", domain.ErrCertificateStoreRequired)
 	}
 	if s.deps.Issuer == nil {
-		return fmt.Errorf("certificate issuer is nil")
+		return fmt.Errorf("%w: certificate issuer is nil", domain.ErrCertificateIssuerRequired)
 	}
 	// Collect due certs from in-memory cache under s.mu (no store lock needed).

M internal/usecase/publictls/renewal_test.go +14 -0


@@ -56,6 +56,20 @@ func TestShouldRenew(t *testing.T) {
56	56		}
57	57		}
58	58
	59	+	func TestRenewDueCertificatesRequiresDependencies(t *testing.T) {
	60	+	cfg := Config{Enabled: true}
	61	+	svc := NewService(cfg, ServiceDeps{})
	62	+
	63	+	err := svc.renewDueCertificates(context.Background(), time.Now())
	64	+	require.Error(t, err)
	65	+	assert.ErrorIs(t, err, domain.ErrCertificateStoreRequired)
	66	+
	67	+	svc.deps.Store, _ = newMockCertificateStore(t)
	68	+	err = svc.renewDueCertificates(context.Background(), time.Now())
	69	+	require.Error(t, err)
	70	+	assert.ErrorIs(t, err, domain.ErrCertificateIssuerRequired)
	71	+	}
	72	+
59	73		// ---------------------------------------------------------------------------
60	74		// RenewalLoopStops
61	75		// ---------------------------------------------------------------------------


@@ -56,6 +56,20 @@ func TestShouldRenew(t *testing.T) {
56	}	56		}
57	}	57		}
58		58
		59	+	func TestRenewDueCertificatesRequiresDependencies(t *testing.T) {
		60	+	cfg := Config{Enabled: true}
		61	+	svc := NewService(cfg, ServiceDeps{})
		62	+
		63	+	err := svc.renewDueCertificates(context.Background(), time.Now())
		64	+	require.Error(t, err)
		65	+	assert.ErrorIs(t, err, domain.ErrCertificateStoreRequired)
		66	+
		67	+	svc.deps.Store, _ = newMockCertificateStore(t)
		68	+	err = svc.renewDueCertificates(context.Background(), time.Now())
		69	+	require.Error(t, err)
		70	+	assert.ErrorIs(t, err, domain.ErrCertificateIssuerRequired)
		71	+	}
		72	+
59	// ---------------------------------------------------------------------------	73		// ---------------------------------------------------------------------------
60	// RenewalLoopStops	74		// RenewalLoopStops
61	// ---------------------------------------------------------------------------	75		// ---------------------------------------------------------------------------

M internal/usecase/publictls/service.go +33 -20


@@ -91,17 +91,8 @@ func (s *Service) Load(ctx context.Context) error {
91	91
92	92		for i := range stored {
93	93		cert := &stored[i]
94		-	// If the tls.Certificate is empty but PEM data is available, parse it.
95		-	if len(cert.Certificate.Certificate) == 0 && len(cert.FullchainPEM) > 0 && len(cert.PrivateKeyPEM) > 0 {
96		-	parsed, parseErr := tls.X509KeyPair(cert.FullchainPEM, cert.PrivateKeyPEM)
97		-	if parseErr != nil {
98		-	s.lastErr[cert.ID] = fmt.Sprintf("parse stored certificate: %v", parseErr)
99		-	continue
100		-	}
101		-	cert.Certificate = parsed
102		-	}
103		-	if len(cert.Certificate.Certificate) == 0 {
104		-	s.lastErr[cert.ID] = "stored certificate is empty"
	94	+	if parseErr := populateStoredCertificate(cert); parseErr != nil {
	95	+	s.lastErr[cert.ID] = parseErr.Error()
105	96		continue
106	97		}
107	98		s.certs[cert.ID] = cert

@@ -116,16 +107,16 @@ func (s *Service) Load(ctx context.Context) error {
116	107		// Reconcile ensures all desired certificates are obtained and cached.
117	108		// If ACME is disabled, it is a no-op.
118	109		func (s *Service) Reconcile(ctx context.Context) error {
119		-	if !s.cfg.Enabled {
120		-	s.log.Debug().Msg("acme disabled, skipping reconcile")
121		-	return nil
122		-	}
123		-
124	110		ctx = zerowrap.CtxWithFields(ctx, map[string]any{
125	111		zerowrap.FieldLayer: "usecase",
126	112		zerowrap.FieldUseCase: "Reconcile",
127	113		})
128	114		log := zerowrap.FromCtx(ctx)
	115	+
	116	+	if !s.cfg.Enabled {
	117	+	log.Debug().Msg("acme disabled, skipping reconcile")
	118	+	return nil
	119	+	}
129	120		log.Debug().Msg("starting certificate reconciliation")
130	121
131	122		if s.deps.Store == nil {

@@ -144,12 +135,11 @@ func (s *Service) Reconcile(ctx context.Context) error {
144	135		// Determine effective challenge mode.
145	136		effective := s.deps.Effective
146	137		if effective.Mode == "" {
147		-	// Derive conservatively from config.
148		-	mode, err := domain.ParseACMEChallengeMode(s.cfg.Challenge)
	138	+	resolved, err := ResolveEffectiveChallenge(ctx, s.cfg, nil)
149	139		if err != nil {
150		-	mode = domain.ACMEChallengeHTTP01
	140	+	return fmt.Errorf("resolve effective challenge: %w", err)
151	141		}
152		-	effective.Mode = mode
	142	+	effective = resolved
153	143		}
154	144
155	145		// Get route hosts early to build required hosts set before target derivation.

@@ -214,6 +204,23 @@ func (s *Service) Reconcile(ctx context.Context) error {
214	204		return nil
215	205		}
216	206
	207	+	func populateStoredCertificate(cert *out.StoredCertificate) error {
	208	+	if cert == nil {
	209	+	return fmt.Errorf("stored certificate is nil")
	210	+	}
	211	+	if len(cert.Certificate.Certificate) == 0 && len(cert.FullchainPEM) > 0 && len(cert.PrivateKeyPEM) > 0 {
	212	+	parsed, err := tls.X509KeyPair(cert.FullchainPEM, cert.PrivateKeyPEM)
	213	+	if err != nil {
	214	+	return fmt.Errorf("parse stored certificate: %w", err)
	215	+	}
	216	+	cert.Certificate = parsed
	217	+	}
	218	+	if len(cert.Certificate.Certificate) == 0 {
	219	+	return fmt.Errorf("stored certificate is empty")
	220	+	}
	221	+	return nil
	222	+	}
	223	+
217	224		func canonicalHostSet(hosts []string) map[string]struct{} {
218	225		required := make(map[string]struct{}, len(hosts))
219	226		for _, host := range hosts {

@@ -259,6 +266,12 @@ func (s *Service) reconcileMissingTargets(ctx context.Context, missing []Certifi
259	266		s.mu.Unlock()
260	267		continue
261	268		}
	269	+	if err := populateStoredCertificate(stored); err != nil {
	270	+	s.mu.Lock()
	271	+	s.lastErr[target.ID] = err.Error()
	272	+	s.mu.Unlock()
	273	+	continue
	274	+	}
262	275
263	276		if err := s.deps.Store.Save(ctx, *stored); err != nil {
264	277		s.mu.Lock()

 	for i := range stored {
 		cert := &stored[i]
-		// If the tls.Certificate is empty but PEM data is available, parse it.
+		if parseErr := populateStoredCertificate(cert); parseErr != nil {
-		if len(cert.Certificate.Certificate) == 0 && len(cert.FullchainPEM) > 0 && len(cert.PrivateKeyPEM) > 0 {
+			s.lastErr[cert.ID] = parseErr.Error()
-			parsed, parseErr := tls.X509KeyPair(cert.FullchainPEM, cert.PrivateKeyPEM)
-			if parseErr != nil {
-				s.lastErr[cert.ID] = fmt.Sprintf("parse stored certificate: %v", parseErr)
-				continue
-			}
-			cert.Certificate = parsed
-		}
-		if len(cert.Certificate.Certificate) == 0 {
-			s.lastErr[cert.ID] = "stored certificate is empty"
 			continue
 		}
 		s.certs[cert.ID] = cert
 // Reconcile ensures all desired certificates are obtained and cached.
 // If ACME is disabled, it is a no-op.
 func (s *Service) Reconcile(ctx context.Context) error {
-	if !s.cfg.Enabled {
-		s.log.Debug().Msg("acme disabled, skipping reconcile")
-		return nil
-	}
 	ctx = zerowrap.CtxWithFields(ctx, map[string]any{
 		zerowrap.FieldLayer:   "usecase",
 		zerowrap.FieldUseCase: "Reconcile",
 	})
 	log := zerowrap.FromCtx(ctx)
+	if !s.cfg.Enabled {
+		log.Debug().Msg("acme disabled, skipping reconcile")
+		return nil
+	}
 	log.Debug().Msg("starting certificate reconciliation")
 	if s.deps.Store == nil {
 	// Determine effective challenge mode.
 	effective := s.deps.Effective
 	if effective.Mode == "" {
-		// Derive conservatively from config.
+		resolved, err := ResolveEffectiveChallenge(ctx, s.cfg, nil)
-		mode, err := domain.ParseACMEChallengeMode(s.cfg.Challenge)
 		if err != nil {
-			mode = domain.ACMEChallengeHTTP01
+			return fmt.Errorf("resolve effective challenge: %w", err)
 		}
-		effective.Mode = mode
+		effective = resolved
 	}
 	// Get route hosts early to build required hosts set before target derivation.
 	return nil
 }
+func populateStoredCertificate(cert *out.StoredCertificate) error {
+	if cert == nil {
+		return fmt.Errorf("stored certificate is nil")
+	}
+	if len(cert.Certificate.Certificate) == 0 && len(cert.FullchainPEM) > 0 && len(cert.PrivateKeyPEM) > 0 {
+		parsed, err := tls.X509KeyPair(cert.FullchainPEM, cert.PrivateKeyPEM)
+		if err != nil {
+			return fmt.Errorf("parse stored certificate: %w", err)
+		}
+		cert.Certificate = parsed
+	}
+	if len(cert.Certificate.Certificate) == 0 {
+		return fmt.Errorf("stored certificate is empty")
+	}
+	return nil
+}
 func canonicalHostSet(hosts []string) map[string]struct{} {
 	required := make(map[string]struct{}, len(hosts))
 	for _, host := range hosts {
 			s.mu.Unlock()
 			continue
 		}
+		if err := populateStoredCertificate(stored); err != nil {
+			s.mu.Lock()
+			s.lastErr[target.ID] = err.Error()
+			s.mu.Unlock()
+			continue
+		}
 		if err := s.deps.Store.Save(ctx, *stored); err != nil {
 			s.mu.Lock()

M internal/usecase/publictls/service_test.go +26 -0


@@ -309,6 +309,32 @@ func TestServiceReconcileObtainsMissingHTTP01Cert(t *testing.T) {
309	309		assert.NotEmpty(t, stored[0].Certificate.Certificate)
310	310		}
311	311
	312	+	func TestServiceReconcileResolvesMissingEffectiveChallenge(t *testing.T) {
	313	+	ctx := context.Background()
	314	+	routes := &fakeRoutes{routes: []domain.Route{{Domain: "app.example.com"}}}
	315	+	issuer, _ := newMockPublicCertificateIssuer(t, nil, nil)
	316	+	store, _ := newMockCertificateStore(t)
	317	+	cfg := Config{
	318	+	Enabled: true,
	319	+	Email: "admin@example.com",
	320	+	Challenge: "http-01",
	321	+	HTTPPort: 8088,
	322	+	// Missing TLSPort should be caught by ResolveEffectiveChallenge rather
	323	+	// than silently defaulting an effective mode.
	324	+	}
	325	+
	326	+	svc := NewService(cfg, ServiceDeps{
	327	+	Config: cfg,
	328	+	Routes: routes,
	329	+	Issuer: issuer,
	330	+	Store: store,
	331	+	})
	332	+
	333	+	err := svc.Reconcile(ctx)
	334	+	require.Error(t, err)
	335	+	assert.ErrorIs(t, err, domain.ErrACMEChallengeInvalid)
	336	+	}
	337	+
312	338		func TestServiceStatusReportsCoverage(t *testing.T) {
313	339		ctx := context.Background()
314	340


@@ -309,6 +309,32 @@ func TestServiceReconcileObtainsMissingHTTP01Cert(t *testing.T) {
309	assert.NotEmpty(t, stored[0].Certificate.Certificate)	309		assert.NotEmpty(t, stored[0].Certificate.Certificate)
310	}	310		}
311		311
		312	+	func TestServiceReconcileResolvesMissingEffectiveChallenge(t *testing.T) {
		313	+	ctx := context.Background()
		314	+	routes := &fakeRoutes{routes: []domain.Route{{Domain: "app.example.com"}}}
		315	+	issuer, _ := newMockPublicCertificateIssuer(t, nil, nil)
		316	+	store, _ := newMockCertificateStore(t)
		317	+	cfg := Config{
		318	+	Enabled: true,
		319	+	Email: "admin@example.com",
		320	+	Challenge: "http-01",
		321	+	HTTPPort: 8088,
		322	+	// Missing TLSPort should be caught by ResolveEffectiveChallenge rather
		323	+	// than silently defaulting an effective mode.
		324	+	}
		325	+
		326	+	svc := NewService(cfg, ServiceDeps{
		327	+	Config: cfg,
		328	+	Routes: routes,
		329	+	Issuer: issuer,
		330	+	Store: store,
		331	+	})
		332	+
		333	+	err := svc.Reconcile(ctx)
		334	+	require.Error(t, err)
		335	+	assert.ErrorIs(t, err, domain.ErrACMEChallengeInvalid)
		336	+	}
		337	+
312	func TestServiceStatusReportsCoverage(t *testing.T) {	338		func TestServiceStatusReportsCoverage(t *testing.T) {
313	ctx := context.Background()	339		ctx := context.Background()
314		340