From 92c4c6ada445c1925a8b397f4171ca7735cbda16 Mon Sep 17 00:00:00 2001 From: James Short Date: Wed, 18 Jan 2023 10:47:17 -0800 Subject: [PATCH] Logfile open mode and permission plus location configurability. (#556) * Unify logfile names and create with more secure open options and perms. Fixes #555 (Tribute to Colin McRae) * Logfile location configurability. Fixes #555 --- .circleci/config.yml | 8 +++ etc/et.cfg | 1 + src/base/LogHandler.cpp | 89 +++++++++++++++++++++-------- src/base/LogHandler.hpp | 13 ++++- src/htm/HtmClientMain.cpp | 9 +-- src/htm/HtmServerMain.cpp | 9 +-- src/terminal/TerminalClientMain.cpp | 25 ++++---- src/terminal/TerminalMain.cpp | 28 ++++----- src/terminal/TerminalServerMain.cpp | 29 ++++++---- test/Main.cpp | 10 +--- 10 files changed, 127 insertions(+), 94 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index fad233f18..25eaea95d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -53,6 +53,10 @@ jobs: - run: name: Kill server command: sudo pkill etserver + - run: + name: Debug info/logs if failed + when: always + command: ls -la /tmp/et*; sudo awk 'FNR==1 {print "XXXXXX", FILENAME, "XXXXXX"}{print}' /tmp/et*.log connect_with_jumphost: machine: @@ -84,6 +88,10 @@ jobs: - run: name: Kill servers command: sudo pkill etserver + - run: + name: Debug info/logs if failed + when: always + command: ls -la /tmp/et*; sudo awk 'FNR==1 {print "XXXXXX", FILENAME, "XXXXXX"}{print}' /tmp/et*.log workflows: version: 2 diff --git a/etc/et.cfg b/etc/et.cfg index 4a7d3443b..b3fc41dfd 100644 --- a/etc/et.cfg +++ b/etc/et.cfg @@ -10,3 +10,4 @@ verbose = 0 silent = 0 logsize = 20971520 telemetry = true +logdirectory = /tmp diff --git a/src/base/LogHandler.cpp b/src/base/LogHandler.cpp index ad0cde429..33d421845 100644 --- a/src/base/LogHandler.cpp +++ b/src/base/LogHandler.cpp @@ -4,8 +4,9 @@ INITIALIZE_EASYLOGGINGPP namespace et { el::Configurations LogHandler::setupLogHandler(int *argc, char ***argv) { - // easylogging parse verbose arguments, see [Application Arguments] + // easylogging parses verbose arguments, see [Application Arguments] // in https://github.com/muflihun/easyloggingpp/blob/master/README.md + // but it is non-intuitive so we explicitly set verbosity based on cxxopts START_EASYLOGGINGPP(*argc, *argv); // Easylogging configurations @@ -23,13 +24,43 @@ el::Configurations LogHandler::setupLogHandler(int *argc, char ***argv) { return defaultConf; } -void LogHandler::setupLogFile(el::Configurations *defaultConf, string filename, - string maxlogsize) { +void LogHandler::setupLogFiles(el::Configurations *defaultConf, + const string &path, const string &filenamePrefix, + bool logToStdout, bool redirectStderrToFile, + bool appendPid, string maxlogsize) { + time_t rawtime; + struct tm *timeinfo; + char buffer[80]; + time(&rawtime); + timeinfo = localtime(&rawtime); + strftime(buffer, sizeof(buffer), "%Y-%m-%d_%H-%M-%S", timeinfo); + string current_time(buffer); + string logFilename = filenamePrefix + "-" + current_time; + string stderrFilename = filenamePrefix + "-stderr-" + current_time; + if (appendPid) { + string pid = std::to_string(getpid()); + logFilename.append("_" + pid); + stderrFilename.append("_" + pid); + } + logFilename.append(".log"); + stderrFilename.append(".log"); + string fullFname = createLogFile(path, logFilename); + // Enable strict log file size check el::Loggers::addFlag(el::LoggingFlag::StrictLogFileSizeCheck); - defaultConf->setGlobally(el::ConfigurationType::Filename, filename); + defaultConf->setGlobally(el::ConfigurationType::Filename, fullFname); defaultConf->setGlobally(el::ConfigurationType::ToFile, "true"); defaultConf->setGlobally(el::ConfigurationType::MaxLogFileSize, maxlogsize); + + if (logToStdout) { + defaultConf->setGlobally(el::ConfigurationType::ToStandardOutput, "true"); + } else { + defaultConf->setGlobally(el::ConfigurationType::ToStandardOutput, "false"); + } + + if (redirectStderrToFile) { + stderrToFile(path, stderrFilename); + } } void LogHandler::rolloutHandler(const char *filename, std::size_t size) { @@ -38,27 +69,6 @@ void LogHandler::rolloutHandler(const char *filename, std::size_t size) { remove(filename); } -string LogHandler::stderrToFile(const string &pathPrefix) { - time_t rawtime; - struct tm *timeinfo; - char buffer[80]; - time(&rawtime); - timeinfo = localtime(&rawtime); - strftime(buffer, sizeof(buffer), "%Y-%m-%d_%I-%M", timeinfo); - string current_time(buffer); - string stderrFilename = pathPrefix + "_stderr_" + current_time; - FILE *stderr_stream = freopen(stderrFilename.c_str(), "w", stderr); - fs::permissions( - stderrFilename, - fs::perms::owner_read | fs::perms::owner_write | fs::perms::group_read, - fs::perm_options::replace); - if (!stderr_stream) { - STFATAL << "Invalid filename " << stderrFilename; - } - setvbuf(stderr_stream, NULL, _IOLBF, BUFSIZ); // set to line buffering - return stderrFilename; -} - void LogHandler::setupStdoutLogger() { el::Logger *stdoutLogger = el::Loggers::getLogger("stdout"); // Easylogging configurations @@ -70,4 +80,33 @@ void LogHandler::setupStdoutLogger() { stdoutConf.setGlobally(el::ConfigurationType::ToFile, "false"); el::Loggers::reconfigureLogger(stdoutLogger, stdoutConf); } + +string LogHandler::createLogFile(const string &path, const string &filename) { + string fullFname = path + "/" + filename; + try { + fs::create_directories(path); + } catch (const fs::filesystem_error &fse) { + CLOG(ERROR, "stdout") << "Cannot create logfile directory: " << fse.what() + << endl; + exit(1); + } +#ifdef WIN32 + // O_NOFOLLOW does not exist on windows + FATAL_FAIL(::open(fullFname.c_str(), O_EXCL | O_CREAT, 0600)); +#else + FATAL_FAIL(::open(fullFname.c_str(), O_NOFOLLOW | O_EXCL | O_CREAT, 0600)); +#endif + return fullFname; +} + +void LogHandler::stderrToFile(const string &path, + const string &stderrFilename) { + string fullFname = createLogFile(path, stderrFilename); + FILE *stderr_stream = freopen(fullFname.c_str(), "w", stderr); + if (!stderr_stream) { + STFATAL << "Invalid filename " << stderrFilename; + } + setvbuf(stderr_stream, NULL, _IOLBF, BUFSIZ); // set to line buffering +} + } // namespace et diff --git a/src/base/LogHandler.hpp b/src/base/LogHandler.hpp index 8fcf8e6e7..1fa9088cb 100644 --- a/src/base/LogHandler.hpp +++ b/src/base/LogHandler.hpp @@ -7,11 +7,18 @@ namespace et { class LogHandler { public: static el::Configurations setupLogHandler(int *argc, char ***argv); - static void setupLogFile(el::Configurations *defaultConf, string filename, - string maxlogsize = "20971520"); + static void setupLogFiles(el::Configurations *defaultConf, const string &path, + const string &filenamePrefix, + bool logToStdout = false, + bool redirectStderrToFile = false, + bool appendPid = false, + string maxlogsize = "20971520"); static void rolloutHandler(const char *filename, std::size_t size); - static string stderrToFile(const string &pathPrefix); static void setupStdoutLogger(); + + private: + static void stderrToFile(const string &path, const string &stderrFilename); + static string createLogFile(const string &path, const string &filename); }; } // namespace et #endif // __ET_LOG_HANDLER__ diff --git a/src/htm/HtmClientMain.cpp b/src/htm/HtmClientMain.cpp index 46fc23cc7..e1b705b2e 100644 --- a/src/htm/HtmClientMain.cpp +++ b/src/htm/HtmClientMain.cpp @@ -61,14 +61,9 @@ int main(int argc, char** argv) { // Setup easylogging configurations el::Configurations defaultConf = LogHandler::setupLogHandler(&argc, &argv); - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false"); el::Loggers::setVerboseLevel(3); - // default max log file size is 20MB for etserver - string maxlogsize = "20971520"; - LogHandler::setupLogFile(&defaultConf, GetTempDirectory() + "htm.log", - maxlogsize); - // Redirect std streams to a file - LogHandler::stderrToFile(GetTempDirectory() + "htm"); + LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(), "htm", false, + true); // Reconfigure default logger to apply settings above el::Loggers::reconfigureLogger("default", defaultConf); diff --git a/src/htm/HtmServerMain.cpp b/src/htm/HtmServerMain.cpp index 8eae2accb..a79a9e435 100644 --- a/src/htm/HtmServerMain.cpp +++ b/src/htm/HtmServerMain.cpp @@ -13,14 +13,9 @@ int main(int argc, char **argv) { // Setup easylogging configurations el::Configurations defaultConf = et::LogHandler::setupLogHandler(&argc, &argv); - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false"); el::Loggers::setVerboseLevel(3); - // default max log file size is 20MB for etserver - string maxlogsize = "20971520"; - LogHandler::setupLogFile(&defaultConf, GetTempDirectory() + "htmd.log", - maxlogsize); - // Redirect std streams to a file - LogHandler::stderrToFile(GetTempDirectory() + "htmd"); + LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(), "htmd", false, + true); // Reconfigure default logger to apply settings above el::Loggers::reconfigureLogger("default", defaultConf); diff --git a/src/terminal/TerminalClientMain.cpp b/src/terminal/TerminalClientMain.cpp index 2ce996e04..0823a923f 100644 --- a/src/terminal/TerminalClientMain.cpp +++ b/src/terminal/TerminalClientMain.cpp @@ -88,11 +88,13 @@ int main(int argc, char** argv) { ("v,verbose", "Enable verbose logging", cxxopts::value()->default_value("0")) // ("k,keepalive", "Client keepalive duration in seconds", - cxxopts::value()) // - ("logtostdout", "Write log to stdout") // - ("silent", "Disable logging") // - ("N,no-terminal", "Do not create a terminal") // - ("f,forward-ssh-agent", "Forward ssh-agent socket") // + cxxopts::value()) // + ("l,logdir", "Base directory for log files.", + cxxopts::value()->default_value(tmpDir)) // + ("logtostdout", "Write log to stdout") // + ("silent", "Disable logging") // + ("N,no-terminal", "Do not create a terminal") // + ("f,forward-ssh-agent", "Forward ssh-agent socket") // ("ssh-socket", "The ssh-agent socket to forward", cxxopts::value()) // ("telemetry", @@ -119,21 +121,14 @@ int main(int argc, char** argv) { el::Loggers::setVerboseLevel(result["verbose"].as()); - if (result.count("logtostdout")) { - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "true"); - } else { - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false"); - // Redirect std streams to a file - LogHandler::stderrToFile((tmpDir + "/etclient")); - } - // silent Flag, since etclient doesn't read /etc/et.cfg file if (result.count("silent")) { defaultConf.setGlobally(el::ConfigurationType::Enabled, "false"); } - LogHandler::setupLogFile( - &defaultConf, (tmpDir + "/etclient-%datetime{%Y-%M-%d_%H_%m_%s}.log")); + LogHandler::setupLogFiles(&defaultConf, result["logdir"].as(), + "etclient", result.count("logtostdout"), + !result.count("logtostdout")); el::Loggers::reconfigureLogger("default", defaultConf); // set thread name diff --git a/src/terminal/TerminalMain.cpp b/src/terminal/TerminalMain.cpp index 046713a4a..df02ea014 100644 --- a/src/terminal/TerminalMain.cpp +++ b/src/terminal/TerminalMain.cpp @@ -48,7 +48,9 @@ int main(int argc, char** argv) { // Not used by etterminal but easylogging uses this flag under the hood ("v,verbose", "Enable verbose logging", cxxopts::value()->default_value("0")) // - ("logtostdout", "Write log to stdout") // + ("l,logdir", "Base directory for log files.", + cxxopts::value()->default_value(GetTempDirectory())) // + ("logtostdout", "Write log to stdout") // ("serverfifo", "If set, connects to the etserver instance listening on the matching " "fifo name", // @@ -63,15 +65,6 @@ int main(int argc, char** argv) { el::Loggers::setVerboseLevel(result["verbose"].as()); - if (result.count("logtostdout")) { - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "true"); - } else { - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false"); - } - - // default max log file size is 20MB for etserver - string maxlogsize = "20971520"; - GOOGLE_PROTOBUF_VERIFY_VERSION; srand(1); @@ -147,10 +140,9 @@ int main(int argc, char** argv) { string username = string(ssh_get_local_username()); if (result.count("jump")) { // etserver with --jump cannot write to the default log file(root) - LogHandler::setupLogFile( - &defaultConf, - GetTempDirectory() + "etjump-" + username + "-" + id + ".log", - maxlogsize); + LogHandler::setupLogFiles(&defaultConf, result["logdir"].as(), + ("etjump-" + username + "-" + id), + result.count("logtostdout"), false); // Reconfigure default logger to apply settings above el::Loggers::reconfigureLogger("default", defaultConf); // set thread name @@ -177,10 +169,10 @@ int main(int argc, char** argv) { } // etserver with --idpasskey cannot write to the default log file(root) - LogHandler::setupLogFile( - &defaultConf, - GetTempDirectory() + "etterminal-" + username + "-" + id + ".log", - maxlogsize); + LogHandler::setupLogFiles(&defaultConf, result["logdir"].as(), + ("etterminal-" + username + "-" + id), + result.count("logtostdout"), false); + // Reconfigure default logger to apply settings above el::Loggers::reconfigureLogger("default", defaultConf); // set thread name diff --git a/src/terminal/TerminalServerMain.cpp b/src/terminal/TerminalServerMain.cpp index 3d10552d4..6c62776ad 100644 --- a/src/terminal/TerminalServerMain.cpp +++ b/src/terminal/TerminalServerMain.cpp @@ -37,7 +37,9 @@ int main(int argc, char **argv) { ("daemon", "Daemonize the server") // ("cfgfile", "Location of the config file", cxxopts::value()->default_value("")) // - ("logtostdout", "log to stdout") // + ("l,logdir", "Base directory for log files.", + cxxopts::value()) // + ("logtostdout", "log to stdout") // ("pidfile", "Location of the pid file", cxxopts::value()->default_value( "/var/run/etserver.pid")) // @@ -70,14 +72,6 @@ int main(int argc, char **argv) { } } - if (result.count("logtostdout")) { - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "true"); - } else { - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false"); - // Redirect std streams to a file - LogHandler::stderrToFile(GetTempDirectory() + "etserver"); - } - ServerFifoPath serverFifo; // default max log file size is 20MB for etserver @@ -86,6 +80,7 @@ int main(int argc, char **argv) { int port = 0; string bindIp = ""; bool telemetry = false; + string logDirectory = GetTempDirectory(); if (result.count("cfgfile")) { // Load the config file CSimpleIniA ini(true, false, false); @@ -128,6 +123,7 @@ int main(int argc, char **argv) { if (silent && atoi(silent) != 0) { defaultConf.setGlobally(el::ConfigurationType::Enabled, "false"); } + // read log file size limit const char *logsize = ini.GetValue("Debug", "logsize", NULL); if (logsize && atoi(logsize) != 0) { @@ -135,6 +131,11 @@ int main(int argc, char **argv) { maxlogsize = string(logsize); } + // log file directory (TODO path validation and trailing slash cleanup) + const char *logdir = ini.GetValue("Debug", "logdirectory", NULL); + if (logdir) { + logDirectory = string(logdir); + } } else { STFATAL << "Invalid config file: " << cfgfilename; } @@ -157,6 +158,10 @@ int main(int argc, char **argv) { telemetry = result["telemetry"].as(); } + if (result.count("logdir")) { + logDirectory = result["logdir"].as(); + } + GOOGLE_PROTOBUF_VERIFY_VERSION; srand(1); @@ -165,9 +170,9 @@ int main(int argc, char **argv) { } // Set log file for etserver process here. - LogHandler::setupLogFile(&defaultConf, - GetTempDirectory() + "etserver-%datetime.log", - maxlogsize); + LogHandler::setupLogFiles( + &defaultConf, logDirectory, "etserver", result.count("logtostdout"), + !result.count("logtostdout"), true /* appendPid */, maxlogsize); // Reconfigure default logger to apply settings above el::Loggers::reconfigureLogger("default", defaultConf); // set thread name diff --git a/test/Main.cpp b/test/Main.cpp index eb6016baa..537961dbf 100644 --- a/test/Main.cpp +++ b/test/Main.cpp @@ -13,17 +13,14 @@ int main(int argc, char **argv) { el::Configurations defaultConf = et::LogHandler::setupLogHandler(&argc, &argv); et::LogHandler::setupStdoutLogger(); - defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "true"); - defaultConf.setGlobally(el::ConfigurationType::ToFile, "true"); // el::Loggers::setVerboseLevel(9); et::HandleTerminate(); string logDirectoryPattern = GetTempDirectory() + string("et_test_XXXXXXXX"); string logDirectory = string(mkdtemp(&logDirectoryPattern[0])); - string logPath = string(logDirectory) + "/log"; - CLOG(INFO, "stdout") << "Writing log to " << logPath << endl; - et::LogHandler::setupLogFile(&defaultConf, logPath); + CLOG(INFO, "stdout") << "Writing log to " << logDirectory << endl; + et::LogHandler::setupLogFiles(&defaultConf, logDirectory, "log", true, true); // Reconfigure default logger to apply settings above el::Loggers::reconfigureLogger("default", defaultConf); @@ -35,7 +32,6 @@ int main(int argc, char **argv) { TelemetryService::get()->shutdown(); TelemetryService::destroy(); - FATAL_FAIL(::remove(logPath.c_str())); - FATAL_FAIL(::remove(logDirectory.c_str())); + FATAL_FAIL(fs::remove_all(logDirectory.c_str())); return result; }