> On 一月 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 > > Madhan Neethiraj wrote: > 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 - in this case, the automatic process should know its zone , or it just want to find in unzoned guids. A compromise method is that we add a parameter to indicate whether to search in all zones or in the unzoned range. Such as "ZoneName=_ALL_" or "ZoneName=_UNZONED_" ? - Kirby ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74268/#review225068 ----------------------------------------------------------- On 一月 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 一月 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 > >
