From ba0c6890744f5d0c4173d8e8e73d1b1809ec97b7 Mon Sep 17 00:00:00 2001 From: Joshua Wang Date: Mon, 15 Jul 2024 10:42:58 -0700 Subject: [PATCH 1/6] commandrun: keep track of newly created files Signed-off-by: Joshua Wang --- attestation/commandrun/tracing_linux.go | 26 +++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/attestation/commandrun/tracing_linux.go b/attestation/commandrun/tracing_linux.go index cd473c47..7132b7b8 100644 --- a/attestation/commandrun/tracing_linux.go +++ b/attestation/commandrun/tracing_linux.go @@ -74,6 +74,8 @@ func (r *CommandRun) trace(c *exec.Cmd, actx *attestation.AttestationContext) ([ } func (p *ptraceContext) runTrace() error { + defer p.retryOpenedFiles() + runtime.LockOSThread() defer runtime.UnlockOSThread() status := unix.WaitStatus(0) @@ -121,6 +123,26 @@ func (p *ptraceContext) runTrace() error { } } +func (p *ptraceContext) retryOpenedFiles() { + // after tracing, look through opened files to try to resolve any newly created files + procInfo := p.getProcInfo(p.parentPid) + + for file, digestSet := range procInfo.OpenedFiles { + if digestSet != nil { + continue + } + + newDigest, err := cryptoutil.CalculateDigestSetFromFile(file, p.hash) + + if err != nil { + delete(procInfo.OpenedFiles, file) + continue + } + + procInfo.OpenedFiles[file] = newDigest + } +} + func (p *ptraceContext) nextSyscall(pid int) error { regs := unix.PtraceRegs{} if err := unix.PtraceGetRegs(pid, ®s); err != nil { @@ -213,6 +235,10 @@ func (p *ptraceContext) handleSyscall(pid int, regs unix.PtraceRegs) error { procInfo := p.getProcInfo(pid) digestSet, err := cryptoutil.CalculateDigestSetFromFile(file, p.hash) if err != nil { + if _, isPathErr := err.(*os.PathError); isPathErr { + procInfo.OpenedFiles[file] = nil + } + return err } From 4508b6d3b307d434fb2fac8d4c5bc4be35eb173b Mon Sep 17 00:00:00 2001 From: Joshua Wang Date: Mon, 15 Jul 2024 12:52:33 -0700 Subject: [PATCH 2/6] product: only attest for opened files when tracing is enabled Signed-off-by: Joshua Wang --- attestation/commandrun/commandrun.go | 8 ++++---- attestation/product/product.go | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/attestation/commandrun/commandrun.go b/attestation/commandrun/commandrun.go index 5fc2007e..8a41e020 100644 --- a/attestation/commandrun/commandrun.go +++ b/attestation/commandrun/commandrun.go @@ -70,7 +70,7 @@ func WithMaterials(materials map[string]cryptoutil.DigestSet) Option { func WithTracing(enabled bool) Option { return func(cr *CommandRun) { - cr.enableTracing = enabled + cr.EnableTracing = enabled } } @@ -117,10 +117,10 @@ type CommandRun struct { Stderr string `json:"stderr,omitempty"` ExitCode int `json:"exitcode"` Processes []ProcessInfo `json:"processes,omitempty"` + EnableTracing bool silent bool materials map[string]cryptoutil.DigestSet - enableTracing bool environmentBlockList map[string]struct{} } @@ -176,7 +176,7 @@ func (r *CommandRun) runCmd(ctx *attestation.AttestationContext) error { stderrWriter := io.MultiWriter(stderrWriters...) c.Stdout = stdoutWriter c.Stderr = stderrWriter - if r.enableTracing { + if r.EnableTracing { enableTracing(c) } @@ -185,7 +185,7 @@ func (r *CommandRun) runCmd(ctx *attestation.AttestationContext) error { } var err error - if r.enableTracing { + if r.EnableTracing { r.Processes, err = r.trace(c, ctx) } else { err = c.Wait() diff --git a/attestation/product/product.go b/attestation/product/product.go index 9e834f32..4e370c0f 100644 --- a/attestation/product/product.go +++ b/attestation/product/product.go @@ -23,6 +23,7 @@ import ( "github.com/gabriel-vasile/mimetype" "github.com/gobwas/glob" "github.com/in-toto/go-witness/attestation" + "github.com/in-toto/go-witness/attestation/commandrun" "github.com/in-toto/go-witness/attestation/file" "github.com/in-toto/go-witness/cryptoutil" "github.com/in-toto/go-witness/registry" @@ -186,6 +187,25 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error { return err } + for _, completedAttestor := range ctx.CompletedAttestors() { + attestor := completedAttestor.Attestor + if commandRunAttestor, ok := attestor.(*commandrun.CommandRun); ok && commandRunAttestor.EnableTracing { + openedFileSet := map[string]bool{} + + for _, process := range commandRunAttestor.Processes { + for file := range process.OpenedFiles { + openedFileSet[file] = true; + } + } + + for file := range products { + if _, ok := openedFileSet[file]; !ok { + delete(products, file) + } + } + } + } + a.products = fromDigestMap(ctx.WorkingDir(), products) return nil } From ac7897d1ae8ec009059daf6dc8407950b8a221a8 Mon Sep 17 00:00:00 2001 From: Joshua Wang Date: Wed, 17 Jul 2024 16:20:06 -0700 Subject: [PATCH 3/6] file: do not attempt to record an artifact if it was not opened by the process Signed-off-by: Joshua Wang --- attestation/file/file.go | 14 +++++++++----- attestation/file/file_test.go | 6 +++--- attestation/material/material.go | 2 +- attestation/product/product.go | 20 +++++++++----------- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/attestation/file/file.go b/attestation/file/file.go index 36a37e4e..14065d6f 100644 --- a/attestation/file/file.go +++ b/attestation/file/file.go @@ -26,7 +26,7 @@ import ( // recordArtifacts will walk basePath and record the digests of each file with each of the functions in hashes. // If file already exists in baseArtifacts and the two artifacts are equal the artifact will not be in the // returned map of artifacts. -func RecordArtifacts(basePath string, baseArtifacts map[string]cryptoutil.DigestSet, hashes []cryptoutil.DigestValue, visitedSymlinks map[string]struct{}) (map[string]cryptoutil.DigestSet, error) { +func RecordArtifacts(basePath string, baseArtifacts map[string]cryptoutil.DigestSet, hashes []cryptoutil.DigestValue, visitedSymlinks map[string]struct{}, processWasTraced bool, openedFiles map[string]bool) (map[string]cryptoutil.DigestSet, error) { artifacts := make(map[string]cryptoutil.DigestSet) err := filepath.Walk(basePath, func(path string, info fs.FileInfo, err error) error { if err != nil { @@ -57,7 +57,7 @@ func RecordArtifacts(basePath string, baseArtifacts map[string]cryptoutil.Digest } visitedSymlinks[linkedPath] = struct{}{} - symlinkedArtifacts, err := RecordArtifacts(linkedPath, baseArtifacts, hashes, visitedSymlinks) + symlinkedArtifacts, err := RecordArtifacts(linkedPath, baseArtifacts, hashes, visitedSymlinks, processWasTraced, openedFiles) if err != nil { return err } @@ -65,7 +65,7 @@ func RecordArtifacts(basePath string, baseArtifacts map[string]cryptoutil.Digest for artifactPath, artifact := range symlinkedArtifacts { // all artifacts in the symlink should be recorded relative to our basepath joinedPath := filepath.Join(relPath, artifactPath) - if shouldRecord(joinedPath, artifact, baseArtifacts) { + if shouldRecord(joinedPath, artifact, baseArtifacts, processWasTraced, openedFiles) { artifacts[filepath.Join(relPath, artifactPath)] = artifact } } @@ -78,7 +78,7 @@ func RecordArtifacts(basePath string, baseArtifacts map[string]cryptoutil.Digest return err } - if shouldRecord(relPath, artifact, baseArtifacts) { + if shouldRecord(relPath, artifact, baseArtifacts, processWasTraced, openedFiles) { artifacts[relPath] = artifact } @@ -89,9 +89,13 @@ func RecordArtifacts(basePath string, baseArtifacts map[string]cryptoutil.Digest } // shouldRecord determines whether artifact should be recorded. +// if the process was traced and the artifact was not one of the opened files, return false // if the artifact is already in baseArtifacts, check if it's changed // if it is not equal to the existing artifact, return true, otherwise return false -func shouldRecord(path string, artifact cryptoutil.DigestSet, baseArtifacts map[string]cryptoutil.DigestSet) bool { +func shouldRecord(path string, artifact cryptoutil.DigestSet, baseArtifacts map[string]cryptoutil.DigestSet, processWasTraced bool, openedFiles map[string]bool) bool { + if _, ok := openedFiles[path]; !ok && processWasTraced { + return false + } if previous, ok := baseArtifacts[path]; ok && artifact.Equal(previous) { return false } diff --git a/attestation/file/file_test.go b/attestation/file/file_test.go index 73344bff..436e4a4b 100644 --- a/attestation/file/file_test.go +++ b/attestation/file/file_test.go @@ -38,13 +38,13 @@ func TestBrokenSymlink(t *testing.T) { symTestDir := filepath.Join(dir, "symTestDir") require.NoError(t, os.Symlink(testDir, symTestDir)) - _, err := RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}) + _, err := RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool) require.NoError(t, err) // remove the symlinks and make sure we don't get an error back require.NoError(t, os.RemoveAll(testDir)) require.NoError(t, os.RemoveAll(testFile)) - _, err = RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}) + _, err = RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool) require.NoError(t, err) } @@ -58,6 +58,6 @@ func TestSymlinkCycle(t *testing.T) { require.NoError(t, os.Symlink(dir, symTestDir)) // if a symlink cycle weren't properly handled this would be an infinite loop - _, err := RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}) + _, err := RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool) require.NoError(t, err) } diff --git a/attestation/material/material.go b/attestation/material/material.go index 74f047c0..6b99a4e3 100644 --- a/attestation/material/material.go +++ b/attestation/material/material.go @@ -90,7 +90,7 @@ func (a *Attestor) Schema() *jsonschema.Schema { } func (a *Attestor) Attest(ctx *attestation.AttestationContext) error { - materials, err := file.RecordArtifacts(ctx.WorkingDir(), nil, ctx.Hashes(), map[string]struct{}{}) + materials, err := file.RecordArtifacts(ctx.WorkingDir(), nil, ctx.Hashes(), map[string]struct{}{}, false, map[string]bool{}) if err != nil { return err } diff --git a/attestation/product/product.go b/attestation/product/product.go index 4e370c0f..d70987e7 100644 --- a/attestation/product/product.go +++ b/attestation/product/product.go @@ -182,30 +182,28 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error { a.compiledExcludeGlob = compiledExcludeGlob a.baseArtifacts = ctx.Materials() - products, err := file.RecordArtifacts(ctx.WorkingDir(), a.baseArtifacts, ctx.Hashes(), map[string]struct{}{}) - if err != nil { - return err - } + + processWasTraced := false + openedFileSet := map[string]bool{} for _, completedAttestor := range ctx.CompletedAttestors() { attestor := completedAttestor.Attestor if commandRunAttestor, ok := attestor.(*commandrun.CommandRun); ok && commandRunAttestor.EnableTracing { - openedFileSet := map[string]bool{} + processWasTraced = true for _, process := range commandRunAttestor.Processes { for file := range process.OpenedFiles { openedFileSet[file] = true; } } - - for file := range products { - if _, ok := openedFileSet[file]; !ok { - delete(products, file) - } - } } } + products, err := file.RecordArtifacts(ctx.WorkingDir(), a.baseArtifacts, ctx.Hashes(), map[string]struct{}{}, processWasTraced, openedFileSet) + if err != nil { + return err + } + a.products = fromDigestMap(ctx.WorkingDir(), products) return nil } From 90b02beb93e799f2baa349b765933211dc6724c2 Mon Sep 17 00:00:00 2001 From: Joshua Wang Date: Thu, 18 Jul 2024 11:15:29 -0700 Subject: [PATCH 4/6] chore: fix lint and don't export enableTracing Signed-off-by: Joshua Wang --- attestation/commandrun/commandrun.go | 12 ++++++++---- attestation/file/file_test.go | 6 +++--- attestation/product/product.go | 6 +++--- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/attestation/commandrun/commandrun.go b/attestation/commandrun/commandrun.go index 8a41e020..35bbbf39 100644 --- a/attestation/commandrun/commandrun.go +++ b/attestation/commandrun/commandrun.go @@ -70,7 +70,7 @@ func WithMaterials(materials map[string]cryptoutil.DigestSet) Option { func WithTracing(enabled bool) Option { return func(cr *CommandRun) { - cr.EnableTracing = enabled + cr.enableTracing = enabled } } @@ -117,10 +117,10 @@ type CommandRun struct { Stderr string `json:"stderr,omitempty"` ExitCode int `json:"exitcode"` Processes []ProcessInfo `json:"processes,omitempty"` - EnableTracing bool silent bool materials map[string]cryptoutil.DigestSet + enableTracing bool environmentBlockList map[string]struct{} } @@ -160,6 +160,10 @@ func (rc *CommandRun) RunType() attestation.RunType { return RunType } +func (rc *CommandRun) EnableTracing() bool { + return rc.enableTracing +} + func (r *CommandRun) runCmd(ctx *attestation.AttestationContext) error { c := exec.Command(r.Cmd[0], r.Cmd[1:]...) c.Dir = ctx.WorkingDir() @@ -176,7 +180,7 @@ func (r *CommandRun) runCmd(ctx *attestation.AttestationContext) error { stderrWriter := io.MultiWriter(stderrWriters...) c.Stdout = stdoutWriter c.Stderr = stderrWriter - if r.EnableTracing { + if r.enableTracing { enableTracing(c) } @@ -185,7 +189,7 @@ func (r *CommandRun) runCmd(ctx *attestation.AttestationContext) error { } var err error - if r.EnableTracing { + if r.enableTracing { r.Processes, err = r.trace(c, ctx) } else { err = c.Wait() diff --git a/attestation/file/file_test.go b/attestation/file/file_test.go index 436e4a4b..5379a487 100644 --- a/attestation/file/file_test.go +++ b/attestation/file/file_test.go @@ -38,13 +38,13 @@ func TestBrokenSymlink(t *testing.T) { symTestDir := filepath.Join(dir, "symTestDir") require.NoError(t, os.Symlink(testDir, symTestDir)) - _, err := RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool) + _, err := RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool{}) require.NoError(t, err) // remove the symlinks and make sure we don't get an error back require.NoError(t, os.RemoveAll(testDir)) require.NoError(t, os.RemoveAll(testFile)) - _, err = RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool) + _, err = RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool{}) require.NoError(t, err) } @@ -58,6 +58,6 @@ func TestSymlinkCycle(t *testing.T) { require.NoError(t, os.Symlink(dir, symTestDir)) // if a symlink cycle weren't properly handled this would be an infinite loop - _, err := RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool) + _, err := RecordArtifacts(dir, map[string]cryptoutil.DigestSet{}, []cryptoutil.DigestValue{{Hash: crypto.SHA256}}, map[string]struct{}{}, false, map[string]bool{}) require.NoError(t, err) } diff --git a/attestation/product/product.go b/attestation/product/product.go index d70987e7..2e15e526 100644 --- a/attestation/product/product.go +++ b/attestation/product/product.go @@ -188,12 +188,12 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error { for _, completedAttestor := range ctx.CompletedAttestors() { attestor := completedAttestor.Attestor - if commandRunAttestor, ok := attestor.(*commandrun.CommandRun); ok && commandRunAttestor.EnableTracing { + if commandRunAttestor, ok := attestor.(*commandrun.CommandRun); ok && commandRunAttestor.EnableTracing() { processWasTraced = true for _, process := range commandRunAttestor.Processes { - for file := range process.OpenedFiles { - openedFileSet[file] = true; + for fname := range process.OpenedFiles { + openedFileSet[fname] = true } } } From 5f73308bafb82f462137356793b66ea196af6ed0 Mon Sep 17 00:00:00 2001 From: Joshua Wang Date: Fri, 19 Jul 2024 10:38:46 -0700 Subject: [PATCH 5/6] chore: fix linting Signed-off-by: Joshua Wang --- attestation/commandrun/tracing_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/attestation/commandrun/tracing_linux.go b/attestation/commandrun/tracing_linux.go index 7132b7b8..90eb9c24 100644 --- a/attestation/commandrun/tracing_linux.go +++ b/attestation/commandrun/tracing_linux.go @@ -75,7 +75,7 @@ func (r *CommandRun) trace(c *exec.Cmd, actx *attestation.AttestationContext) ([ func (p *ptraceContext) runTrace() error { defer p.retryOpenedFiles() - + runtime.LockOSThread() defer runtime.UnlockOSThread() status := unix.WaitStatus(0) From fa3f5715212134f18bef81fc71f4116a1f63cd42 Mon Sep 17 00:00:00 2001 From: Joshua Wang Date: Mon, 29 Jul 2024 10:38:51 -0700 Subject: [PATCH 6/6] change EnableTracing to TracingEnabled Signed-off-by: Joshua Wang --- attestation/commandrun/commandrun.go | 2 +- attestation/product/product.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/attestation/commandrun/commandrun.go b/attestation/commandrun/commandrun.go index 35bbbf39..431d9ca7 100644 --- a/attestation/commandrun/commandrun.go +++ b/attestation/commandrun/commandrun.go @@ -160,7 +160,7 @@ func (rc *CommandRun) RunType() attestation.RunType { return RunType } -func (rc *CommandRun) EnableTracing() bool { +func (rc *CommandRun) TracingEnabled() bool { return rc.enableTracing } diff --git a/attestation/product/product.go b/attestation/product/product.go index 2e15e526..8c9d6c34 100644 --- a/attestation/product/product.go +++ b/attestation/product/product.go @@ -188,7 +188,7 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error { for _, completedAttestor := range ctx.CompletedAttestors() { attestor := completedAttestor.Attestor - if commandRunAttestor, ok := attestor.(*commandrun.CommandRun); ok && commandRunAttestor.EnableTracing() { + if commandRunAttestor, ok := attestor.(*commandrun.CommandRun); ok && commandRunAttestor.TracingEnabled() { processWasTraced = true for _, process := range commandRunAttestor.Processes {