From ebc799522c09ed3048756520259fb5a391d1b92e Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Wed, 11 Sep 2024 18:30:49 -0400 Subject: [PATCH] squash. Hunting down bug with acAclPolicy being executed. Restored ability to run iRODS 4 server. --- .../irods/irods_client_server_negotiation.hpp | 1 + lib/core/src/irods_client_negotiation.cpp | 2 +- server/core/include/irods/initServer.hpp | 2 +- server/core/src/initServer.cpp | 33 ++++++++++++++++--- server/core/src/irods_server_negotiation.cpp | 25 +++++++------- server/core/src/rodsAgent.cpp | 6 ++-- server/core/src/rodsConnect.cpp | 1 + server/main_server/src/agent_main.cpp | 8 ++--- server/main_server/src/main.cpp | 10 ++++-- 9 files changed, 61 insertions(+), 27 deletions(-) diff --git a/lib/core/include/irods/irods_client_server_negotiation.hpp b/lib/core/include/irods/irods_client_server_negotiation.hpp index 1789a3e1c5..d4a976108c 100644 --- a/lib/core/include/irods/irods_client_server_negotiation.hpp +++ b/lib/core/include/irods/irods_client_server_negotiation.hpp @@ -69,6 +69,7 @@ namespace irods error client_server_negotiation_for_server( irods::network_object_ptr, // server connection handle std::string&, // results of negotiation + bool, // boolean indicating whether client-server negotiation is needed RsComm* = nullptr); // optional RsComm initialized during agent startup (maintains compatibility with rodsAgent.cpp) /// =-=-=-=-=-=-=- diff --git a/lib/core/src/irods_client_negotiation.cpp b/lib/core/src/irods_client_negotiation.cpp index 4336af8e3f..046a60b855 100644 --- a/lib/core/src/irods_client_negotiation.cpp +++ b/lib/core/src/irods_client_negotiation.cpp @@ -237,7 +237,7 @@ namespace irods #else // if it is set then check for our magic token which requests // the negotiation, if it is not the magic token, move on - return _neg && std::string_view{_neg}.find(REQ_SVR_NEG) != std::string_view::npos; + return _neg && std::string_view{_neg} == REQ_SVR_NEG; #endif } // do_client_server_negotiation_for_server diff --git a/server/core/include/irods/initServer.hpp b/server/core/include/irods/initServer.hpp index 15749d45af..90650a01e0 100644 --- a/server/core/include/irods/initServer.hpp +++ b/server/core/include/irods/initServer.hpp @@ -14,7 +14,7 @@ int initAgent(int processType, rsComm_t* rsComm); int initHostConfigByFile(); int initRsComm(rsComm_t* rsComm); -int initRsCommWithStartupPack(rsComm_t* rsComm, startupPack_t* startupPack); +int initRsCommWithStartupPack(rsComm_t* rsComm, startupPack_t* startupPack, bool& require_cs_neg); int chkAllowedUser(const char* userName, const char* rodsZone); int setRsCommFromRodsEnv(rsComm_t* rsComm); void close_all_l1_descriptors(RsComm& _comm); diff --git a/server/core/src/initServer.cpp b/server/core/src/initServer.cpp index 7ec184900d..0d5bfb09a1 100644 --- a/server/core/src/initServer.cpp +++ b/server/core/src/initServer.cpp @@ -3,6 +3,7 @@ #include "irods/genQuery.h" #include "irods/getRemoteZoneResc.h" #include "irods/getRescQuota.h" +#include "irods/irods_client_server_negotiation.hpp" #include "irods/irods_configuration_keywords.hpp" #include "irods/irods_stacktrace.hpp" #include "irods/miscServerFunct.hpp" @@ -48,10 +49,12 @@ #include #include +#include #include #include #include #include +#include #include namespace @@ -126,8 +129,9 @@ int initServerInfo(int processType, rsComm_t* rsComm) /* queue the local zone */ status = queueZone(zone_name.c_str(), zone_port, nullptr, nullptr); + rodsLog(LOG_NOTICE, "initServerInfo - queueZone failed %d", status); if (status < 0) { - rodsLog(LOG_DEBUG, "initServerInfo - queueZone failed %d", status); + rodsLog(LOG_NOTICE, "initServerInfo - queueZone failed %d", status); // do not error out } } @@ -179,6 +183,8 @@ int initServerInfo(int processType, rsComm_t* rsComm) } } + // TODO This function invokes acAclPolicy which guarantees GenQuery1 honors + // the permission model. status = initZone( rsComm ); if ( status < 0 ) { rodsLog( LOG_SYS_FATAL, @@ -689,17 +695,20 @@ initRsComm( rsComm_t *rsComm ) { } int -initRsCommWithStartupPack( rsComm_t *rsComm, startupPack_t *startupPack ) { +initRsCommWithStartupPack( rsComm_t *rsComm, startupPack_t *startupPack, bool& require_cs_neg) { char *tmpStr; static char tmpStr2[LONG_NAME_LEN]; /* always use NATIVE_PROT as a client. e.g., server to server comm */ snprintf( tmpStr2, LONG_NAME_LEN, "%s=%d", IRODS_PROT, NATIVE_PROT ); putenv( tmpStr2 ); - if ( startupPack != NULL ) { + if (startupPack) { rsComm->connectCnt = startupPack->connectCnt; + log_agent::info("{}: startupPack->connectCnt = [{}]", __func__, startupPack->connectCnt); rsComm->irodsProt = startupPack->irodsProt; + log_agent::info("{}: startupPack->irodsProt = [{}]", __func__, startupPack->irodsProt); rsComm->reconnFlag = startupPack->reconnFlag; + log_agent::info("{}: startupPack->reconnFlag = [{}]", __func__, startupPack->reconnFlag); rstrcpy( rsComm->proxyUser.userName, startupPack->proxyUser, NAME_LEN ); if ( strcmp( startupPack->proxyUser, PUBLIC_USER_NAME ) == 0 ) { rsComm->proxyUser.authInfo.authFlag = PUBLIC_USER_AUTH; @@ -712,7 +721,19 @@ initRsCommWithStartupPack( rsComm_t *rsComm, startupPack_t *startupPack ) { rstrcpy( rsComm->clientUser.rodsZone, startupPack->clientRodsZone, NAME_LEN ); rstrcpy( rsComm->cliVersion.relVersion, startupPack->relVersion, NAME_LEN ); rstrcpy( rsComm->cliVersion.apiVersion, startupPack->apiVersion, NAME_LEN ); - rstrcpy( rsComm->option, startupPack->option, LONG_NAME_LEN ); } + + std::string_view opt_str = startupPack->option; + const auto pos = opt_str.find(REQ_SVR_NEG); + require_cs_neg = (std::string_view::npos != pos); + if (require_cs_neg) { + const auto client_app_name = opt_str.substr(0, pos); + client_app_name.copy(rsComm->option, sizeof(RsComm::option)); + } + else { + opt_str.copy(rsComm->option, sizeof(RsComm::option)); + } + log_agent::info("{}: rsComm->option = [{}]", __func__, rsComm->option); + } else { /* have to depend on env variable */ tmpStr = getenv( SP_NEW_SOCK ); if ( tmpStr == NULL ) { @@ -824,6 +845,10 @@ initRsCommWithStartupPack( rsComm_t *rsComm, startupPack_t *startupPack ) { else { rstrcpy( rsComm->option, tmpStr, LONG_NAME_LEN ); } + + if (std::getenv(irods::RODS_CS_NEG)) { + require_cs_neg = true; + } } if (const auto ec = setLocalAddr(rsComm->sock, &rsComm->localAddr); ec == USER_RODS_HOSTNAME_ERR) { diff --git a/server/core/src/irods_server_negotiation.cpp b/server/core/src/irods_server_negotiation.cpp index 23c887f0db..c8290d4806 100644 --- a/server/core/src/irods_server_negotiation.cpp +++ b/server/core/src/irods_server_negotiation.cpp @@ -71,12 +71,15 @@ namespace irods /// @brief function which manages the TLS and Auth negotiations with the client error client_server_negotiation_for_server( irods::network_object_ptr _ptr, - std::string& _result, - RsComm* _comm) + std::string& _result, + bool _require_cs_neg, + RsComm* _comm) // TODO Consider making this a required argument. { // =-=-=-=-=-=-=- // manufacture an rei for the applyRule ruleExecInfo_t rei{}; +#if 0 + RsComm rsComm{}; if (_comm) { rei.rsComm = _comm; @@ -85,7 +88,6 @@ namespace irods // If issues arise from this local rsComm either from lack of information, // such as the missing rsComm.myEnv, or environment variable mismatch, use // the existing rsComm higher up the call stack. - rsComm_t rsComm{}; auto comm_status{initRsCommWithStartupPack(&rsComm, nullptr)}; if (comm_status < 0) { @@ -94,6 +96,9 @@ namespace irods rei.rsComm = &rsComm; } +#else + rei.rsComm = _comm; +#endif std::string rule_result; std::list params; @@ -111,21 +116,18 @@ namespace irods // =-=-=-=-=-=-=- // check to see if a negotiation was requested - if (!do_client_server_negotiation_for_server(rei.rsComm->option)) { + if (!_require_cs_neg) { // =-=-=-=-=-=-=- // if it was not but we require SSL then error out if ( CS_NEG_REQUIRE == rule_result ) { std::stringstream msg; msg << "SSL is required by the server but not requested by the client"; return ERROR( SYS_INVALID_INPUT_PARAM, msg.str() ); - - } - else { - // =-=-=-=-=-=-=- - // a negotiation was not requested and we do not require SSL - we good - return SUCCESS(); } + // =-=-=-=-=-=-=- + // a negotiation was not requested and we do not require SSL - we good + return SUCCESS(); } // =-=-=-=-=-=-=- @@ -215,5 +217,4 @@ namespace irods msg.str() ); } // client_server_negotiation_for_server - -}; // namespace irods +} // namespace irods diff --git a/server/core/src/rodsAgent.cpp b/server/core/src/rodsAgent.cpp index 948d4af9de..63db07889e 100644 --- a/server/core/src/rodsAgent.cpp +++ b/server/core/src/rodsAgent.cpp @@ -590,10 +590,10 @@ int runIrodsAgentFactory(sockaddr_un agent_addr) log_ns::set_error_object(&rsComm.rError); irods::at_scope_exit release_error_stack{[] { log_ns::set_error_object(nullptr); }}; - //std::memset(&rsComm, 0, sizeof(RsComm)); rsComm.thread_ctx = static_cast(std::malloc(sizeof(thread_context))); - status = initRsCommWithStartupPack(&rsComm, nullptr); + bool require_cs_neg = false; + status = initRsCommWithStartupPack(&rsComm, nullptr, require_cs_neg); // manufacture a network object for comms irods::network_object_ptr net_obj; @@ -670,7 +670,7 @@ int runIrodsAgentFactory(sockaddr_un agent_addr) // this scope block makes valgrind happy { std::string neg_results; - ret = irods::client_server_negotiation_for_server(net_obj, neg_results); + ret = irods::client_server_negotiation_for_server(net_obj, neg_results, require_cs_neg); if (!ret.ok() || neg_results == irods::CS_NEG_FAILURE) { // send a 'we failed to negotiate' message here?? // or use the error stack rule engine thingie diff --git a/server/core/src/rodsConnect.cpp b/server/core/src/rodsConnect.cpp index e2f6d75a6e..6a73c33275 100644 --- a/server/core/src/rodsConnect.cpp +++ b/server/core/src/rodsConnect.cpp @@ -296,6 +296,7 @@ int queueZone(const char* zoneName, if (primaryServerHost == NULL) { rodsLog(LOG_DEBUG, "queueZone: primaryServerHost for %s is NULL", zoneName); + // TODO This is where iRODS 5 currently fails. This leads to acAclPolicy not running. return SYS_INVALID_SERVER_HOST; } else { diff --git a/server/main_server/src/agent_main.cpp b/server/main_server/src/agent_main.cpp index 9842d003d9..fac5dfd5aa 100644 --- a/server/main_server/src/agent_main.cpp +++ b/server/main_server/src/agent_main.cpp @@ -836,10 +836,10 @@ namespace rsComm.thread_ctx = static_cast(std::malloc(sizeof(thread_context))); //int status = initRsCommWithStartupPack(&rsComm, nullptr); // TODO This call relies on reading env variables to init rsComm. - int status = initRsCommWithStartupPack(&rsComm, startupPack); // This version just uses the startupPack. + bool require_cs_neg = false; + int status = initRsCommWithStartupPack(&rsComm, startupPack, require_cs_neg); // This version just uses the startupPack. // manufacture a network object for comms - //irods::network_object_ptr net_obj; ret = irods::network_factory(&rsComm, net_obj); if (!ret.ok()) { log_agent::error(PASS(ret).result()); @@ -917,8 +917,8 @@ namespace // this scope block makes valgrind happy { std::string neg_results; - ret = irods::client_server_negotiation_for_server(net_obj, neg_results, &rsComm); - if (!ret.ok() || neg_results == irods::CS_NEG_FAILURE) { + ret = irods::client_server_negotiation_for_server(net_obj, neg_results, require_cs_neg, &rsComm); + if (!ret.ok() || irods::CS_NEG_FAILURE == neg_results) { // send a 'we failed to negotiate' message here?? // or use the error stack rule engine thingie log_agent::error(PASS(ret).result()); diff --git a/server/main_server/src/main.cpp b/server/main_server/src/main.cpp index aed50bd9be..ce111af766 100644 --- a/server/main_server/src/main.cpp +++ b/server/main_server/src/main.cpp @@ -313,6 +313,12 @@ int main(int _argc, char* _argv[]) } } #endif + // TODO This proves there is a bug in the delay server migration logic. + // The following lines will be removed before merging. + // The issue with the migration logic is that it leads to an unused forked process when the successor is set to an invalid server. The iRODS server still works, it's just we get an extra child process that serves no purpose. + std::this_thread::sleep_for(std::chrono::seconds{1}); + continue; + // // Execute delay server migration algorithm. // @@ -361,7 +367,7 @@ int main(int _argc, char* _argv[]) } } - if (launch_delay_server) { + if (launch_delay_server || 0 == g_pid_ds) { log_server::info("{}: Launching Delay Server.", __func__); g_pid_ds = fork(); if (0 == g_pid_ds) { @@ -483,7 +489,7 @@ int main(int _argc, char* _argv[]) } } - if (launch_delay_server) { + if (launch_delay_server || 0 == g_pid_ds) { log_server::info("{}: Launching Delay Server.", __func__); g_pid_ds = fork(); if (0 == g_pid_ds) {