diff --git a/proxy/internal/acme/locker.go b/proxy/internal/acme/locker.go index 2f0f18885..f42324736 100644 --- a/proxy/internal/acme/locker.go +++ b/proxy/internal/acme/locker.go @@ -2,12 +2,14 @@ package acme import ( "context" + "fmt" "path/filepath" log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/proxy/internal/flock" "github.com/netbirdio/netbird/proxy/internal/k8s" + "github.com/netbirdio/netbird/shared/management/domain" ) // certLocker provides distributed mutual exclusion for certificate operations. @@ -74,9 +76,15 @@ func newFlockLocker(certDir string, logger *log.Logger) *flockLocker { return &flockLocker{certDir: certDir, logger: logger} } -// Lock acquires an advisory file lock for the given domain. -func (l *flockLocker) Lock(ctx context.Context, domain string) (func(), error) { - lockPath := filepath.Join(l.certDir, domain+".lock") +// Lock acquires an advisory file lock for the given domain. The domain must +// be a valid hostname so the lock file always resolves to a direct child of +// certDir; anything else is rejected before touching the filesystem. +func (l *flockLocker) Lock(ctx context.Context, name string) (func(), error) { + if !domain.IsValidDomainNoWildcard(name) { + return nil, fmt.Errorf("invalid domain %q for lock file", name) + } + + lockPath := filepath.Join(l.certDir, name+".lock") lockFile, err := flock.Lock(ctx, lockPath) if err != nil { return nil, err @@ -89,7 +97,7 @@ func (l *flockLocker) Lock(ctx context.Context, domain string) (func(), error) { return func() { if err := flock.Unlock(lockFile); err != nil { - l.logger.Debugf("release cert lock for domain %q: %v", domain, err) + l.logger.Debugf("release cert lock for domain %q: %v", name, err) } }, nil } diff --git a/proxy/internal/acme/locker_test.go b/proxy/internal/acme/locker_test.go index 39245df0c..f131f64f3 100644 --- a/proxy/internal/acme/locker_test.go +++ b/proxy/internal/acme/locker_test.go @@ -63,3 +63,33 @@ func TestNewCertLockerK8sFallsBackToFlock(t *testing.T) { _, ok := locker.(*flockLocker) assert.True(t, ok, "k8s-lease without SA should fall back to flockLocker") } + +func TestFlockLockerRejectsUnsafeDomain(t *testing.T) { + root := t.TempDir() + certDir := filepath.Join(root, "certs") + require.NoError(t, os.Mkdir(certDir, 0o700)) + locker := newFlockLocker(certDir, nil) + + for _, d := range []string{ + "", + ".", + "..", + "../escape", + "../../etc/cron.d/attacker", + "sub/dir.example.com", + `back\slash.example.com`, + "*.example.com", + } { + unlock, err := locker.Lock(context.Background(), d) + assert.Error(t, err, "domain %q", d) + assert.Nil(t, unlock, "domain %q", d) + } + + assert.NoFileExists(t, filepath.Join(root, "escape.lock")) + certEntries, err := os.ReadDir(certDir) + require.NoError(t, err) + assert.Empty(t, certEntries) + rootEntries, err := os.ReadDir(root) + require.NoError(t, err) + assert.Len(t, rootEntries, 1) +}