Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

[BUG] Change of behavior between KoP 0.3.0 (2.6.2) and 2.7+ using SASL #415

Closed
jpenning-tibco opened this issue Mar 25, 2021 · 4 comments · Fixed by #662
Closed

[BUG] Change of behavior between KoP 0.3.0 (2.6.2) and 2.7+ using SASL #415

jpenning-tibco opened this issue Mar 25, 2021 · 4 comments · Fixed by #662

Comments

@jpenning-tibco
Copy link
Contributor

Describe the bug
We noticed a change in behavior between KoP 0.3.0 (2.6.2) and 2.7+ using SASL with token authentication (#206?)

  • In 0.3.0, when multiple clients connected using different SASL credentials (username=tenant/namespace) there was no cross-talk between them: Client1 with SASL1 on TopicA did not see data from Client2 with SASL2 on TopicA.
  • With 2.7.x this has changed, and Client1 and Client2 can freely exchange data.

To Reproduce
Steps to reproduce the behavior:

  1. Configure Pulsar and KoP to use SASL w/ token authentication.
  2. Run Kafka producers/consumers using different SASL credentials. The Kafka clients will cross-talk (Pulsar clients are not affected by this).

Expected behavior
The desired behavior is to have SASL credentials prevent cross-talk between Kafka clients.

Additional context
We understand that Kafka is not normally multi-tenant, but we think the behavior seen in 0.3.0 is very useful. We are interested to know what the intentions/goals are here. Is the behavior we rely on with 0.3.0 the intended behavior, a side-effect of the original implementation, etc?

@BewareMyPower
Copy link
Collaborator

I don't understand well. Could you give a more detail explanation on how two clients exchange data?

@jpenning-tibco
Copy link
Contributor Author

I don't understand well. Could you give a more detail explanation on how two clients exchange data?

By "two clients exchange data", I just mean messages sent by a producer app are received by a consumer app.

The test apps I run are basic and very similar to your Sarama/Confluent integration tests. The big difference is that my test apps have SASL configured. I can share my test apps with you if it will help.

In my test case the producer has a different SASL username and password than the consumer. In 0.3.0 if the producer app and the consumer app have different SASL username, then the consumer will not receive messages from the producer.

@jpenning-tibco
Copy link
Contributor Author

If you need log files, config or any other data please let me know.

@BewareMyPower
Copy link
Collaborator

I get it. Currently the SASL/PLAIN mechanism for KoP only checks if the role has the permission for the specific namespace (username). However, the client could produce/consume to another namespace since there we have no checks for if the username that represents the namespace is what the topic belongs.

I'll take a look at how to fix it when I'm free.

@BewareMyPower BewareMyPower mentioned this issue Jun 18, 2021
9 tasks
Demogorgon314 added a commit that referenced this issue Aug 21, 2021
Add authorization to handleTopicMetadataRequest(#236 ).

Fix #415 and #571 

## Motivation
When client fetch metadata need check topic permission, so we need add authorization in handleTopicMetadataRequest, and do not perform role verification in authentication.

## Modifications
Add a common method in `KafkaRequestHandler#authorize` , this method use `authorizer` to authorization.
Modify the authentication behavior, and do not verify the role during authentication, verify the role in fetch metadata(#571  )
wangjialing218 pushed a commit to wangjialing218/kop that referenced this issue Aug 24, 2021
…ative#662)

Add authorization to handleTopicMetadataRequest(streamnative#236 ).

Fix streamnative#415 and streamnative#571 

## Motivation
When client fetch metadata need check topic permission, so we need add authorization in handleTopicMetadataRequest, and do not perform role verification in authentication.

## Modifications
Add a common method in `KafkaRequestHandler#authorize` , this method use `authorizer` to authorization.
Modify the authentication behavior, and do not verify the role during authentication, verify the role in fetch metadata(streamnative#571  )
BewareMyPower pushed a commit that referenced this issue Aug 25, 2021
Add authorization to handleTopicMetadataRequest(#236 ).

Fix #415 and #571 

## Motivation
When client fetch metadata need check topic permission, so we need add authorization in handleTopicMetadataRequest, and do not perform role verification in authentication.

## Modifications
Add a common method in `KafkaRequestHandler#authorize` , this method use `authorizer` to authorization.
Modify the authentication behavior, and do not verify the role during authentication, verify the role in fetch metadata(#571  )
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants