Skip to content

Commit

Permalink
feat(secret): add line from dockerfile where secret was added to secr…
Browse files Browse the repository at this point in the history 
…et result (#2780)

Co-authored-by: knqyf263 <knqyf263@gmail.com>
  • Loading branch information
@DmitriyLewen @knqyf263
DmitriyLewen and knqyf263 committed Sep 15, 2022
1 parent 9f6680a commit b6e394d
Show file tree
Hide file tree
Showing 17 changed files with 508 additions and 192 deletions.
11 changes: 3 additions & 8 deletions pkg/fanal/applier/docker.go
View file
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
View file
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
79 changes: 60 additions & 19 deletions pkg/fanal/artifact/image/image.go
View file
Expand Up @@ -37,6 +37,11 @@ type Artifact struct {
artifactOption artifact.Option
}

type LayerInfo struct {
DiffID string
CreatedBy string // can be empty
}

func NewArtifact(img types.Image, c cache.ArtifactCache, opt artifact.Option) (artifact.Artifact, error) {
// Initialize handlers
handlerManager, err := handler.NewManager(opt)
Expand Down Expand Up @@ -90,11 +95,14 @@ 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, err := a.calcCacheKeys(imageID, diffIDs)
if err != nil {
return types.ArtifactReference{}, err
}

// Parse histories and extract a list of "created_by"
layerKeyMap := a.consolidateCreatedBy(diffIDs, layerKeys, configFile)

missingImage, missingLayers, err := a.cache.MissingBlobs(imageKey, layerKeys)
if err != nil {
return types.ArtifactReference{}, xerrors.Errorf("unable to get missing layers: %w", err)
Expand Down Expand Up @@ -130,45 +138,77 @@ 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) (string, []string, 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
return "", nil, err
}

layerKeyMap := map[string]string{}
hookVersions := a.handlerManager.Versions()
var layerKeys []string
for _, diffID := range diffIDs {
blobKey, err := cache.CalcKey(diffID, a.analyzer.AnalyzerVersions(), hookVersions, a.artifactOption)
if err != nil {
return "", nil, nil, err
return "", nil, err
}
layerKeys = append(layerKeys, blobKey)
layerKeyMap[blobKey] = diffID
}
return imageKey, layerKeys, layerKeyMap, nil
return imageKey, layerKeys, nil
}

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

// If history detected incorrect - use only diffID
// TODO: our current logic may not detect empty layers correctly in rare cases.
validCreatedBy := len(diffIDs) == len(createdBy)

layerKeyMap := map[string]LayerInfo{}
for i, diffID := range diffIDs {

c := ""
if validCreatedBy {
c = createdBy[i]
}

layerKey := layerKeys[i]
layerKeyMap[layerKey] = LayerInfo{
DiffID: diffID,
CreatedBy: c,
}
}
return layerKeyMap
}

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]
layer := 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, layer.DiffID) {
disabledAnalyers = append(disabledAnalyers, analyzer.TypeSecret)
}

layerInfo, err := a.inspectLayer(ctx, diffID, disabledAnalyers)
layerInfo, err := a.inspectLayer(ctx, layer, 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 @@ -202,12 +242,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 @@ -236,15 +276,16 @@ 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,
OpaqueDirs: opqDirs,
WhiteoutFiles: whFiles,
OS: result.OS,
Repository: result.Repository,
PackageInfos: result.PackageInfos,
Applications: result.Applications,
Secrets: result.Secrets,
Licenses: result.Licenses,
OpaqueDirs: opqDirs,
WhiteoutFiles: whFiles,
CustomResources: result.CustomResources,

// For Red Hat
Expand Down
11 changes: 11 additions & 0 deletions pkg/fanal/artifact/image/image_test.go
View file
Expand Up @@ -65,6 +65,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 @@ -275,6 +276,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 @@ -342,6 +344,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 @@ -408,6 +411,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 @@ -440,6 +444,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 @@ -625,6 +630,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02",
CreatedBy: "bazel build ...",
},
},
},
Expand All @@ -635,6 +641,7 @@ func TestArtifact_Inspect(t *testing.T) {
SchemaVersion: types.BlobJSONSchemaVersion,
Digest: "",
DiffID: "sha256:dffd9992ca398466a663c87c92cfea2a2db0ae0cf33fcb99da60eec52addbfc5",
CreatedBy: "bazel build ...",
},
},
},
Expand All @@ -645,6 +652,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 @@ -656,6 +664,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 @@ -780,6 +789,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 @@ -911,6 +921,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
21 changes: 17 additions & 4 deletions pkg/fanal/image/daemon/containerd.go
View file
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,
history: 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
}

0 comments on commit b6e394d

Please sign in to comment.