michaeljmarshall commented on PR #19455: URL: https://github.com/apache/pulsar/pull/19455#issuecomment-1464036418
> In a pulsar cluster there are not only super roles, but also non-super roles, so authorization is still required Sure, and that is why this PR is necessary for the older branches. If the proxy has a super user role that is not in the `proxyRoles` configured by the broker and the proxy is using mTLS to authenticate with the broker, all clients going through the proxy inherit the proxy's superuser role. > Before this change, if a client wanted to connect to the cluster using the super role, the role had to be configured in the proxy's superUserRoles, otherwise the proxy would not be able to authenticate it, after this change, if a client connected to the cluster using the super role, the role could not appear in superUserRoles of the proxy and proxyRoles of the broker, otherwise the connect also will fail, which seems to be an opposite behavior This analysis does not match my understanding of the PR. The proxy's `superUserRoles` has nothing to do with this change. This PR only changes the broker's logic to require an `originalPrincipal` to be supplied, the `role` must be one of the `proxyRoles` in the broker.conf. The core logic is here: https://github.com/apache/pulsar/blob/d4be954dedcc7537b3d65b9a1d7b5662e6062fdf/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java#L332-L335 It sounds like the problem you're encountering is that your proxy's role is not in the `proxyRoles` list. The tests modified by this PR support my understanding of this change. > I think this is a security enhancement, not a vulnerability (I'm sure you agree) I disagree that this is only an enhancement. This change protects operators from dangerous misconfigurations. The only way this is not a vulnerability is if we agree that a proxy is supposed to connect with a role in the `proxyRoles`. It is a vulnerability if we decide that the `proxyRoles` list is irrelevant because it's not the user misconfiguring things but rather pulsar doing the wrong thing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
