[WIP] Alpine support via Alpine-SecDB #272

jzelinskie · 2016-11-18T21:42:10Z

I'm making this PR now just to let people know that it's finally happening. Data is sourced from Alpine SecDB (MIT licensed). This fetcher adds git as a dependency on the system path.

Namespace Detector based on /etc/alpine-release
Feature detector based on /lib/apk/db/installed
Fetcher using alpine-secdb via git
Unit tests for all
Ensure everything actually works 😜

Quentin-M · 2016-11-18T21:56:29Z

README (source, license)
Dockerfile (git dep)
GIF centering (swag)

barakmich · 2016-11-18T21:57:27Z

GIF centering

🚎

betawaffle · 2016-11-18T22:34:06Z

Umm, the GIF doesn't even load!

Quentin-M · 2016-11-18T23:39:14Z

updater/fetchers/alpine/alpine.go

+
+	// Ask the database for the latest commit we successfully applied.
+	var dbCommit string
+	dbCommit, err = db.GetKeyValue("alpineUpdater")


Quentin-M · 2016-11-18T23:40:34Z

updater/fetchers/alpine/alpine.go

+	// Append any changed vulnerabilities to the response.
+	if commit == dbCommit {
+		log.Debug("no alpine update")
+	} else {


set FlagName/FlagValue before; then no need for else, just return; saves an indentation level.

Quentin-M · 2016-11-19T00:10:24Z

updater/fetchers/alpine/alpine.go

+		for _, fix := range pkg.Fixes {
+			version, err := types.NewVersion(pkg.Version)
+			if err != nil {
+				continue


log.Warningf("could not parse package version '%s': %s. skipping", pkg.version, err.Error())

Quentin-M · 2016-11-19T00:15:27Z

updater/fetchers/alpine/alpine.go

+				continue
+			}
+
+			var vuln database.Vulnerability


should we use the database.Vulnerability{} notation?

Quentin-M · 2016-11-19T00:15:41Z

updater/fetchers/alpine/alpine.go

+			var vuln database.Vulnerability
+			vuln.Name = fix
+			vuln.Link = nvdURLPrefix + fix
+			vuln.FixedIn = append(vuln.FixedIn, database.FeatureVersion{


why append() if you just created the object?

Quentin-M · 2016-11-19T00:27:52Z

worker/detectors/feature/apk/apk.go

+			// one.
+			if ipkg.Feature.Name != "" && ipkg.Version.String() != "" {
+				pkgSet[ipkg.Feature.Name+"#"+ipkg.Version.String()] = ipkg
+				ipkg = database.FeatureVersion{}


ipkg = intermediary package, once you have both the name and version, we reset it so that we can parse a new one.

Quentin-M · 2016-11-19T00:29:02Z

worker/detectors/feature/apk/apk.go

+
+func (d *detector) Detect(data map[string][]byte) ([]database.FeatureVersion, error) {
+	file, exists := data["lib/apk/db/installed"]
+	if exists {


f, hasFile := data["lib/apk/db/installed"] if !hasFile { return []database.FeatureVersion{}, nil }

simpler, avoids one level of indentation

mattmoor · 2016-11-19T03:17:56Z

Instead of adding a dependency on Git, did you consider simply fetching the tarball at HEAD?

http://git.alpinelinux.org/cgit/alpine-secdb/snapshot/alpine-secdb-master.tar.bz2

Quentin-M · 2016-11-19T10:48:58Z

It makes a lot of sense now because their database is extremely small. As the database gets bigger (hopefully.. but maybe never big enough to make sense), git pull will become faster and download less data after few update loops. However using the tarball would surely make adapting the fetcher easier when we'll want to adapt Clair to be used without external network connectivity.

jzelinskie · 2016-11-19T17:15:23Z

However using the tarball would surely make adapting the fetcher easier when we'll want to adapt Clair to be used without external network connectivity.

I'm absolutely open to changing to the tarball download if this a goal for the future. Do we have a plan for migrating existing fetchers to systems that could support this?

mattmoor · 2016-11-19T20:02:10Z

FWIW, we run in contexts where shelling out to tools like bzr, git, rpm simply isn't practical. In our pseudo air-gapped environment we can reach outside by swapping out http.DefaultTransport in-process for certain sites, but tool invocations create trouble (harder to do the same, especially without carrying patches).

jzelinskie · 2016-11-28T21:51:11Z

@mattmoor Have you been unable to use Ubuntu fetchers for this same reason? They depend on bzr.

mattmoor · 2016-11-28T21:55:35Z

@jzelinskie Yes, we have a variant (that we were supposed to upstream, which I'm running down) that basically makes the mechanism pluggable.

The tl;dr is that we define an interface for the core methods that shell out to bzr and implement it one way with bzr and another with the bzr tarball url.

mwarkentin · 2016-11-28T21:56:21Z

Does anyone have a feel for how maintained alpine-secdb is? It looks like it was last updated ~7 weeks ago?

Really hoping for a good solution here for monitoring for vulnerable packages in Alpine, just wondering if there are any plans for how the upstream database will be kept up to date?

Quentin-M · 2016-11-28T22:01:49Z

@mwarkentin That's a very good question. We have contacted the main committer to ask about the license and whether we should expect this database to be a first-time citizen / updated regularly or not. We now know that the database is licensed under MIT but as far as I know, we have had no update on that second question yet. We hope that our implementation will increase demand on this important topic at Alpine.

mwarkentin · 2016-11-28T22:08:01Z

@Quentin-M Thanks! I agree that having an implementation like this could be a good driver for keeping the db updated.

Please update if you get any further information about this. :)

jzelinskie · 2016-11-29T01:17:41Z

@mattmoor any timeline on upstreaming this work? If we could settle on a streamlined a process for backchanneling data sources for air-gapped Clair instances, that'd be great!

mattmoor · 2016-11-29T20:19:07Z

FWIW, I wouldn't characterize it as a general air-gap solution so much as a pattern that enables it. No timeframe yet, so don't block this for it.

Quentin-M · 2016-12-14T23:41:59Z

@jzelinskie Have any e2e tested it on an image that actually contains one of the managed vulnerability? Also, I am aware that an existing namespace detector catched Alpine (probably wrongly) on some images, that'd be good to make sure they don't collide and that only the actual new Alpine detector gets it rather than the other one.

Quentin-M · 2016-12-15T17:45:24Z

updater/fetchers/alpine/alpine.go

+)
+
+const (
+	secdbGitURL  = "http://git.alpinelinux.org/cgit/alpine-secdb"


Can we add a note for 'httpss://' when supported?

Quentin-M · 2016-12-15T17:50:03Z

updater/fetchers/alpine/alpine.go

+	}
+
+	// Append any changed vulnerabilities to the response.
+	for _, namespace := range []string{"v3.3", "v3.4"} {


nit: It'd be neat to extract the version->parser mapping outside of this function, at the top.

Quentin-M · 2016-12-15T17:53:56Z

updater/fetchers/alpine/alpine.go

+		}
+	}
+
+	out, err := utils.Exec(f.repositoryLocalPath, "git", "rev-parse", "HEAD")


So the first time, we clone. The next times, we just rev-parse HEAD. Shouldn't we pull before? Otherwise even if HEAD moved, we'd still be reading the old files when opening them.

Quentin-M · 2016-12-15T18:22:48Z

updater/fetchers/alpine/alpine.go

+				continue
+			}
+
+			var vuln database.Vulnerability


Is not using database.Vulnerability{Name: "", Link: ""} style a conscious decision?

Quentin-M · 2016-12-16T00:01:35Z

Besides my comment regarding to potential Alpine detections with the existing detectors above, this looks fine.

I think my idea around range []string{"v3.3", "v3.4"} was to have a map Version->Parsing function at the top and avoid having these hardcoded in a function (in which we range and switch over). So we could simply modify the map and add a new parsing function whenever we need to. Now they are hardcoded in two functions (parseVulnsFromNamespace and FetchUpdate). It doesn't matter too much though, just style.

Quentin-M · 2016-12-16T21:52:58Z

updater/fetchers/alpine/alpine.go

+
+	parseFunc, exists := parsers[namespace]
+	if !exists {
+		return


All these are much better. Please just add a Warning here as well as an updater Note for that so we can be alerted when we need to take action. See https://github.com/coreos/clair/blob/master/updater/fetchers/ubuntu/ubuntu.go#L139

Quentin-M · 2016-12-16T22:53:51Z

LGTM. Users will then be able to figure out action is needed by reading Prometheus.

This change adjusts some names of types being exported and adds some documentation.

adityacs · 2017-05-31T13:33:25Z

@jzelinskie Could you please provide me a alpine docker image which has some CVEs?
I scanned many images with alpine but always getting 0 vulnerabilities.

boydgreenfield · 2018-01-26T02:45:49Z

@adityacs Very late, but node:6.9.4-alpine has a bunch of vulnerabilities (first one I googled, was also worried as the latest few all returned 0 and I wasn't sure if any real checks were being executed):

2018/01/25 18:44:33 [WARN] ▶ Image [node:6.9.4-alpine] contains 7 total vulnerabilities
2018/01/25 18:44:33 [ERRO] ▶ Image [node:6.9.4-alpine] contains 7 unapproved vulnerabilities
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+
| STATUS     | CVE SEVERITY          | PACKAGE NAME | PACKAGE VERSION | CVE DESCRIPTION                                               |
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+
| Unapproved | High CVE-2016-9843    | zlib         | 1.2.8-r2        |                                                               |
|            |                       |              |                 | https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-9843  |
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+
| Unapproved | High CVE-2016-9841    | zlib         | 1.2.8-r2        |                                                               |
|            |                       |              |                 | https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-9841  |
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+
| Unapproved | Medium CVE-2017-16544 | busybox      | 1.24.2-r12      |                                                               |
|            |                       |              |                 | https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-16544 |
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+
| Unapproved | Medium CVE-2017-15650 | musl         | 1.1.14-r14      |                                                               |
|            |                       |              |                 | https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15650 |
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+
| Unapproved | Medium CVE-2016-9842  | zlib         | 1.2.8-r2        |                                                               |
|            |                       |              |                 | https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-9842  |
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+
| Unapproved | Medium CVE-2016-9840  | zlib         | 1.2.8-r2        |                                                               |
|            |                       |              |                 | https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-9840  |
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+
| Unapproved | Medium CVE-2017-15873 | busybox      | 1.24.2-r12      |                                                               |
|            |                       |              |                 | https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15873 |
+------------+-----------------------+--------------+-----------------+---------------------------------------------------------------+

jzelinskie added component/fetcher kind/feature request wishes for new functionality/docs reviewed/needs rework will be closed if review not addressed labels Nov 18, 2016

jzelinskie assigned Quentin-M Nov 18, 2016

Quentin-M reviewed Nov 18, 2016

View reviewed changes

Quentin-M reviewed Nov 19, 2016

View reviewed changes

Quentin-M reviewed Dec 15, 2016

View reviewed changes

Quentin-M approved these changes Dec 16, 2016

View reviewed changes

Quentin-M reviewed Dec 16, 2016

View reviewed changes

jzelinskie added 13 commits December 19, 2016 11:32

detectors/namespace: support pointers in tests

0b2a9ab

This change adjusts some names of types being exported and adds some documentation.

detectors/namespace: add alpine-release detector

1d5a9dd

detectors/feature: consistent naming and godoc

e4b5930

detectors/feature: add apk feature detector

fc908e6

updater/fetchers: add alpine secdb fetcher

0cb8fc9

dockerfile: add git dependency

c1e0f61

README: add alpine data sources

805f620

alpine: add support for v3.4 YAML schema

3d90cac

alpine: compile alpine into clair binary

f8457b9

alpine: truncate namespace to "vMAJOR.MINOR"

9be305d

alpine: refactor fetcher & git pull on update

59e6c62

fetchers/alpine: auto detect namespaces

3be8dfc

fetchers/alpine: add notes for untracked namespaces

f74cd35

jzelinskie merged commit d62bddd into quay:master Dec 19, 2016

jzelinskie deleted the alpine branch December 19, 2016 16:39

jzelinskie mentioned this pull request Dec 29, 2016

Add fully support for Alpine based containers #12

Closed

boydgreenfield unassigned Quentin-M Jan 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Alpine support via Alpine-SecDB #272

[WIP] Alpine support via Alpine-SecDB #272

jzelinskie commented Nov 18, 2016 •

edited

Quentin-M commented Nov 18, 2016 •

edited by jzelinskie

barakmich commented Nov 18, 2016 •

edited by jzelinskie

betawaffle commented Nov 18, 2016

Quentin-M Nov 18, 2016

Quentin-M Nov 18, 2016

Quentin-M Nov 19, 2016 •

edited

Quentin-M Nov 19, 2016 •

edited

Quentin-M Nov 19, 2016 •

edited

Quentin-M Nov 19, 2016

jzelinskie Nov 28, 2016

Quentin-M Nov 19, 2016 •

edited

mattmoor commented Nov 19, 2016

Quentin-M commented Nov 19, 2016 •

edited

jzelinskie commented Nov 19, 2016 •

edited

mattmoor commented Nov 19, 2016

jzelinskie commented Nov 28, 2016

mattmoor commented Nov 28, 2016

mwarkentin commented Nov 28, 2016

Quentin-M commented Nov 28, 2016 •

edited

mwarkentin commented Nov 28, 2016

jzelinskie commented Nov 29, 2016

mattmoor commented Nov 29, 2016

Quentin-M commented Dec 14, 2016

Quentin-M Dec 15, 2016

Quentin-M Dec 15, 2016

Quentin-M Dec 15, 2016

Quentin-M Dec 15, 2016

Quentin-M commented Dec 16, 2016 •

edited

Quentin-M Dec 16, 2016

Quentin-M commented Dec 16, 2016

adityacs commented May 31, 2017

boydgreenfield commented Jan 26, 2018

[WIP] Alpine support via Alpine-SecDB #272

[WIP] Alpine support via Alpine-SecDB #272

Conversation

jzelinskie commented Nov 18, 2016 • edited

Quentin-M commented Nov 18, 2016 • edited by jzelinskie

barakmich commented Nov 18, 2016 • edited by jzelinskie

betawaffle commented Nov 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Quentin-M Nov 19, 2016 • edited

Choose a reason for hiding this comment

Quentin-M Nov 19, 2016 • edited

Choose a reason for hiding this comment

Quentin-M Nov 19, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Quentin-M Nov 19, 2016 • edited

Choose a reason for hiding this comment

mattmoor commented Nov 19, 2016

Quentin-M commented Nov 19, 2016 • edited

jzelinskie commented Nov 19, 2016 • edited

mattmoor commented Nov 19, 2016

jzelinskie commented Nov 28, 2016

mattmoor commented Nov 28, 2016

mwarkentin commented Nov 28, 2016

Quentin-M commented Nov 28, 2016 • edited

mwarkentin commented Nov 28, 2016

jzelinskie commented Nov 29, 2016

mattmoor commented Nov 29, 2016

Quentin-M commented Dec 14, 2016

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

Quentin-M commented Dec 16, 2016 • edited

Choose a reason for hiding this comment

Quentin-M commented Dec 16, 2016

adityacs commented May 31, 2017

boydgreenfield commented Jan 26, 2018

jzelinskie commented Nov 18, 2016 •

edited

Quentin-M commented Nov 18, 2016 •

edited by jzelinskie

barakmich commented Nov 18, 2016 •

edited by jzelinskie

Quentin-M Nov 19, 2016 •

edited

Quentin-M Nov 19, 2016 •

edited

Quentin-M Nov 19, 2016 •

edited

Quentin-M Nov 19, 2016 •

edited

Quentin-M commented Nov 19, 2016 •

edited

jzelinskie commented Nov 19, 2016 •

edited

Quentin-M commented Nov 28, 2016 •

edited

Quentin-M commented Dec 16, 2016 •

edited