[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 f8b333c12b7924f33a9a09703b0b16b25b7fa22e..66b399c1580c77b8652b918fc1b88f62155fad49 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 771f42021e11f83991bd332f77665f1a6ca33929..775722f2fdc1e7883e6c04d79267a4681f6cecc7 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 b08a64f353330256de941fd93c7642b8a48eda87..5aea97181b6243e1e1c5d42b0fd0108ff54d657b 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 b73881d2bcb26cb22a541c2ac1e1ef0867542614..3834b8ab56c9022abc352dbfa5c6669e3f723c96 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);
 }