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
Remove github.com/dgrijalva/jwt-go #3459
Conversation
How are we planning to release v2.7.2? I see other milestone issues have been merged into main but if we want v2.7.2 to still not be a module then we'll have to backport those changes into a different branch. This change will look very different without modules. |
github.com/dgrijalva/jwt-go us unmaintained and vulnerable to CVE-2020-26160. Updating github.com/Azure/go-autorest removes the dependency. Signed-off-by: Bracken Dawson <abdawson@gmail.com>
The direct dependency that ultimately brings in github.com/dgrijalva/jwt-go is github.com/Azure/azure-sdk-for-go but that hasn't been updated yet, so updating the indirect dependency github.com/Azure/go-autorest is needed. |
thanks @brackendawson , this PR should be backported to v2.7.2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This made me think of #3138 which updates autorest
and crypto
indirect deps, etc.
We should probably close that one if we merge this one in CC: @brackendawson @wy65701436
By updating github.com/Azure/go-autorest/autorest to v.0.11.19 (10e0b31633f168ce1a329dcbdd0ab9842e533fb5) Backport of github.com/distribution#3459
By updating github.com/Azure/go-autorest/autorest to v.0.11.19 (10e0b31633f168ce1a329dcbdd0ab9842e533fb5) Backport of github.com/distribution#3459 Signed-off-by: Bracken Dawson <abdawson@gmail.com>
@@ -43,6 +53,8 @@ github.com/docker/libtrust v0.0.0-20150114040149-fa567046d9b1 h1:ZClxb8laGDf5arX | |||
github.com/docker/libtrust v0.0.0-20150114040149-fa567046d9b1/go.mod h1:cyGadeNEkKy96OOhEzfZl+yxihPEzKnqJwvfuSUqbZE= | |||
github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ= | |||
github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= | |||
github.com/form3tech-oss/jwt-go v3.2.2+incompatible h1:TcekIExNqud5crz4xD2pavyTgWiPvpYe4Xau31I0PRk= | |||
github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this on the version for 2.7; https://github.com/distribution/distribution/pull/3465/files#r679126223
This looks to be pulling in v3.2.2, which is missing a security fix in v3.2.4; form3tech-oss/jwt-go@v3.2.2...v3.2.4 (see https://github.com/form3tech-oss/jwt-go/tree/v3.2.4)
To get a more current version, you can add a //indirect
to the go.mod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I was just checking the same thing. I think in this branch as long as github.com/Azure/go-autorest/autorest still uses github.com/form3tech-oss/jwt-go then using an //indirect to bump it to 3.2.4 is good.
@@ -14,7 +14,6 @@ require ( | |||
github.com/bugsnag/osext v0.0.0-20130617224835-0dd3f918b21b // indirect | |||
github.com/bugsnag/panicwrap v0.0.0-20151223152923-e2c28503fcd0 // indirect | |||
github.com/denverdino/aliyungo v0.0.0-20190125010748-a747050bb1ba | |||
github.com/dgrijalva/jwt-go v0.0.0-20170104182250-a601269ab70c // indirect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a // indirect
for the jwt-go module, to force it to the most current (v3.2.4) version (see my other comment)
github.com/form3tech-oss/jwt-go v3.2.4+incompatible // indirect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly it's not that easy, v3.2.4 introduced an empty mod file which means the import line has to change to github.com/form3tech-oss/jwt-go/v3 now, which is a code change in github.com/Azure/go-autorest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think there is no valid way to import 3.2.4 with that mod file in there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps someone should open a PR to remove it; looks like it may have been unintentional that it was added.
Either that and/or forward-port the security fix to the upstream (form3tech-oss/jwt-go#14)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in the main branch, because form3tech-oss/jwt-go@v3.2.4 contains an invalid go.mod file (the module string should be versioned). Our only options are:
- Use a replace to swap the module for golang-jwt/jwt, I don't like replace directives in releases.
- Get Azure/go-autorest to change to golang-jwt/jwt (issue here Replace github.com/form3tech-oss/jwt-go with https://github.com/golang-jwt/jwt? Azure/go-autorest#642) then update that here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a workaround, it's possible to use a commit as version; in that case I think go mod
should work (run go mod tidy afterwards to generate the pseudo version)
github.com/form3tech-oss/jwt-go a211650c6ae1cff6d7347d3e24070d65dcfb1122 // indirect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also opened form3tech-oss/jwt-go#17 to remove the go.mod 😅
Haven't come round to it yet, but I was planning to open a PR in Because the https://github.com/form3tech-oss/jwt-go has now been archived (see form3tech-oss/jwt-go#17 (comment)) |
I opened Azure/go-autorest#645 to switch Azure/go-autorest to use the new module. Unfortunately couldn't update it to |
Heads up; Azure/go-autorest#645 was merged, and now switches Azure/go-autorest to |
We should close this PR as #3138 addressed this. |
Yes, looks like this was resolved in #3138 Thanks @brackendawson ! |
github.com/dgrijalva/jwt-go is unmaintained and vulnerable to
CVE-2020-26160. Updating github.com/Azure/go-autorest removes
the dependency.
Signed-off-by: Bracken Dawson abdawson@gmail.com
fixes #3361