swift does not return when max object size is exceeded

[Stuart McLaren]

The swift max object size is ~5GB.

If a user attempts to upload an object of a size > 5GB from stdin all input data is read by the server.
In this example 7GB was uploaded.

$ cat dd.7000 | curl -i -X PUT -T - -H 'x-auth-token: XXX' https://swift.com:443/v1/AUTH_XXX/tmp/dd.7000 >/dev/null
  % Total % Received % Xferd Average Speed Time Time Time Current
                                 Dload Upload Total Spent Left Speed
100 7003M 0 108 0 7003M 0 37.5M --:--:-- 0:03:06 --:--:-- 35.8M

Instead the server should return when the max object size is hit.

I haven't looked in detail at the swift code, but in the case of glance (https://bugs.launchpad.net/glance/+bug/1284246)
it seems to be down to this section of wsgi code in /usr/lib/python2.7/dist-packages/eventlet/wsgi.py which 'reads and discards' the incoming request body:

 420 finally:
 421 if hasattr(result, 'close'):
 422 result.close()
 423 if (self.environ['eventlet.input'].chunked_input or
 424 self.environ['eventlet.input'].position \
 425 < self.environ['eventlet.input'].content_length):
 426 ## Read and discard body if there was no pending 100-continue
 427 if not self.environ['eventlet.input'].wfile:
 428 # NOTE: MINIMUM_CHUNK_SIZE is used here for purpose different than chunking.
 429 # We use it only cause it's at hand and has reasonable value in terms of
 430 # emptying the buffer.
 431 start = time.time()
 432 while self.environ['eventlet.input'].read(MINIMUM_CHUNK_SIZE):
 433 pass
 434 finish = time.time()

Marking as security related in case people consider it DOS risk. Feel free to mark public if appropriate.

[Grant Murphy (gmurphy)]

I performed an isolated test and it does appear to read all input data from the request as indicated. (Even for 404 not found).
I would suggest that we raise this bug with <email address hidden> or maybe <email address hidden> as it really needs to be handled and fixed by the eventlet people. We can track the progress of the fix accordingly.

Sound good?

[clayg (clay-gerrard)]

So this is for transfer-encoding chunked...

swift.proxy.controllers.obj is watching bytes_transferred and it will throw a HTTPRequestEntityTooLarge, but I could see eventlet needing to follow spec and finish reading the request before sending the response...

We could try the good 'ol nuke_from_orbit trick like we do in bufferedhttp - I'm sure the real _sock is down there somewhere.

That's assuming we care more about not reading bytes forever than getting the 413 back to client? Once it's closed, I think you're done.

But, I'm not sure that solution will work upstream. Maybe they could make an optional max input size. But really once you start looking at eventlet's implementation for chunked encoding you'll be more scared of the line buffering anyway - so it's probably best if we don't get in there at all.

I'm not so sure I understand the DOS? I mean the clients are going to have to actually send those bits right? There's no amplification? If they can DOS you with a 7GB upload why can't they DOS you with two 4GB uploads? Is it just like they're trying to tie up connections? LIke slowloris but without the part where the client doesn't have to have unlimited bandwidth?

[Thierry Carrez (ttx)]

I guess the difference between the 7Gb upload and the 4Gb upload is that the 7Gb upload is never stored, so it never ends up costing you money. That said I agree with you that there is no amplification, and that it's not very different from uploading/deleting "normal" 4Gb bits.

The limit between normal usage and DoS is generally blurry in the IaaS world. I generally consider that if (1) there is no amplification and (2) you have the option to mitigate infinite POST requests at the network border using specialized equipment, then it falls under the "normal usage" side.

[Thierry Carrez (ttx)]

I would be inclined to consider this particular variation as a weak DoS vector that resembles normal usage. Not talking about the other variants you submitted. Thoughts ?

[Stuart McLaren (stuart-mclaren)]

Hi Thierry,

That sounds reasonable to me (FWIW I'm by no means a security guru).

[Stuart McLaren (stuart-mclaren)]

Hi Thierry,

That sounds reasonable to me (FWIW I'm no security guru).

[John Dickinson (notmyname)]

I removed Swift from bug https://bugs.launchpad.net/glance/+bug/1202785 (which is more about when a 4xx response is sent).

This particular bug report here is about what happens when the swift limits have been reached and Swift knows the PUT will fail.

Questions to consider:

1) upload 7GB with the expect header and a content length: This should return a 4xx immediately after the request headers are sent and not read the body of the request. All socket resources should be freed for other connections.

2) upload 7GB without the expect header and with a content-length: I'd think this should terminate the connection after 5GB are uploaded and free any socket resources.

3) upload 7GB with the expect header and no content length: same as (1), but including it for completeness

4) upload 7GB without the expect header and with no content length: similar to (2), Swift should terminate the connection after 5GB are uploaded and free any socket resources

These may be reduced to only 2 use cases, but I've expanded it for completeness. I can imagine treating (2) and (4) differently (or not).

We need to know what these scenarios do, and we should also document what happens in the Swift sourcetree docs.

While I'm not convinced this is a security risk, there is a small concern (from an efficiency standpoint) that resources should be freed when possible to allow other requests to use the server resources.

[Thierry Carrez (ttx)]

OK, unless someone complains soon I'll make this one public, remove the vulnerability task and rename it "Swift should return ASAP when..." for clarity.

[John Dickinson (notmyname)]

I do not think this should be made public until the questions above are answered.

[Tristan Cacqueray (tristan-cacqueray)]

@John I'm also not convinced this is a security risk.

The threat would be bandwidth/resources exhaustion. But even if swift would efficiently release those resources for objects > 5GB, this does not prevent an attacker to perform several < 5GB upload.

Then the questions you ask are relevant from an efficiency point of view (when does those resources should be released) and thus could be taken care of publicly.

I suggest we do as ttx said in #8...

[Christian Schwede (cschwede)]

Today I tried to find the reason for this bug, but I couldn't simulate this?

In my tests Swift aborted the transfer, though not immediately. I used the latest checkout from master for these tests, eventlet version was 0.9.16 (from Ubuntu 12.04 LTS) and 0.14 (latest release).

To make testing a little bit easier, I lowered the maximum object size in /etc/swift/swift.conf to 10 MB:

 [swift-constraints]
 max_file_size = 10485760

Now I tried to upload a 100MB object:

vagrant@saio:~$ dd if=/dev/zero bs=1k count=200k | curl -w "\nTransferred bytes: %{size_upload}\n" -v -X PUT -T - -H "Expect:" -H 'x-auth-token: AUTH_tk32acbc4891434aefb749ce657a24503d' http://localhost:8080/v1/AUTH_test/test/test
* About to connect() to localhost port 8080 (#0)
* Trying 127.0.0.1... connected
> PUT /v1/AUTH_test/test/test HTTP/1.1
> User-Agent: curl/7.22.0 (x86_64-pc-linux-gnu) libcurl/7.22.0 OpenSSL/1.0.1 zlib/1.2.3.4 libidn/1.23 librtmp/2.3
> Host: localhost:8080
> Accept: */*
> Transfer-Encoding: chunked
> x-auth-token: AUTH_tk32acbc4891434aefb749ce657a24503d
>
< HTTP/1.1 413 Request Entity Too Large
< Content-Length: 108
< Content-Type: text/html; charset=UTF-8
< X-Trans-Id: txf6aaabb7be5b488a832b4-00533a65d3
< Date: Tue, 01 Apr 2014 07:08:03 GMT
* HTTP error before end of send, stop sending
<
* Closing connection #0
<html><h1>Request Entity Too Large</h1><p>The body of your request was too large for this server.</p></html>
Transferred bytes: 12099140

As you can see from the last line, ~12MB were transferred. This number varies from request to request, but in my test it was approx. 1-2MB more than the limit set in /etc/swift/swift.conf (no matter how much that number was, ie ~ 102MB were transferred if I set max_file_size to 100MB).

I tested this with and without the expect header set, results were the same for me. To me this behavior looks ok?

[Tristan Cacqueray (tristan-cacqueray)]

Thank you christian! Considering your investigation, swift is not impacted, at least since Icehouse.

I don't think this warrant an OSSA, and should be opened/marked as closed. Anyone against it ?

[John Dickinson (notmyname)]

agreed