Skip to content

Commit 0951902

Browse files
authored
logs: enable NoFollow for process_log-discovered paths
Turn on the no-follow enforcement plumbed through in the previous stack entries, for the one provider it's meant for: - pkg/logs/schedulers/ad/scheduler.go sets cfg.NoFollow = true for sources whose config.Provider is names.ProcessLog. process_log paths come from readlink(/proc/<pid>/fd/<n>); the kernel resolves all symlinks at file-open time, so the string in /proc/fd is already canonical - any symlink appearing later at that path was planted after discovery and indicates an attacker-controlled swap. Other providers (file, kubernetes, ...) are unaffected: their paths are explicitly specified by the user, and it's up to the user to ensure such a path isn't swapped for a symlink by an untrusted party. - comp/core/autodiscovery/providers/process_log.go's checkFileReadable now opens with privilegedlogsclient.OpenNoFollow instead of Open, to match what the tailer does when it actually reads the discovered file. Closes DSCVR-475. Final PR in the stack split out of PR #51746 for easier review; depends on the tailer/fingerprinter NoFollow-plumbing PR. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Vincent Whitchurch <vincent.whitchurch@datadoghq.com>
1 parent 8a4b41e commit 0951902

3 files changed

Lines changed: 65 additions & 1 deletion

File tree

comp/core/autodiscovery/providers/process_log.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func checkFileReadable(logPath string) error {
219219
// Check readability with the privileged logs client to match what the
220220
// log tailer uses. That client can use the privileged logs module in
221221
// system-probe if it is available.
222-
file, err := privilegedlogsclient.Open(logPath)
222+
file, err := privilegedlogsclient.OpenNoFollow(logPath)
223223
if err != nil {
224224
log.Infof("Discovered log file %s could not be opened: %v", logPath, err)
225225
return err

pkg/logs/schedulers/ad/scheduler.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,14 @@ func CreateSources(config integration.Config) ([]*sourcesPkg.LogSource, error) {
262262

263263
cfg.IntegrationSourceIndex = index
264264
cfg.IntegrationSource = config.Source
265+
if config.Provider == names.ProcessLog {
266+
// process_log paths come from readlink(/proc/<pid>/fd/<n>). The Linux
267+
// kernel resolves all symlinks at file-open time, so the string in
268+
// /proc/fd is already canonical — any symlink that appears later was
269+
// planted after discovery and indicates an attacker-controlled swap.
270+
// NoFollow tells the file tailer to open with O_NOFOLLOW on each component.
271+
cfg.NoFollow = true
272+
}
265273

266274
if service != nil {
267275
// a config defined in a container label or a pod annotation does not always contain a type,

pkg/logs/schedulers/ad/scheduler_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,3 +496,59 @@ func TestCreateSourcesWithNilConfigurations(t *testing.T) {
496496
})
497497
}
498498
}
499+
500+
// TestNoFollowFlagSetOnConfig verifies that sources created from a process_log
501+
// provider have NoFollow=true, and that sources from other providers do not.
502+
// This flag is used by the file tailer to enforce O_NOFOLLOW opens, preventing
503+
// symlink-swap attacks on process_log-discovered paths.
504+
func TestNoFollowFlagSetOnConfig(t *testing.T) {
505+
tests := []struct {
506+
name string
507+
provider string
508+
expectedFlag bool
509+
}{
510+
{
511+
name: "process_log provider sets NoFollow=true",
512+
provider: names.ProcessLog,
513+
expectedFlag: true,
514+
},
515+
{
516+
name: "file provider does not set NoFollow",
517+
provider: names.File,
518+
expectedFlag: false,
519+
},
520+
{
521+
name: "kubernetes provider does not set NoFollow",
522+
provider: names.Kubernetes,
523+
expectedFlag: false,
524+
},
525+
}
526+
527+
for _, tt := range tests {
528+
t.Run(tt.name, func(t *testing.T) {
529+
var logsConfig []byte
530+
var serviceID string
531+
532+
if tt.provider == names.File {
533+
logsConfig = []byte("logs:\n - type: file\n path: /var/log/app.log\n service: svc\n")
534+
serviceID = ""
535+
} else {
536+
logsConfig = []byte(`[{"type":"file","path":"/var/log/app.log","service":"svc"}]`)
537+
serviceID = tt.provider + ":///var/log/app.log"
538+
}
539+
540+
cfg := integration.Config{
541+
LogsConfig: logsConfig,
542+
Provider: tt.provider,
543+
Name: "test-config",
544+
ServiceID: serviceID,
545+
}
546+
547+
sources, err := CreateSources(cfg)
548+
require.NoError(t, err)
549+
require.Len(t, sources, 1)
550+
assert.Equal(t, tt.expectedFlag, sources[0].Config.NoFollow,
551+
"Config.NoFollow should be %v for provider %q", tt.expectedFlag, tt.provider)
552+
})
553+
}
554+
}

0 commit comments

Comments
 (0)