Query strings are problematic

[jnardone]

I wouldn't classify this as a bug per se, but there is a fundamental problem with one of the components used in the canonical string.

Right now, this library requires the entire path including query parameters (both for input and output). However, there is some fundamental ambiguity on whether spaces in query strings should be encoded with %20 or with a +. Different libraries on different platforms take different paths (python's urllib uses +, but ruby grape sees %20, etc.) This means for queries that have spaces, we will never generate an HMAC match because we don't know which format the caller used on the server side.

Amazon sidesteps all of this with their HMAC spec, by requiring both sides to ignore any query params e.g. everything from the ? on in a URL. This, however, would be a breaking change for this library.

Another possibility would be for the server-side to try all combinations of %20 and + (in the query string portion) when evaluating claims, though it's probably reasonable to expect that either it's all %20 or all +, and not some mix-and-match.

Thoughts?

[samtgarson]

+1, Amazon's approach seems the most logical, this could at least be given as an option in some config.

[octalmage]

I think we're going to fake it by building our own request without query strings then check it with authentic?, but it would be nice to have built-in support.

[jraede]

Are there any plans to make this configurable?

[kjg]

Do you have any examples that show why this doesn't work as is?

My understanding is that the server side should get the path and query string exactly as it was requested by the client. I can't think of a case where the server would parse out the query params, and then have to re-construct the query params back into the string that was used in the url.

[jnardone]

You don't know if the caller encoded spaces as + or %20 when they computed the value. We specifically hit this with our Ruby back-end and a QA test harness in Python.

[kjg]

Do you have any example code that shows this?

I struggling to understand how we don't know how the caller encoded spaces. We should have the raw uri used in the request passed on to our library from the server, and the caller should have generated the uri query string and all that before computing the value as well.

[jraede]

Seems like the problem is this library doesn't take the raw URL including the query string but relies on another library's parsed version of it. For example a request like ?foo%5B%5D=bar actually turns into ?foo[]=bar when this library constructs the signature payload. Sent via Mobile

…

________________________________ From: Kevin Glowacz <notifications@github.com> Sent: Thursday, September 28, 2017 2:33:07 PM To: mgomes/api_auth Cc: Jason Raede; Comment Subject: Re: [mgomes/api_auth] Query strings are problematic (#123) Do you have any example code that shows this? I struggling to understand how we don't know how the caller encoded spaces. We should have the raw uri used in the request passed on to our library from the server, and the caller should have generated the uri query string and all that before computing the value as well. — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#123 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEXD4oPR4Yc0BNXklyuVJf9sa2mf1im_ks5sm-ZjgaJpZM4JZ4dh>.

[jraede]

Oh, I'm wrong, I was just running into that issue in testing but it seems like the query string is used as-provided. Non-issue for me.

[DaKaZ]

This issue gets even sportier. Look at the difference in Rails 5.1.3 versus Rails 5.1.4:

Rails 5.1.3 ApiAuth canonical_string:'GET,application/json,,/api/v1/employees?select=epulse_id%2Cfirst_name%2Clast_name,Thu, 14 Dec 2017 16:19:48 GMT'

Rails 5.1.4 ApiAuth canonical_string:'GET,application/json,,/api/v1/employees?select=epulse_id,first_name,last_name,Thu, 14 Dec 2017 16:20:57 GMT'

[fwininger]

@DaKaZ can you give me the code to see the difference between Rails 5.1.3 and 5.1.4. I don't be able to reproduce your problem. Thanks

[DaKaZ]

@fwininger interesting.... I built a new project and I am getting consistent returns from rails now. So there is something weird going on with my production app.

This stuff is always fun. Either way, we should either require the params to be escaped or unescaped, right? If we are not declarative here, the ambiguity will allow errors like the one I have encountered.

[fogle]

@kjg, you said

My understanding is that the server side should get the path and query string exactly as it was requested by the client. I can't think of a case where the server would parse out the query params, and then have to re-construct the query params back into the string that was used in the url.

Unfortunately, if you're running serverless on AWS, API Gateway does not provide the query string to your lambda function, just an unordered map of key value pairs.