Enhancement: get vulnerabilities by image's non-empty top layer #7

supereagle · 2016-11-05T14:20:17Z

@hashmap I have finished the enhancement. PTAL, thanks.

Fix #6

…ayer

hashmap · 2016-11-08T08:56:45Z

Thanks @supereagle I'm testing it

hashmap · 2016-11-09T11:43:27Z

Do you have a (public) example of an image with vulnerabilities? Your version always says Found 0 vulnerabilities, perhaps the current version gives false alarms.

supereagle · 2016-11-10T06:38:51Z

Do you clean up your postgres? If you use an old postgres, maybe the relationship between layers stored in it is not correct. As the current version depends on these relationships to ensure that the vulnerabilities can be got by non-empty top layer.

supereagle · 2016-11-10T09:31:36Z

You can have a try on the public image docker.io/nginx:1.9.

supereagle · 2016-11-22T05:30:38Z

@hashmap Have you tested again?

supereagle · 2016-12-04T23:14:19Z

Hi Alexey Miroshkin, I have explained your test results in my comments. Have you tested this PR again?

…

________________________________ 发件人: Alexey Miroshkin <notifications@github.com> 发送时间: 2016年11月9日 19:43 收件人: optiopay/klar 抄送: Robin Yue; Mention 主题: Re: [optiopay/klar] enhancement: get vulnerabilities by image's non-empty top layer (#7) Do you have a (public) example of an image with vulnerabilities? Your version always says Found 0 vulnerabilities, perhaps the current version gives false alarms. ― You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#7 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ADO7fRwGHo1gTzW81SL5hwLXew7l_qDNks5q8bHggaJpZM4KqRgi>.

hashmap · 2016-12-05T14:10:47Z

clair/clair.go

 	var vs []Vulnerability
-	for i := range image.FsLayers {
+	for i := layerLength - 1; i >= 0; i-- {


I thought you want to analyze just the top level, in code I see that empty layers a filtered out and we reversed the direction of iteration. Did I miss something?

filterEmptyLayers() just filters the empty layters, not reverses the layer order.

hashmap · 2016-12-05T14:11:59Z

clair/clair.go

-	if index > 0 {
-		parentName = image.FsLayers[index-1].BlobSum
+	if index < len(image.FsLayers)-1 {
+		parentName = image.FsLayers[index+1].BlobSum


We reversed the loop, but layers are still stored in the original order, why we take upper layer as a parent?

hashmap · 2016-12-05T14:16:02Z

clair/clair.go

+	vs, err := c.analyzeLayer(image.FsLayers[0])
+	if err != nil {
+		fmt.Printf("Analyse image %s/%s:%s failed: %s", image.Registry, image.Name, image.Tag, err.Error())
+		return nil


Clair sometimes returns error, not sure we should stop in this case.

The GET layer API will be called only once with the the top layer as it is out of loop. If it has error, should return with no vulnerability.

hashmap · 2016-12-05T14:17:12Z

clair/clair.go

 	return vs
 }

-func (c *Clair) analyzeLayer(layer *layer) (*[]Vulnerability, error) {
-	url := fmt.Sprintf("%s/v1/layers/%s?vulnerabilities", c.url, layer.Name)
+func (c *Clair) analyzeLayer(layer docker.FsLayer) ([]Vulnerability, error) {


The idea was to work with Clair layers here, not with Docker ones, any reason to break this encapsulation?

They are the same, as both of them are for the layer id. The vulnerabilities can be got by image top layer, which can be directly got by image.FsLayers[0], so use Docker layer instead.

hashmap · 2016-12-05T14:17:36Z

clair/clair.go

@@ -131,7 +153,7 @@ func (c *Clair) analyzeLayer(layer *layer) (*[]Vulnerability, error) {
 			vs = append(vs, v)
 		}
 	}
-	return &vs, nil
+	return vs, nil


I like this change, thanks

hashmap · 2016-12-06T15:27:53Z

Cool, thanks for your efforts! The only question from my side why do you need to reverse order while pushing layers to Clair?

supereagle · 2016-12-07T05:14:22Z

The revered order is the order from parents to children. image.FsLayers[n-1] is the child layer, and image.FsLayers[n] is the parent layer.

hashmap · 2016-12-13T14:39:34Z

Could you point me to the spec which defines the order of layers for pull? For some reason I thought that order is different, but could not find any specs now.

supereagle · 2016-12-14T09:26:24Z

Take docker.io/nginx:latest as an example, its Dockerfile is mainline/jessie/Dockerfile

And its manifest is:

   "schemaVersion": 1,
   "name": "library/nginx",
   "tag": "latest",
   "architecture": "amd64",
   "fsLayers": [
      {
         "blobSum": "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4"
      },
      {
         "blobSum": "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4"
      },
      {
         "blobSum": "sha256:d685e39ac8a4ccea462d489b503f9de952f0b8c7a0b0a0548f7a5c20b272668b"
      },
      {
         "blobSum": "sha256:386dc9762af975db201ed66aebd3f8b5f2c24389db4744d54ec47667dcdae26a"
      },
      {
         "blobSum": "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4"
      },
      {
         "blobSum": "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4"
      },
      {
         "blobSum": "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4"
      },
      {
         "blobSum": "sha256:386a066cd84a33a04d560c42bef66d1dd64ebfc76de78550e5fd0f8d57778bca"
      }
   ],
...
}

Compare its dockerfile and manifest, you will find just two non-empty layers(d685e39ac8a4ccea462d489b503f9de952f0b8c7a0b0a0548f7a5c20b272668b, 386dc9762af975db201ed66aebd3f8b5f2c24389db4744d54ec47667dcdae26a) for two RUN commands in dockerfile. The order of commands in dockerfile are reversed for their layers in manifest.

supereagle · 2016-12-18T06:01:05Z

@hashmap Any points still not clear?

enhancement: get vulnerabilities according to image's non-empty top l…

9a94f0d

…ayer

i1skn changed the title ~~enhancement: get vulnerabilities by image's non-empty top layer~~ Enhancement: get vulnerabilities by image's non-empty top layer Nov 10, 2016

i1skn added the enhancement label Nov 10, 2016

hashmap reviewed Dec 5, 2016

View reviewed changes

hashmap merged commit b088466 into optiopay:master Dec 19, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancement: get vulnerabilities by image's non-empty top layer #7

Enhancement: get vulnerabilities by image's non-empty top layer #7

supereagle commented Nov 5, 2016 •

edited

Loading

hashmap commented Nov 8, 2016

hashmap commented Nov 9, 2016

supereagle commented Nov 10, 2016

supereagle commented Nov 10, 2016

supereagle commented Nov 22, 2016

supereagle commented Dec 4, 2016 via email

hashmap Dec 5, 2016

supereagle Dec 6, 2016

hashmap Dec 5, 2016

hashmap Dec 5, 2016

supereagle Dec 6, 2016

hashmap Dec 5, 2016

supereagle Dec 6, 2016

hashmap Dec 5, 2016

hashmap commented Dec 6, 2016

supereagle commented Dec 7, 2016

hashmap commented Dec 13, 2016

supereagle commented Dec 14, 2016

supereagle commented Dec 18, 2016

Enhancement: get vulnerabilities by image's non-empty top layer #7

Enhancement: get vulnerabilities by image's non-empty top layer #7

Conversation

supereagle commented Nov 5, 2016 • edited Loading

hashmap commented Nov 8, 2016

hashmap commented Nov 9, 2016

supereagle commented Nov 10, 2016

supereagle commented Nov 10, 2016

supereagle commented Nov 22, 2016

supereagle commented Dec 4, 2016 via email

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

hashmap commented Dec 6, 2016

supereagle commented Dec 7, 2016

hashmap commented Dec 13, 2016

supereagle commented Dec 14, 2016

supereagle commented Dec 18, 2016

supereagle commented Nov 5, 2016 •

edited

Loading