From a5d4f7fbd9e08966775a94c0c0f2f43dbebf6b32 Mon Sep 17 00:00:00 2001 From: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com> Date: Mon, 15 Aug 2022 23:40:54 +0600 Subject: [PATCH] feat(secret): detect secrets removed or overwritten in upper layer (#2611) Co-authored-by: knqyf263 --- pkg/fanal/applier/docker.go | 50 +++++++++- pkg/fanal/applier/docker_test.go | 161 ++++++++++++++++++++++++++++++ pkg/fanal/artifact/image/image.go | 2 +- pkg/fanal/types/secret.go | 3 +- pkg/report/table/secret.go | 9 +- pkg/report/table/secret_test.go | 3 +- 6 files changed, 217 insertions(+), 11 deletions(-) diff --git a/pkg/fanal/applier/docker.go b/pkg/fanal/applier/docker.go index 7feab70d000..ac5f00be7dc 100644 --- a/pkg/fanal/applier/docker.go +++ b/pkg/fanal/applier/docker.go @@ -89,6 +89,7 @@ func lookupOriginLayerForLib(filePath string, lib types.Package, layers []types. func ApplyLayers(layers []types.BlobInfo) types.ArtifactDetail { sep := "/" nestedMap := nested.Nested{} + secretsMap := map[string]types.Secret{} var mergedLayer types.ArtifactDetail for _, layer := range layers { @@ -132,12 +133,11 @@ func ApplyLayers(layers []types.BlobInfo) types.ArtifactDetail { // Apply secrets for _, secret := range layer.Secrets { - secret.Layer = types.Layer{ + l := types.Layer{ Digest: layer.Digest, DiffID: layer.DiffID, } - key := fmt.Sprintf("%s/type:secret", secret.FilePath) - nestedMap.SetByString(key, sep, secret) + secretsMap = mergeSecrets(secretsMap, secret, l) } // Apply license files @@ -170,8 +170,6 @@ func ApplyLayers(layers []types.BlobInfo) types.ArtifactDetail { mergedLayer.Applications = append(mergedLayer.Applications, v) case types.Misconfiguration: mergedLayer.Misconfigurations = append(mergedLayer.Misconfigurations, v) - case types.Secret: - mergedLayer.Secrets = append(mergedLayer.Secrets, v) case types.LicenseFile: mergedLayer.Licenses = append(mergedLayer.Licenses, v) case types.CustomResource: @@ -180,6 +178,16 @@ func ApplyLayers(layers []types.BlobInfo) types.ArtifactDetail { return nil }) + lastDiffID := layers[len(layers)-1].DiffID + for _, s := range secretsMap { + for i, finding := range s.Findings { + if finding.Layer.DiffID != lastDiffID { + s.Findings[i].Deleted = true // This secret is deleted in the upper layer + } + } + mergedLayer.Secrets = append(mergedLayer.Secrets, s) + } + // Extract dpkg licenses // The license information is not stored in the dpkg database and in a separate file, // so we have to merge the license information into the package. @@ -260,3 +268,35 @@ func aggregate(detail *types.ArtifactDetail) { // Overwrite Applications detail.Applications = apps } + +// We must save secrets from all layers even though they are removed in the uppler layer. +// If the secret was changed at the top level, we need to overwrite it. +func mergeSecrets(secretsMap map[string]types.Secret, newSecret types.Secret, layer types.Layer) map[string]types.Secret { + for i := range newSecret.Findings { // add layer to the Findings from the new secret + newSecret.Findings[i].Layer = layer + } + + secret, ok := secretsMap[newSecret.FilePath] + if !ok { + // Add the new finding if its file doesn't exist before + secretsMap[newSecret.FilePath] = newSecret + } else { + // If the new finding has the same `RuleID` as the finding in the previous layers - use the new finding + for _, previousFinding := range secret.Findings { // secrets from previous layers + if !secretFindingsContains(newSecret.Findings, previousFinding) { + newSecret.Findings = append(newSecret.Findings, previousFinding) + } + } + secretsMap[newSecret.FilePath] = newSecret + } + return secretsMap +} + +func secretFindingsContains(findings []types.SecretFinding, finding types.SecretFinding) bool { + for _, f := range findings { + if f.RuleID == finding.RuleID { + return true + } + } + return false +} diff --git a/pkg/fanal/applier/docker_test.go b/pkg/fanal/applier/docker_test.go index 46e5c551942..60d4c724ad8 100644 --- a/pkg/fanal/applier/docker_test.go +++ b/pkg/fanal/applier/docker_test.go @@ -328,6 +328,167 @@ func TestApplyLayers(t *testing.T) { }, }, }, + { + name: "happy path with removed and updated secret", + inputLayers: []types.BlobInfo{ + { + SchemaVersion: 2, + Digest: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02", + DiffID: "sha256:a187dde48cd289ac374ad8539930628314bc581a481cdb41409c9289419ddb72", + Secrets: []types.Secret{ + { + FilePath: "usr/secret.txt", + Findings: []types.SecretFinding{ + { + RuleID: "aws-access-key-id", + Category: "AWS", + Severity: "CRITICAL", + Title: "AWS Access Key ID", + StartLine: 1, + EndLine: 1, + Match: "AWS_ACCESS_KEY_ID=********************", + Code: types.Code{ + Lines: []types.Line{ + { + Number: 1, + Content: "AWS_ACCESS_KEY_ID=********************", + IsCause: true, + Highlighted: "AWS_ACCESS_KEY_ID=********************", + FirstCause: true, + LastCause: true, + }, + }, + }, + }, + }, + }, + }, + }, + { + SchemaVersion: 2, + Digest: "sha256:24df0d4e20c0f42d3703bf1f1db2bdd77346c7956f74f423603d651e8e5ae8a7", + DiffID: "sha256:aad63a9339440e7c3e1fff2b988991b9bfb81280042fa7f39a5e327023056819", + Secrets: []types.Secret{ + { + FilePath: "usr/secret.txt", + Findings: []types.SecretFinding{ + { + RuleID: "github-pat", + Category: "GitHub", + Severity: "CRITICAL", + Title: "GitHub Personal Access Token", + StartLine: 1, + EndLine: 1, + Match: "GITHUB_PAT=****************************************", + Code: types.Code{ + Lines: []types.Line{ + { + Number: 1, + Content: "GITHUB_PAT=****************************************", + IsCause: true, + Highlighted: "GITHUB_PAT=****************************************", + FirstCause: true, + LastCause: true, + }, + }, + }, + }, + { + RuleID: "aws-access-key-id", + Category: "AWS", + Severity: "CRITICAL", + Title: "AWS Access Key ID", + StartLine: 2, + EndLine: 2, + Match: "AWS_ACCESS_KEY_ID=********************", + Code: types.Code{ + Lines: []types.Line{ + { + Number: 1, + Content: "AWS_ACCESS_KEY_ID=********************", + IsCause: true, + Highlighted: "AWS_ACCESS_KEY_ID=********************", + FirstCause: true, + LastCause: true, + }, + }, + }, + }, + }, + }, + }, + }, + { + SchemaVersion: 2, + Digest: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02", + DiffID: "sha256:a187dde48cd289ac374ad8539930628314bc581a481cdb41409c9289419ddb72", + WhiteoutFiles: []string{ + "usr/secret.txt", + }, + }, + }, + want: types.ArtifactDetail{ + Secrets: []types.Secret{ + { + FilePath: "usr/secret.txt", + Findings: []types.SecretFinding{ + { + RuleID: "github-pat", + Category: "GitHub", + Severity: "CRITICAL", + Title: "GitHub Personal Access Token", + Deleted: true, + StartLine: 1, + EndLine: 1, + Match: "GITHUB_PAT=****************************************", + Layer: types.Layer{ + Digest: "sha256:24df0d4e20c0f42d3703bf1f1db2bdd77346c7956f74f423603d651e8e5ae8a7", + DiffID: "sha256:aad63a9339440e7c3e1fff2b988991b9bfb81280042fa7f39a5e327023056819", + }, + Code: types.Code{ + Lines: []types.Line{ + { + Number: 1, + Content: "GITHUB_PAT=****************************************", + IsCause: true, + Highlighted: "GITHUB_PAT=****************************************", + FirstCause: true, + LastCause: true, + }, + }, + }, + }, + { + RuleID: "aws-access-key-id", + Category: "AWS", + Severity: "CRITICAL", + Title: "AWS Access Key ID", + Deleted: true, + StartLine: 2, + EndLine: 2, + Match: "AWS_ACCESS_KEY_ID=********************", + Layer: types.Layer{ + Digest: "sha256:24df0d4e20c0f42d3703bf1f1db2bdd77346c7956f74f423603d651e8e5ae8a7", + DiffID: "sha256:aad63a9339440e7c3e1fff2b988991b9bfb81280042fa7f39a5e327023056819", + }, + Code: types.Code{ + Lines: []types.Line{ + { + Number: 1, + Content: "AWS_ACCESS_KEY_ID=********************", + IsCause: true, + Highlighted: "AWS_ACCESS_KEY_ID=********************", + FirstCause: true, + LastCause: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, { name: "happy path with status.d and opaque dirs without the trailing slash", inputLayers: []types.BlobInfo{ diff --git a/pkg/fanal/artifact/image/image.go b/pkg/fanal/artifact/image/image.go index 660d1b01f86..43049d5f2ef 100644 --- a/pkg/fanal/artifact/image/image.go +++ b/pkg/fanal/artifact/image/image.go @@ -357,7 +357,7 @@ func (a Artifact) guessBaseLayers(diffIDs []string, configFile *v1.ConfigFile) [ return nil } - var baseImageIndex int + baseImageIndex := -1 var foundNonEmpty bool for i := len(configFile.History) - 1; i >= 0; i-- { h := configFile.History[i] diff --git a/pkg/fanal/types/secret.go b/pkg/fanal/types/secret.go index e96b264fd44..3d2e3ca5f70 100644 --- a/pkg/fanal/types/secret.go +++ b/pkg/fanal/types/secret.go @@ -5,7 +5,6 @@ type SecretRuleCategory string type Secret struct { FilePath string Findings []SecretFinding - Layer Layer `json:",omitempty"` } type SecretFinding struct { @@ -17,4 +16,6 @@ type SecretFinding struct { EndLine int Code Code Match string + Deleted bool + Layer Layer `json:",omitempty"` } diff --git a/pkg/report/table/secret.go b/pkg/report/table/secret.go index e96fe922f40..ee8804108e7 100644 --- a/pkg/report/table/secret.go +++ b/pkg/report/table/secret.go @@ -5,11 +5,10 @@ import ( "fmt" "strings" - dbTypes "github.com/aquasecurity/trivy-db/pkg/types" - "github.com/liamg/tml" "golang.org/x/crypto/ssh/terminal" + dbTypes "github.com/aquasecurity/trivy-db/pkg/types" "github.com/aquasecurity/trivy/pkg/fanal/types" ) @@ -120,7 +119,11 @@ func (r *secretRenderer) renderCode(secret types.SecretFinding) { lineInfo = tml.Sprintf("%s-%d", lineInfo, secret.EndLine) } } - r.printf(" %s%s\r\n", r.target, lineInfo) + var note string + if secret.Deleted { + note = " (deleted in the intermediate layer)" + } + r.printf(" %s%s%s\r\n", r.target, lineInfo, note) r.printSingleDivider() for i, line := range lines { if line.Truncated { diff --git a/pkg/report/table/secret_test.go b/pkg/report/table/secret_test.go index fd87f7f10aa..a853c34d9ed 100644 --- a/pkg/report/table/secret_test.go +++ b/pkg/report/table/secret_test.go @@ -68,6 +68,7 @@ this is a title Category: ftypes.SecretRuleCategory("category"), Title: "this is a title", Severity: "HIGH", + Deleted: true, StartLine: 3, EndLine: 4, Code: ftypes.Code{ @@ -114,7 +115,7 @@ HIGH: category (rule-id) ════════════════════════════════════════ this is a title ──────────────────────────────────────── - my-file:3-4 + my-file:3-4 (deleted in the intermediate layer) ──────────────────────────────────────── 1 #!/bin/bash 2