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

rgw: Use decoded URI when verifying TempURL #13007

Merged
merged 1 commit into from Jan 26, 2017

Conversation

Projects
None yet
4 participants
@Werkov
Contributor

Werkov commented Jan 19, 2017

Instead of calliing url_decode directly, we reuse s->decoded_uri that is
initialized in RGWREST::preprocess().

Fixes: http://tracker.ceph.com/issues/18590
Signed-off-by: Michal Koutný mkoutny@suse.com

@Werkov

This comment has been minimized.

Contributor

Werkov commented Jan 19, 2017

@smithfarm: IIUC, this PR against master should be the more important one (and consider #12986 be its backport to kraken).

@smithfarm smithfarm requested review from oritwas and yehudasa Jan 20, 2017

rgw: Use decoded URI when verifying TempURL
Instead of calliing url_decode directly, we reuse s->decoded_uri that is
initialized in RGWREST::preprocess().

Fixes: http://tracker.ceph.com/issues/18590
Signed-off-by: Michal Koutný <mkoutny@suse.com>
@yehudasa

This comment has been minimized.

Member

yehudasa commented Jan 20, 2017

@rzarzynski can you take a look at this one?

@rzarzynski rzarzynski self-assigned this Jan 23, 2017

@rzarzynski

Looks good. I will merge the PR after testing.

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Jan 25, 2017

Manually verified:

  • unfixed RadosGW:
curl -i "${publicURL}/cont/obj%61%61?temp_url_sig=03e36445c6942215dedb052ba2be86889d2ff6ae&temp_url_expires=1485373186" -X GET
HTTP/1.1 401 Unauthorized
Content-Length: 12
X-Trans-Id: tx000000000000000000004-005888f549-1013-default
Accept-Ranges: bytes
Content-Type: text/plain; charset=utf-8
Date: Wed, 25 Jan 2017 18:58:17 GMT

AccessDenied
  • fixed RadosGW:
$ curl -i "${publicURL}/cont/obj%61%61?temp_url_sig=03e36445c6942215dedb052ba2be86889d2ff6ae&temp_url_expires=1485373186" -X GET
HTTP/1.1 200 OK
Content-Length: 3
Accept-Ranges: bytes
Last-Modified: Wed, 25 Jan 2017 18:39:08 GMT
X-Timestamp: 1485369548.54302
etag: d16fb36f0911f878998c136191af705e
Content-Disposition: attachment; filename="objaa"
X-Trans-Id: tx000000000000000000002-005888f5fb-1016-default
Content-Type: application/x-www-form-urlencoded
Date: Wed, 25 Jan 2017 19:01:15 GMT

xyz
  • Swift:
$ curl -i "${publicURL}/cont/obj%61%61?temp_url_sig=673dca9dc0b3b82c6db44acd2156381af2693618&temp_url_expires=1485374027" -X GET 
HTTP/1.1 200 OK
Content-Length: 3
Accept-Ranges: bytes
Last-Modified: Wed, 25 Jan 2017 18:50:32 GMT
Etag: d16fb36f0911f878998c136191af705e
X-Timestamp: 1485370231.51584
Content-Type: application/x-www-form-urlencoded
Content-Disposition: attachment; filename="objaa"
X-Trans-Id: txc5eafd9cafee44eca4237-005888f4dd
Date: Wed, 25 Jan 2017 18:56:29 GMT

xyz

Also Tempest is perfectly fine with this PR. Still waiting for Teuthology.

@yehudasa yehudasa merged commit b6f1ba8 into ceph:master Jan 26, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@smithfarm smithfarm changed the title from rgw: Use decoded URI when verifying TempURL to [DNM] rgw: Use decoded URI when verifying TempURL Jan 26, 2017

@smithfarm smithfarm changed the title from [DNM] rgw: Use decoded URI when verifying TempURL to rgw: Use decoded URI when verifying TempURL Jan 26, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jan 29, 2017

@Werkov, I looked at backporting this to jewel and hammer, but quickly reached the conclusion that it's non-trivial (at least for me). Can you help?

(git cherry-pick -x, describe conflict resolution, put the backport tracker URL in the PR description, and ping me in the PR - I'll take care of the rest)

Thanks!

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jan 29, 2017

@Werkov If the backport needs to be done manually, just write "This is a manual backport of 4e1318f" and explain why it could not be cherry-picked.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jan 29, 2017

@Werkov A jewel backport PR is open already #13145 but it hasn't passed Jenkins CI yet.

@Werkov

This comment has been minimized.

Contributor

Werkov commented Jan 31, 2017

FTR, hammer backport PR is #13198.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jan 31, 2017

@Werkov Thanks, looks good. I'll test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment