Avoid AuthenticationDataSource mutation for subscription name #16065

merlimat · 2022-06-14T22:43:39Z

Motivation

The authenticationData field in ServerCnx is being mutated to add the subscription field that will be passed on to the authorization plugin. The problem is that authenticationData is scoped to the whole connection and it should be getting mutated for each consumer that is created on the connection.

The current code leads to a race condition where the subscription name used in the authz plugin is already modified while we're looking at it. Instead, we should create a new object and enforce the final modifier.

The `authenticationData` field in `ServerCnx` is being mutated to add the `subscription` field that will be passed on to the authorization plugin. The problem is that `authenticationData` is scoped to the whole connection and it should be getting mutated for each consumer that is created on the connection. The current code leads to a race condition where the subscription name used in the authz plugin is already modified while we're looking at it. Instead, we should create a new object and enforce the final modifier. (cherry picked from commit e6b12c6)

### Motivation The `authenticationData` field in `ServerCnx` is being mutated to add the `subscription` field that will be passed on to the authorization plugin. The problem is that `authenticationData` is scoped to the whole connection and it should be getting mutated for each consumer that is created on the connection. The current code leads to a race condition where the subscription name used in the authz plugin is already modified while we're looking at it. Instead, we should create a new object and enforce the final modifier. (cherry picked from commit e6b12c6)

### Motivation #16065 fixes the race condition issue, but introduces a new issue. This issue is triggered when the Proxy and Broker work together, when we use the proxy to request the broker to do lookup/subscribe/produce operation, the broker always uses the original authentication data for authorization, not proxy authentication data, which causes this issue. ### Modification - Fix passing authentication data, differentiate between original auth data and connected auth data by avoid to use the `getAuthenticationData()`, this method name is easy to cause confusion and can not correctly get the authentication data

### Motivation #16065 fixes the race condition issue, but introduces a new issue. This issue is triggered when the Proxy and Broker work together, when we use the proxy to request the broker to do lookup/subscribe/produce operation, the broker always uses the original authentication data for authorization, not proxy authentication data, which causes this issue. ### Modification - Fix passing authentication data, differentiate between original auth data and connected auth data by avoid to use the `getAuthenticationData()`, this method name is easy to cause confusion and can not correctly get the authentication data (cherry picked from commit 936bbbc)

### Motivation apache#16065 fixes the race condition issue, but introduces a new issue. This issue is triggered when the Proxy and Broker work together, when we use the proxy to request the broker to do lookup/subscribe/produce operation, the broker always uses the original authentication data for authorization, not proxy authentication data, which causes this issue. ### Modification - Fix passing authentication data, differentiate between original auth data and connected auth data by avoid to use the `getAuthenticationData()`, this method name is easy to cause confusion and can not correctly get the authentication data (cherry picked from commit 936bbbc) Signed-off-by: Zixuan Liu <nodeces@gmail.com>

…#16065) ### Motivation The `authenticationData` field in `ServerCnx` is being mutated to add the `subscription` field that will be passed on to the authorization plugin. The problem is that `authenticationData` is scoped to the whole connection and it should be getting mutated for each consumer that is created on the connection. The current code leads to a race condition where the subscription name used in the authz plugin is already modified while we're looking at it. Instead, we should create a new object and enforce the final modifier. (cherry picked from commit e6b12c6) (cherry picked from commit 5cc3649)

### Motivation apache#16065 fixes the race condition issue, but introduces a new issue. This issue is triggered when the Proxy and Broker work together, when we use the proxy to request the broker to do lookup/subscribe/produce operation, the broker always uses the original authentication data for authorization, not proxy authentication data, which causes this issue. ### Modification - Fix passing authentication data, differentiate between original auth data and connected auth data by avoid to use the `getAuthenticationData()`, this method name is easy to cause confusion and can not correctly get the authentication data (cherry picked from commit 936bbbc) (cherry picked from commit adf5ce7)

### Motivation apache#16065 fixes the race condition issue, but introduces a new issue. This issue is triggered when the Proxy and Broker work together, when we use the proxy to request the broker to do lookup/subscribe/produce operation, the broker always uses the original authentication data for authorization, not proxy authentication data, which causes this issue. ### Modification - Fix passing authentication data, differentiate between original auth data and connected auth data by avoid to use the `getAuthenticationData()`, this method name is easy to cause confusion and can not correctly get the authentication data (cherry picked from commit 936bbbc) Signed-off-by: Zixuan Liu <nodeces@gmail.com>

) ### Motivation #16065 fixes the race condition issue, but introduces a new issue. This issue is triggered when the Proxy and Broker work together, when we use the proxy to request the broker to do lookup/subscribe/produce operation, the broker always uses the original authentication data for authorization, not proxy authentication data, which causes this issue. ### Modification - Fix passing authentication data, differentiate between original auth data and connected auth data by avoid to use the `getAuthenticationData()`, this method name is easy to cause confusion and can not correctly get the authentication data (cherry picked from commit 936bbbc) Signed-off-by: Zixuan Liu <nodeces@gmail.com>

…#16065) The `authenticationData` field in `ServerCnx` is being mutated to add the `subscription` field that will be passed on to the authorization plugin. The problem is that `authenticationData` is scoped to the whole connection and it should be getting mutated for each consumer that is created on the connection. The current code leads to a race condition where the subscription name used in the authz plugin is already modified while we're looking at it. Instead, we should create a new object and enforce the final modifier. (cherry picked from commit e6b12c6) (cherry picked from commit 88b51b1)

### Motivation apache#16065 fixes the race condition issue, but introduces a new issue. This issue is triggered when the Proxy and Broker work together, when we use the proxy to request the broker to do lookup/subscribe/produce operation, the broker always uses the original authentication data for authorization, not proxy authentication data, which causes this issue. ### Modification - Fix passing authentication data, differentiate between original auth data and connected auth data by avoid to use the `getAuthenticationData()`, this method name is easy to cause confusion and can not correctly get the authentication data (cherry picked from commit 936bbbc) Signed-off-by: Zixuan Liu <nodeces@gmail.com>

) ### Motivation #16065 fixes the race condition issue, but introduces a new issue. This issue is triggered when the Proxy and Broker work together, when we use the proxy to request the broker to do lookup/subscribe/produce operation, the broker always uses the original authentication data for authorization, not proxy authentication data, which causes this issue. ### Modification - Fix passing authentication data, differentiate between original auth data and connected auth data by avoid to use the `getAuthenticationData()`, this method name is easy to cause confusion and can not correctly get the authentication data (cherry picked from commit 936bbbc) Signed-off-by: Zixuan Liu <nodeces@gmail.com>

Avoid AuthenticationDataSource mutation for subscription name

b25ed40

merlimat added type/bug The PR fixed a bug or issue reported a bug release/2.8.4 release/2.10.2 release/2.9.4 labels Jun 14, 2022

merlimat added this to the 2.11.0 milestone Jun 14, 2022

merlimat requested review from codelipenghui, BewareMyPower and aloyszhang June 14, 2022 22:43

merlimat self-assigned this Jun 14, 2022

Removed imports

8097a61

nodece approved these changes Jun 15, 2022

View reviewed changes

BewareMyPower approved these changes Jun 15, 2022

View reviewed changes

michaeljmarshall approved these changes Jun 15, 2022

View reviewed changes

michaeljmarshall requested review from eolivelli and lhotari June 15, 2022 06:11

codelipenghui approved these changes Jun 15, 2022

View reviewed changes

codelipenghui merged commit e6b12c6 into apache:master Jun 15, 2022

codelipenghui added the release/2.7.6 label Jun 15, 2022

codelipenghui added cherry-picked/branch-2.7 Archived: 2.7 is end of life cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 labels Jun 15, 2022

merlimat deleted the fix-auth-data-mutation branch June 15, 2022 18:04

nodece mentioned this pull request Jun 23, 2022

[fix][broker] Fix passing incorrect authentication data #16201

Merged

2 tasks

nodece mentioned this pull request Jul 28, 2022

[fix][broker][branch-2.8] Fix passing incorrect authentication data (#16201) #16840

Merged

4 tasks

Jason918 added release/2.7.5 and removed release/2.7.6 labels Jul 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid AuthenticationDataSource mutation for subscription name #16065

Avoid AuthenticationDataSource mutation for subscription name #16065

merlimat commented Jun 14, 2022

Avoid AuthenticationDataSource mutation for subscription name #16065

Avoid AuthenticationDataSource mutation for subscription name #16065

Conversation

merlimat commented Jun 14, 2022

Motivation