> I think the right solution to this problem is better documentation. Sounds good, but I worry that the user cannot set up the correct configuration.
As infrastructure, we need to ensure the security of users at all times. > The main users that need to know how to configure broker side authorization are using MultiRolesTokenAuthorizationProvider or their own custom implementation of the AuthorizationProvider interface. Good idea to improve the documentation! Michael Marshall <mmarsh...@apache.org> 于2022年11月2日周三 13:42写道： > > We have a flag to control the value of authentication data. See > > > https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java#L316-L322 > > Great point. I missed the `forwardAuthorizationCredentials` setting in > my analysis, and I made the wrong conclusion. > > > I suggest we remove this config to make logic clean, don't edit this by > the > > user. > > I think we should follow the principle of least privilege and only > forward the client's auth data when the operator has configured the > proxy to do so. In the default authorization framework, the Pulsar > Broker doesn't need the auth data in order to perform authorization. > Interestingly, the recent Proxy to Broker man in the middle > vulnerability would technically have been worse (in some cases) for > operators that had forwardAuthorizationCredentials=true. > > I think the right solution to this problem is better documentation. > The main users that need to know how to configure broker side > authorization are using MultiRolesTokenAuthorizationProvider or their > own custom implementation of the AuthorizationProvider interface. > > Thanks, > Michael > > On Mon, Oct 31, 2022 at 10:58 PM Zixuan Liu <node...@gmail.com> wrote: > > > > > This is already the case for both HTTP and pulsar protocols > > > > We have a flag to control the value of authentication data. See > > > https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java#L316-L322 > > . > > > > > Is it sufficient to enable authenticateOriginalAuthData? > > > > We need to set the `forwardAuthorizationCredentials=true` in the > > `proxy.conf` and `authenticateOriginalAuthData` in the `broker.conf`. > > > > I suggest we remove this config to make logic clean, don't edit this by > the > > user. > > > > > > Michael Marshall <mmarsh...@apache.org> 于2022年11月1日周二 04:25写道： > > > > > Thanks for starting this thread, Zixuan. > > > > > > For additional context, I provided some related feedback in comments > > > on this PR: https://github.com/apache/pulsar/pull/18130. > > > > > > > So I suggest the proxy should always forward the authentication data > from > > > > the client. > > > > > > This is already the case for both HTTP and pulsar protocols: > > > > > > > https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/DirectProxyHandler.java#L327-L328 > > > > > > > https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyClientCnx.java#L62-L64 > > > > > > I investigated the code snippet referenced above (one example linked > > > here [0]), and I noticed that the main difference comes from this > > > broker setting "authenticateOriginalAuthData". > > > > > > When `authenticateOriginalAuthData` is set to false, > > > `originalAuthDataSource` is always null in the ServerCnx. This looks > > > like a consequence of how the `originalAuthDataSource` is built > > > because the authentication provider builds the `originalAuthState`, > > > which then builds the `originalAuthDataSource`. See [1]. > > > > > > Is it sufficient to enable authenticateOriginalAuthData? > > > > > > Thanks, > > > Michael > > > > > > [0] > > > > https://github.com/apache/pulsar/blob/8f8637a75e05f271bdc8fa2081284d39bc5de972/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L407-L409 > > > [1] > > > > https://github.com/apache/pulsar/blob/8f8637a75e05f271bdc8fa2081284d39bc5de972/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L906-L942 > > > > > > On Mon, Oct 31, 2022 at 5:29 AM Zixuan Liu <node...@gmail.com> wrote: > > > > > > > > Hi all, > > > > > > > > I want to discuss the authentication data issue, which affects the > > > > authorization operation. > > > > > > > > For the default to authorization provider, we only used the role to > check > > > > the permission, the authentication data was ignored. When a user > wants to > > > > customize an authorization provider, the user can care for the > > > > authentication data and role, sometimes the Pulsar cannot pass the > > > correct > > > > authentication data to the authorization provider. > > > > > > > > So like: > > > > ``` > > > > if (originalPrincipal != null) { > > > > isProxyAuthorizedFuture = > > > > service.getAuthorizationService().allowTopicOperationAsync( > > > > topicName, operation, originalPrincipal, > > > > originalAuthDataSource != null ? originalAuthDataSource : > > > > authDataSource); > > > > } > > > > ``` > > > > > > > > For the above code, when `originalAuthDataSource` is null, use the > > > > `authDataSource` instead. This results in a mismatch between the > > > > authentication data and the role. > > > > > > > > The `originalAuthDataSource` is the authentication data of the user > > > client > > > > forwarded by the proxy. When the proxy doesn't forward this > > > authentication > > > > data, we cannot get the correct authentication data in the > authorization > > > > provider. > > > > > > > > So I suggest the proxy should always forward the authentication data > from > > > > the client. Another important reason is that we usually check the > > > > permission of the user client, not the proxy client. > > > > > > > > Please let me know your idea. > > > > > > > > Thanks, > > > > Zixuan > > > >
