Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(secret): detect secrets removed or overwritten in upper layer #2611

Merged
merged 7 commits into from
Aug 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 45 additions & 5 deletions pkg/fanal/applier/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
161 changes: 161 additions & 0 deletions pkg/fanal/applier/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion pkg/fanal/artifact/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
3 changes: 2 additions & 1 deletion pkg/fanal/types/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ type SecretRuleCategory string
type Secret struct {
FilePath string
Findings []SecretFinding
Layer Layer `json:",omitempty"`
}

type SecretFinding struct {
Expand All @@ -17,4 +16,6 @@ type SecretFinding struct {
EndLine int
Code Code
Match string
Deleted bool
Layer Layer `json:",omitempty"`
}
9 changes: 6 additions & 3 deletions pkg/report/table/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -120,7 +119,11 @@ func (r *secretRenderer) renderCode(secret types.SecretFinding) {
lineInfo = tml.Sprintf("%s<blue>-%d", lineInfo, secret.EndLine)
}
}
r.printf(" <blue>%s%s\r\n", r.target, lineInfo)
var note string
if secret.Deleted {
note = " (deleted in the intermediate layer)"
}
r.printf(" <blue>%s%s<magenta>%s\r\n", r.target, lineInfo, note)
r.printSingleDivider()
for i, line := range lines {
if line.Truncated {
Expand Down
3 changes: 2 additions & 1 deletion pkg/report/table/secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down