Skip to content

RGW: allow user disabling presigned urls in rgw configuration#56044

Merged
cbodley merged 2 commits intoceph:mainfrom
pr0ton11:rgw-disable-signature-url
Mar 20, 2024
Merged

RGW: allow user disabling presigned urls in rgw configuration#56044
cbodley merged 2 commits intoceph:mainfrom
pr0ton11:rgw-disable-signature-url

Conversation

@pr0ton11
Copy link
Copy Markdown

@pr0ton11 pr0ton11 commented Mar 7, 2024

Fixes: https://tracker.ceph.com/issues/64797

For security reasons we would like to disallow presigned urls in our S3 cluster.
This is a patch allowing us to set the configuration value rgw_s3_auth_disable_signature_url in the rgw config to disable presigned URLs in the cluster.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@pr0ton11 pr0ton11 requested a review from a team as a code owner March 7, 2024 18:02
Copy link
Copy Markdown
Contributor

@cbodley cbodley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Signed-off-by: Marc Singer <marc@singer.services>
Copy link
Copy Markdown
Contributor

@cbodley cbodley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great. a lab outage is preventing the checks from running. we'll want to make sure the builds/tests succeed before starting qa

@jmundack
Copy link
Copy Markdown
Contributor

jmundack commented Mar 7, 2024

would there be some documentation on what the security concerns are? if not, is this something we can document as a part of this configuration? (ie a use case where cluster admins might want to disable pre-signed urls)

@pr0ton11
Copy link
Copy Markdown
Author

pr0ton11 commented Mar 7, 2024

would there be some documentation on what the security concerns are? if not, is this something we can document as a part of this configuration? (ie a use case where cluster admins might want to disable pre-signed urls)

What we encountered was that pre-signed urls allowed users to bypass the bucket policies in our version, see https://lists.ceph.io/hyperkitty/list/ceph-users@ceph.io/thread/HVLFGODPWVICWGQGD3IEEBDPK7RXZ7CA/#XL3DH52HSDWN47PLBKKPNXQNQ2QVYGUB
Unfortunately I did not get any responses anymore if this could be reproduced by anyone else. But it would be good anyway to disable this feature if we don't want to utilize it.

@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Mar 7, 2024

jenkins test this please

@pr0ton11
Copy link
Copy Markdown
Author

jenkins please test again

@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Mar 20, 2024

@cbodley cbodley merged commit 0bcea93 into ceph:main Mar 20, 2024
tobias-urdin added a commit to tobias-urdin/ceph that referenced this pull request Sep 25, 2024
We have special error handling for these cases but we
don't check the negative value of the error so it
instead gets caught as an error not defined with a
response and throws a http 500.

This was implement by [1] and the same way done
with [2].

[1] ceph#55371
[2] ceph#56044

Signed-off-by: Tobias Urdin <tobias.urdin@binero.com>
samakshd pushed a commit to samakshd/ceph that referenced this pull request Oct 11, 2024
We have special error handling for these cases but we
don't check the negative value of the error so it
instead gets caught as an error not defined with a
response and throws a http 500.

This was implement by [1] and the same way done
with [2].

[1] ceph#55371
[2] ceph#56044

Signed-off-by: Tobias Urdin <tobias.urdin@binero.com>
oshrey16 pushed a commit to oshrey16/ceph that referenced this pull request Oct 13, 2024
We have special error handling for these cases but we
don't check the negative value of the error so it
instead gets caught as an error not defined with a
response and throws a http 500.

This was implement by [1] and the same way done
with [2].

[1] ceph#55371
[2] ceph#56044

Signed-off-by: Tobias Urdin <tobias.urdin@binero.com>
piyushagarwal1411 pushed a commit to piyushagarwal1411/ceph that referenced this pull request Nov 26, 2024
We have special error handling for these cases but we
don't check the negative value of the error so it
instead gets caught as an error not defined with a
response and throws a http 500.

This was implement by [1] and the same way done
with [2].

[1] ceph#55371
[2] ceph#56044

Signed-off-by: Tobias Urdin <tobias.urdin@binero.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants