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

Replace github.com/dgrijalva/jwt-go with github.com/golang-jwt/jwt #13378

Merged
merged 2 commits into from
Oct 4, 2021

Conversation

ysksuzuki
Copy link

@ysksuzuki ysksuzuki commented Oct 1, 2021

This PR is related to #13254.

etcd v3.4 still depends on github.com/dgrijalva/jwt-go which has CVE GHSA-w73w-5m7g-f7qc and is already archived. etcd v3.4 should use a community maintained fork github.com/golang-jwt/jwt which provides the fixed version of the CVE.

Signed-off-by: Yusuke Suzuki yusuke-suzuki@cybozu.co.jp

@ysksuzuki
Copy link
Author

ysksuzuki commented Oct 1, 2021

github.com/golang-jwt/jwt v3.2.2 added ed25519 support but go 1.12 doesn't support crypto/ed25519 package. So I replaced it with github.com/golang-jwt/jwt v3.2.1 which also has the CVE fix.

@ysksuzuki
Copy link
Author

I fixed a commit message format error and confirmed that the test following passed on my local ubuntu 20.04 with go 1.15.13.

$ GOARCH=amd64 PASSES='fmt bom dep' ./test
$ ./build && GOARCH=amd64 PASSES='functional' RACE="false" ./test
$ GOARCH=amd64 CPU=4 PASSES='integration' RACE="false" ./test

…lang-jwt/jwt

github.com/dgrijalva/jwt-go has CVE GHSA-w73w-5m7g-f7qc
and is already archived. etcd v3.4 should use a community maintained fork
github.com/golang-jwt/jwt which provides the fixed version of the CVE.

Signed-off-by: Yusuke Suzuki <yusuke-suzuki@cybozu.co.jp>
Update go to 1.15.15 which is the latest of 1.15 because linux-amd64-fmt fails with go 1.15.13.

Signed-off-by: Yusuke Suzuki <yusuke-suzuki@cybozu.co.jp>
@ysksuzuki
Copy link
Author

I also updated go to 1.15.15 on the workflow file because linux-amd64-fmt doesn't pass with go 1.15.13 anymore.

@ysksuzuki
Copy link
Author

Now I could confirm that all jobs passed on my forked repository.
https://github.com/ysksuzuki/etcd/actions/runs/1297153927

@ysksuzuki
Copy link
Author

It seems that some jobs failed because of flaky tests. Please rerun the jobs.

@ysksuzuki
Copy link
Author

ysksuzuki commented Oct 3, 2021

One job failed. Please rerun it or let me rerun jobs by myself if I can.

As far as I know this flakiness is a known issue and is being handled on #13167

Copy link
Contributor

@hexfusion hexfusion left a comment

Choose a reason for hiding this comment

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

LGTM can you please raise a followup to 3.4 CHANGELOG of this change in dependency. Thanks!

@hexfusion hexfusion merged commit 8ea187e into etcd-io:release-3.4 Oct 4, 2021
@ysksuzuki
Copy link
Author

Thank you for reviewing my PR!

can you please raise a followup to 3.4 CHANGELOG of this change in dependency.

Do you mean to add a v3.4.17 section and describe the change in dependency on the main branch something like this?

https://github.com/etcd-io/etcd/compare/main...ysksuzuki:update-changelog-3.4?expand=1#diff-c42e2429a0d21c0484779bb9249f21076776247124f9789f6485e2084911b280

@ysksuzuki
Copy link
Author

Thank you for releasing v3.4.17 including this fix!

@hexfusion
Copy link
Contributor

I added the CL no worries, thanks for the assist!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants