Skip to content

Commit

Permalink
feat(secret): detect secrets removed or overwritten in upper layer (#…
Browse files Browse the repository at this point in the history 
…2611)

Co-authored-by: knqyf263 <knqyf263@gmail.com>
  • Loading branch information
@DmitriyLewen @knqyf263
DmitriyLewen and knqyf263 committed Aug 15, 2022
1 parent ddffb1b commit a5d4f7f
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 11 deletions.
50 changes: 45 additions & 5 deletions pkg/fanal/applier/docker.go
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit a5d4f7f

Please sign in to comment.