Merge pull request #12725 from ghalliday/issue22415

HPCC-22415 Remove redundant security checking from dali

Reviewed-By: Anthony Fishbeck <anthony.fishbeck@lexisnexis.com>
Reviewed-By: Russ Whitehead <william.whitehead@lexisnexis.com>
Reviewed-By: Jake Smith <jake.smith@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

common/workunit/workunit.cpp

@@ -6770,8 +6770,7 @@ void CLocalWorkUnit::remoteCheckAccess(IUserDescriptor *user, bool writeaccess)
     if (scopename&&*scopename) {
         if (!user)
             user = queryUserDescriptor();
-        CDateTime now;
-        perm = querySessionManager().getPermissionsLDAP("workunit",scopename,user,auditflags,nullptr,now);
+        perm = querySessionManager().getPermissionsLDAP("workunit",scopename,user,auditflags);
         if (perm<0) {
             if (perm == SecAccess_Unavailable)
                 perm = SecAccess_Full;

dali/base/dadfs.cpp

@@ -1261,12 +1261,7 @@ static SecAccessFlags getScopePermissions(const char *scopename,IUserDescriptor
             user = queryDistributedFileDirectory().queryDefaultUser();
         }
 
-        //Create signature
-        CDateTime now;
-        StringBuffer b64sig;
-        createDaliSignature(scopename, user, now, b64sig);
-
-        perms = querySessionManager().getPermissionsLDAP(queryDfsXmlBranchName(DXB_Scope),scopename,user,auditflags, b64sig.str(), now);
+        perms = querySessionManager().getPermissionsLDAP(queryDfsXmlBranchName(DXB_Scope),scopename,user,auditflags);
         if (perms<0) {
             if (perms == SecAccess_Unavailable) {
                 scopePermissionsAvail=false;

dali/base/dasess.cpp

@@ -129,7 +129,7 @@ interface ISessionManagerServer: implements IConnectionMonitor
     virtual void addSession(SessionId id) = 0;
     virtual SessionId lookupProcessSession(INode *node) = 0;
     virtual INode *getProcessSessionNode(SessionId id) =0;
-    virtual SecAccessFlags getPermissionsLDAP(const char *key,const char *obj,IUserDescriptor *udesc,unsigned flags, const char * reqSignature, CDateTime & reqUTCTimestamp, int *err)=0;
+    virtual SecAccessFlags getPermissionsLDAP(const char *key,const char *obj,IUserDescriptor *udesc,unsigned flags, int *err)=0;
     virtual bool clearPermissionsCache(IUserDescriptor *udesc) = 0;
     virtual void stopSession(SessionId sessid,bool failed) = 0;
     virtual void setClientAuth(IDaliClientAuthConnection *authconn) = 0;
@@ -496,24 +496,6 @@ public:
     }
 };
 
-bool createDaliSignature(const char * scope, IUserDescriptor *udesc, CDateTime &now, StringBuffer &b64sig)
-{
-    IDigitalSignatureManager * pDSM = queryDigitalSignatureManagerInstanceFromEnv();
-    if (pDSM && pDSM->isDigiSignerConfigured())
-    {
-        StringBuffer username;
-        udesc->getUserName(username);
-        StringBuffer timeStr;
-        now.setNow();
-        now.getString(timeStr, false);//get UTC timestamp
-        VStringBuffer toSign("%s;%s;%s", scope, username.str(), timeStr.str());
-
-        pDSM->digiSign(b64sig, toSign);//Sign "scope;username;timeStamp"
-        return true;
-    }
-    return false;
-}
-
 class CSessionRequestServer: public Thread
 {
     bool stopped;
@@ -651,15 +633,15 @@ public:
                 if (mb.length()-mb.getPos()>=sizeof(auditflags))
                     mb.read(auditflags);
 
-                StringBuffer reqSignature;
-                CDateTime reqUTCTimestamp;
                 if (mb.remaining() > 0)
                 {
+                    StringBuffer reqSignature; // now ignored
+                    CDateTime reqUTCTimestamp; // also ignored
                     mb.read(reqSignature);
                     reqUTCTimestamp.deserialize(mb);
                 }
                 int err = 0;
-                SecAccessFlags perms = manager.getPermissionsLDAP(key,obj,udesc,auditflags,reqSignature.str(),reqUTCTimestamp,&err);
+                SecAccessFlags perms = manager.getPermissionsLDAP(key,obj,udesc,auditflags,&err);
                 mb.clear().append((int)perms);
                 if (err)
                     mb.append(err);
@@ -936,7 +918,7 @@ public:
     }
 
 
-    SecAccessFlags getPermissionsLDAP(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags,const char * reqSignature, CDateTime & reqUTCTimestamp, int *err)
+    SecAccessFlags getPermissionsLDAP(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags, int *err)
     {
         if (err)
             *err = 0;
@@ -969,21 +951,10 @@ public:
         //Serialize signature. If not provided, compute it
         if (queryDaliServerVersion().compare("3.15") >= 0)
         {
-            if (isEmptyString(reqSignature))
-            {
-                CDateTime now;
-                StringBuffer b64sig;
-                if (createDaliSignature(obj, udesc, now, b64sig))
-                {
-                    mb.append(b64sig.str());
-                    now.serialize(mb);
-                }
-            }
-            else
-            {
-                mb.append(reqSignature);
-                reqUTCTimestamp.serialize(mb);
-            }
+            //Backwards compatibility in case another parameter is added later, otherwise could be removed
+            CDateTime reqUTCTimestamp;
+            mb.append(""); // previously a base64 encoded signature
+            reqUTCTimestamp.serialize(mb);
         }
 
 
@@ -1196,8 +1167,6 @@ class CLdapWorkItem : public Thread
 {
     StringAttr key;
     StringAttr obj;
-    StringAttr reqSignature;
-    CDateTime  reqUTCTimestamp;
     Linked<IUserDescriptor> udesc;
     Linked<IDaliLdapConnection> ldapconn;
     unsigned flags;
@@ -1212,13 +1181,10 @@ public:
     {
         running = false;
     }
-    void start(const char *_key,const char *_obj,IUserDescriptor *_udesc,unsigned _flags,const char * _reqSignature, CDateTime & _reqUTCTimestamp)
+    void start(const char *_key,const char *_obj,IUserDescriptor *_udesc,unsigned _flags)
     {
         key.set(_key);
         obj.set(_obj); 
-        reqSignature.set(_reqSignature);
-        if (!_reqUTCTimestamp.isNull())
-            reqUTCTimestamp.set(_reqUTCTimestamp);
 
 #ifdef NULL_DALIUSER_STACKTRACE
         StringBuffer sb;
@@ -1247,7 +1213,7 @@ public:
             if (!running)
                 break;
             try {
-                ret = ldapconn->getPermissions(key,obj,udesc,flags,reqSignature.str(),reqUTCTimestamp);
+                ret = ldapconn->getPermissions(key,obj,udesc,flags);
             }
             catch(IException *e) {
                 LOG(MCoperatorError, unknownJob, e, "CLdapWorkItem"); 
@@ -1476,7 +1442,7 @@ public:
 
     //ISessionManagerServer
     //Dali method to handle permission request
-    virtual SecAccessFlags getPermissionsLDAP(const char *key,const char *obj,IUserDescriptor *udesc,unsigned flags,const char * reqSignature, CDateTime & reqUTCTimestamp, int *err)
+    virtual SecAccessFlags getPermissionsLDAP(const char *key,const char *obj,IUserDescriptor *udesc,unsigned flags, int *err)
     {
         if (err)
             *err = 0;
@@ -1496,7 +1462,7 @@ public:
         }
 #endif
         if ((ldapconn->getLDAPflags()&(DLF_SAFE|DLF_ENABLED))!=(DLF_SAFE|DLF_ENABLED))
-            return ldapconn->getPermissions(key,obj,udesc,flags,reqSignature,reqUTCTimestamp);
+            return ldapconn->getPermissions(key,obj,udesc,flags);
         atomic_inc(&ldapwaiting);
         unsigned retries = 0;
         while (!stopping) {
@@ -1505,7 +1471,7 @@ public:
                 if (!ldapworker)
                     ldapworker.setown(CLdapWorkItem::get(ldapconn,workthreadsem));
                 if (ldapworker) {
-                    ldapworker->start(key,obj,udesc,flags,reqSignature,reqUTCTimestamp);
+                    ldapworker->start(key,obj,udesc,flags);
                     for (unsigned i=0;i<10;i++) {
                         if (i)
                             OWARNLOG("LDAP stalled(%d) - retrying",i);
@@ -1530,7 +1496,7 @@ public:
                             ldapworker.clear();
                             ldapworker.setown(CLdapWorkItem::get(ldapconn,workthreadsem));
                             if (ldapworker)
-                                ldapworker->start(key,obj,udesc,flags,reqSignature,reqUTCTimestamp);
+                                ldapworker->start(key,obj,udesc,flags);
                         }
                     }
                     if (ldapworker)
@@ -1876,7 +1842,6 @@ ISessionManager &querySessionManager()
         assertex(!isCovenActive()||!queryCoven().inCoven()); // Check not Coven server (if occurs - not initialized correctly;
                                                    // If !coven someone is checking for dali so allow
         SessionManager = new CClientSessionManager();
-    
     }
     return *SessionManager;
 }

dali/base/dasess.hpp

@@ -127,7 +127,7 @@ interface ISessionManager: extends IInterface
     virtual StringBuffer &getClientProcessEndpoint(SessionId id,StringBuffer &buf)=0; // for diagnostics
     virtual unsigned queryClientCount() = 0; // for SNMP
 
-    virtual SecAccessFlags getPermissionsLDAP(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags, const char * reqSignature, CDateTime & reqUTCTimestamp, int *err=NULL)=0;
+    virtual SecAccessFlags getPermissionsLDAP(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags, int *err=NULL)=0;
     virtual bool checkScopeScansLDAP()=0;
     virtual unsigned getLDAPflags()=0;
     virtual void setLDAPflags(unsigned flags)=0;
@@ -149,8 +149,6 @@ extern da_decl ISessionManager &querySessionManager();
 
 #define myProcessSession() (querySessionManager().lookupProcessSession())
 
-extern da_decl bool createDaliSignature(const char * scope, IUserDescriptor *udesc, CDateTime &now, StringBuffer &b64sig);
-
 interface IMessageWrapper
 {
 public:

dali/server/daldap.cpp

@@ -53,8 +53,6 @@ class CDaliLdapConnection: implements IDaliLdapConnection, public CInterface
     StringAttr              filesdefaultuser;
     StringAttr              filesdefaultpassword;
     unsigned                ldapflags;
-    unsigned                requestSignatureExpiryMinutes;//Age at which a dali permissions request signature becomes invalid
-    unsigned                requestSignatureAllowedClockVarianceSeconds;//Number of seconds that timestamps can vary between nodes
     IDigitalSignatureManager * pDSM = nullptr;
 
     void createDefaultScopes()
@@ -86,8 +84,6 @@ public:
     {
         ldapflags = 0;
         if (ldapprops) {
-            requestSignatureExpiryMinutes = ldapprops->getPropInt("@reqSignatureExpiry", 10);
-            requestSignatureAllowedClockVarianceSeconds = ldapprops->getPropInt("@allowedClockVariance", 5);
             if (ldapprops->getPropBool("@checkScopeScans",true))
                 ldapflags |= DLF_SCOPESCANS;
             if (ldapprops->getPropBool("@safeLookup",true))
@@ -126,7 +122,7 @@ public:
     }
 
 
-    SecAccessFlags getPermissions(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags,const char * reqSignature, CDateTime & reqUTCTimestamp)
+    SecAccessFlags getPermissions(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags)
     {
         if (!ldapsecurity||((getLDAPflags()&DLF_ENABLED)==0)) 
             return SecAccess_Full;
@@ -147,80 +143,10 @@ public:
             username.append(filesdefaultuser);
             decrypt(password, filesdefaultpassword);
             OWARNLOG("Missing credentials, injecting deprecated filesdefaultuser");
-            reqSignature = nullptr;
         }
 
         Owned<ISecUser> user = ldapsecurity->createUser(username);
-        user->credentials().setPassword(password);
-
-        //Check that the digital signature provided by the caller (signature of
-        //caller's "scope;username;timeStamp") matches what we expect it to be
-        if (!isEmptyString(reqSignature))
-        {
-            if (nullptr == pDSM)
-                pDSM = queryDigitalSignatureManagerInstanceFromEnv();
-            if (pDSM && pDSM->isDigiVerifierConfigured())
-            {
-                StringBuffer requestTimestamp;
-                reqUTCTimestamp.getString(requestTimestamp, false);//extract timestamp string from Dali request
-
-                CDateTime now;
-                now.setNow();
-                CDateTime daliTime(now);
-                if (requestSignatureAllowedClockVarianceSeconds)//allow for clock variance between machines
-                    daliTime.adjustTimeSecs(requestSignatureAllowedClockVarianceSeconds);
-
-                if (daliTime.compare(reqUTCTimestamp, false) < 0)//timestamp from the future?
-                {
-                    StringBuffer localDaliTimeUTC;
-                    now.getString(localDaliTimeUTC, false);//get UTC timestamp
-                    OERRLOG("LDAP: getPermissions(%s) scope=%s user=%s Request digital signature UTC timestamp %s from the future (Dali UTC time %s)",key?key:"NULL",obj?obj:"NULL",username.str(), requestTimestamp.str(), localDaliTimeUTC.str());
-                    return SecAccess_None;//deny
-                }
-
-                CDateTime expiry(now);
-                expiry.adjustTime(-1 * requestSignatureExpiryMinutes);//compute expiration timestamp
-                if (requestSignatureAllowedClockVarianceSeconds)//allow for clock variance between machines
-                    expiry.adjustTimeSecs(-1 * requestSignatureAllowedClockVarianceSeconds);
-
-                if (reqUTCTimestamp.compare(expiry, false) < 0)//timestamp too far in the past?
-                {
-                    StringBuffer localDaliTimeUTC;
-                    now.getString(localDaliTimeUTC, false);//get UTC timestamp
-                    OERRLOG("LDAP: getPermissions(%s) scope=%s user=%s Expired request digital signature UTC timestamp %s (Dali UTC time %s, configured expiry %d minutes)",key?key:"NULL",obj?obj:"NULL",username.str(), requestTimestamp.str(), localDaliTimeUTC.str(), requestSignatureExpiryMinutes);
-                    return SecAccess_None;//deny
-                }
-
-                VStringBuffer expectedStr("%s;%s;%s", obj, username.str(), requestTimestamp.str());
-                StringBuffer b64Signature(reqSignature);// signature of scope;user;timestamp
-
-                if (!pDSM->digiVerify(b64Signature, expectedStr))//does the digital signature match what we expect?
-                {
-                    OERRLOG("LDAP: getPermissions(%s) scope=%s user=%s fails digital signature verification",key?key:"NULL",obj?obj:"NULL",username.str());
-                    return SecAccess_None;//deny
-                }
-
-                //Mark user as authenticated. The call below to authenticateUser
-                //will add this user to the LDAP cache
-                user->setAuthenticateStatus(AS_AUTHENTICATED);
-            }
-            else
-                OERRLOG("LDAP: getPermissions(%s) scope=%s user=%s Dali received signed request, however Dali is not configured to verify digital signatures",key?key:"NULL",obj?obj:"NULL",username.str());
-        }
-
-        if (!isEmptyString(user->credentials().getPassword()) && !isWorkunitDAToken(user->credentials().getPassword()))
-        {
-            if (!ldapsecurity->authenticateUser(*user, NULL))
-            {
-                const char * extra = "";
-                if (isEmptyString(reqSignature))
-                    extra = " (Password or Dali Signature not provided)";
-                OERRLOG("LDAP: getPermissions(%s) scope=%s user=%s fails LDAP authentication%s",key?key:"NULL",obj?obj:"NULL",username.str(), extra);
-                return SecAccess_None;//deny
-            }
-        }
-        else
-            user->setAuthenticateStatus(AS_AUTHENTICATED);
+        user->setAuthenticateStatus(AS_AUTHENTICATED);
 
         bool filescope = stricmp(key,"Scope")==0;
         bool wuscope = stricmp(key,"workunit")==0;

dali/server/daldap.hpp

@@ -26,7 +26,7 @@ interface IUserDescriptor;
 
 interface IDaliLdapConnection: extends IInterface
 {
-    virtual SecAccessFlags getPermissions(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags,const char * reqSignature, CDateTime & reqUTCTimestamp)=0;
+    virtual SecAccessFlags getPermissions(const char *key,const char *obj,IUserDescriptor *udesc,unsigned auditflags)=0;
     virtual bool checkScopeScans() = 0;
     virtual unsigned getLDAPflags() = 0;
     virtual void setLDAPflags(unsigned flags) = 0;

initfiles/componentfiles/configxml/dali.xsd

@@ -402,20 +402,6 @@
         </xs:appinfo>
       </xs:annotation>
     </xs:attribute>
-    <xs:attribute name="reqSignatureExpiry" use="optional" type="xs:string" default="10">
-      <xs:annotation>
-        <xs:appinfo>
-          <tooltip>Lifetime in minutes of a permissions request digital signature.</tooltip>
-        </xs:appinfo>
-      </xs:annotation>
-    </xs:attribute>
-    <xs:attribute name="allowedClockVariance" use="optional" type="xs:string" default="5">
-      <xs:annotation>
-        <xs:appinfo>
-          <tooltip>Maximum number of seconds that client clocks can vary from Dali clock, used when checking permissions request digital signature.</tooltip>
-        </xs:appinfo>
-      </xs:annotation>
-    </xs:attribute>
     <xs:attribute name="checkScopeScans" type="xs:boolean" use="optional" default="true">
       <xs:annotation>
         <xs:appinfo>

initfiles/componentfiles/configxml/dali.xsl

@@ -240,7 +240,7 @@
         </xsl:element>
         <xsl:if test="string(@ldapServer) != ''">
           <xsl:element name="ldapSecurity">
-            <xsl:copy-of select="@ldapProtocol | @authMethod | @maxConnections | @workunitsBasedn | @filesDefaultUser | @filesDefaultPassword | @reqSignatureExpiry | @allowedClockVariance"/>
+            <xsl:copy-of select="@ldapProtocol | @authMethod | @maxConnections | @workunitsBasedn | @filesDefaultUser | @filesDefaultPassword"/>
             <xsl:variable name="ldapServerName" select="@ldapServer"/>
             <xsl:attribute name="filesBasedn">
                 <xsl:value-of select="/Environment/Software/LDAPServerProcess[@name=$ldapServerName]/@filesBasedn"/>

plugins/workunitservices/workunitservices.cpp

@@ -219,12 +219,7 @@ static bool checkScopeAuthorized(IUserDescriptor *user, const char *scopename)
     SecAccessFlags perm = SecAccess_Full;
     if (scopename && *scopename)
     {
-        //Create signature
-        CDateTime now;
-        StringBuffer b64sig;
-        createDaliSignature(scopename, user, now, b64sig);
-
-        perm = querySessionManager().getPermissionsLDAP("workunit",scopename,user,auditflags, b64sig.str(), now);
+        perm = querySessionManager().getPermissionsLDAP("workunit",scopename,user,auditflags);
         if (perm<0)
         {
             if (perm == SecAccess_Unavailable)