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

Incorrect value of code_verifier parameter for /v2/oauth/token endpoint #60

Open
Gadicuz opened this issue Feb 2, 2020 · 6 comments
Open

Comments

@Gadicuz
Copy link

Gadicuz commented Feb 2, 2020

Bug

Current EVE SSO implementation of /v2/oauth/token endpoint for PKCE uses unnecessary BASE64URL-ENCODING for code_verifier parameter.

According to RFC 7636 Proof Key for Code Exchange by OAuth Public Clients code_verifier and code_challenge parameters are created in following manner:

STRING code_verifier = random STRING of characters [A-Z]/[a-z]/[0-9]/-/./_/~
STRING code_challenge = BASE64URL-ENCODE( SHA256( ASCII(code_verifier) ) )

No additional encoding is required for the parameters sent to the server. RFC 7636, Appendix B provides an example:

code_verifier = random_string => "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"
d = SHA256(code_verifier) => 0x13d31e961a1ad8ec2f16b10c4c982e0876a878ad6df144566ee1894acb70f9c3
code_challenge = BASE64URL-ENCODE(d) => "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"

In the example, the authorization request includes
code_challenge=E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM&code_challenge_method=S256

and the request to the token_endpoint includes
code_verifier=dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk

There is no additional encoding for the parameters.
This procedure doesn't work for EVE SSO and results in status 500 for POST request to /v2/oauth/token enpoint.

Actual Behaviour

Actual EVE SSO Authorization Code with PKCE behaviour can be reconstructed by python example available here. The example does work with EVE SSO server!
According to the example code_verifier and code_challenge parameters are created in following manner:

random = BASE64URL-ENCODE( <32 random bytes> )
d = SHA256( random )
code_challenge = BASE64URL-ENCODE(d)
code_verifier = BASE64URL-ENCODE(random)

First three lines follow RFC 7636 procedure for code_challenge creation.
To follow the RFC, code_verifier value should be equal to value of random string. But in fourth line the example makes additional BASE64URL-ENCODE to calc code_verifier value and this violates RFC 7636.

Expected Behaviour

No additional BASE64URL-ENCODE for code_verifier parameter required on /v2/oauth/token endpoint.

Workaround

One can't use standard OAuth 2.0 RFC 7636 compatible libraries to work with EVE SSO because of the bug. To make them work one can intercept POST request to /v2/oauth/token enpoint and replace code_verifier body parameter with BASE64URL-ENCODE(code_verifier) value.

@CarbonAlabel
Copy link
Contributor

I'm unsure what to make of this report. The example script definitely does base64url encode the code verifier, but by doing that, it doesn't follow https://docs.esi.evetech.net/docs/sso/native_sso_flow.html. This alone would make this an issue for the esi-docs repo.

However, I can confirm that the script works both in its current state, and with the spec violating base64url encode removed, meaning the SSO server will accept both a raw code verifier, and one that has been base64url encoded. This is non-standard, undocumented behavior, but one that shouldn't prevent spec compliant libraries from working with the SSO. I would be interested to know why it was introduced in the first place.

Finally, you state in your report that the SSO responded with a 500 status code. If there was an issue with the code verifier, the SSO should have responded with a 400 status code, leading me to believe the issue you report was most likely caused by some temporary SSO outage.

@Gadicuz
Copy link
Author

Gadicuz commented Feb 18, 2020

You are right. The python example script works fine with additional base64urlencoding and without it. But...

According to RFC 7636 4.1
code_verifier = high-entropy cryptographic random STRING using the unreserved characters [A-Z] / [a-z] / [0-9] / "-" / "." / "_" / "~" from Section 2.3 of [RFC3986], with a minimum length of 43 characters and a maximum length of 128 characters

Let's play with random value in example python script. Instead of
random = base64.urlsafe_b64encode(secrets.token_bytes(32))
set
random = 'U7rzA8Pc2UfF9Kr7_-YcfdAm6cAwWyIK1OpaSL5ttPosY'.encode()
so random.decode() is a nice 'random' 45 characters STRING.

With new random value the script works fine for
code_verifier = base64.urlsafe_b64encode(random).decode().replace("=", "")
but if I remove additional base64urlencoding and set
code_verifier = random.decode()
the POST request executes too long (>1 sec instead of 50-ish ms) and I get the output:

Request sent to URL https://login.eveonline.com/v2/oauth/token with headers {'Content-Type': 'application/x-www-form-urlencoded', 'Host': 'login.eveonline.com'} and form values: {'grant_type': 'authorization_code', 'client_id': '**my client id**', 'code': '1qfR75VciEeXW7ondbUHGw', 'code_verifier': 'U7rzA8Pc2UfF9Kr7_-YcfdAm6cAwWyIK1OpaSL5ttPosY'}
500 Server Error: Internal Server Error for url: https://login.eveonline.com/v2/oauth/token

Something went wrong! Re read the comment at the top of this file and make sure you completed all the prerequisites then try again. Here's some debug info to help you out:

Sent request with url: https://login.eveonline.com/v2/oauth/token
body: grant_type=authorization_code&client_id=**my client id**&code=1qfR75VciEeXW7ondbUHGw&code_verifier=U7rzA8Pc2UfF9Kr7_-YcfdAm6cAwWyIK1OpaSL5ttPosY
headers: {'User-Agent': 'python-requests/2.22.0', 'Accept-Encoding': 'gzip, deflate', 'Accept': '*/*', 'Connection': 'keep-alive', 'Content-Type': 'application/x-www-form-urlencoded', 'Host': 'login.eveonline.com', 'Content-Length': '160'}
SSO response code is: 500
SSO response JSON is: {'Message': 'An error has occurred.'}

If i replay the failed request with the same headers and body I get
Status: 400
Value: {"error":"invalid_grant","error_description":"Authorization code is invalid."}
So my code has been used and is not valid any more (just as expected).

PS. Some response headres are differ too (CORS related), see the picture below.
sso_h

PPS. Again, for random = base64.urlsafe_b64encode(secrets.token_bytes(32)) everything works fine.

PPPS. My application uses 45 characters STRING and it fails for every random string, not just for the value in this example.

@Gadicuz
Copy link
Author

Gadicuz commented Feb 18, 2020

I have a hypothesis.

On server side:

  • RFC 7636 compliant code was added after extra base64url_decode(code_verifier)

  • base64url_decode() uses 'happy way' coding style and doesn't expect 'bad/invalid' code_verifier value

base64url_encode() encodes:

  • 3k octets to 4n characters
  • 3k+1 octets to 4n+2 characters
  • 3k+2 octets to 4n+3 characters

So there are no 'good' base64url encoded strings length of 4*n+1!

If 'happy way' styled base64url_decode() expects 'well-coded' base64 character string it might be unaware of strings 4*n+1 characters length and results in terrible behaviour (infinite loops, buffer overflow and so on). In such case the server terminates process on timeout with status 500 and execution simply doesn't reach RFC compiant case for the 'bad' string.

I checked several code_verifier length:

44 characters. => Success!
45 characters. => 500, {'Message': 'An error has occurred.'}
46 characters. => Success!
47 characters. => Success!
48 characters. => Success!
49 characters. => 500, {'Message': 'An error has occurred.'}
50 characters. => Success!

PS. If a clent uses code_verifier of random length then 25% of token requests will fail with status 500 and CORS error. It might be reason of issue #57 .

@stebet
Copy link
Contributor

stebet commented Feb 20, 2020

This is the best issue report I've ever received! Thank you. I'll work on a fix :)

@stebet
Copy link
Contributor

stebet commented Feb 20, 2020

I'd like to point this as an FYI to make sure the code verifiers are correctly generated as well:

The client SHOULD create a "code_verifier" with a minimum of 256 bits
of entropy. This can be done by having a suitable random number
generator create a 32-octet sequence. The octet sequence can then be
base64url-encoded to produce a 43-octet URL safe string to use as a
"code_challenge" that has the required entropy.

This is documented here: https://tools.ietf.org/html/rfc7636#section-7.1

@Gadicuz
Copy link
Author

Gadicuz commented Feb 20, 2020

Just a tiny note. I'm sure they mean "code_verifier" instead of "code_challenge" in the last sentence,

According to RFC7636 4.2 Client Creates the Code Challenge

plain
code_challenge = code_verifier
S256
code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier)))

One can use a 43-octet URL safe string, which is in fact a "code_verifier" with required entropy, as a "code_challenge" only for "plain" method. For "S256" method the string is used "to produce a code_challenge", not "as a code_challenge"

For example in section 7.3 they say

Concatenating a publicly known value to a code verifier (containing 256 bits of entropy) and then hashing it with SHA256 to produce a code challenge...

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

No branches or pull requests

3 participants