Skip to content
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

Fixed: find tran coordinator was not ACL verified #5370

Merged

Conversation

dlex
Copy link
Contributor

@dlex dlex commented Jul 7, 2022

Cover letter

The code to handle FindCoordinator request for transaction coordinator type
appeared before the caller is checked for authorization for this operation
against the ACL. That allowed execution of find_coordinator(transaction)
by a client that does not have the describe access to the transactional-id.

The check has now been moved before any other handling.

Release notes

  • none

The code to handle FindCoordinator request for transaction coordinator type
appeared before the caller is checked for authorization for this operation
against the ACL. Now the chech has been moved before any other handling.
@dlex dlex requested a review from rystsov July 7, 2022 00:36
@CLAassistant
Copy link

CLAassistant commented Jul 7, 2022

CLA assistant check
All committers have signed the CLA.

@dlex dlex changed the title kafka: fixed: find tran coordinator was not ACL verified Fixed: find tran coordinator was not ACL verified Jul 7, 2022
@dlex dlex added kind/bug Something isn't working area/kafka labels Jul 7, 2022
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

nice catch.

do you think it would be possible to add a test for this around here: https://github.com/redpanda-data/redpanda/blob/dev/tests/rptest/tests/acls_test.py#L162

@dlex
Copy link
Contributor Author

dlex commented Jul 8, 2022

do you think it would be possible to add a test for this around here: https://github.com/redpanda-data/redpanda/blob/dev/tests/rptest/tests/acls_test.py#L162

I don't think so. To my knowledge, there is no kafka client function to issue FindCoordinator(txn) request alone, it is always followed by another txn related request (like InitProducerId) which would be properly verified against ACLs. So unless there is a way to issue raw kafka API requests from ducktape tests, there is no way to isolate this bug with a test.

@rystsov rystsov merged commit 14647aa into redpanda-data:dev Jul 12, 2022
@dlex dlex deleted the find-coordinator-acl-authorization-fix branch July 13, 2022 18:40
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.

None yet

5 participants