Comment 36 for bug 1449212

Jeremy: OK, well I like the idea mainly because the dlo-manifest-with-put-temp-url-to-probe-other-containers issue will require back-ports to older releases while the container-temp-url-key scoping just needs to go back to kilo.

If we can co-ordinate the disclosure date for both it seems manageable. You said you're not against it right?

Unless someone beats me to it - or argues against it - I'll make a separate private security bug for the use dlo-manifest-with-put-temp-url-to-probe-other-containers issue and add my functional test that will describe the issue.

But I sorta wondering, if you can't PUT SLO manifests with tempurls (because of the questionable query-arg stripping) and we decide to strip DLO headers on PUT tempurls to address the account-tempurl issue - does the container scoping issue close itself - or are we still worried about a compromised app that has write access to a container and the ability to create pre-authed GET requests with a container-temp-url key creating a manifest they wouldn't normally authorize to cross-container read? Still seems like a reasonable vector to close with tempurl scoping...

Although we need to make sure we have good docs describing the limitations of container-tempurls and segmented objects - since I think we fully plan to support account-tempurls for manifests whose segments are in another container and we're explicitly deciding that if you have a valid cross-container SLO in a container and you try to create a container-tempurl for it - it doesn't work - on purpose.

FWIW, I think sam's swift.info patch is fine, the functests look good, I think sending a copy of the request with the swift.authorized popped into the app preserves existing behavior and expectations pretty well. I was worried about backports of the test fallout from that patch - but since we'd only backport it to Kilo I think it's fine - +2.

However, I still feel like it's conflating all of our grief with the get_info caching and this security issue - which I'm sure we want to be as surgical of a fix as possible. I'm ok with going the route sam has suggested (+2), but still wondering if there might be something we like better for this fix... asides from the "don't pop authorize if it's a override authorize" trick I already suggested - another way to fake out the authorize pop would be to have the scoped authorized callbacks return an 403 unless the acls have been populated on the request so you make sure you're responding None to the delay denial callback and the not the early check. Or only pop authorized if swift.owner has been populated into the environment would work, since the whole pop trick is just an optimization anyway. OTOH, both of those are hacks, not modifying the request that was handed into the app seems like a reasonable and desireable behavior. Using a copy of the request to pull the authorize callback out of seems like a reasonable way to do that (don't modify the original request) but also still support the weird double auth check delay denial thing where we call authorize twice, ignore the denied response, but on None try to avoid calling authorize again after acl's have been fetched since the authorize callback was already able to authorize the request without them originally. Has anyone else looked at sam's patch - did the commit message update help grok the swift.info cache trick which enables the use of the copied request instead of modifying the original? Does that look good to folks?