feat(secret): add line from dockerfile where secret was added to secret result #2780

DmitriyLewen · 2022-08-25T12:14:20Z

Description

For images: secrets contain the string from dockerfile where this secret was added from.
Also, this line is contained in the table report.
e.g.

/secret.txt (secrets)

Total: 1 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 0, CRITICAL: 1)

CRITICAL: AWS (aws-access-key-id)
═════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
AWS Access Key ID
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 /secret.txt:1
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 This secret is added in 'echo "AWS_ACCESS_KEY_ID=AKIA0123456789ABCDEF" > secret.txt'
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1 [ AWS_ACCESS_KEY_ID=********************
   2   
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Related PRs

fix(image): add logic to detect empty layers #2790

Checklist

I've read the guidelines for contributing to this repository.
I've followed the conventions in the PR title.
I've added tests that prove my fix is effective or that my feature works.
I've updated the documentation with the relevant information (if needed).
I've added usage information (if the PR introduces new options)
I've included a "before" and "after" example to the description (if the PR is a user interface change).

knqyf263 · 2022-08-28T14:12:24Z

pkg/fanal/types/secret.go

 	Layer     Layer `json:",omitempty"`
+	CreatedBy string


What if we put CreatedBy into Layer?

knqyf263 · 2022-08-28T14:16:02Z

pkg/report/table/secret_test.go

+ my-file:3-4
+────────────────────────────────────────
+ This secret is added in 'COPY my-file my-file'


After some consideration, I came up with another idea. One line looks better. If it has a file hash, what if we take the first 7 characters like git rev-parse --short HEAD?

Suggested change

my-file:3-4

────────────────────────────────────────

This secret is added in 'COPY my-file my-file'

my-file:3-4 (added by 'COPY my-file my-file')

Then, if it is still too long, we can cut it like the first 40 chars, etc.

knqyf263 · 2022-08-28T17:24:11Z

pkg/fanal/artifact/image/image.go

-	layerKeyMap := map[string]string{}
+	var createdBy []string
+	// save createdBy fields in order of layers
+	for i := 0; i < len(configFile.History); i++ {


Suggested change

for i := 0; i < len(configFile.History); i++ {

for i, history := range configFile.History {

knqyf263 · 2022-08-28T17:26:22Z

pkg/fanal/artifact/image/image.go

-		layerKeyMap[blobKey] = diffID
+
+		c := ""
+		if len(createdBy) > 0 {


Suggested change

if len(createdBy) > 0 {

if len(createdBy) > i {

knqyf263 · 2022-08-28T17:28:00Z

pkg/fanal/artifact/image/image.go

 	hookVersions := a.handlerManager.Versions()
 	var layerKeys []string
-	for _, diffID := range diffIDs {
+	for i, diffID := range diffIDs {


I think we should compare len(diffIDs) and len(createdBy). They should be the same. If they don't match, we should warn it. It is probably our bug.

CLAassistant · 2022-08-29T04:34:07Z

All committers have signed the CLA.

ankk13

The deleted secret can be removed from secret map, if respective OpaqueDirs/Whiteoutfiles are found.

DmitriyLewen · 2022-08-30T09:14:51Z

Hello @ankk13
You want to remove secret from secretMap, if it was removed in upper layer, right?

if so - we can't do that, because image still contain this secret.
It is not safely. User needs to remove secret from layer where this secret was added

…rfile-line-for-secrets

ankk13 · 2022-09-07T18:56:08Z

ove secret from secretMap, if it was removed in upper layer, right?

if so - we can't do that, because image still contain this secret.

Yes, even if image still contain layer. I believe it is correct to tag secret as "delete in intermediate layer", as we are not commenting that secret is deleted. Let me know if you feel otherwise

add lines from dockerfile to secrets

0b570bc

DmitriyLewen changed the title ~~feat(secrets): add line from dockerfile where secret was added to secret~~ feat(secret): add line from dockerfile where secret was added to secret Aug 25, 2022

DmitriyLewen added 3 commits August 26, 2022 13:48

refactor

91d7cef

refactor

ab8ea1a

add skip empty history

67c70da

knqyf263 reviewed Aug 28, 2022

View reviewed changes

move createdby to layer

70384d9

add history for podman and containerd

c0b43d2

ankk13 reviewed Aug 29, 2022

View reviewed changes

use diffId when wrong histories

894c8c4

DmitriyLewen changed the title ~~feat(secret): add line from dockerfile where secret was added to secret~~ feat(secret): add line from dockerfile where secret was added to secret result Aug 30, 2022

Merge branch 'main' of github.com:aquasecurity/trivy into feat/docke…

f8c6387

…rfile-line-for-secrets

DmitriyLewen self-assigned this Aug 31, 2022

knqyf263 added 7 commits September 13, 2022 20:10

Merge branch 'main' into feat/dockerfile-line-for-secrets

7a889eb

refactor: separate a function

1e073b2

use v1.History

bc6837c

remove cruft

68d584b

do not show note when diff_id is empty

be1bddc

rpc: use reserved for deprecated fields

5817684

fix a bug

605c02e

knqyf263 marked this pull request as ready for review September 13, 2022 18:40

ankk13 approved these changes Sep 14, 2022

View reviewed changes

knqyf263 added this to the v0.32.0 milestone Sep 15, 2022

tonaim approved these changes Sep 15, 2022

View reviewed changes

knqyf263 approved these changes Sep 15, 2022

View reviewed changes

knqyf263 merged commit b6e394d into main Sep 15, 2022

knqyf263 deleted the feat/dockerfile-line-for-secrets branch September 15, 2022 07:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(secret): add line from dockerfile where secret was added to secret result #2780

feat(secret): add line from dockerfile where secret was added to secret result #2780

DmitriyLewen commented Aug 25, 2022 •

edited

knqyf263 Aug 28, 2022

DmitriyLewen Aug 29, 2022

knqyf263 Aug 28, 2022

DmitriyLewen Aug 29, 2022

knqyf263 Aug 28, 2022

DmitriyLewen Aug 29, 2022

knqyf263 Aug 28, 2022

knqyf263 Aug 28, 2022

CLAassistant commented Aug 29, 2022 •

edited

ankk13 left a comment

DmitriyLewen commented Aug 30, 2022

ankk13 commented Sep 7, 2022

	for i := 0; i < len(configFile.History); i++ {
	for i, history := range configFile.History {

feat(secret): add line from dockerfile where secret was added to secret result #2780

feat(secret): add line from dockerfile where secret was added to secret result #2780

Conversation

DmitriyLewen commented Aug 25, 2022 • edited

Description

Related PRs

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Aug 29, 2022 • edited

ankk13 left a comment

Choose a reason for hiding this comment

DmitriyLewen commented Aug 30, 2022

ankk13 commented Sep 7, 2022

DmitriyLewen commented Aug 25, 2022 •

edited

CLAassistant commented Aug 29, 2022 •

edited