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

Changed the package dep version to fix security vulternability #2844

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

nikita15p
Copy link
Contributor

@nikita15p nikita15p commented Sep 28, 2021

Summary

The github.com/golang-jwt/jwt/v4 package is the active package with fix of vulnerability.

Full changelog

With this change, secure version of jwt-go package is used

Issues resolved

Fix #XXX

Documentation

NA

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

  • Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

@nikita15p nikita15p requested a review from a team as a code owner September 28, 2021 12:59
@nikita15p nikita15p changed the title Changed the package dep version due to security vulternability fix in latest bersion Changed the package dep version to fix security vulternability Sep 28, 2021
Signed-off-by: nikita15p <nikita15p@gmail.com>
@mmorel-35
Copy link
Contributor

mmorel-35 commented Sep 28, 2021

Hi! Isn't it advised to move from github.com/dgrijalva/jwt-go to github.com/golang-jwt/jwt/v4 instead ?

@jpeach
Copy link
Contributor

jpeach commented Sep 28, 2021

Hi! Isn't it advised to move from github.com/dgrijalva/jwt-go to github.com/golang-jwt/jwt/v4 instead ?

Yeh, if this repo has vulnerabilities, best to switch to something that is maintained.

@nikita15p
Copy link
Contributor Author

@mmorel-35 @jpeach agreed and made changes to this PR. Thanks !

@mmorel-35
Copy link
Contributor

It's seems like tests are failing. And did you go mod tidy? I don't see the deletion of the old library in the go.sum

Signed-off-by: nikita15p <nikita15p@gmail.com>
@nikita15p
Copy link
Contributor Author

And did you go mod tidy? I don't see the deletion of the old library in the go.sum

Yes I did. (Its there in make check also). But it will remain there since some dependencies which we use still use that package. For eg: github.com/spiffe/spire@v0.12.3
go: github.com/spiffe/spire@v0.12.3 requires
github.com/dgrijalva/jwt-go@v3.2.0+incompatible: missing go.sum entry; to add it:
go mod download github.com/dgrijalva/jwt-go
make: *** [mk/build.mk:72: build/kuma-cp] Error 1

@mmorel-35
Copy link
Contributor

mmorel-35 commented Sep 29, 2021

Alright! I see 😊!

@codecov-commenter
Copy link

Codecov Report

Merging #2844 (69ac093) into master (47b2cbd) will increase coverage by 0.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2844      +/-   ##
==========================================
+ Coverage   52.34%   52.35%   +0.01%     
==========================================
  Files         888      888              
  Lines       51798    51798              
==========================================
+ Hits        27113    27120       +7     
+ Misses      22534    22524      -10     
- Partials     2151     2154       +3     
Impacted Files Coverage Δ
app/kuma-dp/pkg/config/validate.go 77.77% <ø> (ø)
pkg/tokens/builtin/zoneingress/issuer.go 5.55% <0.00%> (ø)
pkg/tokens/builtin/issuer/issuer.go 82.22% <100.00%> (ø)
pkg/core/resources/manager/cache.go 81.81% <0.00%> (-2.60%) ⬇️
pkg/core/bootstrap/autoconfig.go 54.46% <0.00%> (+8.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47b2cbd...69ac093. Read the comment docs.

@jpeach jpeach self-assigned this Sep 30, 2021
@jpeach jpeach self-requested a review September 30, 2021 06:59
@jpeach jpeach removed their assignment Sep 30, 2021
@jakubdyszkiewicz jakubdyszkiewicz merged commit 1d52756 into kumahq:master Sep 30, 2021
mergify bot pushed a commit that referenced this pull request Sep 30, 2021
Signed-off-by: nikita15p <nikita15p@gmail.com>
(cherry picked from commit 1d52756)
jakubdyszkiewicz pushed a commit that referenced this pull request Oct 4, 2021
Signed-off-by: nikita15p <nikita15p@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants