Reintroduce image scanning for openSUSE and SLE #506

flavio · 2018-01-09T20:42:21Z

As promised inside of #503... handle scanning of openSUSE and SUSE Linux Enterprise images.

The unit tests are passing but I wasn't able to test the code against real images because neither clair-scanner nor reg work against master's new API. Suggestions about how to test the changes are welcome 😃

jzelinskie · 2018-01-11T20:02:37Z

Thank you so much for reworking all of these changes! I'm really sorry for the slow response.

We don't really have an e2e test framework, and the API changed this release. You can do manual testing by simply using cURL against the API server. It handles both gRPC and JSON over REST.

I'd also like to see some changes to the documentation to include the licensing information for the data in the openSUSE database.

jzelinskie

This mostly looks good since it's a lot of copypasta from the the RHEL vulnsrc. Ideally after this is merged, we can refactor some of the OVAL parsing into a library decoupled from the particular distribution.

jzelinskie · 2018-07-07T17:37:41Z

ext/vulnsrc/suse/suse.go

+
+	log "github.com/sirupsen/logrus"
+
+	"fmt"


this should be grouped in the first section of imports

jzelinskie · 2018-07-07T17:38:56Z

ext/vulnsrc/suse/suse.go

+		up.UpdaterFlag = "openSUSEUpdater"
+		up.FileRegexp = regexp.MustCompile(`opensuse.leap.(\d+\.*\d*).xml`)
+	default:
+		panic("Unrecognized flavor")


panic("tried to create an updater for an unrecognized flavor of suse")

jzelinskie · 2018-07-07T17:44:05Z

ext/vulnsrc/suse/suse.go

+	if err != nil {
+		return resp, err
+	}
+	log.WithField("flagvalue", flagValue)


This probably needs to be something like Debug level if it stays at all.

jzelinskie · 2018-07-07T17:45:00Z

ext/vulnsrc/suse/suse.go

+
+	// this contains the modification time of the most recent
+	// file expressed as unix time (int64)
+	latestOval, _ := strconv.ParseInt(flagValue, 10, 64)


Why ignore this error?

jzelinskie · 2018-07-07T17:49:05Z

ext/vulnsrc/suse/suse.go

+			log.Fields{
+				"ovalFile": ovalFile,
+				"updater":  u.Name,
+			}).Debug("file to check")


log.WithFields(log.Fields{ "ovalFile": ovalFile, "updater": u.Name, }).Debug("file to check")

jzelinskie · 2018-07-07T18:34:01Z

ext/vulnsrc/suse/suse.go

+	}
+
+	// timestamp format 2017-10-23T04:07:14
+	layout := "2006-1-2T15:04:05"


pull this out into a constant timeFormatOVAL

jzelinskie · 2018-07-09T15:18:13Z

ext/vulnsrc/suse/suse.go

+				Vulnerability: database.Vulnerability{
+					Name: name(definition),
+					Link: link(definition),
+					//TODO: handle that once openSUSE/SLE OVAL files have severity info


pull this out into a function severity(definition) that doesn't do anything but return UnknownSeverity, and has this comment.

jzelinskie · 2018-07-09T15:24:52Z

ext/vulnsrc/suse/suse.go

+		// Attempt to parse package data from trees of criterions.
+		for _, c := range criterions {
+			if match := suseInstalledCommentRegexp.FindStringSubmatch(c.Comment); match != nil {
+				if len(match) != 4 {


it's easier to read this conditional if you keep it if (positive case) else negative case.

if len(match) == 4 { osVerison = ... } else { log.WithField().Warning() }

jzelinskie · 2018-07-09T15:34:19Z

ext/vulnsrc/suse/suse.go

+	re := regexp.MustCompile(`-\d+\.`)
+
+	matches := re.FindStringSubmatchIndex(fullname)
+


get rid of this new line

jzelinskie · 2018-07-09T15:36:51Z

ext/vulnsrc/suse/suse_test.go

+		"libfreerdp2-2.0.0~git.1463131968.4e66df7-11.69 is installed": []string{"libfreerdp2", "2.0.0~git.1463131968.4e66df7-11.69"},
+		"ruby2.1-rubygem-sle2docker-0.2.3-5.1 is installed":           []string{"ruby2.1-rubygem-sle2docker", "0.2.3-5.1"},
+		"xen-libs-4.4.1_06-2.2 is installed":                          []string{"xen-libs", "4.4.1_06-2.2"},
+		"runc-0.1.1+gitr2816_02f8fa7 is installed":                    []string{"runc", "0.1.1+gitr2816_02f8fa7"},


add some tests for the failure scenario

flavio · 2018-09-25T19:26:31Z

@jzelinskie sorry for the late response... I've implemented all the changes you requested. Once you are fine with the changes I'll rebase this branch against master and squash all the commits into a single one.

❤️

jzelinskie

couple of tiny nitpicks, but feel free to rebase now

jzelinskie · 2018-11-14T18:42:39Z

ext/vulnsrc/suse/suse.go

+	// Fetch the update list.
+	r, err := http.Get(ovalURI)
+	if err != nil {
+		log.Fatal(err)


This shouldn't be fatal.

jzelinskie · 2018-11-14T18:47:02Z

ext/vulnsrc/suse/suse.go

+		// Collect vulnerabilities.
+		for _, v := range vs {
+			resp.Vulnerabilities = append(resp.Vulnerabilities, v)
+		}


This for-loop can be done in one line:

resp.Vulnerabilities = append(resp.Vulnerabilities, vs...)

jzelinskie · 2018-11-14T18:54:41Z

ext/vulnsrc/suse/suse.go

+			}
+			for _, p := range pkgs {
+				vulnerability.Affected = append(vulnerability.Affected, p)
+			}


This for-loop can be done in one line:

vulnerability.Affected = append(vulnerability.Affected, pkgs...)

jzelinskie · 2018-11-14T19:07:38Z

ext/vulnsrc/suse/suse.go

+			}
+
+			if suseOpenSUSEInstalledCommentRegexp.FindStringSubmatch(c.Comment) == nil && strings.HasSuffix(c.Comment, " is installed") {
+				name, version, err := splitPackageNameAndVersion(c.Comment[:len(c.Comment)-13])


In order to make it obvious where 13 is coming from: len(c.Comment)-len(" is installed")

flavio · 2018-11-15T09:57:17Z

@jzelinskie I've update the code with the changes you proposed. Take a look at the changes I made and gimme the final green light for the rebase.

jzelinskie · 2018-11-19T16:36:44Z

I don't want this to hang any longer. Rebase away!

flavio · 2018-11-20T07:21:08Z

I don't want this to hang any longer. Rebase away!

@jzelinskie done!

flavio · 2018-11-22T16:13:50Z

I did another forced push to address the style check errors reported by goftm.

Payback159 · 2018-12-17T12:39:57Z

Hi Guys, any forecast when this feature will be available?

jzelinskie · 2019-01-04T19:18:35Z

Sorry for the silence around the holidays. This code all looks fine. I think all this PR needs is to add OpenSUSE to the documentation in a few places. Most importantly here: https://github.com/coreos/clair/blob/master/Documentation/drivers-and-data-sources.md#data-sources-for-the-built-in-drivers

Handle scanning of openSUSE and SUSE Linux Enterprise images. Signed-off-by: Flavio Castelli <fcastelli@suse.com>

Expand the documentation about the available data sources to mention openSUSE and SLE. Signed-off-by: Flavio Castelli <fcastelli@suse.com>

flavio · 2019-01-07T17:50:09Z

@jzelinskie I did a rebase against master and then added a new commit which extends the documentation. Thanks for pointing that out. AFAIK this is the only place of the documentation that needs to be extended.

jzelinskie

Yay! Let's get this ball rolling!

jzelinskie self-requested a review January 11, 2018 19:55

jzelinskie added component/ext/vulnsrc labels Jan 11, 2018

flavio mentioned this pull request Mar 5, 2018

Add openSUSE/SLES support #199

Merged

4 tasks

jzelinskie suggested changes Jul 9, 2018

View reviewed changes

jzelinskie added the reviewed/needs rework will be closed if review not addressed label Jul 9, 2018

jzelinskie added lacking/review needs to be reviewed by a maintainer and removed reviewed/needs rework will be closed if review not addressed labels Sep 26, 2018

jzelinskie suggested changes Nov 14, 2018

View reviewed changes

flavio force-pushed the reintroduce-suse-opensuse branch from bcf33b3 to fd554e6 Compare November 20, 2018 07:20

flavio force-pushed the reintroduce-suse-opensuse branch from fd554e6 to 47c6299 Compare November 22, 2018 16:13

flavio added 2 commits January 7, 2019 18:48

Reintroduce image scanning for openSUSE and SLE

5a4d491

Handle scanning of openSUSE and SUSE Linux Enterprise images. Signed-off-by: Flavio Castelli <fcastelli@suse.com>

Update documentation: talk about SUSE support

1105102

Expand the documentation about the available data sources to mention openSUSE and SLE. Signed-off-by: Flavio Castelli <fcastelli@suse.com>

flavio force-pushed the reintroduce-suse-opensuse branch from 47c6299 to 1105102 Compare January 7, 2019 17:49

jzelinskie approved these changes Jan 7, 2019

View reviewed changes

jzelinskie merged commit b08ad9b into quay:master Jan 7, 2019

flavio deleted the reintroduce-suse-opensuse branch January 8, 2019 07:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reintroduce image scanning for openSUSE and SLE #506

Reintroduce image scanning for openSUSE and SLE #506

flavio commented Jan 9, 2018

jzelinskie commented Jan 11, 2018

jzelinskie left a comment

jzelinskie Jul 7, 2018

jzelinskie Jul 7, 2018

jzelinskie Jul 7, 2018

jzelinskie Jul 7, 2018

jzelinskie Jul 7, 2018

jzelinskie Jul 7, 2018

jzelinskie Jul 9, 2018

jzelinskie Jul 9, 2018

jzelinskie Jul 9, 2018

jzelinskie Jul 9, 2018

flavio commented Sep 25, 2018

jzelinskie left a comment

jzelinskie Nov 14, 2018

jzelinskie Nov 14, 2018

jzelinskie Nov 14, 2018

jzelinskie Nov 14, 2018

flavio commented Nov 15, 2018

jzelinskie commented Nov 19, 2018

flavio commented Nov 20, 2018

flavio commented Nov 22, 2018

Payback159 commented Dec 17, 2018

jzelinskie commented Jan 4, 2019

flavio commented Jan 7, 2019

jzelinskie left a comment

		re := regexp.MustCompile(`-\d+\.`)

		matches := re.FindStringSubmatchIndex(fullname)

Reintroduce image scanning for openSUSE and SLE #506

Reintroduce image scanning for openSUSE and SLE #506

Conversation

flavio commented Jan 9, 2018

jzelinskie commented Jan 11, 2018

jzelinskie left a 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

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

flavio commented Sep 25, 2018

jzelinskie left a 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

flavio commented Nov 15, 2018

jzelinskie commented Nov 19, 2018

flavio commented Nov 20, 2018

flavio commented Nov 22, 2018

Payback159 commented Dec 17, 2018

jzelinskie commented Jan 4, 2019

flavio commented Jan 7, 2019

jzelinskie left a comment

Choose a reason for hiding this comment