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

proposal: standardize on marking versions as vulnerable #29537

Closed
Zemnmez opened this issue Jan 3, 2019 · 15 comments
Closed

proposal: standardize on marking versions as vulnerable #29537

Zemnmez opened this issue Jan 3, 2019 · 15 comments

Comments

@Zemnmez
Copy link

Zemnmez commented Jan 3, 2019

Abstract

Go modules have been extremely useful for dependency analysis. We need a way of marking certain revisions as vulnerable so that they can be picked up by tooling and patched appropriately. In other languages, this is something that is done at worst by parsing natural language CVE disclosures. I hope to do better than this :)

My proposal is to introduce two git tag forms, which look like: vulnerability CVE-2018-0001 in v1.0.1 for findings and fix vulnerability CVE-2018-0001 in v1.0.2 for fixes. Packages that do not have versions work via go.mod versions: vulnerability CVE-2018-0001 in v0.0.0-20180517173623-c85619274f5d.

I think this would be a huge boon to Go security overall. I expect my proposal won't be perfect and I welcome feedback.

Proposal

See also: go mod version definition.
NB: as go.mod versions do not require annotated semver version tags, this system also works with repos that only have git refs.

tag: vulnerability [identifier] in [go mod version]

Used to mark a go mod version as having some security bug. The identifier is expected to be unique -- it could, for example be a CVE ID or a JIRA ticket ID.

The tag is expected to be on the first ref known to be vulnerable.

tag: fix vulnerability [identifier] in [go mod version]

Used to mark a go mod version as not vulnerable to a security bug. The unique identifier is said to be fixed.

The tag is expected to be on the first ref known to introduce a fix.

Determining if a version is vulnerable

  1. Enumerate all fix vulnerability tags that are before the go mod version, collect their identifiers.
  2. Enumerate all vulnerability tags that are before the go mod version and whose identifiers are not in the fix vulnerability set.
  3. If the resulting set is not empty, it is the set of vulnerabilities affecting this version.

EBNF sketch

space = " " | "-"

nonspacing = ? unicode categories: Letter, Number, Mark, Punctuation, Symbol ?

identifier = { nonspacing }

vulnerability_proposal_tag = vulnerability_tag | vulnerability_fix_tag

vulnerability_tag = "vulnerability", space, identifier, space, "in", space, go_mod_version
vulnerability_fix_tag = "fix vulnerability" space, identifier, space, "in", space, go_mod_version

Other ideas I'd like feedback on

  • using tag annotations instead of whole tags, as per @v3n's comment. This potentially means old tags can be 'marked' as vulnerable.
  • optional [HIGH] [LOW] etc CVSS score annotations?
@gopherbot gopherbot added this to the Proposal milestone Jan 3, 2019
@myitcv
Copy link
Member

myitcv commented Jan 3, 2019

I think this conversation is probably best added to #24031?

@Zemnmez
Copy link
Author

Zemnmez commented Jan 3, 2019

I think #24031 is a subset of the problems this proposal is an attempt to solve

@v3n
Copy link

v3n commented Jan 3, 2019

I would love to see this, it's really hard to tell what is vulnerable where. My only qualm is it requires opt-in from the repository maintainers and I'm honestly not sure how many would do that. I'd love to see some system where CVE's can be centrally referenced to identify vulns in a similiar system.

EDIT: after thinking, it may make sense to put this metadata instead an annotated tag for the release instead of using a whole tag. This would allow do you do something in release 1, say:

git tag -a v1.0.0 -m "gomod vuln CVE-XXXXXX"

And follow up later with something like:

go tag -a v1.0.1 -m "gomod fixes CVE-XXXXXX

@Zemnmez
Copy link
Author

Zemnmez commented Jan 3, 2019

Damn. That's an incredible idea

@Zemnmez
Copy link
Author

Zemnmez commented Jan 3, 2019

OK. I've looked into it and while tag annotations are great in that they can be GPG signed and have authorship etc, one can't append an annotation to an existing tag. It'd frankly be incredible if a go user could append a mark saying that the tag is vulnerable to something, but I don't think this concept of accumulating data in a single tag is well supported. I'll add it to the other options for feedback. Maybe someone has a workaround

@bitfield
Copy link

bitfield commented Jan 5, 2019

Yes, you're more likely to get traction by adding your suggestion to #24031 rather than opening a duplicate issue here.

@bcmills
Copy link
Contributor

bcmills commented Jan 8, 2019

Anything that relies on git tags alone seems like a non-starter. Whatever the solution is, it must also work with the module proxy protocol.

@bcmills bcmills added the modules label Jan 8, 2019
@Zemnmez
Copy link
Author

Zemnmez commented Jan 8, 2019

Anything that relies on git tags alone seems like a non-starter.

why?

Whatever the solution is, it must also work with the module proxy protocol.

this would be metadata under the module proxy protocol. as it says in the docs, the metadata is intended to be expanded

@bcmills
Copy link
Contributor

bcmills commented Jan 9, 2019

Anything that relies on git tags alone seems like a non-starter.

why?

Not everyone distributes through git. (The Go command itself supports several different VCS systems plus the module proxy protocol.)

@Zemnmez
Copy link
Author

Zemnmez commented Jan 9, 2019

I still don't understand. As we both know, the Go modules system is the accepted way forward for Go package versioning. My proposal is virtually identical in implementation to the versioning system the Go modules system uses, which also relies on tagging to attach metadata. If these tags were not portable, we could not be having this conversation about compatibility in the first place!

@bcmills
Copy link
Contributor

bcmills commented Jan 10, 2019

The only current tag syntax we require tools to support is the semver.org syntax with the v letter as a prefix. This proposal would expand that: for example, do all VCS systems support tags that contain spaces?

@bcmills
Copy link
Contributor

bcmills commented Jan 10, 2019

At any rate, I think this discussion is getting too fragmented. Let's consolidate into #24031.

@bcmills bcmills closed this as completed Jan 10, 2019
@Zemnmez
Copy link
Author

Zemnmez commented Jan 10, 2019

here's the list of supported VCS:

// vcsList lists the known version control systems
var vcsList = []*Cmd{
	vcsHg,
	vcsGit,
	vcsSvn,
	vcsBzr,
}

Git, as we know supports these.

Mercurial:

A tag is a symbolic identifier for a changeset. It can contain any characters except ":" (colon), "\r" (Carriage Return) or "\n" (Line Feed). Mercurial has two kinds of tags: local and regular.

SVN uses a tag including spaces as the canon example of creating tags:

$ svn copy http://svn.example.com/repos/calc/trunk \
           http://svn.example.com/repos/calc/tags/release-1.0 \
           -m "Tagging the 1.0 release of the 'calc' project."

Committed revision 902.

However, bazaar doesn't support spaces in tags:

Tag names are non-whitespace Unicode strings starting with a letter. It's recommended that the tag name start with a project identifier: for example, bzr-release-0.9 or hp-ijs-1.2.12.

I propose, then that dashes, '-' instead of spaces may be used. This falls in line with how bazaar separates spaces in their examples. I'll alter the proposal

@Zemnmez
Copy link
Author

Zemnmez commented Jan 10, 2019

OK. I'll put my proposal there.

@Zemnmez
Copy link
Author

Zemnmez commented Jan 10, 2019

this comment now supersedes this proposal

@golang golang locked and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants