Client generates Faraday request URLs with leading double slashes which trips up AWS signing

[julik]

I am experimenting with the ES client in combo with AWS v4 and I use a request to create an index:

client.indices.create(index: index_name, body: INDEX_STRUCTURE)

using the workflow recommended in the README for AWS ES servers (using the Faraday signers). I was receiving an error that the canonical string used for signature generation was not matching the expected one. After some time and after having added some "putsie debugging statements" to the Aws::Signer::V4 I figured that it was getting the URL with a path having a double slash at the start. This made the index creation request fail. So it looks like the client is sending the request to Faraday with a double-slash at the start of the path, and that works fine with a bare ES, but does not work when you need to use this path to compute a request signature.

I think this might be related to how the -api gem works, and is probably easy to fix - but I don't currently have time to dive into it myself.

[karmi]

This really shouldn't happen, the Utils.__pathify method used in the API methods explicitely protects against double slashes using squeeze.

When I try it with logging, I get:

client.indices.create(index: 'test')
2016-02-23 08:09:48 -0800: PUT http://localhost:9200/test [status:200, request:0.266s, query:n/a]

Would you have some time to identify where the double slashes occur, and post some reproducible code?

[julik]

Sure thing. Here's how I set up Faraday (my correctional middleware is commented out):

creds = Aws::Credentials.new(ENV.fetch('AWS_ACCESS_KEY_ID'), ENV.fetch('AWS_SECRET_ACCESS_KEY'))
Elasticsearch::Client.new(url: ENV.fetch('ENDPOINT_URL')) do |faraday|
  # faraday.request :suppress_double_root
  faraday.request :aws_signers_v4,
    credentials: creds,
    service_name: 'es',
    region: ENV.fetch('AWS_REGION')
  faraday.response :logger
  faraday.adapter Faraday.default_adapter
end

and all I ever use are the following operations:

client.indices.create(index: index_name, body: INDEX_STRUCTURE)
...
client.bulk(body: bulk_requests)

and when I run this within a spec I get the following output:

I, [2016-02-23T21:35:16.553958 #24538]  INFO -- : put https://<MY_AWS_DOMAIN>.eu-west-1.es.amazonaws.com//xferevents_daily_20160223
D, [2016-02-23T21:35:16.554045 #24538] DEBUG -- request: User-Agent: "Faraday v0.9.2"
X-Amz-Date: "20160223T203516Z"
Host: "search-wt-xferevents-experimental1-ns3qfcxnxhcdbtezk3iff4m57e.eu-west-1.es.amazonaws.com"
X-Amz-Content-Sha256: "a5a9dc9cab87574b8c3e6548e25ed92b33a44cbfa8cbd1048c43924b347e4526"
Authorization: "AWS4-HMAC-SHA256 Credential=<MY_AWS_KEY>/20160223/eu-west-1/es/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=3f8d37d706219e51f7edf9992f05949656d67c163722b62a5f6c09313e3601c6"
I, [2016-02-23T21:35:17.196294 #24538]  INFO -- Status: 403
D, [2016-02-23T21:35:17.196450 #24538] DEBUG -- response: content-type: "application/json"
server: "Jetty(8.1.12.v20130726)"
x-amzn-requestid: "f27f736f-da6c-11e5-aa72-d1b898b36949"
content-length: "785"
connection: "Close"

and the error I get contains

The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.\n\nThe Canonical String for this request should have been\n'PUT\n/

If I change my suppress_double_root middleware to this:

class SuppressDoubleRoot < Faraday::Middleware
  def call(env)
    raise env.url.inspect
  end
  Faraday::Request.register_middleware :suppress_double_root => self
end

it raises (in the same spec)

#<URI::HTTPS:0x007fd48ee76980 URL:https://<my-aws-domain>.eu-west-1.es.amazonaws.com//xferevents_daily_20160223>

There clearly is a double slash in there. Knowing the index does not get created, I think it is clear it is the index create request that is adding the double slashes. My lockfile lists the following versions:

elasticsearch (1.0.15)
      elasticsearch-api (= 1.0.15)
      elasticsearch-transport (= 1.0.15)

Note that I am using both the AWS SDK and Faraday on the default adapter, which should be Net::HTTP. To reproduce, try to perform a request like this against a (very tiny) AWS ES domain, which shouldn't cost all that much.

If I use this to create a client that talks to my default install

Elasticsearch::Client.new do |f|
      f.response :logger
      f.adapter Faraday.default_adapter
 end

there are no double slashes added. If I use the :url option:

Elasticsearch::Client.new(url: 'http://localhost:9200') do |f|
   f.response :logger
   f.adapter Faraday.default_adapter
end

also not. This only happens in combination with the AWS URL I need to use (it has no trailing slash)
and the signer middleware. However, the double root suppressor thing I made is before the signer in the middleware chain and already shows a URL having a double slash root in path.

[julik]

And no, sorry - I got no idea where this double-slash gets prepended, but I hope the above comment provides a repro guide.

[julik]

I am almost certain I have tried both with and without the trailing slash.

On 11 mrt. 2016, at 21:44, Mike Waldner notifications@github.com wrote:

Hi @julik. I just noticed I had a trailing slash in my aws url I was passing into the client url: parameter. When I removed the trailing / the problem went away.

[karmi]

So, I have tried with a real AWS Elasticsearch service endpoint, and the problem indeed is with the trailing slash. Observe:

When I pass a URL with the trailing slash:

client = Elasticsearch::Client.new url: 'https://search-mytestdomain-XXX.us-east-1.es.amazonaws.com/'

$ ruby -I lib ./tmp/problem_double_slashes.rb 
I, [2016-03-13T09:12:31.474463 #22155]  INFO -- : put https://search-mytestdomain-XXX.us-east-1.es.amazonaws.com//test

# [403] {"message":"The request signature we calculated does not match ...

When I pass a URL without the trailing slash:

client = Elasticsearch::Client.new url: 'https://search-mytestdomain-XXX.us-east-1.es.amazonaws.com'

I, [2016-03-13T09:19:05.924465 #22202]  INFO -- : put https://search-mytestdomain-XXX.us-east-1.es.amazonaws.com/test

# [403] {"Message":"User: arn:aws:iam::995337243610:root is not authorized ...

I'm usually trying really hard for making the life of the library users as easy as possible, and was considering removing the trailing slash from passed URL, but I'm resistant to such a change, to be honest. First, the AWS console does give you a URL without the trailing slash, so if you simply copy&paste that, you should be fine. Second, this change would require another set unit tests (we cannot just squeeze the whole URL of course, because of http://), might be something hard to explain in code without a comment... and for what kind of real benefit? Note that (as usual), I'm not against it, just that I think the cost-benefit ratio here is not that big. What does everybody else think?

[julik]

I need to verify this, because I remember I had problems without the trailing slash as well. It might be that Faraday is involved there as well. If it turns out that it does work without a trailing slash then maybe we should put the trailing slash thing in the documentation and close the issue. I will have more info in a couple of days, when I have time to play with this again. Would everyone be OK with this?

[karmi]

Sure thing, no stress, let's just not forget about it and potentially close it. Regarding documentation -- that has an even higher cost that adding the automatic slash removal :)

I'm quite sure the problem is with the trailing slash, and only AWS complains about the double slashes, Elasticsearch doesn't care, so if anybody feels strongly about the double slash removal, go for it, otherwise I'd just keep it like it is for the moment.

[julik]

Ok it is correct - without the trailing slash I don't see the problem occurring. Very odd because I triple-verified it was happening before, maybe it's a heisenbug. I will set up a documentation PR.

[karmi]

Closing for now -- as I suggested, if we feel this is a common source of problems, let's add automatic removing of trailing slashes in code!

[chikamichi]

Hi,

This tiny cleanup would go a long way making the lib more usable from the get go. End users shouldn't have to troubleshoot this kind of typo-based "errors", should they? 🌵

[lupine]

We're experiencing this at GitLab: https://gitlab.com/gitlab-org/gitlab-ee/issues/2039

   elasticsearch (5.0.3)
      elasticsearch-api (= 5.0.3)
      elasticsearch-transport (= 5.0.3)
    elasticsearch-model (0.1.9)
    elasticsearch-rails (0.1.9)

We can fix it at input time, but I'd prefer to submit a patch upstream. Given this issue has been closed with a docfix, would a PR be welcomed?