From 0f369b6abae678ba311c216b9cf9fb9978de1eee Mon Sep 17 00:00:00 2001 From: Aditya Sirish Date: Mon, 12 Dec 2022 16:35:43 -0500 Subject: [PATCH] Fix handling of symlinks in artifact recording * Records source path instead of destination path for symlinks * Adds explicit parameter to decide if symlinks to directories must be followed Signed-off-by: Aditya Sirish --- cmd/record.go | 13 +++++++++-- cmd/root.go | 1 + cmd/run.go | 11 ++++++++- doc/in-toto_record.md | 3 +++ doc/in-toto_record_start.md | 3 +++ doc/in-toto_record_stop.md | 3 +++ doc/in-toto_run.md | 3 +++ in_toto/runlib.go | 46 +++++++++++++++++++++++++------------ in_toto/runlib_test.go | 28 +++++++++++----------- in_toto/verifylib.go | 2 +- 10 files changed, 80 insertions(+), 33 deletions(-) diff --git a/cmd/record.go b/cmd/record.go index 28dd1ebf..cca47270 100644 --- a/cmd/record.go +++ b/cmd/record.go @@ -126,6 +126,15 @@ operating systems. It is done by replacing all line separators with a new line character.`, ) + recordCmd.PersistentFlags().BoolVar( + &followSymlinkDirs, + "follow-symlink-dirs", + false, + `Follow symlinked directories to their targets. Note: this parameter +toggles following linked directories only, linked files are always +recorded independently of this parameter.`, + ) + recordCmd.MarkPersistentFlagRequired("name") // Record Start Command @@ -156,7 +165,7 @@ command is executed. Symlinks are followed.`, } func recordStart(cmd *cobra.Command, args []string) error { - block, err := intoto.InTotoRecordStart(recordStepName, recordMaterialsPaths, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization) + block, err := intoto.InTotoRecordStart(recordStepName, recordMaterialsPaths, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization, followSymlinkDirs) if err != nil { return fmt.Errorf("failed to create start link file: %w", err) } @@ -179,7 +188,7 @@ func recordStop(cmd *cobra.Command, args []string) error { return fmt.Errorf("failed to load start link file at %s: %w", prelimLinkName, err) } - linkMb, err := intoto.InTotoRecordStop(prelimLinkMb, recordProductsPaths, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization) + linkMb, err := intoto.InTotoRecordStop(prelimLinkMb, recordProductsPaths, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization, followSymlinkDirs) if err != nil { return fmt.Errorf("failed to create stop link file: %w", err) } diff --git a/cmd/root.go b/cmd/root.go index 02391f08..c9842b88 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -23,6 +23,7 @@ var ( exclude []string outDir string lineNormalization bool + followSymlinkDirs bool ) var rootCmd = &cobra.Command{ diff --git a/cmd/run.go b/cmd/run.go index 1985be1e..38aad731 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -140,6 +140,15 @@ with a new line character.`, `Indicate that there is no command to be executed for the step.`, ) + runCmd.PersistentFlags().BoolVar( + &followSymlinkDirs, + "follow-symlink-dirs", + false, + `Follow symlinked directories to their targets. Note: this parameter +toggles following linked directories only, linked files are always +recorded independently of this parameter.`, + ) + runCmd.Flags().StringVar( &spiffeUDS, "spiffe-workload-api-path", @@ -158,7 +167,7 @@ func run(cmd *cobra.Command, args []string) error { return fmt.Errorf("no command arguments passed, please specify or use --no-command option") } - block, err := intoto.InTotoRun(stepName, runDir, materialsPaths, productsPaths, args, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization) + block, err := intoto.InTotoRun(stepName, runDir, materialsPaths, productsPaths, args, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization, followSymlinkDirs) if err != nil { return fmt.Errorf("failed to create link metadata: %w", err) } diff --git a/doc/in-toto_record.md b/doc/in-toto_record.md index ebba196b..a7c5c007 100644 --- a/doc/in-toto_record.md +++ b/doc/in-toto_record.md @@ -18,6 +18,9 @@ failure and zero otherwise. -e, --exclude stringArray Path patterns to match paths that should not be recorded as ‘materials’ or ‘products’. Passed patterns override patterns defined in environment variables or config files. See Config docs for details. + --follow-symlink-dirs Follow symlinked directories to their targets. Note: this parameter + toggles following linked directories only, linked files are always + recorded independently of this parameter. -h, --help help for record -k, --key string Path to a private key file to sign the resulting link metadata. The keyid prefix is used as an infix for the link metadata filename, diff --git a/doc/in-toto_record_start.md b/doc/in-toto_record_start.md index 8fb8464c..8e0a82ef 100644 --- a/doc/in-toto_record_start.md +++ b/doc/in-toto_record_start.md @@ -30,6 +30,9 @@ in-toto record start [flags] -e, --exclude stringArray Path patterns to match paths that should not be recorded as ‘materials’ or ‘products’. Passed patterns override patterns defined in environment variables or config files. See Config docs for details. + --follow-symlink-dirs Follow symlinked directories to their targets. Note: this parameter + toggles following linked directories only, linked files are always + recorded independently of this parameter. -k, --key string Path to a private key file to sign the resulting link metadata. The keyid prefix is used as an infix for the link metadata filename, i.e. ‘..link’. See ‘–key-type’ for available diff --git a/doc/in-toto_record_stop.md b/doc/in-toto_record_stop.md index 666e3637..1e0fe86e 100644 --- a/doc/in-toto_record_stop.md +++ b/doc/in-toto_record_stop.md @@ -31,6 +31,9 @@ in-toto record stop [flags] -e, --exclude stringArray Path patterns to match paths that should not be recorded as ‘materials’ or ‘products’. Passed patterns override patterns defined in environment variables or config files. See Config docs for details. + --follow-symlink-dirs Follow symlinked directories to their targets. Note: this parameter + toggles following linked directories only, linked files are always + recorded independently of this parameter. -k, --key string Path to a private key file to sign the resulting link metadata. The keyid prefix is used as an infix for the link metadata filename, i.e. ‘..link’. See ‘–key-type’ for available diff --git a/doc/in-toto_run.md b/doc/in-toto_run.md index a5f5f656..e08f635c 100644 --- a/doc/in-toto_run.md +++ b/doc/in-toto_run.md @@ -22,6 +22,9 @@ in-toto run [flags] -e, --exclude stringArray Path patterns to match paths that should not be recorded as 0 ‘materials’ or ‘products’. Passed patterns override patterns defined in environment variables or config files. See Config docs for details. + --follow-symlink-dirs Follow symlinked directories to their targets. Note: this parameter + toggles following linked directories only, linked files are always + recorded independently of this parameter. -h, --help help for run -k, --key string Path to a PEM formatted private key file used to sign the resulting link metadata. diff --git a/in_toto/runlib.go b/in_toto/runlib.go index 87e69050..aa8b99d8 100644 --- a/in_toto/runlib.go +++ b/in_toto/runlib.go @@ -92,10 +92,10 @@ the following format: If recording an artifact fails the first return value is nil and the second return value is the error. */ -func RecordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns []string, lStripPaths []string, lineNormalization bool) (evalArtifacts map[string]interface{}, err error) { +func RecordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns []string, lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (evalArtifacts map[string]interface{}, err error) { // Make sure to initialize a fresh hashset for every RecordArtifacts call visitedSymlinks = NewSet() - evalArtifacts, err = recordArtifacts(paths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization) + evalArtifacts, err = recordArtifacts(paths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs) // pass result and error through return evalArtifacts, err } @@ -118,7 +118,7 @@ the following format: If recording an artifact fails the first return value is nil and the second return value is the error. */ -func recordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns []string, lStripPaths []string, lineNormalization bool) (map[string]interface{}, error) { +func recordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns []string, lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (map[string]interface{}, error) { artifacts := make(map[string]interface{}) for _, path := range paths { err := filepath.Walk(path, @@ -160,18 +160,35 @@ func recordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns if err != nil { return err } + info, err := os.Stat(evalSym) + if err != nil { + return err + } + targetIsDir := false + if info.IsDir() { + if !followSymlinkDirs { + // We don't follow symlinked directories + return nil + } + targetIsDir = true + } // add symlink to visitedSymlinks set // this way, we know which link we have visited already // if we visit a symlink twice, we have detected a symlink cycle visitedSymlinks.Add(path) - // We recursively call RecordArtifacts() to follow + // We recursively call recordArtifacts() to follow // the new path. - evalArtifacts, evalErr := recordArtifacts([]string{evalSym}, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization) + evalArtifacts, evalErr := recordArtifacts([]string{evalSym}, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs) if evalErr != nil { return evalErr } for key, value := range evalArtifacts { - artifacts[key] = value + if targetIsDir { + symlinkPath := filepath.Join(path, strings.TrimPrefix(key, evalSym)) + artifacts[symlinkPath] = value + } else { + artifacts[path] = value + } } return nil } @@ -189,8 +206,7 @@ func recordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns } } // Check if path is unique - _, existingPath := artifacts[path] - if existingPath { + if _, exists := artifacts[path]; exists { return fmt.Errorf("left stripping has resulted in non unique dictionary key: %s", path) } artifacts[path] = artifact @@ -295,10 +311,10 @@ return value is an empty Metablock and the second return value is the error. */ func InTotoRun(name string, runDir string, materialPaths []string, productPaths []string, cmdArgs []string, key Key, hashAlgorithms []string, gitignorePatterns []string, - lStripPaths []string, lineNormalization bool) (Metablock, error) { + lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (Metablock, error) { var linkMb Metablock - materials, err := RecordArtifacts(materialPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization) + materials, err := RecordArtifacts(materialPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs) if err != nil { return linkMb, err } @@ -312,7 +328,7 @@ func InTotoRun(name string, runDir string, materialPaths []string, productPaths } } - products, err := RecordArtifacts(productPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization) + products, err := RecordArtifacts(productPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs) if err != nil { return linkMb, err } @@ -347,9 +363,9 @@ in order to provide evidence for supply chain steps that cannot be carries out by a single command. InTotoRecordStart collects the hashes of the materials before any commands are run, signs the unfinished link, and returns the link. */ -func InTotoRecordStart(name string, materialPaths []string, key Key, hashAlgorithms, gitignorePatterns []string, lStripPaths []string, lineNormalization bool) (Metablock, error) { +func InTotoRecordStart(name string, materialPaths []string, key Key, hashAlgorithms, gitignorePatterns []string, lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (Metablock, error) { var linkMb Metablock - materials, err := RecordArtifacts(materialPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization) + materials, err := RecordArtifacts(materialPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs) if err != nil { return linkMb, err } @@ -380,7 +396,7 @@ created by InTotoRecordStart and records the hashes of any products creted by commands run between InTotoRecordStart and InTotoRecordStop. The resultant finished link metablock is then signed by the provided key and returned. */ -func InTotoRecordStop(prelimLinkMb Metablock, productPaths []string, key Key, hashAlgorithms, gitignorePatterns []string, lStripPaths []string, lineNormalization bool) (Metablock, error) { +func InTotoRecordStop(prelimLinkMb Metablock, productPaths []string, key Key, hashAlgorithms, gitignorePatterns []string, lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (Metablock, error) { var linkMb Metablock if err := prelimLinkMb.VerifySignature(key); err != nil { return linkMb, err @@ -391,7 +407,7 @@ func InTotoRecordStop(prelimLinkMb Metablock, productPaths []string, key Key, ha return linkMb, errors.New("invalid metadata block") } - products, err := RecordArtifacts(productPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization) + products, err := RecordArtifacts(productPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs) if err != nil { return linkMb, err } diff --git a/in_toto/runlib_test.go b/in_toto/runlib_test.go index 377d3115..c82da62b 100644 --- a/in_toto/runlib_test.go +++ b/in_toto/runlib_test.go @@ -104,7 +104,7 @@ func TestGitPathSpec(t *testing.T) { "*.pub", // Match all .pub files (even the ones in subdirectories) "beta/foo.tar.gz", // Match full path "alpha/**", // Match all directories and files beneath alpha - }, nil, testOSisWindows()) + }, nil, testOSisWindows(), false) if !reflect.DeepEqual(result, expected) { t.Errorf("RecordArtifacts returned '(%s, %s)', expected '(%s, nil)'", result, err, expected) @@ -128,11 +128,11 @@ func TestSymlinkToFile(t *testing.T) { } expected := map[string]interface{}{ - "foo.tar.gz": map[string]interface{}{ + "foo.tar.gz.sym": map[string]interface{}{ "sha256": "52947cb78b91ad01fe81cd6aef42d1f6817e92b9e6936c1e5aabb7c98514f355", }, } - result, err := RecordArtifacts([]string{"foo.tar.gz.sym"}, []string{"sha256"}, nil, nil, testOSisWindows()) + result, err := RecordArtifacts([]string{"foo.tar.gz.sym"}, []string{"sha256"}, nil, nil, testOSisWindows(), false) if !reflect.DeepEqual(result, expected) { t.Errorf("RecordArtifacts returned '(%s, %s)', expected '(%s, nil)'", result, err, expected) @@ -171,7 +171,7 @@ func TestIndirectSymlinkCycles(t *testing.T) { } // provoke "symlink cycle detected" error - _, err = RecordArtifacts([]string{"symTestA/linkToB.sym", "symTestB/linkToA.sym", "foo.tar.gz"}, []string{"sha256"}, nil, nil, testOSisWindows()) + _, err = RecordArtifacts([]string{"symTestA/linkToB.sym", "symTestB/linkToA.sym", "foo.tar.gz"}, []string{"sha256"}, nil, nil, testOSisWindows(), true) if !errors.Is(err, ErrSymCycle) { t.Errorf("we expected: %s, we got: %s", ErrSymCycle, err) } @@ -214,13 +214,13 @@ func TestSymlinkToFolder(t *testing.T) { t.Errorf("could not write symTmpfile: %s", err) } - result, err := RecordArtifacts([]string{"symTmpfile.sym"}, []string{"sha256"}, nil, nil, testOSisWindows()) + result, err := RecordArtifacts([]string{"symTmpfile.sym"}, []string{"sha256"}, nil, nil, testOSisWindows(), true) if err != nil { t.Error(err) } expected := map[string]interface{}{ - p: map[string]interface{}{ + filepath.Join("symTmpfile.sym", "symTmpfile"): map[string]interface{}{ "sha256": "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", }, } @@ -266,7 +266,7 @@ func TestSymlinkCycle(t *testing.T) { } // provoke "symlink cycle detected" error - _, err = RecordArtifacts([]string{"symlinkCycle/symCycle.sym", "foo.tar.gz"}, []string{"sha256"}, nil, nil, testOSisWindows()) + _, err = RecordArtifacts([]string{"symlinkCycle/symCycle.sym", "foo.tar.gz"}, []string{"sha256"}, nil, nil, testOSisWindows(), true) if !errors.Is(err, ErrSymCycle) { t.Errorf("we expected: %s, we got: %s", ErrSymCycle, err) } @@ -289,7 +289,7 @@ func TestRecordArtifacts(t *testing.T) { t.Errorf("could not write tmpfile: %s", err) } result, err := RecordArtifacts([]string{"foo.tar.gz", - "tmpdir/tmpfile"}, []string{"sha256"}, nil, []string{"tmpdir/"}, testOSisWindows()) + "tmpdir/tmpfile"}, []string{"sha256"}, nil, []string{"tmpdir/"}, testOSisWindows(), false) expected := map[string]interface{}{ "foo.tar.gz": map[string]interface{}{ "sha256": "52947cb78b91ad01fe81cd6aef42d1f6817e92b9e6936c1e5aabb7c98514f355", @@ -307,7 +307,7 @@ func TestRecordArtifacts(t *testing.T) { t.Errorf("could not write tmpfile: %s", err) } _, err = RecordArtifacts([]string{"foo.tar.gz", - "tmpdir/foo.tar.gz"}, []string{"sha256"}, nil, []string{"tmpdir/"}, testOSisWindows()) + "tmpdir/foo.tar.gz"}, []string{"sha256"}, nil, []string{"tmpdir/"}, testOSisWindows(), false) if err == nil { t.Error("duplicated path error expected") } @@ -317,7 +317,7 @@ func TestRecordArtifacts(t *testing.T) { } // Test error by recording nonexistent artifact - result, err = RecordArtifacts([]string{"file-does-not-exist"}, []string{"sha256"}, nil, nil, testOSisWindows()) + result, err = RecordArtifacts([]string{"file-does-not-exist"}, []string{"sha256"}, nil, nil, testOSisWindows(), false) if !os.IsNotExist(err) { t.Errorf("RecordArtifacts returned '(%s, %s)', expected '(nil, %s)'", result, err, os.ErrNotExist) @@ -464,7 +464,7 @@ func TestInTotoRun(t *testing.T) { } for _, table := range tablesCorrect { - result, err := InTotoRun(linkName, "", table.materialPaths, table.productPaths, table.cmdArgs, table.key, table.hashAlgorithms, nil, nil, testOSisWindows()) + result, err := InTotoRun(linkName, "", table.materialPaths, table.productPaths, table.cmdArgs, table.key, table.hashAlgorithms, nil, nil, testOSisWindows(), false) if !reflect.DeepEqual(result, table.result) { t.Errorf("InTotoRun returned '(%s, %s)', expected '(%s, nil)'", result, err, table.result) } else { @@ -508,7 +508,7 @@ func TestInTotoRun(t *testing.T) { } for _, table := range tablesInvalid { - result, err := InTotoRun(linkName, "", table.materialPaths, table.productPaths, table.cmdArgs, table.key, table.hashAlgorithms, nil, nil, testOSisWindows()) + result, err := InTotoRun(linkName, "", table.materialPaths, table.productPaths, table.cmdArgs, table.key, table.hashAlgorithms, nil, nil, testOSisWindows(), false) if err == nil { t.Errorf("InTotoRun returned '(%s, %s)', expected error", result, err) @@ -578,10 +578,10 @@ func TestInTotoRecord(t *testing.T) { } for _, table := range tablesCorrect { - result, err := InTotoRecordStart(linkName, table.materialPaths, table.key, table.hashAlgorithms, nil, nil, testOSisWindows()) + result, err := InTotoRecordStart(linkName, table.materialPaths, table.key, table.hashAlgorithms, nil, nil, testOSisWindows(), false) assert.Nil(t, err, "unexpected error while running record start") assert.Equal(t, table.startResult, result, "result from record start did not match expected result") - stopResult, err := InTotoRecordStop(result, table.productPaths, table.key, table.hashAlgorithms, nil, nil, testOSisWindows()) + stopResult, err := InTotoRecordStop(result, table.productPaths, table.key, table.hashAlgorithms, nil, nil, testOSisWindows(), false) assert.Nil(t, err, "unexpected error while running record stop") assert.Equal(t, table.stopResult, stopResult, "result from record stop did not match expected result") } diff --git a/in_toto/verifylib.go b/in_toto/verifylib.go index 2302040f..4f30a5c9 100644 --- a/in_toto/verifylib.go +++ b/in_toto/verifylib.go @@ -52,7 +52,7 @@ func RunInspections(layout Layout, runDir string, lineNormalization bool) (map[s } linkMb, err := InTotoRun(inspection.Name, runDir, paths, paths, - inspection.Run, Key{}, []string{"sha256"}, nil, nil, lineNormalization) + inspection.Run, Key{}, []string{"sha256"}, nil, nil, lineNormalization, false) if err != nil { return nil, err