From 6678ceb51b2b9411cab88a5d031a71d4bc97199a Mon Sep 17 00:00:00 2001 From: Nathan Heldt-Sheller Date: Wed, 23 Nov 2016 12:20:52 -0800 Subject: [PATCH] [IOT-1595] Change Policy Engine to us ACE Union behavior. The current Policy Engine logic is to assess the permissions on the first matching ACE for a request (matched via Subject and Resource), and respond to the request (Grant or Deny) based on that ACE. The new OCF 1.0 behavior specifies that if any ACE allows a request, it should be Granted (so-called "Union" behavior). To allow consistency we must fix this in 1.2.1. This patch changes the Policy Engine to keep searching for an ACE that Grants the request, until either the request is granted, or the end of the ACL is reached. Change-Id: Idd4e90c37c7e0fcf963105b34b3e82dfde2ccfd2 Signed-off-by: Nathan Heldt-Sheller Reviewed-on: https://gerrit.iotivity.org/gerrit/14701 Reviewed-by: Kevin Kane Tested-by: jenkins-iotivity Reviewed-by: Greg Zaverucha (cherry picked from commit 9524976da93a87f1b74e550a672a431e63e858f3) Reviewed-on: https://gerrit.iotivity.org/gerrit/14717 Reviewed-by: Randeep Singh --- resource/csdk/security/include/internal/policyengine.h | 1 - resource/csdk/security/src/amsmgr.c | 2 +- resource/csdk/security/src/policyengine.c | 9 ++++----- resource/csdk/security/unittest/policyengine.cpp | 1 - 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/resource/csdk/security/include/internal/policyengine.h b/resource/csdk/security/include/internal/policyengine.h index f8b333c..66b399c 100644 --- a/resource/csdk/security/include/internal/policyengine.h +++ b/resource/csdk/security/include/internal/policyengine.h @@ -46,7 +46,6 @@ typedef struct PEContext char resource[MAX_URI_LENGTH]; OicSecSvrType_t resourceType; uint16_t permission; - bool matchingAclFound; bool amsProcessing; SRMAccessResponse_t retVal; AmsMgrContext_t *amsMgrContext; diff --git a/resource/csdk/security/src/amsmgr.c b/resource/csdk/security/src/amsmgr.c index 771f420..775722f 100644 --- a/resource/csdk/security/src/amsmgr.c +++ b/resource/csdk/security/src/amsmgr.c @@ -398,7 +398,7 @@ void ProcessAMSRequest(PEContext_t *context) OIC_LOG_V(INFO, TAG, "Entering %s", __func__); if (NULL != context) { - if((false == context->matchingAclFound) && (false == context->amsProcessing)) + if((ACCESS_GRANTED != context->retVal) && (false == context->amsProcessing)) { context->amsProcessing = true; diff --git a/resource/csdk/security/src/policyengine.c b/resource/csdk/security/src/policyengine.c index b08a64f..5aea971 100644 --- a/resource/csdk/security/src/policyengine.c +++ b/resource/csdk/security/src/policyengine.c @@ -99,7 +99,6 @@ void SetPolicyEngineState(PEContext_t *context, const PEState_t state) memset(&context->subject, 0, sizeof(context->subject)); memset(&context->resource, 0, sizeof(context->resource)); context->permission = 0x0; - context->matchingAclFound = false; context->amsProcessing = false; context->retVal = ACCESS_DENIED_POLICY_ENGINE_ERROR; @@ -508,7 +507,6 @@ static void ProcessAccessRequest(PEContext_t *context) if (IsResourceInAce(context->resource, currentAce)) { OIC_LOG_V(INFO, TAG, "%s:found matching resource in ACE" ,__func__); - context->matchingAclFound = true; // Found the resource, so it's down to valid period & permission. context->retVal = ACCESS_DENIED_INVALID_PERIOD; @@ -526,7 +524,7 @@ static void ProcessAccessRequest(PEContext_t *context) { OIC_LOG_V(INFO, TAG, "%s:no ACL found matching subject for resource %s",__func__, context->resource); } - } while ((NULL != currentAce) && (false == context->matchingAclFound)); + } while ((NULL != currentAce) && (ACCESS_GRANTED != context->retVal)); if (IsAccessGranted(context->retVal)) { @@ -608,8 +606,9 @@ SRMAccessResponse_t CheckPermission(PEContext_t *context, ProcessAccessRequest(context); - // If matching ACL not found, and subject != wildcard, try wildcard. - if ((false == context->matchingAclFound) && \ + // If access not already granted, and requested subject != wildcard, + // try looking for a wildcard ACE that grants access. + if ((ACCESS_GRANTED != context->retVal) && \ (false == IsWildCardSubject(&context->subject))) { //Saving subject for Amacl check diff --git a/resource/csdk/security/unittest/policyengine.cpp b/resource/csdk/security/unittest/policyengine.cpp index b73881d..3834b8a 100644 --- a/resource/csdk/security/unittest/policyengine.cpp +++ b/resource/csdk/security/unittest/policyengine.cpp @@ -113,6 +113,5 @@ TEST(PolicyEngineCore, DeInitPolicyEngine) DeInitPolicyEngine(&g_peContext); EXPECT_EQ(STOPPED, g_peContext.state); EXPECT_EQ((uint16_t)0, g_peContext.permission); - EXPECT_FALSE(g_peContext.matchingAclFound); EXPECT_EQ(ACCESS_DENIED_POLICY_ENGINE_ERROR, g_peContext.retVal); } -- 2.7.4