ccpgames / sso-issues

Please file issues with the CCP SSO (login.eveonline.com) here.
17 stars 1 forks source link

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

Open Gadicuz opened 4 years ago

Gadicuz commented 4 years ago

Bug

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

According to RFC 7636 Proof Key for Code Exchange by OAuth Public Clients _codeverifier and _codechallenge 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 _codeverifier and _codechallenge 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 _codechallenge creation. To follow the RFC, _codeverifier value should be equal to value of random string. But in fourth line the example makes additional BASE64URL-ENCODE to calc _codeverifier value and this violates RFC 7636.

Expected Behaviour

No additional BASE64URL-ENCODE for _codeverifier 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 _codeverifier body parameter with BASE64URL-ENCODE(_codeverifier) value.

CarbonAlabel commented 4 years ago

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 commented 4 years ago

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.tokenbytes(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 commented 4 years ago

I have a hypothesis.

On server side:

_base64urlencode() encodes:

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

If 'happy way' styled _base64urldecode() 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 _codeverifier 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 _codeverifier of random length then 25% of token requests will fail with status 500 and CORS error. It might be reason of issue #57 .

stebet commented 4 years ago

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

stebet commented 4 years ago

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 commented 4 years ago

Just a tiny note. I'm sure they mean _"codeverifier" instead of _"codechallenge" 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...