Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Aug 15, 2022
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f538030
save secrets from all layers
DmitriyLewen Jul 28, 2022
99a0633
move layer to SecretFinding
DmitriyLewen Aug 3, 2022
8ed5bc2
refactor
DmitriyLewen Aug 3, 2022
c5c4c43
remove an unused field
knqyf263 Aug 15, 2022
20998c4
fix: guess base layers
knqyf263 Aug 15, 2022
c2038d4
feat: add deleted flag
knqyf263 Aug 15, 2022
0967cb7
feat: display the secret is deleted
knqyf263 Aug 15, 2022
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
6 changes: 2 additions & 4 deletions pkg/fanal/applier/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ func ApplyLayers(layers []types.BlobInfo) types.ArtifactDetail {
Digest: layer.Digest,
DiffID: layer.DiffID,
}
key := fmt.Sprintf("%s/type:secret", secret.FilePath)
nestedMap.SetByString(key, sep, secret)
// we must save secrets from all layers
mergedLayer.Secrets = append(mergedLayer.Secrets, secret)
}

// 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 Down
138 changes: 138 additions & 0 deletions pkg/fanal/applier/docker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,144 @@ func TestApplyLayers(t *testing.T) {
},
},
},
{
name: "happy path with removed and updated secret",
inputLayers: []types.BlobInfo{
{
SchemaVersion: 2,
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,
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,
},
},
},
},
},
},
},
},
{
SchemaVersion: 2,
DiffID: "sha256:a187dde48cd289ac374ad8539930628314bc581a481cdb41409c9289419ddb72",
WhiteoutFiles: []string{
"usr/secret.txt",
},
},
},
want: types.ArtifactDetail{
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,
},
},
},
},
},
Layer: types.Layer{
DiffID: "sha256:a187dde48cd289ac374ad8539930628314bc581a481cdb41409c9289419ddb72",
},
},
{
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,
},
},
},
},
},
Layer: types.Layer{
DiffID: "sha256:aad63a9339440e7c3e1fff2b988991b9bfb81280042fa7f39a5e327023056819",
},
},
},
},
},
{
name: "happy path with status.d and opaque dirs without the trailing slash",
inputLayers: []types.BlobInfo{
Expand Down
1 change: 1 addition & 0 deletions pkg/scanner/local/scan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ func (s Scanner) secretsToResults(secrets []ftypes.Secret) types.Results {
results = append(results, types.Result{
Target: secret.FilePath,
Class: types.ClassSecret,
Layer: secret.Layer.DiffID,
Secrets: secret.Findings,
})
}
Expand Down
1 change: 1 addition & 0 deletions pkg/types/report.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type Result struct {
Target string `json:"Target"`
Class ResultClass `json:"Class,omitempty"`
Type string `json:"Type,omitempty"`
Layer string `json:"Layer,omitempty"` // Used only for secrets, as Trivy keeps secrets from all layers.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have Layer here rather than in Result.

trivy/pkg/fanal/types/secret.go

Lines 11 to 20 in 7699153

type SecretFinding struct {
RuleID string
Category SecretRuleCategory
Severity string
Title string
StartLine int
EndLine int
Code Code
Match string
}

If the finding is the same, the first introduced layer should be taken.

  • Layer 1
    • no secret found
  • Layer 2
    • secret.txt
      • GitHub PAT
  • Layer 3
    • secret.txt
      • GitHub PAT
      • AWS Access Key
  • Layer 4
    • remove secret.txt

The result would be as below.

{
  "Target": "secret.txt",
  "Secrets": [
    {
      "RuleID": "github-pat",
      ...
      "Layer": "Layer 2",
    },
    {
      "RuleID": "aws-access-key",
      ...
      "Layer": "Layer 3",
    }
  ]
}

We should not have github-pat in Layer 3.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that Layer actually consists of Digest and DiffID. The above example is for sake of simplicity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Packages []ftypes.Package `json:"Packages,omitempty"`
Vulnerabilities []DetectedVulnerability `json:"Vulnerabilities,omitempty"`
MisconfSummary *MisconfSummary `json:"MisconfSummary,omitempty"`
Expand Down