Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guess go main module version based on binary contents #2608

Merged
merged 7 commits into from
Feb 9, 2024

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Feb 8, 2024

Today the go main module version cannot be determined in all cases. We have a few heuristics in place to try and guess what the main module version is (by looking at ldflags and build setting info), however, there are still cases that are missed:

# as it is today...
❯ syft -q registry.k8s.io/ingress-nginx/controller:v1.6.4 | grep k8s.io/ingress-nginx
k8s.io/ingress-nginx                              (devel)                               go-module

This PR adds an additional heuristic to look at the binary contents and use a simple semver regex pattern matcher like we do with the binary cataloger to find matching version strings:

# with the code change...
❯ go run ./cmd/syft -q registry.k8s.io/ingress-nginx/controller:v1.6.4 | grep k8s.io/ingress-nginx
k8s.io/ingress-nginx                              (devel)                               go-module
k8s.io/ingress-nginx                              v1.6.4                                go-module

You'll note that it seems like it only "kind of" worked -- now we have two k8s.io/ingress-nginx packages, one that is correct and another that still shows devel. In this particular case, this is the best we can do. Let's look at what was found in detail:

# with the code change...
❯ go run ./cmd/syft -q registry.k8s.io/ingress-nginx/controller:v1.6.4 -o json | jq '.artifacts[] | select(.name == "k8s.io/ingress-nginx") | {name: .name, version: .version, locations: .locations}'
{
  "name": "k8s.io/ingress-nginx",
  "version": "(devel)",
  "locations": [
    {
      "path": "/dbg",
      "layerID": "sha256:fcc6781a7edaf8fa9971199d0c361dd3a0bebd6d76805a2d3cd41ff5b477a380",
      "accessPath": "/dbg",
      "annotations": {
        "evidence": "primary"
      }
    }
  ]
}
{
  "name": "k8s.io/ingress-nginx",
  "version": "(devel)",
  "locations": [
    {
      "path": "/wait-shutdown",
      "layerID": "sha256:ac97db116c0d5573f76be38bfa782acf196ec1ed83ba753546479a4732f8e5cc",
      "accessPath": "/wait-shutdown",
      "annotations": {
        "evidence": "primary"
      }
    }
  ]
}
{
  "name": "k8s.io/ingress-nginx",
  "version": "v1.6.4",
  "locations": [
    {
      "path": "/nginx-ingress-controller",
      "layerID": "sha256:56b71709a8bf999f9a65d21e58cad1be7a521c0718981b98a0032b38eb9c5338",
      "accessPath": "/nginx-ingress-controller",
      "annotations": {
        "evidence": "primary"
      }
    }
  ]
}

There are three different binaries that have the same k8s.io/ingress-nginx package: /nginx-ingress-controller, /wait-shutdown, /dbg.

It seems that only the main /nginx-ingress-controller binary has any indication of a version baked into the binary:

❯ strings nginx-ingress-controller | grep '1\.6\.4'
v1.6.4
❯ strings dbg | grep '1\.6\.4'
❯ strings wait-shutdown | grep '1\.6\.4'

In the future we might be able to correlate claimed package h1 information across all main packages discovered to try and get a better answer for all packages, but that is out of scope for this particular enhancement (and may be questionable to do , thus requires some discussion first). -- nevermind, I forgot there is no h1 digest for the main module

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
@wagoodman wagoodman added the enhancement New feature or request label Feb 8, 2024
@wagoodman
Copy link
Contributor Author

Question for reviewers: since this has the potential to raise up an incorrect version on a package, should this at least have a configuration option that a user can opt out of this heuristic?

@westonsteimel
Copy link
Contributor

A config option seems like a good idea to me; however should it also apply or have subcomponents to also enable/disable the parsing of version from ldflags etc since that also has the potential to be incorrect?

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
@wagoodman
Copy link
Contributor Author

I've added configuration options for all version guessing heuristics since they can all really have downsides. They all default to enabled.

@@ -37,15 +38,16 @@ var (
// inject the correct version into the main module of the build process

knownBuildFlagPatterns = []*regexp.Regexp{
regexp.MustCompile(`(?m)\.([gG]it)?([bB]uild)?[vV]ersion=(\S+/)*(?P<version>v?\d+.\d+.\d+[-\w]*)`),
regexp.MustCompile(`(?m)\.([gG]it)?([bB]uild)?[vV]er(sion)?=(\S+/)*(?P<version>v?\d+.\d+.\d+[-\w]*)`),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adjusts the pattern to be more permissive based on examples in the field. This allows for us to depend on the (probably more dependable) LD flags instead of the binary contents pattern matching in more cases.

@wagoodman wagoodman requested a review from a team February 8, 2024 17:50
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
@wagoodman wagoodman enabled auto-merge (squash) February 9, 2024 19:46
@wagoodman wagoodman merged commit 84576b9 into main Feb 9, 2024
11 checks passed
@wagoodman wagoodman deleted the get-go-devel-versin branch February 9, 2024 19:52
disc pushed a commit to disc/syft that referenced this pull request Feb 11, 2024
* guess go main module version based on binary contents

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* add configuration options for golang main module version heuristics

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* fix test setup for go bin cataloger

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* fix unit test

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* fix incorrect test assert ordering

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* handle error from seek

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
* guess go main module version based on binary contents

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* add configuration options for golang main module version heuristics

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* fix test setup for go bin cataloger

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* fix unit test

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* fix incorrect test assert ordering

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* handle error from seek

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants