Re: [DISCUSS] Remove the need for ADMIN permissions for listDecommissionedRegionServers

2024-02-27 Thread Rushabh Shah 
Thank you Viraj and Andrew for the replies.
I have merged HBASE-28391
  to all the active
branches.

On Tue, Feb 27, 2024 at 9:05 AM Andrew Purtell  wrote:

> We have had this discussion before about other list* methods and we have
> sometimes decided to restrict them to ADMIN. The reason for that was the
> information returned by the method might leak sensitive information. For
> example, listing table descriptors will include all arbitrary and
> potentially sensitive user set attributes in the schema.
> I think here the information listed is not sensitive in the same way. Host
> cluster membership, and especially decommissioned hosts, is not secret.
>
> Compatibility should be fine. Someone granted ADMIN permission will still
> be able to invoke this method if the security check is relaxed.
>
> +1
>
> On Tue, Feb 27, 2024 at 8:32 AM Viraj Jasani  wrote:
>
> > +1 for relaxing the permission. While I haven't gone through the history,
> > it seems that requiring ADMIN for listDecomm operation might be an
> > oversight.
> >
> > Unless it is really big deal from compatibility viewpoint, I think we
> > should be fine relaxing this.
> >
> >
> >
> > On Mon, Feb 26, 2024 at 8:55 PM Rushabh Shah
> >  wrote:
> >
> > > Hi hbase-dev,
> > >
> > > Why do we need ADMIN permissions for
> > > AccessController#preListDecommissionedRegionServers
> > > API ?
> > >
> > > From Phoenix, we are calling Admin#getRegionServers(true) where the
> > > argument excludeDecommissionedRS is set to true. [1]
> > > If excludeDecommissionedRS  is set to true and if we have
> > > AccessController co-proc
> > > attached, it requires ADMIN permissions to execute
> > > listDecommissionedRegionServers RPC. [2]
> > >  Snippet below
> > >
> > >   @Override
> > >   public void
> > >
> >
> preListDecommissionedRegionServers(ObserverContext
> > > ctx)
> > > throws IOException {
> > > requirePermission(ctx, "listDecommissionedRegionServers",
> > > Action.ADMIN);
> > >   }
> > >
> > > I understand that we need ADMIN permissions
> > > for preDecommissionRegionServers and preRecommissionRegionServers
> because
> > > it changes the membership of regionservers but I don’t see any need for
> > > ADMIN permissions for listDecommissionedRegionServers.
> > >
> > > Does anyone have objections if we relax the requirement to READ
> > permissions
> > > instead of ADMIN permissions?
> > >
> > > I have created HBASE-28391
> > > <
> https://urldefense.com/v3/__https://issues.apache.org/jira/browse/HBASE-28391__;!!DCbAVzZNrAf4!BkLK4S-sG0Soms4PFYK9G420e4QQdVUdjjz7x0PfOtSq0cvPOLehISZxTqx2Y1tc-bIayELNPNlK6q08dBVJJ4o$
> > to implement this.
> > > Thank you !
> > >
> > >
> > > 1.
> > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/hbase/blob/branch-2.5/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java*L1721-L1730__;Iw!!DCbAVzZNrAf4!BkLK4S-sG0Soms4PFYK9G420e4QQdVUdjjz7x0PfOtSq0cvPOLehISZxTqx2Y1tc-bIayELNPNlK6q085ZMR_0M$
> > >
> > > 2.
> > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/hbase/blob/branch-2.5/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java*L1205-L1207__;Iw!!DCbAVzZNrAf4!BkLK4S-sG0Soms4PFYK9G420e4QQdVUdjjz7x0PfOtSq0cvPOLehISZxTqx2Y1tc-bIayELNPNlK6q08LXwSMKo$
> > >
> >
>
>
> --
> Best regards,
> Andrew
>
> Unrest, ignorance distilled, nihilistic imbeciles -
> It's what we’ve earned
> Welcome, apocalypse, what’s taken you so long?
> Bring us the fitting end that we’ve been counting on
>- A23, Welcome, Apocalypse
>

Re: [DISCUSS] Remove the need for ADMIN permissions for listDecommissionedRegionServers

2024-02-27 Thread Andrew Purtell 
We have had this discussion before about other list* methods and we have
sometimes decided to restrict them to ADMIN. The reason for that was the
information returned by the method might leak sensitive information. For
example, listing table descriptors will include all arbitrary and
potentially sensitive user set attributes in the schema.
I think here the information listed is not sensitive in the same way. Host
cluster membership, and especially decommissioned hosts, is not secret.

Compatibility should be fine. Someone granted ADMIN permission will still
be able to invoke this method if the security check is relaxed.

+1

On Tue, Feb 27, 2024 at 8:32 AM Viraj Jasani  wrote:

> +1 for relaxing the permission. While I haven't gone through the history,
> it seems that requiring ADMIN for listDecomm operation might be an
> oversight.
>
> Unless it is really big deal from compatibility viewpoint, I think we
> should be fine relaxing this.
>
>
>
> On Mon, Feb 26, 2024 at 8:55 PM Rushabh Shah
>  wrote:
>
> > Hi hbase-dev,
> >
> > Why do we need ADMIN permissions for
> > AccessController#preListDecommissionedRegionServers
> > API ?
> >
> > From Phoenix, we are calling Admin#getRegionServers(true) where the
> > argument excludeDecommissionedRS is set to true. [1]
> > If excludeDecommissionedRS  is set to true and if we have
> > AccessController co-proc
> > attached, it requires ADMIN permissions to execute
> > listDecommissionedRegionServers RPC. [2]
> >  Snippet below
> >
> >   @Override
> >   public void
> >
> preListDecommissionedRegionServers(ObserverContext
> > ctx)
> > throws IOException {
> > requirePermission(ctx, "listDecommissionedRegionServers",
> > Action.ADMIN);
> >   }
> >
> > I understand that we need ADMIN permissions
> > for preDecommissionRegionServers and preRecommissionRegionServers because
> > it changes the membership of regionservers but I don’t see any need for
> > ADMIN permissions for listDecommissionedRegionServers.
> >
> > Does anyone have objections if we relax the requirement to READ
> permissions
> > instead of ADMIN permissions?
> >
> > I have created HBASE-28391
> >  to implement this.
> > Thank you !
> >
> >
> > 1.
> >
> >
> https://github.com/apache/hbase/blob/branch-2.5/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java#L1721-L1730
> >
> > 2.
> >
> >
> https://github.com/apache/hbase/blob/branch-2.5/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java#L1205-L1207
> >
>


-- 
Best regards,
Andrew

Unrest, ignorance distilled, nihilistic imbeciles -
It's what we’ve earned
Welcome, apocalypse, what’s taken you so long?
Bring us the fitting end that we’ve been counting on
   - A23, Welcome, Apocalypse

Re: [DISCUSS] Remove the need for ADMIN permissions for listDecommissionedRegionServers

2024-02-27 Thread Viraj Jasani 
+1 for relaxing the permission. While I haven't gone through the history,
it seems that requiring ADMIN for listDecomm operation might be an
oversight.

Unless it is really big deal from compatibility viewpoint, I think we
should be fine relaxing this.



On Mon, Feb 26, 2024 at 8:55 PM Rushabh Shah
 wrote:

> Hi hbase-dev,
>
> Why do we need ADMIN permissions for
> AccessController#preListDecommissionedRegionServers
> API ?
>
> From Phoenix, we are calling Admin#getRegionServers(true) where the
> argument excludeDecommissionedRS is set to true. [1]
> If excludeDecommissionedRS  is set to true and if we have
> AccessController co-proc
> attached, it requires ADMIN permissions to execute
> listDecommissionedRegionServers RPC. [2]
>  Snippet below
>
>   @Override
>   public void
> preListDecommissionedRegionServers(ObserverContext
> ctx)
> throws IOException {
> requirePermission(ctx, "listDecommissionedRegionServers",
> Action.ADMIN);
>   }
>
> I understand that we need ADMIN permissions
> for preDecommissionRegionServers and preRecommissionRegionServers because
> it changes the membership of regionservers but I don’t see any need for
> ADMIN permissions for listDecommissionedRegionServers.
>
> Does anyone have objections if we relax the requirement to READ permissions
> instead of ADMIN permissions?
>
> I have created HBASE-28391
>  to implement this.
> Thank you !
>
>
> 1.
>
> https://github.com/apache/hbase/blob/branch-2.5/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java#L1721-L1730
>
> 2.
>
> https://github.com/apache/hbase/blob/branch-2.5/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java#L1205-L1207
>

[DISCUSS] Remove the need for ADMIN permissions for listDecommissionedRegionServers

2024-02-26 Thread Rushabh Shah 
Hi hbase-dev,

Why do we need ADMIN permissions for
AccessController#preListDecommissionedRegionServers
API ?

>From Phoenix, we are calling Admin#getRegionServers(true) where the
argument excludeDecommissionedRS is set to true. [1]
If excludeDecommissionedRS  is set to true and if we have
AccessController co-proc
attached, it requires ADMIN permissions to execute
listDecommissionedRegionServers RPC. [2]
 Snippet below

  @Override
  public void 
preListDecommissionedRegionServers(ObserverContext
ctx)
throws IOException {
requirePermission(ctx, "listDecommissionedRegionServers", Action.ADMIN);
  }

I understand that we need ADMIN permissions
for preDecommissionRegionServers and preRecommissionRegionServers because
it changes the membership of regionservers but I don’t see any need for
ADMIN permissions for listDecommissionedRegionServers.

Does anyone have objections if we relax the requirement to READ permissions
instead of ADMIN permissions?

I have created HBASE-28391
 to implement this.
Thank you !


1.
https://github.com/apache/hbase/blob/branch-2.5/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java#L1721-L1730

2.
https://github.com/apache/hbase/blob/branch-2.5/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java#L1205-L1207