> On Jan. 6, 2023, 3:58 a.m., Kirby Zhou wrote: > > Ship It! > > Ramachandran Krishnan wrote: > Hi Madhan, > Based on Kirby Zhou review comments ,we addressed the security constraint > when serviceName and zoneName is not passed > I believe this fix will cover all the edge cases as well . > > Madhan Neethiraj wrote: > When both serviceName and zoneName are not provided, the lookup should > only be based on guid. Why restrict to only UNZONED_SECURITY_ZONE_ID? > > @Kirby - can you please share more details of security issue in looking > for policy with guid only? > > Ramachandran Krishnan wrote: > Hi Madhan/Kirby, > Security Zone is a feature that provides an ability in Ranger to separate > resource policies into different zones. > It also enables multiple administrators to setup different policies, > based on the zones that they are assigned to. > In this case ,we might fetch the policies which are tagged with some > security zone .I feel this could be security thread > when Sales Admin see the policy which tagged with Finance Admin.Due to > that ,we added defaulut zone Id which is not tied to any security Zone > Please correct me If I am wrong > > Madhan Neethiraj wrote: > Ramachandran - before a policy is returned to the caller, Ranger ensures > that the caller has appropriate privileges. If caller doesn't have privilege > to view a policy, error code 403 (SC_FORBIDDEN) will be returned - refer to > ServiceREST.ensureAdminAndAuditAccess(policy). There is no security concern > here. > > Ramachandran Krishnan wrote: > Thanks Madhan for pointing out .If the caller doesn't have privilege to > view a policy, error code 403 (SC_FORBIDDEN) will be returned. > So it will not leak any security contraint.It makes sense > Kirby, > It would be great if you elobrate a bit where this will create security > concenrn when we are not passing default Zone Id > > Kirby Zhou wrote: > Madhan is right, ServiceREST.ensureAdminAndAuditAccess seems is enough to > prenvet unauthorized access when there is no zone id passed. > It is not a security risk now. > > But my concern is that the semantics of API have been changed and > inconsistent. > > In the old code: > if guid, serviceName and zoneName is given, it returns policy match guid, > serviceName and zoneName together, > if only guid and serviceName is given, it returns policy match guid, > serviceName and RANGER_UNZONED_SECURITY_ZONE_ID together. > > I think guid+zoneName / guid only based queries should follow the same > principle as above. > > It may confuse some automatic processes which believe that the returned > policies are always in the given zone ( or unzoned ). > > Ramachandran Krishnan wrote: > Madhan/Kirby, > It would be great if we finalize the things whether we can keep the > default zoneId along with guid when zoneName is not passed like old code way > or Do we need to strict to guid only when zoneName is not passed
This change in REST API to retrieve a policy by GUI was necessary to deal with the case where the caller doesn't know the service name. How does this patch deal with cases where the caller doesn't know the zone name? The search is restricted only to UNZONED i.e. will exclude policies in security zones. This doesn't look correct. Hence I suggested to not add zoneId filter in this case. > It may confuse some automatic processes which believe that the returned > policies are always in the given zone ( or unzoned ). Kirby - in this case, would the automatic process know of the security-zone in which to find the policies? If not, it will fail to retrieve the policy simply by searching for guid - as the search will only look for policies in UNZONED. Is is alright? - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74268/#review225068 ----------------------------------------------------------- On Jan. 5, 2023, 10:15 a.m., Ramachandran Krishnan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74268/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2023, 10:15 a.m.) > > > Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, > Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, > Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy. > > > Bugs: RANGER-4031 > https://issues.apache.org/jira/browse/RANGER-4031 > > > Repository: ranger > > > Description > ------- > > Not able to fetch Policy details using guid /api/policy/guid/{guid} without > service name > > Request without servicename > > curl -s -L -X GET > "https://q************/service/public/v2/api/policy/guid/****-2f42-4451-9edf-****"; > -H "Content-Type: application/json" -H "Accept: application/json" -H > "Authorization: Basic *********DEyMw==" > Response : 404 > > Request with servicename > > curl -s -L -X GET > "https://****************/service/public/v2/api/policy/guid/*****-2f42-4451-9edf-****?serviceName=hive"; > -H "Content-Type: application/json" -H "Accept: application/json" -H > "Authorization: Basic ***************==" > Response Proper : 200 with proper details > > Code : > https://github.com/apache/ranger/blob/master/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java#L505 > > @GET @Path("/api/policy/guid/{guid}") > @Produces({ "application/json", "application/xml" }) > public RangerPolicy > getPolicyByGUIDAndServiceNameAndZoneName(@PathParam("guid") String guid, > > @DefaultValue("") > @QueryParam("serviceName") String serviceName, > > @DefaultValue("") @QueryParam("ZoneName") String zoneName) { > return > serviceREST.getPolicyByGUIDAndServiceNameAndZoneName(guid, serviceName, > zoneName); } > As query parameters are optional it should give proper response > > Expected : User should be able to get policy details using only guid in path > params > > > As part of the current design, Ranger expects both serviceName,guid should be > mandatory and zoneName can be optional > Proposal: > Add the logic to fetch the RangerPolicy by guid from the backend > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > 6b9604817 > security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java > 37d7561d4 > security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java > c7a6ea0a6 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > e17494fa9 > security-admin/src/main/resources/META-INF/jpa_named_queries.xml 85c8b6213 > security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java > 7f1ec6d3e > security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java > 2a123de93 > security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java > 7b15810e0 > > > Diff: https://reviews.apache.org/r/74268/diff/6/ > > > Testing > ------- > > > Thanks, > > Ramachandran Krishnan > >
