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): add line from dockerfile where secret was added to secret result #2780

Merged
merged 15 commits into from Sep 15, 2022
Merged
11 changes: 3 additions & 8 deletions pkg/fanal/applier/docker.go
Expand Up @@ -134,8 +134,9 @@ func ApplyLayers(layers []types.BlobInfo) types.ArtifactDetail {
// Apply secrets
for _, secret := range layer.Secrets {
l := types.Layer{
Digest: layer.Digest,
DiffID: layer.DiffID,
Digest: layer.Digest,
DiffID: layer.DiffID,
CreatedBy: layer.CreatedBy,
}
secretsMap = mergeSecrets(secretsMap, secret, l)
}
Expand Down Expand Up @@ -178,13 +179,7 @@ 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)
}

Expand Down
15 changes: 9 additions & 6 deletions pkg/fanal/applier/docker_test.go
Expand Up @@ -335,6 +335,7 @@ func TestApplyLayers(t *testing.T) {
SchemaVersion: 2,
Digest: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02",
DiffID: "sha256:a187dde48cd289ac374ad8539930628314bc581a481cdb41409c9289419ddb72",
CreatedBy: "Line_1",
Secrets: []types.Secret{
{
FilePath: "usr/secret.txt",
Expand Down Expand Up @@ -368,6 +369,7 @@ func TestApplyLayers(t *testing.T) {
SchemaVersion: 2,
Digest: "sha256:24df0d4e20c0f42d3703bf1f1db2bdd77346c7956f74f423603d651e8e5ae8a7",
DiffID: "sha256:aad63a9339440e7c3e1fff2b988991b9bfb81280042fa7f39a5e327023056819",
CreatedBy: "Line_2",
Secrets: []types.Secret{
{
FilePath: "usr/secret.txt",
Expand Down Expand Up @@ -422,6 +424,7 @@ func TestApplyLayers(t *testing.T) {
SchemaVersion: 2,
Digest: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02",
DiffID: "sha256:a187dde48cd289ac374ad8539930628314bc581a481cdb41409c9289419ddb72",
CreatedBy: "Line_3",
WhiteoutFiles: []string{
"usr/secret.txt",
},
Expand All @@ -437,13 +440,13 @@ func TestApplyLayers(t *testing.T) {
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",
Digest: "sha256:24df0d4e20c0f42d3703bf1f1db2bdd77346c7956f74f423603d651e8e5ae8a7",
DiffID: "sha256:aad63a9339440e7c3e1fff2b988991b9bfb81280042fa7f39a5e327023056819",
CreatedBy: "Line_2",
},
Code: types.Code{
Lines: []types.Line{
Expand All @@ -463,13 +466,13 @@ func TestApplyLayers(t *testing.T) {
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",
Digest: "sha256:24df0d4e20c0f42d3703bf1f1db2bdd77346c7956f74f423603d651e8e5ae8a7",
DiffID: "sha256:aad63a9339440e7c3e1fff2b988991b9bfb81280042fa7f39a5e327023056819",
CreatedBy: "Line_2",
},
Code: types.Code{
Lines: []types.Line{
Expand Down
55 changes: 40 additions & 15 deletions pkg/fanal/artifact/image/image.go
Expand Up @@ -39,6 +39,11 @@ type Artifact struct {
artifactOption artifact.Option
}

type LayerInfo struct {
DiffID string
CreatedBy string
}

func NewArtifact(img types.Image, c cache.ArtifactCache, opt artifact.Option) (artifact.Artifact, error) {
misconf := opt.MisconfScannerOption
// Register config analyzers
Expand Down Expand Up @@ -93,7 +98,7 @@ func (a Artifact) Inspect(ctx context.Context) (types.ArtifactReference, error)
log.Logger.Debugf("Base Layers: %v", baseDiffIDs)

// Convert image ID and layer IDs to cache keys
imageKey, layerKeys, layerKeyMap, err := a.calcCacheKeys(imageID, diffIDs)
imageKey, layerKeys, layerKeyMap, err := a.calcCacheKeys(imageID, diffIDs, configFile)
if err != nil {
return types.ArtifactReference{}, err
}
Expand Down Expand Up @@ -133,45 +138,64 @@ func (Artifact) Clean(_ types.ArtifactReference) error {
return nil
}

func (a Artifact) calcCacheKeys(imageID string, diffIDs []string) (string, []string, map[string]string, error) {
func (a Artifact) calcCacheKeys(imageID string, diffIDs []string, configFile *v1.ConfigFile) (string, []string, map[string]LayerInfo, error) {
// Pass an empty config scanner option so that the cache key can be the same, even when policies are updated.
imageKey, err := cache.CalcKey(imageID, a.analyzer.ImageConfigAnalyzerVersions(), nil, artifact.Option{})
if err != nil {
return "", nil, nil, err
}

layerKeyMap := map[string]string{}
var createdBy []string
// save createdBy fields in order of layers
for _, h := range configFile.History {
// skip histories for empty layers
if h.EmptyLayer {
continue
}
c := strings.TrimPrefix(strings.TrimPrefix(h.CreatedBy, "/bin/sh -c "), "#(nop) ")
createdBy = append(createdBy, c)
}

layerKeyMap := map[string]LayerInfo{}
hookVersions := a.handlerManager.Versions()
var layerKeys []string
for _, diffID := range diffIDs {
for i, diffID := range diffIDs {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

blobKey, err := cache.CalcKey(diffID, a.analyzer.AnalyzerVersions(), hookVersions, a.artifactOption)
if err != nil {
return "", nil, nil, err
}
layerKeys = append(layerKeys, blobKey)
layerKeyMap[blobKey] = diffID

c := ""
if len(createdBy) > 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if len(createdBy) > 0 {
if len(createdBy) > i {

c = createdBy[i]
}
layerKeyMap[blobKey] = LayerInfo{
DiffID: diffID,
CreatedBy: c,
}
}
return imageKey, layerKeys, layerKeyMap, nil
}

func (a Artifact) inspect(ctx context.Context, missingImage string, layerKeys, baseDiffIDs []string, layerKeyMap map[string]string) error {
func (a Artifact) inspect(ctx context.Context, missingImage string, layerKeys, baseDiffIDs []string, layerKeyMap map[string]LayerInfo) error {
done := make(chan struct{})
errCh := make(chan error)

var osFound types.OS
for _, k := range layerKeys {
go func(ctx context.Context, layerKey string) {
diffID := layerKeyMap[layerKey]
lInfo := layerKeyMap[layerKey]

// If it is a base layer, secret scanning should not be performed.
var disabledAnalyers []analyzer.Type
if slices.Contains(baseDiffIDs, diffID) {
if slices.Contains(baseDiffIDs, lInfo.DiffID) {
disabledAnalyers = append(disabledAnalyers, analyzer.TypeSecret)
}

layerInfo, err := a.inspectLayer(ctx, diffID, disabledAnalyers)
layerInfo, err := a.inspectLayer(ctx, lInfo, disabledAnalyers)
if err != nil {
errCh <- xerrors.Errorf("failed to analyze layer: %s : %w", diffID, err)
errCh <- xerrors.Errorf("failed to analyze layer: %s : %w", layerInfo.DiffID, err)
return
}
if err = a.cache.PutBlob(layerKey, layerInfo); err != nil {
Expand Down Expand Up @@ -205,12 +229,12 @@ func (a Artifact) inspect(ctx context.Context, missingImage string, layerKeys, b

}

func (a Artifact) inspectLayer(ctx context.Context, diffID string, disabled []analyzer.Type) (types.BlobInfo, error) {
log.Logger.Debugf("Missing diff ID in cache: %s", diffID)
func (a Artifact) inspectLayer(ctx context.Context, layerInfo LayerInfo, disabled []analyzer.Type) (types.BlobInfo, error) {
log.Logger.Debugf("Missing diff ID in cache: %s", layerInfo.DiffID)

layerDigest, r, err := a.uncompressedLayer(diffID)
layerDigest, r, err := a.uncompressedLayer(layerInfo.DiffID)
if err != nil {
return types.BlobInfo{}, xerrors.Errorf("unable to get uncompressed layer %s: %w", diffID, err)
return types.BlobInfo{}, xerrors.Errorf("unable to get uncompressed layer %s: %w", layerInfo.DiffID, err)
}

// Prepare variables
Expand Down Expand Up @@ -239,7 +263,8 @@ func (a Artifact) inspectLayer(ctx context.Context, diffID string, disabled []an
blobInfo := types.BlobInfo{
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: layerDigest,
DiffID: diffID,
DiffID: layerInfo.DiffID,
CreatedBy: layerInfo.CreatedBy,
OS: result.OS,
Repository: result.Repository,
PackageInfos: result.PackageInfos,
Expand Down
11 changes: 11 additions & 0 deletions pkg/fanal/artifact/image/image_test.go
Expand Up @@ -63,6 +63,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:beee9f30bc1f711043e78d4a2be0668955d4b761d587d6f60c2c8dc081efb203",
CreatedBy: "ADD file:0c4555f363c2672e350001f1293e689875a3760afe7b3f9146886afe67121cba in / ",
OS: &types.OS{
Family: "alpine",
Name: "3.11.5",
Expand Down Expand Up @@ -273,6 +274,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02",
CreatedBy: "bazel build ...",
OS: &types.OS{
Family: "debian",
Name: "9.9",
Expand Down Expand Up @@ -340,6 +342,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:dffd9992ca398466a663c87c92cfea2a2db0ae0cf33fcb99da60eec52addbfc5",
CreatedBy: "bazel build ...",
PackageInfos: []types.PackageInfo{
{
FilePath: "var/lib/dpkg/status.d/libc6",
Expand Down Expand Up @@ -406,6 +409,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:24df0d4e20c0f42d3703bf1f1db2bdd77346c7956f74f423603d651e8e5ae8a7",
CreatedBy: "COPY file:842584685f26edb24dc305d76894f51cfda2bad0c24a05e727f9d4905d184a70 in /php-app/composer.lock ",
Applications: []types.Application{
{
Type: "composer", FilePath: "php-app/composer.lock",
Expand Down Expand Up @@ -438,6 +442,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:a4595c43a874856bf95f3bfc4fbf78bbaa04c92c726276d4f64193a47ced0566",
CreatedBy: "COPY file:c6d0373d380252b91829a5bb3c81d5b1afa574c91cef7752d18170a231c31f6d in /ruby-app/Gemfile.lock ",
Applications: []types.Application{
{
Type: types.Bundler, FilePath: "ruby-app/Gemfile.lock",
Expand Down Expand Up @@ -623,6 +628,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02",
CreatedBy: "bazel build ...",
},
},
},
Expand All @@ -633,6 +639,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:dffd9992ca398466a663c87c92cfea2a2db0ae0cf33fcb99da60eec52addbfc5",
CreatedBy: "bazel build ...",
},
},
},
Expand All @@ -643,6 +650,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:24df0d4e20c0f42d3703bf1f1db2bdd77346c7956f74f423603d651e8e5ae8a7",
CreatedBy: "COPY file:842584685f26edb24dc305d76894f51cfda2bad0c24a05e727f9d4905d184a70 in /php-app/composer.lock ",
OpaqueDirs: []string{"php-app/"},
},
},
Expand All @@ -654,6 +662,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:a4595c43a874856bf95f3bfc4fbf78bbaa04c92c726276d4f64193a47ced0566",
CreatedBy: "COPY file:c6d0373d380252b91829a5bb3c81d5b1afa574c91cef7752d18170a231c31f6d in /ruby-app/Gemfile.lock ",
OpaqueDirs: []string{"ruby-app/"},
},
},
Expand Down Expand Up @@ -778,6 +787,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:beee9f30bc1f711043e78d4a2be0668955d4b761d587d6f60c2c8dc081efb203",
CreatedBy: "ADD file:0c4555f363c2672e350001f1293e689875a3760afe7b3f9146886afe67121cba in / ",
OS: &types.OS{
Family: "alpine",
Name: "3.11.5",
Expand Down Expand Up @@ -909,6 +919,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:beee9f30bc1f711043e78d4a2be0668955d4b761d587d6f60c2c8dc081efb203",
CreatedBy: "ADD file:0c4555f363c2672e350001f1293e689875a3760afe7b3f9146886afe67121cba in / ",
OS: &types.OS{
Family: "alpine",
Name: "3.11.5",
Expand Down
25 changes: 19 additions & 6 deletions pkg/fanal/image/daemon/containerd.go
Expand Up @@ -19,6 +19,7 @@ import (
api "github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/go-connections/nat"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/samber/lo"
Expand Down Expand Up @@ -90,14 +91,15 @@ func ContainerdImage(ctx context.Context, imageName string) (Image, func(), erro
_ = os.Remove(f.Name())
}

insp, err := inspect(ctx, img, ref)
insp, history, err := inspect(ctx, img, ref)
if err != nil {
return nil, nil, xerrors.Errorf("inspect error: %w", err)
}

return &image{
opener: imageOpener(ctx, ref.String(), f, imageWriter(client, img)),
inspect: insp,
opener: imageOpener(ctx, ref.String(), f, imageWriter(client, img)),
inspect: insp,
convertedHistory: history,
}, cleanup, nil
}

Expand All @@ -121,7 +123,7 @@ func readImageConfig(ctx context.Context, img containerd.Image) (ocispec.Image,
}

// ported from https://github.com/containerd/nerdctl/blob/d110fea18018f13c3f798fa6565e482f3ff03591/pkg/inspecttypes/dockercompat/dockercompat.go#L279-L321
func inspect(ctx context.Context, img containerd.Image, ref docker.Named) (api.ImageInspect, error) {
func inspect(ctx context.Context, img containerd.Image, ref docker.Named) (api.ImageInspect, []v1.History, error) {
var tag string
if tagged, ok := ref.(refdocker.Tagged); ok {
tag = tagged.Tag()
Expand All @@ -130,14 +132,25 @@ func inspect(ctx context.Context, img containerd.Image, ref docker.Named) (api.I

imgConfig, imgConfigDesc, err := readImageConfig(ctx, img)
if err != nil {
return api.ImageInspect{}, err
return api.ImageInspect{}, nil, err
}

var lastHistory ocispec.History
if len(imgConfig.History) > 0 {
lastHistory = imgConfig.History[len(imgConfig.History)-1]
}

var history []v1.History
for _, h := range imgConfig.History {
history = append(history, v1.History{
Author: h.Author,
Created: v1.Time{Time: *h.Created},
CreatedBy: h.CreatedBy,
Comment: h.Comment,
EmptyLayer: h.EmptyLayer,
})
}

portSet := make(nat.PortSet)
for k := range imgConfig.Config.ExposedPorts {
portSet[nat.Port(k)] = struct{}{}
Expand Down Expand Up @@ -168,5 +181,5 @@ func inspect(ctx context.Context, img containerd.Image, ref docker.Named) (api.I
return d.String()
}),
},
}, nil
}, history, nil
}