Comment 11 for bug 981332

Revision history for this message
Mike Lundy (novas0x2a) wrote :

The content-length can only be considered a hint, you can never actually trust it. In the current code, using the filesystem store, you can send a content-length of 4 and then dump 100PB on glance and it'll accept it. The code that actually reads from the socket needs to stop when it hits IMAGE_SIZE_CAP; this is a minor security problem, but it requires auth, so it's not too bad.

In any case, I see the lack of input validation on content-length to be a separate bug; it will require some amount of rearchitecting because the store assumes the image_size/content-length can be trusted, but only the store knows how big the data actually is. Note also that chunked-encoding in glance is the fallback path; any performant implementation should be using the sendfile api anyway. I might even go so far as to say that glance should die immediately on startup when sendfile is not supported, unless the user passes a flag; sendfile has so much better performance that it's no contest.

Swift does not violate the HTTP spec that I can see, it only sends chunked if it doesn't send content-length: https://github.com/openstack/swift/blob/master/swift/common/client.py#L682 and 690. As far as I know, there's no max object size for swift, so there's no reasonable point at which the server can reject input ahead of time. Instead, it fallocates the content-length (https://github.com/openstack/swift/blob/master/swift/obj/server.py#L560) which would be rejected if it were larger than the filesystem could handle. It also validates that the upload_size matches the content-length (and refuses to add the object to the hash if violated: https://github.com/openstack/swift/blob/master/swift/obj/server.py#L576). I don't understand the intricacies of swift, but seeing that leads me to believe that someone specifically thought about it and handled it appropriately.

Does this set your fears to rest so you'll +1 the review?