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

[issue 675] oauth2 use golang-jwt address CVE-2020-26160 #713

Merged
merged 2 commits into from
Jan 20, 2022

Conversation

zzzming
Copy link
Contributor

@zzzming zzzming commented Jan 18, 2022

Fixes #675 #675

Motivation

The original CVE
CVE-2020-26160
high severity
Vulnerable versions: <= 3.2.0
Patched version: No fix
jwt-go allows attackers to bypass intended access restrictions in situations with []string{} for m["aud"] (which is allowed by the specification). Because the type assertion fails, "" is the value of aud. This is a security problem if the JWT token is presented to a service that lacks its own audience check. There is no patch available and users of jwt-go are advised to migrate to golang-jwt at version 3.2.1

Modifications

oauth2 go mod uses github.com/form3tech-oss/jwt-go, which is a fork from dgrijalva/jwt-go that is in read-only. The modification is switch to golang-jwt.

Golang 1.15 minimum version support

Set the minimum golang version to 1.15. It is because the recommended golang-jwt library requires golang 1.15+. It's explained in these two issues from the golang-jwt repo.
golang-jwt/jwt#50
golang-jwt/jwt#90

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes)
  • The public API: (no)
  • The schema: ( no )
  • The default values of configurations: ( no)
  • The wire protocol: (no)

Switch to golang-jwt as the CVE suggests.

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@@ -6,7 +6,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
go-version: [1.13, 1.14, 1.15, 1.16, 1.17]
go-version: [1.15, 1.16, 1.17]
Copy link
Member

Choose a reason for hiding this comment

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

The newly fixed jwt-go does not support versions prior to go 1.5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. They do not support. Here are two issues explaining why. I posed in the original motivation too.
golang-jwt/jwt#50
golang-jwt/jwt#90

@wolfstudy
Copy link
Member

cc @zymap please check

Copy link
Member

@zymap zymap left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

Good work @zzzming !

@wolfstudy wolfstudy merged commit 25e5957 into apache:master Jan 20, 2022
@lhotari
Copy link
Member

lhotari commented Feb 7, 2022

@wolfstudy what is the schedule for 0.8.0 release? https://github.com/apache/pulsar/blob/master/pulsar-function-go/go.sum and https://github.com/apache/pulsar/blob/master/pulsar-function-go/examples/go.sum get hi-lighted as vulnerable for CVE-2020-26160 and it would be good to have these issues resolved asap.

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.

should change dgrijalva/jwt-go to golang-jwt/jwt in oauth2/go.mod
4 participants