[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 <nathan.heldt-sheller@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/14701
Reviewed-by: Kevin Kane <kkane@microsoft.com>
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Greg Zaverucha <gregz@microsoft.com>

diff --git a/resource/csdk/security/include/internal/policyengine.h b/resource/csdk/security/include/internal/policyengine.h
index f8b333c..66b399c 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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);
 }