HPCC-16086 Security permissions values are hardcoded

The code for Dali, workunit and plugins etc includes hardcoded values for
permission settings. Instead of using mysterious values like 255, -255, -1
and 0, we should specify the named SecAccess_* enumerations found in
seclib.hpp

Signed-off-by: Russ Whitehead <william.whitehead@lexisnexis.com>

common/workunit/workunit.cpp

@@ -3950,17 +3950,17 @@ void CLocalWorkUnit::remoteCheckAccess(IUserDescriptor *user, bool writeaccess)
     unsigned auditflags = DALI_LDAP_AUDIT_REPORT|DALI_LDAP_READ_WANTED;
     if (writeaccess)
         auditflags |= DALI_LDAP_WRITE_WANTED;
-    int perm = 255;
+    int perm = SecAccess_Full;
     const char *scopename = p->queryProp("@scope");
     if (scopename&&*scopename) {
         if (!user)
             user = queryUserDescriptor();
         perm = querySessionManager().getPermissionsLDAP("workunit",scopename,user,auditflags);
         if (perm<0) {
-            if (perm==-1) 
-                perm = 255;
+            if (perm == SecAccess_Unavailable)
+                perm = SecAccess_Full;
             else 
-                perm = 0;
+                perm = SecAccess_None;
         }
     }
     if (!HASREADPERMISSION(perm))

dali/base/CMakeLists.txt

@@ -63,6 +63,7 @@ include_directories (
          ./../../system/include 
          ./../../system/jlib 
          ./../../rtl/include 
+         ./../../system/security/shared
     )
 
 ADD_DEFINITIONS( -DLOGMSGCOMPONENT=3 -D_USRDLL -DDALI_EXPORTS )

dali/base/dadfs.cpp

@@ -39,6 +39,7 @@
 #include "rmtfile.hpp"
 #include "dadfs.hpp"
 #include "eclhelper.hpp"
+#include "seclib.hpp"
 
 #ifdef _DEBUG
 //#define EXTRA_LOGGING
@@ -1244,7 +1245,7 @@ static int getScopePermissions(const char *scopename,IUserDescriptor *user,unsig
     static bool permissionsavail=true;
     if (auditflags==(unsigned)-1) 
         return permissionsavail?1:0;
-    int ret = 255;
+    int perms = SecAccess_Full;
     if (permissionsavail&&scopename&&*scopename&&((*scopename!='.')||scopename[1])) {
         if (!user)
         {
@@ -1255,17 +1256,17 @@ static int getScopePermissions(const char *scopename,IUserDescriptor *user,unsig
 #endif
             user = queryDistributedFileDirectory().queryDefaultUser();
         }
-        ret = querySessionManager().getPermissionsLDAP(queryDfsXmlBranchName(DXB_Scope),scopename,user,auditflags);
-        if (ret<0) {
-            if (ret==-1) {
+        perms = querySessionManager().getPermissionsLDAP(queryDfsXmlBranchName(DXB_Scope),scopename,user,auditflags);
+        if (perms<0) {
+            if (perms == SecAccess_Unavailable) {
                 permissionsavail=false;
-                ret = 255;
+                perms = SecAccess_Full;
             }
             else 
-                ret = 0;
+                perms = SecAccess_None;
         }
     }
-    return ret;
+    return perms;
 }
 
 static void checkLogicalScope(const char *scopename,IUserDescriptor *user,bool readreq,bool createreq)
@@ -10622,7 +10623,7 @@ int CDistributedFileDirectory::getNodePermissions(const IpAddress &ip,IUserDescr
 int CDistributedFileDirectory::getFDescPermissions(IFileDescriptor *fdesc,IUserDescriptor *user,unsigned auditflags)
 {
     // this checks have access to the nodes in the file descriptor
-    int ret = 255;
+    int retPerms = SecAccess_Full;
     unsigned np = fdesc->numParts();
     for (unsigned i=0;i<np;i++) {
         INode *node = fdesc->queryNode(i);
@@ -10652,16 +10653,16 @@ int CDistributedFileDirectory::getFDescPermissions(IFileDescriptor *fdesc,IUserD
                 dlfn.setExternal(rfn.queryEndpoint(),localpath.str());          
                 StringBuffer scopes;
                 dlfn.getScopes(scopes);
-                int p = getScopePermissions(scopes.str(),user,auditflags);
-                if (p<ret) {
-                    ret = p;
-                    if (ret==0)
-                        return 0;
+                int perm = getScopePermissions(scopes.str(),user,auditflags);
+                if (perm < retPerms) {
+                    retPerms = perm;
+                    if (retPerms == SecAccess_None)
+                        return SecAccess_None;
                 }
             }
         }
     }
-    return ret;
+    return retPerms;
 }
 
 void CDistributedFileDirectory::setDefaultUser(IUserDescriptor *user)

dali/base/dasess.cpp

@@ -31,7 +31,7 @@
 #include "dasds.hpp"
 #include "daclient.hpp"
 #include "daldap.hpp"
-
+#include "seclib.hpp"
 #include "dasess.hpp"
 
 #ifdef _MSC_VER
@@ -887,10 +887,10 @@ public:
         if (err)
             *err = 0;
         if (securitydisabled)
-            return -1;
+            return SecAccess_Unavailable;
         if (queryDaliServerVersion().compare("1.8") < 0) {
             securitydisabled = true;
-            return -1;
+            return SecAccess_Unavailable;
         }
         CMessageBuffer mb;
         mb.append((int)MSR_LOOKUP_LDAP_PERMISSIONS);
@@ -922,7 +922,7 @@ public:
                     throw new CDaliLDAP_Exception(e);
             }
         }
-        if (ret==-1)
+        if (ret == SecAccess_Unavailable)
             securitydisabled = true;
         return ret;
     }
@@ -1405,9 +1405,9 @@ public:
         if (err)
             *err = 0;
         if (!ldapconn)
-            return -1;
+            return SecAccess_Unavailable;
 #ifdef _NO_LDAP
-        return -1;
+        return SecAccess_Unavailable;
 #else
 #ifdef NULL_DALIUSER_STACKTRACE
         StringBuffer sb;

dali/daliadmin/CMakeLists.txt

@@ -39,6 +39,7 @@ include_directories (
          ./../ft 
          ./../../common/workunit
          ./../../common/dllserver
+         ./../../system/security/shared
     )
 
 ADD_DEFINITIONS( -D_CONSOLE )

dali/daliadmin/daliadmin.cpp

@@ -45,6 +45,7 @@
 
 #include "workunit.hpp"
 #include "dllserver.hpp"
+#include "seclib.hpp"
 
 #ifdef _WIN32
 #include <conio.h>
@@ -1521,7 +1522,7 @@ static void listrelationships(const char *primary,const char *secondary)
 
 int dfsperm(const char *obj,IUserDescriptor *user)
 {
-    int perm =0;
+    int perm = SecAccess_None;
     if (strchr(obj,'\\')||strchr(obj,'/')) {
         Owned<IFileDescriptor> fd = createFileDescriptor();
         RemoteFilename rfn;

dali/server/daldap.cpp

@@ -120,13 +120,13 @@ public:
     int getPermissions(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags)
     {
         if (!ldapsecurity||((getLDAPflags()&DLF_ENABLED)==0)) 
-            return 255;
+            return SecAccess_Full;
         bool filescope = stricmp(key,"Scope")==0;
         bool wuscope = stricmp(key,"workunit")==0;
         if (filescope||wuscope) {
             StringBuffer username;
             StringBuffer password;
-            int perm = 0;
+            int perm = SecAccess_None;
             if (udesc) {
                 udesc->getUserName(username);
                 udesc->getPassword(password);
@@ -155,8 +155,8 @@ public:
                         perm=ldapsecurity->authorizeFileScope(*user, obj);
                     else if (wuscope)
                         perm=ldapsecurity->authorizeWorkunitScope(*user, obj);
-                    if (perm==-1)
-                        perm = 0;
+                    if (perm == SecAccess_Unavailable)
+                        perm = SecAccess_None;
                 }
             }
             unsigned taken = msTick()-start;
@@ -181,7 +181,7 @@ public:
             }
             return perm;
         }
-        return 255;
+        return SecAccess_Full;
     }
     bool clearPermissionsCache(IUserDescriptor *udesc)
     {

esp/services/ws_access/ws_accessService.cpp

@@ -3791,7 +3791,7 @@ bool Cws_accessEx::onFilePermission(IEspContext &context, IEspFilePermissionRequ
         if ((!groupName || !*groupName) && (!userName || !*userName))
             throw MakeStringException(ECLWATCH_INVALID_ACCOUNT_NAME, "Either user name or group name has to be specified.");
 
-        int access = -1;
+        int access = SecAccess_Unavailable;
         if (userName && *userName) //for user
         {
             resp.setFileName(fileName);

plugins/workunitservices/CMakeLists.txt

@@ -44,6 +44,7 @@ include_directories (
          ./../../system/jhtree 
          ./../../system/jlib 
          ./../../dali/sasha 
+         ./../../system/security/shared
     )
 
 ADD_DEFINITIONS( -D_USRDLL -DWORKUNITSERVICES_EXPORTS )

plugins/workunitservices/workunitservices.cpp

@@ -53,6 +53,7 @@ Persists changed?
 #include "workunitservices.hpp"
 #include "workunitservices.ipp"
 #include "environment.hpp"
+#include "seclib.hpp"
 
 #define WORKUNITSERVICES_VERSION "WORKUNITSERVICES 1.0.2"
 
@@ -221,19 +222,19 @@ static bool checkScopeAuthorized(IUserDescriptor *user, const char *scopename)
     if (securityDisabled)
         return true;
     unsigned auditflags = DALI_LDAP_AUDIT_REPORT|DALI_LDAP_READ_WANTED;
-    int perm = 255;
+    int perm = SecAccess_Full;
     if (scopename && *scopename)
     {
         perm = querySessionManager().getPermissionsLDAP("workunit",scopename,user,auditflags);
         if (perm<0)
         {
-            if (perm==-1)
+            if (perm == SecAccess_Unavailable)
             {
-                perm = 255;
+                perm = SecAccess_Full;
                 securityDisabled = true;
             }
             else 
-                perm = 0;
+                perm = SecAccess_None;
         }
         if (!HASREADPERMISSION(perm)) 
             return false;

system/security/LdapSecurity/aci.cpp

@@ -587,11 +587,12 @@ public:
     }
 };
 
+//Translates ACI permission settings to SecAccessFlags
 int NewSec2Sec(int newsec)
 {
     int sec = 0;
     if(newsec == -1)
-        return -1;
+        return SecAccess_Unavailable;
     if(newsec == NewSecAccess_Full)
         return SecAccess_Full;
 
@@ -655,7 +656,7 @@ public:
         int perm = 0;
         if(m_acilist.length() == 0)
         {
-            perm = -1;
+            perm = SecAccess_Unavailable;
         }
         else
         {
@@ -1049,7 +1050,7 @@ CSecurityDescriptor* AciProcessor::changePermission(CSecurityDescriptor* initial
 
 StringBuffer& CIPlanetAciProcessor::sec2aci(int secperm, StringBuffer& aciperm)
 {
-    if(secperm == -1 || secperm == SecAccess_None)
+    if(secperm == SecAccess_Unavailable || secperm == SecAccess_None)
         return aciperm;
 
     if(secperm >= SecAccess_Full)
@@ -1134,7 +1135,7 @@ CSecurityDescriptor* CIPlanetAciProcessor::createDefaultSD(ISecUser * const user
 
 StringBuffer& COpenLdapAciProcessor::sec2aci(int secperm, StringBuffer& aciperm)
 {
-    if(secperm == -1 || secperm == SecAccess_None)
+    if(secperm == SecAccess_Unavailable || secperm == SecAccess_None)
         return aciperm;
 
     if(secperm >= SecAccess_Full)

system/security/shared/caching.cpp

@@ -133,7 +133,7 @@ void CResPermissionsCache::add( IArrayOf<ISecResource>& resources )
         if(resource == NULL)
             continue;
         int permissions = secResource->getAccessFlags();
-        if(permissions == -1)
+        if(permissions == SecAccess_Unavailable)
             continue;
 
         MapResAccess::iterator it = m_resAccessMap.find(SecCacheKeyEntry(resource, resourcetype));

system/security/shared/seclib.hpp

@@ -50,6 +50,7 @@ enum NewSecAccessFlags
 
 enum SecAccessFlags
 {
+    SecAccess_Unavailable = -1,
     SecAccess_Unknown = -255,
     SecAccess_None = 0,
     SecAccess_Access = 1,