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

Moving to a fork for jwt-go with a CVE fix (transient dependency) #100401

Closed
PushkarJ opened this issue Mar 19, 2021 · 14 comments · Fixed by #102755
Closed

Moving to a fork for jwt-go with a CVE fix (transient dependency) #100401

PushkarJ opened this issue Mar 19, 2021 · 14 comments · Fixed by #102755
Labels
area/code-organization Issues or PRs related to kubernetes code organization kind/bug Categorizes issue or PR as related to a bug. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@PushkarJ
Copy link
Member

PushkarJ commented Mar 19, 2021

What happened:

github.com/dgrijalva/jwt-go v3.2.0 is known to have a CVE-2020-26160 with severity rating as high. This is a dependency for etcd and heketi which is a dependency for kubernetes. Although the vulnerable function verifyAudience is not used, because of this dependency chain, security scanners tag kubernetes releases as impacted by this CVE. To prevent this false positive alert, it would be a good idea to move a release and fork, that fixes this CVE

What you expected to happen:

Kubernetes to move to a forked release github.com/form3tech-oss/jwt-go/ that has a fix for this CVE

etcd version 3.5.0-alpha.0 also includes this fix.

A separate issue for heketi is opened for them to create a tagged release: heketi/heketi#1841

Related PR: #100382 I will re-open this PR with a fix once heketi and etcd both release a tagged version

How to reproduce it (as minimally and precisely as possible):

x@x kubernetes % go mod why github.com/dgrijalva/jwt-go
# github.com/dgrijalva/jwt-go
k8s.io/kubernetes/pkg/volume/glusterfs
github.com/heketi/heketi/client/api/go-client
github.com/dgrijalva/jwt-go
x@x kubernetes % go mod graph | grep github.com/dgrijalva/jwt-go
go.etcd.io/etcd@v0.5.0-alpha.5.0.20200910180754-dd1b699fc489 github.com/dgrijalva/jwt-go@v3.2.0+incompatible
github.com/spf13/viper@v1.7.0 github.com/dgrijalva/jwt-go@v3.2.0+incompatible

Anything else we need to know?:

Environment:

  • Kubernetes version (use kubectl version): <= 1.21
  • Cloud provider or hardware configuration: any
  • OS (e.g: cat /etc/os-release): n/a
  • Kernel (e.g. uname -a): n/a
  • Install tools: n/a
  • Network plugin and version (if this is a network-related bug): n/a
  • Others: n/a

/area dependency
/kind cleanup
/sig storage

@PushkarJ PushkarJ added the kind/bug Categorizes issue or PR as related to a bug. label Mar 19, 2021
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 19, 2021
@neolit123
Copy link
Member

/sig architecture
/area code-organization
for the dependency.

/sig api-machinery
for etcd.

@k8s-ci-robot k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. area/code-organization Issues or PRs related to kubernetes code organization sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 21, 2021
@fedebongio
Copy link
Contributor

/remove-sig api-machinery
/cc @mikedanese

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Mar 23, 2021
@mikedanese
Copy link
Member

It looks like the project is abandoned.

dgrijalva/jwt-go#428

@kubernetes/dep-approvers what do we do in these scenarios? We are using go-jose for core jwt needs but as long as this library remains in vendor/, there's a risk that usage will sneak in.

@dims
Copy link
Member

dims commented Mar 25, 2021

@mikedanese looks like etcd already moved, and heketi has merged an update to the newer repo, so we can wait a bit for both etcd and heketi releases to come out and then we will be able to remove the old repo.

Also, could we persuade etcd to switch to go-jose?

@navidshaikh
Copy link
Member

Ran snyk on master branch and it lists the CVE as mentioned in the issue

➜  kubernetes git:(master) snyk test
[…]

✗ High severity vulnerability found in github.com/satori/go.uuid
  Description: Insecure Randomness
  Info: https://snyk.io/vuln/SNYK-GOLANG-GITHUBCOMSATORIGOUUID-72488
  Introduced through: k8s.io/legacy-cloud-providers/azure@unknown
  From: k8s.io/legacy-cloud-providers/azure@unknown > github.com/Azure/azure-sdk-for-go/storage@unknown > github.com/satori/go.uuid@1.2.0

✗ High severity vulnerability found in github.com/dgrijalva/jwt-go
  Description: Access Restriction Bypass
  Info: https://snyk.io/vuln/SNYK-GOLANG-GITHUBCOMDGRIJALVAJWTGO-596515
  Introduced through: github.com/heketi/heketi/client/api/go-client@unknown, k8s.io/apiserver/pkg/storage/etcd3/testing@unknown
  From: github.com/heketi/heketi/client/api/go-client@unknown > github.com/dgrijalva/jwt-go@3.2.0
  From: k8s.io/apiserver/pkg/storage/etcd3/testing@unknown > go.etcd.io/etcd/integration@unknown > go.etcd.io/etcd/etcdserver/api/v3rpc@unknown > go.etcd.io/etcd/mvcc@unknown > go.etcd.io/etcd/auth@unknown > github.com/dgrijalva/jwt-go@3.2.0
  Fixed in: 4.0.0-preview1

Seeing another CWE-338 for github.com/satori/go.uuid do we care about CWE too?

@PushkarJ
Copy link
Member Author

Thanks @dims, 100% agree @mikedanese with regards to getting this moving: I have pinged heketi maintainers again to remind to bump a new minor version release that points to the fixed fork. etcd is still on alpha release that has the fix (3.5.0 alpha).

@navidshaikh that's a good finding. Was not aware of the other vulnerable pkg. Maybe worth opening a separate PR about that too? Feel free to correct me if it makes sense to have a single PR that fixes both.

@mikedanese
Copy link
Member

Looks like Azure SDK issue is fixed in Azure/azure-sdk-for-go#14283 and should be resolved with a dependency bump.

@mikedanese
Copy link
Member

@PushkarJ do you have a heketi issue we can reference here?

@mikedanese mikedanese added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Mar 30, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Mar 30, 2021
@PushkarJ
Copy link
Member Author

PushkarJ commented Mar 31, 2021

@PushkarJ do you have a heketi issue we can reference here?

Here you go:
heketi/heketi#1841

@navidshaikh
Copy link
Member

Maybe worth opening a separate PR about that too? Feel free to correct me if it makes sense to have a single PR that fixes both.

@PushkarJ : It was a transient dependency and fixed where it came from in Azure/azure-sdk-for-go#14283, next dep bump should fix it.

go mod why github.com/satori/go.uuid
# github.com/satori/go.uuid
k8s.io/kubernetes/cmd/cloud-controller-manager
k8s.io/legacy-cloud-providers/azure
github.com/Azure/azure-sdk-for-go/storage
github.com/satori/go.uuid

@PushkarJ
Copy link
Member Author

PushkarJ commented Apr 7, 2021

Update on this issue:

  1. heketi maintainers released a new version with the fix https://github.com/heketi/heketi/releases/tag/v10.3.0
  2. etcd fix is in v3.5.0 but still in alpha stage https://github.com/etcd-io/etcd/releases/tag/v3.5.0-alpha.0
  3. azure fix is in place as @navidshaikh mentioned: https://github.com/Azure/azure-sdk-for-go/releases/tag/v53.0.0

Have commits that fixes 1 and 3 in my fork already. Probably worth the wait for opening a PR until etcd release a GA for v3.5.0 ?

@dims
Copy link
Member

dims commented Apr 7, 2021

@PushkarJ yes! thanks.

@PushkarJ
Copy link
Member Author

PushkarJ commented Apr 8, 2021

To keep everyone in the loop, etcd fixes (pt. 2 above) should happen as part of the upgrade to etcd 3.5.x through this PR by @liggitt : #100488 . So the sequence of steps planned are:

  1. etcd 3.5.x is GA
  2. Get PR update etcd, grpc, protobuf dependencies #100488 merged
  3. New PR from me is created to ensure jwt-go transient dependency is removed by bumping heketi and azure-sdk-for-go
  4. The new PR is then merged

@PushkarJ
Copy link
Member Author

PushkarJ commented May 27, 2021

Related: spf13/viper#997 and spf13/viper#1126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code-organization Issues or PRs related to kubernetes code organization kind/bug Categorizes issue or PR as related to a bug. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants