[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-29 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-57773
 
 
   Submitted to all 2.x branches and master, after getting HBASE-20950 also 
backported to branch-2.1.
   
   1.x needs some help with precommit before this change can land there.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-27 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-579031784
 
 
   HBASE-20950 isn't contained on branch-2.1 which means I have to fix up 
TestInfoServersAcl.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-27 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-578803799
 
 
   Any more requests on this?


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-24 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-578303726
 
 
   Rebase'd on top of master and made `/metrics` and `/jmx` privileged.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-24 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-578211471
 
 
   Poking around at a local install -- looks like the DumpServlet is now 
requiring admins even when auth'n for the UI is off :)


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-23 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-577976481
 
 
   Alright, I think the last commit does this right now.
   
   There was a problem in my previous patches in that the API I added -- trying 
to have `privileged` and `unprivileged` methods for adding a servlet to the 
HttpServer were half-baked. I have this working now so that we don't have to be 
injecting authz logic into every servlet we write. Just, when we add it to the 
HttpServer/InfoServer, we call the appropriate method to restrict (or not) 
access to admins only.
   
   I added some more unit tests which show that both the contexts (e.g. 
`/logs/`) and the servlets (e.g. `/dump`) both work for admins and reject it 
for non-admins. There was some trickiness in cleaning this up: we have a bit of 
cruft in the HttpServer logic.
   * We need to add our "default apps"
   * Then add the filters we want to apply globally (e.g. spnego, security 
headers, etc)
   * Then we add all the servlets, optionally adding in the new 
`AdminAuthorizedFilter` when we register that filter
   * * That new Filter is the piece which, added at the end of the filter chain 
(meaning, after all of our other filters we expect to run all the time), will 
stop callers from accessing that protected servlet if you're not an admin 
(`HTTP/403`).


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-22 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-577480036
 
 
   I just need to write a UT to cover this stuff. Just realized I hadn't done 
that.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-22 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-577413620
 
 
   Turns out I'm still missing the critical bits for everything besides the 
`/logs/` http endpoint. Looking in hadoop to see how they do this because I 
have no idea how the existing AdminAuthorizedServlet is supposed to be used (I 
would have thought it should be a Filter..)


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2020-01-17 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-575793728
 
 
   > I don't see the logs, ... pages listed. Those should be limited to admins 
too?
   
   
https://github.com/apache/hbase/blob/978546b2f247b29dd63bad55b17fdc2e7a31e55d/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java#L681-L683
   
   The logs servlet already had the AdminAuthorizedServlet in the chain; it was 
just ineffective because we weren't configuring/setting an `ADMIN_ACL`.
   
   > I don't see the ... debug, ... pages listed. Those should be limited to 
admins too?
   
   Good catch! Was missing this, will fix.
   
   > I don't see the ... zk dump pages listed. Those should be limited to 
admins too?
   
   I'm not finding this, @busbey . I need to go looking to see if this is a 
branch-specific feature.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL

2019-12-12 Thread GitBox
joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
URL: https://github.com/apache/hbase/pull/936#issuecomment-565191500
 
 
   Testing I've done:
   * New unit tests
   * Explicit admin user defined in configuration 
(`hbase.security.authentication.spnego.admin.users`)
   * `curl` with both the admin and a non-admin
   * The above along with `hbase.master.ui.readonly=true` as well.
   
   If admins are set, that will limit who can interact with sensitive 
endpoints. Setting readonly=true will further restrict the system and disallow 
anyone (including admins) from modifying hbase via the UI.
   
   I think this maintains all of the previous semantics folks would expect, 
while letting the security-conscious lock things down.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services