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

Browser/crypto base64 decoding strictly required padding #716

Closed
Quutti opened this issue May 21, 2019 · 2 comments · Fixed by #722
Closed

Browser/crypto base64 decoding strictly required padding #716

Quutti opened this issue May 21, 2019 · 2 comments · Fixed by #722
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@Quutti
Copy link

Quutti commented May 21, 2019

Environment details

  • OS: Windows 10
  • Node.js version: 8.11.1
  • npm version: 5.6
  • google-auth-library version: 4.0.0

Description

Browser implementation of crypto uses npm package base64-js to decode base64 strings. Base64-js package strictly requires padding on base64 strings, so when the base64 string is passed to the function the user is represented with an error "Can't parse token envelope".

For an example OAuth2Client.verifyIdToken (internally calls verifySignedJwtWithCertsAsync) -method uses crypto to decode base64 strings. In this case when id_token is passed, the client implementation might raise an error for user if id_token segment 0 or 1 is non-modular with 4.

NodeJS implementation of crypto uses Buffers which is not strict with padding, so NodeJS implementation is working correctly.

There is an issue on base64-js Github repository issue tracker (beatgammit/base64-js#45), but the issue is quite old. It might be better for you guys to create a simple ADHoC fix for this (I can provide a PR) or change the base64 decoding library.

@Quutti Quutti changed the title Browser/cr Browser/crypto base64 decoding strictly required padding May 21, 2019
@JustinBeckwith JustinBeckwith added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels May 24, 2019
@JustinBeckwith
Copy link
Contributor

Thanks for the heads up! PRs are always welcomed :)

@alexander-fenster
Copy link
Contributor

Oh. I did the padding here

// base64js requires padding, so let's add some '='
while (signature.length % 4 !== 0) {
signature += '=';
}

but I obviously missed some other places. Fixing that now. Thanks @Quutti!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants