michaeljmarshall opened a new issue, #15597: URL: https://github.com/apache/pulsar/issues/15597
## Motivation Pulsar supports subscription level authorization. When combined with topic level authorization, a user can configure Pulsar to limit which roles can consume from which topic subscriptions. However, when this feature is left unconfigured for a subscription, a role that has permission to consume from a topic is, by default, implicitly granted permission to consume from any subscription on that topic. As a consequence, a missed security configuration could lead to accidental privilege escalation. Here is a reference to the code responsible for the current behavior: https://github.com/apache/pulsar/blob/6864b0ae5520e06b9d0fc5dcfa5a0a0a44feee87/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java#L115-L122 ## Goal I propose we add a namespace policy to configure a Pulsar namespace to either allow all or reject all roles when there is no configuration for a specific subscription’s permission. This way, a missed configuration results in a rejected request due to insufficient permission. This PIP will not change the current behavior and will be backwards compatible. It will add a new boolean field to the existing `auth_policies` namespace policy to configure how the `PulsarAuthorizationProvider` handles an empty set of allowed roles in the `canConsume` method. ## Naming I am not settled on the right name for this feature/namespace policy yet. Hopefully this thread can help identify the right name. First, the existing subscription level authorization feature has several names. The Admin API calls this feature `PermissionOnSubscription`, the Pulsar Admin CLI tool calls it `subscription-permission`, the AuthPolicies interface calls it `SubscriptionAuthentication`, and the value is stored in the metadata store as `subscription_auth_roles`. My preferred names for this feature are `implicit_subscription_auth` and `implicitPermissionOnSubscription` because they work well with the “grant” and “revoke” actions, e.g. `grantImplicitPermissionOnSubscription` would be a PUT/POST call to the `/implicitPermissionOnSubscription` endpoint to set the policy value to true. However, that policy name requires the default value to be true to maintain backwards compatibility. Enrico expressed concern that defaulting to true is problematic for the upgrade path: https://github.com/apache/pulsar/pull/15576#discussion_r872045946. Alternatively, we could use the names `PermissionOnSubscriptionRequired` and `subscription_auth_required`. In that case, I would switch the admin API so that the admin API has a single setter endpoint that takes the configuration as a part of the body instead of relying on PUT to mean grant permission and DELETE to mean revoke permission. Please let me know if you have thoughts on what name(s) make sense for this feature. ## API Changes The API changes include updating the Admin API to enable getting and modifying the namespace policy, as well as updating the namespace AuthPolicy interface to store this new metadata field. ## Implementation Draft implementation: https://github.com/apache/pulsar/pull/15576 The core update is to the `PulsarAuthorizationProvider#canConsumeAsync` method so that when `implicit_subscription_auth` is true, a null or empty set of roles for a subscription’s permission will result in granted permission to consume from the subscription, and when `implicit_subscription_auth` is false, a null or empty set of roles for a subscription’s permission will result in rejected permission to consume from the subscription. Note that if we negate the meaning of the variable name, the logic will also be inverted appropriately. ## Rejected Alternatives First, we have already received a PR proposing to change the default behavior for all use cases: https://github.com/apache/pulsar/pull/11113. This PR went stale because changing the default would break many deployments. By making the behavior configurable, we satisfy the requirements requested in that PR without breaking existing deployments. Second, it’s possible to implement a new `AuthorizationProvider` to get this feature. However, I believe this feature will benefit users and is a natural development on the existing Pulsar features around subscription authorization, so I think we should include it in the default `PulsarAuthorizationProvider`. -- 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]
