optiopay / klar

Integration of Clair and Docker Registry
MIT License
506 stars 140 forks source link

Use whitelist for specific image instead of base. #105

Closed mbode closed 6 years ago

mbode commented 6 years ago

If you want to avoid a breaking change to the whitelist semantics, we could also introduce a third section (something like specificImages), but I am not sure if this is necessary.

hashmap commented 6 years ago

Thanks for your contribution! From your changes in whitelist-example.yaml it seems that it's not a breaking change, but an extension of the existing functionality. Did I miss smth?

mbode commented 6 years ago

If I understand it correclty: Say you scan an image called myimage that uses alpine:

So if there are users relying on whitelists, they might need to migrate them. In particular, they need to duplicate entries in case they want to ignore CVEs for seperate images of one os class but not put these CVEs into the general whitelist. The proposed change just makes the whitelist a bit more fine-grained.

(Maybe confusingly) I did not remove the existing entries centos and alpine as there are also docker images (centos, alpine) under these specific names.

mbode commented 6 years ago

@hashmap What do you think?

hashmap commented 6 years ago

@mbode sorry, I was travelling. Sounds good, let's merge it and see how it goes.

mbode commented 6 years ago

Np, thanks for merging! Would you mind doing a release containing the change at a suitable opportunity?

chindou77 commented 6 years ago

Hi there, is there a specific reason for not supporting fully qualified image names (e.g., hostname[:port]/username/reponame[:tag]) for specifying image-specific whitelists?

mbode commented 6 years ago

@chindou77 Not super-specific: we push the same images to separate ECRs for dev/staging/prod and it is convenien not having to duplicate whitelist entries for that use case.

On the other hand, do you think it is common to pull images with the same username/reponame from different repositories with those images being different enough from each other to warrant separate whitelists?

tsuna commented 5 years ago

With this change it's no longer possible to whitelist things for a specific base image. For example say I have 2 images that both derive, indirectly, from some debian base image. There are a bunch of vulnerabilities in debian's glibc that I want to whitelist. I can't factor out the whitelist anymore unless I make it general, right?

The example whitelist is confusing because it seems to imply you can filter both based on base image or based on a specific image. Realistically no one is going to run a vanilla "alpine" image.

tsuna commented 5 years ago

Another little thing that wasn't clear (no pun intended) to me is that the library/ prefix is needed for top-level images, e.g. if I wanted to whitelist some CVEs for https://hub.docker.com/_/flink/ I figured I need:

images:
  library/flink:
    # heap overflow in libXcursor, pulled as a dependency by Java.
    - CVE-2015-9262
mbode commented 5 years ago

@tsuna Yes, you are correct, cf. my earlier post:

So if there are users relying on whitelists, they might need to migrate them. In particular, they need to duplicate entries in case they want to ignore CVEs for seperate images of one os class but not put these CVEs into the general whitelist. The proposed change just makes the whitelist a bit more fine-grained.