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

Signing Error when sending DELETE request with body to Elasticsearch #1733

Closed
ajkerr opened this issue Sep 27, 2017 · 8 comments
Closed

Signing Error when sending DELETE request with body to Elasticsearch #1733

ajkerr opened this issue Sep 27, 2017 · 8 comments
Labels
guidance Question that needs advice or information.

Comments

@ajkerr
Copy link
Contributor

ajkerr commented Sep 27, 2017

We have run into a strange issue with request signatures when sending DELETE requests to the AWS Elasticsearch service.

Much of the research on this issue came from this issue TheDeveloper/http-aws-es#19 and this AWS forum thread https://forums.aws.amazon.com/thread.jspa?threadID=227353

In this example program, we are attempting to use the Clear Scroll Elasticsearch API (https://www.elastic.co/guide/en/elasticsearch/reference/5.6/search-request-scroll.html#_clear_scroll_api)

In the first request, we are using the flavour of the API where a DELETE request is sent with the scroll_id sent in the body of the request. This request fails with a bad signature error. It shouldn't be failing.

In the second request, we deliberately override the X-Amz-Content-Sha256 header with an incorrect value. This time, the request gets through to Elasticsearch, even though the signature should not be valid. Elasticsearch complains that the scroll_id was not sent.

In the third request, we don't send a request body, and send the scroll id on the request path. This request works correctly (gives the correct error from Elasticsearch).

const AWS = require("aws-sdk");

AWS.config.update({region: "us-west-2"});

function signAndSendRequest(requestName, request) {
  const signer = new AWS.Signers.V4(request , "es");
  signer.addAuthorization(AWS.config.credentials, new Date());

  const send = new AWS.NodeHttpClient();
  send.handleRequest(request, null, function(response) {
    let respBody = "";
    response.on("data", function (chunk) {
      respBody += chunk;
    });
    response.on("end", function() {
      console.log("RESPONSE for %s: %j", requestName, respBody);
    });
  }, function(err) {
    console.log("Error: " + err);
  });
}

AWS.config.getCredentials((err) => {
  if (err) {
    console.log(`[error] credentials are not available: ${JSON.stringify(err)}`);
    return;
  }

  const endpoint
    = new AWS.Endpoint("https://search-your-domain-here.us-west-2.es.amazonaws.com");

  // This request fails with an 403 error that states:
  // "The request signature we calculated does not match the signature you provided"
  // It should not.
  let request = new AWS.HttpRequest(endpoint);
  request.method = "DELETE";
  request.path = "/_search/scroll";
  request.region = "us-west-2";
  request.headers["presigned-expires"] = false;
  request.headers["Host"] = endpoint.host;
  request.body = JSON.stringify({scroll_id: "12345"});
  signAndSendRequest("1: DELETE with body", request);

  // This request gets an error from Elasticsearch: "Validation Failed: 1: no scroll ids specified"
  // It should not have accepted the signature due to the invalid X-Amz-Content-Sha256 header.
  request = new AWS.HttpRequest(endpoint);
  request.method = "DELETE";
  request.path = "/_search/scroll";
  request.region = "us-west-2";
  request.headers["presigned-expires"] = false;
  request.headers["Host"] = endpoint.host;
  request.body = JSON.stringify({scroll_id: "12345"});
  request.headers["X-Amz-Content-Sha256"] = AWS.util.crypto.sha256("", "hex");
  signAndSendRequest("2: DELETE with body and incorrect X-Amz-Content-Sha256 header", request);

  // If we use an alternate syntax that doesn't combine a DELETE with a request body,
  // all is good (it's an error, but the correct one, because the scroll_id doesn't exist).
  request = new AWS.HttpRequest(endpoint);
  request.method = "DELETE";
  request.path = "/_search/scroll/12345";
  request.region = "us-west-2";
  request.headers["presigned-expires"] = false;
  request.headers["Host"] = endpoint.host;
  signAndSendRequest("3: DELETE without body", request);
});

It's unclear to me whether the AWS v4 signing code in the Javascript SDK is incorrect, or there's something wrong with the signature validation in the Elasticsearch service, but I figured I'd start here.

@AllanZhengYP
Copy link
Contributor

Hi @ajkerr

I believe the difference is whether body presents. In your second example, the body will be ignored by the signer, and 'X-Amz-Content-Sha256' header will be used as encoded body. And I don't think it's problem of signing the body because native SDK also uses JSON.stringify() to serialize body.
I will contact the service team to verify this issue.

@ajkerr
Copy link
Contributor Author

ajkerr commented Sep 28, 2017

@AllanFly120 Thanks for your response. Ignoring the second example, the big problem for us is that the first example doesn't work for DELETE requests. POSTs, PUTs, etc all seem to work when a body is present, but not DELETEs.

@ajkerr
Copy link
Contributor Author

ajkerr commented Sep 29, 2017

@AllanFly120 UPDATE: After a lot of tinkering, I noticed that if I add the following line of code to the first request above, the request is sent correctly:

request.headers["Content-Length"] = request.body.length;

My guess is that the SDK is not setting this header correctly for DELETEs with a body.

After digging in the SDK code a bit, I verified that modifying the setHeaders() function as follows in lib/signers/v4.js seems to fix this problem:

  addHeaders: function addHeaders(credentials, datetime) {
    this.request.headers['X-Amz-Date'] = datetime;
    if (credentials.sessionToken) {
      this.request.headers['x-amz-security-token'] = credentials.sessionToken;
    }
    if (this.request.body) {
      this.request.headers['Content-Length'] = this.request.body.length;
    }
  },

I notice that the older v2 signer did something similar with this header.

@AllanZhengYP
Copy link
Contributor

AllanZhengYP commented Sep 30, 2017

Hi @ajkerr
I believe that's the case. I will keep following this to the service team. Maybe this service will ignore request body with Content-Length set to 0. But I don't think it's an issue with SDK because the sigV4 should be designed to be compatible with all supported services. We need some more time to find out the root cause and fix this issue.
Thanks a lot for your investigation.

@ajkerr
Copy link
Contributor Author

ajkerr commented Oct 12, 2017

@AllanFly120 Any updates on this?

@jeskew
Copy link
Contributor

jeskew commented Oct 12, 2017

@ajkerr I believe the correct solution is to set the Content-Length header before passing the request to the signer. The SDK's Node HTTP handler is just passing the body to request.end, and something further down the stack is not sending the body for DELETE requests unless Content-Length is set.

If the SDK included a data-plane client for Elasticsearch Service, we would be able to handle this specific case transparently for users. However, SDK internals are being called directly in this case, so the calling code will need to ensure that a Content-Length header is set for DELETE requests with bodies.

@jeskew
Copy link
Contributor

jeskew commented Oct 21, 2017

I did a bit more research to make sure this wasn't a bug and found a justification for the behavior in the HTTP/1.1 spec. Section 4.3 reads in part:

The presence of a message-body in a request is signaled by the inclusion of a Content-Length or Transfer-Encoding header field in the request's message-headers.

Without either a Content-Length or Transfer-Encoding header, the delimiter that normally signals the end of the headers (\r\n\r\n) instead signals the end of the message.

@jeskew jeskew closed this as completed Oct 21, 2017
@srchase srchase added the guidance Question that needs advice or information. label Dec 7, 2018
@lock
Copy link

lock bot commented Sep 29, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
guidance Question that needs advice or information.
Projects
None yet
Development

No branches or pull requests

4 participants