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.

Sign up for GitHub

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

Jump to bottom

Bump go-yaml version to cover fixed ddos heuristic #2821

Merged
merged 1 commit into from Nov 1, 2019
Merged

Bump go-yaml version to cover fixed ddos heuristic #2821

merged 1 commit into from Nov 1, 2019

Conversation

petrkotas
Copy link
Contributor

@petrkotas petrkotas commented Oct 17, 2019 •
edited

What this PR does / why we need it:

This PR bumbs go-yaml to v2.2.4, which has the ddos vulnerability fixed.

Issue:
go-yaml preceding 2.2.4 had vulnerability to ddos attack via billion laughs bomb.
Such attack lead to program to be unresponsive.
Issue has been described in
https://raesene.github.io/blog/2019/10/15/From-stackoverflow-to-CVE/

Moreover dependencies in our vendor have update in progress:

Signed-off-by: Petr Kotas petr.kotas@gmail.com

Release note:

NONE

@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/M labels Oct 17, 2019
@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 17, 2019
@petrkotas
Copy link
Contributor Author

/cc @rmohr

@petrkotas petrkotas force-pushed the go-yaml branch 2 times, most recently from 628dd5a to 2e7e7bc Compare October 17, 2019 12:57
@rmohr
Copy link
Member

rmohr commented Oct 21, 2019

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2019
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rmohr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 21, 2019
@rmohr
Copy link
Member

rmohr commented Oct 21, 2019

/hold

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 21, 2019
@rmohr
Copy link
Member

rmohr commented Oct 21, 2019

@petrkotas @slintes I wonder if we should also bump staging/src/kubevirt.io/client-go/go.mod, since that one is exported independently of the main go.mod.

@petrkotas
Copy link
Contributor Author

petrkotas commented Oct 21, 2019 •
edited

Hi @rmohr we can do this separately, since it will require working on the dependencies. The yaml.v2 is used indirectly via dependencies and these have to be updated.

Or we can try to go with replace, but I am not sure how well it will work.

WDYT?

@slintes
Copy link
Contributor

slintes commented Oct 22, 2019

Hi @rmohr we can do this separately, since it will require working on the dependencies. The yaml.v2 is used indirectly via dependencies and these have to be updated.

Or we can try to go with replace, but I am not sure how well it will work.

WDYT?

I'd vote for replace in client-go. kubevirt itself is fine without it.

msluiter@slintes 🎩︎  ~/dev/work/kubevirt/kubevirt ‹pull/yaml›  ‹kubernetes-admin@kubernetes/default›
$ go list -m all | grep yaml.v2
go: finding kubevirt.io/client-go v0.0.0-00010101000000-000000000000
gopkg.in/yaml.v2 v2.2.4

msluiter@slintes 🎩︎  ~/dev/work/kubevirt/kubevirt/staging/src/kubevirt.io/client-go ‹pull/yaml›  ‹kubernetes-admin@kubernetes/default›
$ go list -m all | grep yaml.v2
gopkg.in/yaml.v2 v2.2.2

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2019
@petrkotas
Copy link
Contributor Author

Hi @slintes I have provided the replace. However it seems it still keeps some older dependencies in the go.sum, I am guessing it will resolve with never version of the dependent libraries.

@slintes
Copy link
Contributor

slintes commented Oct 24, 2019

AFAIK go.sum is supposed to keep old entries.

But this looks good I'd say:

$ go list -m all | grep yaml.v2
gopkg.in/yaml.v2 v2.2.2 => gopkg.in/yaml.v2 v2.2.4

Thanks!

/lgtm

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 24, 2019
@petrkotas
Bump go-yaml version to cover fixed ddos heuristic
40a9bc6 
This PR bumbs go-yaml to v2.2.4, which has the ddos vulnerability fixed.

Issue:
go-yaml preceding 2.2.4 had vulnerability to ddos attack via billion
laughs bomb.
Such attack lead to program to be unresponsive.
Issue has been described in
https://raesene.github.io/blog/2019/10/15/From-stackoverflow-to-CVE/

Signed-off-by: Petr Kotas <petr.kotas@gmail.com>
@kubevirt-bot kubevirt-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 29, 2019
@petrkotas
Copy link
Contributor Author

@slintes Thanks for the review, I have rebased the code.

@slintes
Copy link
Contributor

slintes commented Oct 29, 2019

/lgtm

and we forgot:

/hold cancel

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Oct 29, 2019
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@petrkotas
Copy link
Contributor Author

/test pull-kubevirt-e2e-os-3.11.0-crio

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

7 similar comments
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@petrkotas
Copy link
Contributor Author

/test pull-kubevirt-e2e-os-3.11.0-crio

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-bot kubevirt-bot merged commit b903412 into kubevirt:master Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfranczy mfranczy Awaiting requested review from mfranczy

@phoracek phoracek Awaiting requested review from phoracek

@rmohr rmohr Awaiting requested review from rmohr

Assignees

@slintes slintes

@rmohr rmohr

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@petrkotas @rmohr @kubevirt-bot @slintes @kubevirt-commenter-bot