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

Consider using a different Base64 decoder #143

Closed
omsmith opened this issue Jul 7, 2016 · 3 comments
Closed

Consider using a different Base64 decoder #143

omsmith opened this issue Jul 7, 2016 · 3 comments
Milestone

Comments

@omsmith
Copy link

omsmith commented Jul 7, 2016

As per auth0/node-jsonwebtoken#208, some users of your library are getting stuck on passing in a String as a secret. My initial response was "that's not base64, it should be throwing" however given the decoder used, that doesn't appear to be the case. I expect the experience would be much improved if a proper base64 decoder was used, which complained about invalid base64.

As an example, here's the difference between the current parseBase64Binary and Java 8's Base64:

private static void printWithBadDecoder( String in ) {
  System.out.println(Arrays.toString(
    javax.xml.bind.DatatypeConverter.parseBase64Binary( in )
  ));
}

private static void printWithGoodDecoder( String in ) {
  System.out.println(Arrays.toString(
    Base64.getDecoder().decode( in )
  ));
}

public static void main( String[] args ) {
  printWithBadDecoder( "my-secret-token-to-change-in-production" );
  printWithBadDecoder( "mysecrettokentochangeinproduction" );
  printWithBadDecoder( "mysecrettokentochangeinproductio" );

  printWithGoodDecoder( "my-secret-token-to-change-in-production" );
  printWithGoodDecoder( "mysecrettokentochangeinproduction" );
  printWithGoodDecoder( "mysecrettokentochangeinproductio" );
}
[-101, 43, 30, 114, -73, -83, -74, -119, 30, -98, -38, 28, -123, -87, -32, 122, 41, -23, -82, -121, 110, 114, -40, -88]
[-101, 43, 30, 114, -73, -83, -74, -119, 30, -98, -38, 28, -123, -87, -32, 122, 41, -23, -82, -121, 110, 114, -40, -88]
[-101, 43, 30, 114, -73, -83, -74, -119, 30, -98, -38, 28, -123, -87, -32, 122, 41, -23, -82, -121, 110, 114, -40, -88]
Exception in thread "main" java.lang.IllegalArgumentException: Illegal base64 character 2d
Exception in thread "main" java.lang.IllegalArgumentException: Last unit does not have enough valid bits
[-101, 43, 30, 114, -73, -83, -74, -119, 30, -98, -38, 28, -123, -87, -32, 122, 41, -23, -82, -121, 110, 114, -40, -88]

As you can see, the current decoder skips over illegal characters, and then afterward doesn't require it be correctly sized (ignoring any remaining characters).

@lhazlewood
Copy link
Contributor

Thanks for the issue! I agree this could improve usability, so it's a nice enhancement.

@RobWin
Copy link

RobWin commented Jan 13, 2017

Yes, we run into the same issue that javax.xml.bind.DatatypeConverter.parseBase64Binary decoded a Base64 encoded String not correctly.
java.util.Base64.getDecoder().decode seems to work correctly.

Maybe I'm doing it wrong.
But let's assume I use a weak secret like 012345678901234567890123456789XY.
If I Base64 encode it, the result is MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2Nzg5WFk.

When I decode this Base64 encoded String, I get two different results. See wrongBytes and correctBytes
When I convert the bytes back into a String (inside of the debugger view), you can see that wrongBytes seems to show a wrong secret?

image

Any idea why this happens?

@lhazlewood
Copy link
Contributor

This was resolved in the 0.10.0 release. Documentation here: https://github.com/jwtk/jjwt#base64

@lhazlewood lhazlewood added this to the 0.10.0 milestone Mar 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants