[GitHub] [hbase] joshelser commented on issue #936: HBASE-17115 Define UI admins via an ACL
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
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
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
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
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
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
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
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
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
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