Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/lib/SoftHSM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -574,10 +574,17 @@ CK_RV SoftHSM::C_Initialize(CK_VOID_PTR pInitArgs)
return CKR_GENERAL_ERROR;
}

// Configure log file (empty string = use syslog)
if (!setLogFile(Configuration::i()->getString("log.file", "")))
{
WARNING_MSG("Could not open log file, using syslog");
}

// Configure object store storage backend used by all tokens.
if (!ObjectStoreToken::selectBackend(Configuration::i()->getString("objectstore.backend", DEFAULT_OBJECTSTORE_BACKEND)))
{
ERROR_MSG("Could not set the storage backend");
closeLogFile();
return CKR_GENERAL_ERROR;
}

Expand All @@ -589,6 +596,7 @@ CK_RV SoftHSM::C_Initialize(CK_VOID_PTR pInitArgs)
if (!objectStore->isValid())
{
WARNING_MSG("Could not load the object store");
closeLogFile();
delete objectStore;
objectStore = NULL;
delete sessionObjectStore;
Expand Down Expand Up @@ -637,6 +645,9 @@ CK_RV SoftHSM::C_Finalize(CK_VOID_PTR pReserved)
CryptoFactory::reset();
SecureMemoryRegistry::reset();

// Close log file if open
closeLogFile();

isInitialised = false;

supportedMechanisms.clear();
Expand Down
1 change: 1 addition & 0 deletions src/lib/common/Configuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const struct config Configuration::valid_config[] = {
{ "objectstore.backend", CONFIG_TYPE_STRING },
{ "objectstore.umask", CONFIG_TYPE_INT_OCTAL },
{ "log.level", CONFIG_TYPE_STRING },
{ "log.file", CONFIG_TYPE_STRING },
{ "slots.removable", CONFIG_TYPE_BOOL },
{ "slots.mechanisms", CONFIG_TYPE_STRING },
{ "library.reset_on_fork", CONFIG_TYPE_BOOL },
Expand Down
121 changes: 115 additions & 6 deletions src/lib/common/log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

Implements logging functions. This file is based on the concepts from
SoftHSM v1 but extends the logging functions with support for a variable
argument list as defined in stdarg (3).
argument list as defined in stdarg (3) and logging to file.
*****************************************************************************/

#include "config.h"
Expand All @@ -38,9 +38,19 @@
#include <stdio.h>
#include <sstream>
#include <vector>
#include <time.h>
#ifdef _WIN32
#include <windows.h>
#include <process.h>
#else
#include <unistd.h>
#endif
#include "log.h"
#include "MutexFactory.h"

int softLogLevel = LOG_DEBUG;
static FILE* logFile = nullptr;
static Mutex* logMutex = nullptr;

bool setLogLevel(const std::string &loglevel)
{
Expand Down Expand Up @@ -69,6 +79,95 @@ bool setLogLevel(const std::string &loglevel)
return true;
}

bool setLogFile(const std::string &logFilePath)
{
// Quick return without creating mutex for default configuration
if (logFilePath.empty() && logFile == nullptr)
{
return true;
}

if (logMutex == nullptr)
{
// Create mutex for later access
logMutex = MutexFactory::i()->getMutex();
}

if (logFile != nullptr)
{
fclose(logFile);
logFile = nullptr;
}

if (logFilePath.empty())
{
return true;
}

// This function needs to be called in init so it does not need locking
logFile = fopen(logFilePath.c_str(), "a");
if (logFile == nullptr)
{
syslog(LOG_ERR, "Failed to open log file: %s, using syslog only", logFilePath.c_str());
return false;
}

return true;
}

void closeLogFile()
{
if (logFile != nullptr)
{
fclose(logFile);
logFile = nullptr;
}

if (logMutex != nullptr)
{
MutexFactory::i()->recycleMutex(logMutex);
logMutex = nullptr;
}
}

static const char* getLevelString(int loglevel)
{
switch(loglevel)
{
case LOG_ERR: return "ERROR";
case LOG_WARNING: return "WARNING";
case LOG_INFO: return "INFO";
case LOG_DEBUG: return "DEBUG";
default: return "UNKNOWN";
}
}

static void writeLogToFile(const int loglevel, const char* prependText, const char* msgText)
{
MutexLocker lock(logMutex);

#ifdef _WIN32
SYSTEMTIME st;
GetLocalTime(&st);
fprintf(logFile, "%04d-%02d-%02d %02d:%02d:%02d.%03d [%d] %s: %s%s\n",
st.wYear, st.wMonth, st.wDay, st.wHour, st.wMinute, st.wSecond, st.wMilliseconds,
_getpid(), getLevelString(loglevel), prependText, msgText);
#else
struct timespec ts;
clock_gettime(CLOCK_REALTIME, &ts);
struct tm timeinfo;
localtime_r(&ts.tv_sec, &timeinfo);

char basetime[32];
strftime(basetime, sizeof(basetime), "%Y-%m-%d %H:%M:%S", &timeinfo);
fprintf(logFile, "%s.%03ld [%d] %s: %s%s\n",
basetime, ts.tv_nsec / 1000000,
(int)getpid(), getLevelString(loglevel), prependText, msgText);
#endif

fflush(logFile);
}
Comment on lines +145 to +169
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's explore the structure and understand the logging setup
find . -name "log.cpp" -o -name "log.h" | head -20

Repository: softhsm/SoftHSMv2

Length of output: 108


🏁 Script executed:

# Check the full log.cpp file to understand initialization
wc -l src/lib/common/log.cpp

Repository: softhsm/SoftHSMv2

Length of output: 87


🏁 Script executed:

# Look for logFile and logMutex declarations
rg -n "logFile|logMutex" src/lib/common/log.cpp -B 2 -A 2

Repository: softhsm/SoftHSMv2

Length of output: 2165


🏁 Script executed:

# Check how these are initialized
rg -n "logFile\s*=" src/lib/common/log.cpp | head -20

Repository: softhsm/SoftHSMv2

Length of output: 272


🏁 Script executed:

# Look for the complete log initialization function
rg -n "void.*[Ll]og" src/lib/common/log.cpp | grep -E "(init|Init|setup|Setup)"

Repository: softhsm/SoftHSMv2

Length of output: 43


🏁 Script executed:

# Check if there are other calls to writeLogToFile
rg -n "writeLogToFile" src/lib/common/log.cpp

Repository: softhsm/SoftHSMv2

Length of output: 211


🏁 Script executed:

# Check what platforms are supported - look at build configs
find . -name "*.ac" -o -name "CMakeLists.txt" -o -name "configure" | grep -E "^\./[^/]+$|CMakeLists|configure\.ac" | head -10

Repository: softhsm/SoftHSMv2

Length of output: 321


🏁 Script executed:

# Check for clock_gettime usage and portability handling
rg -n "clock_gettime|CLOCK_REALTIME" src/

Repository: softhsm/SoftHSMv2

Length of output: 124


🏁 Script executed:

# Look for platform-specific defines and portability considerations
rg -n "WIN32|_WIN32|HAVE_CLOCK|sun|solaris|aix|hpux" src/lib/common/log.cpp

Repository: softhsm/SoftHSMv2

Length of output: 95


🏁 Script executed:

# Check the main header to understand platform support declarations
head -50 src/lib/common/log.h

Repository: softhsm/SoftHSMv2

Length of output: 2166


🏁 Script executed:

# Check configure.ac for platform support and clock_gettime checks
cat configure.ac | head -100

Repository: softhsm/SoftHSMv2

Length of output: 2688


🏁 Script executed:

# Check for specific clock_gettime checks in configure
rg -n "clock_gettime|HAVE_CLOCK" configure.ac

Repository: softhsm/SoftHSMv2

Length of output: 43


🏁 Script executed:

# Check CMakeLists.txt for platform detection
cat CMakeLists.txt | head -80

Repository: softhsm/SoftHSMv2

Length of output: 3231


🏁 Script executed:

# Look for config.h template or any portability headers
find . -name "config.h*" -type f

Repository: softhsm/SoftHSMv2

Length of output: 80


🏁 Script executed:

# Check what syslog calls are used elsewhere
rg -n "syslog" src/lib/common/log.cpp | head -20

Repository: softhsm/SoftHSMv2

Length of output: 287


🏁 Script executed:

# Search more thoroughly in configure.ac for clock or time checks
cat configure.ac | grep -i "clock\|time\|posix" -C 2

Repository: softhsm/SoftHSMv2

Length of output: 43


🏁 Script executed:

# Search for AC_CHECK_FUNCS in configure.ac
rg -n "AC_CHECK_FUNCS" configure.ac

Repository: softhsm/SoftHSMv2

Length of output: 93


🏁 Script executed:

# Get full context around configure.ac to see all checks
cat configure.ac | tail -100

Repository: softhsm/SoftHSMv2

Length of output: 2226


🏁 Script executed:

# Check if there are any ifdefs or checks for clock_gettime in the codebase
rg -n "HAVE_CLOCK|HAVE_POSIX|__POSIX__|POSIX" src/

Repository: softhsm/SoftHSMv2

Length of output: 1483


🏁 Script executed:

# Verify the exact context of how writeLogToFile is called
sed -n '200,215p' src/lib/common/log.cpp

Repository: softhsm/SoftHSMv2

Length of output: 458


🏁 Script executed:

# Check README or documentation for supported platforms
find . -name "README*" -o -name "INSTALL*" | head -10

Repository: softhsm/SoftHSMv2

Length of output: 94


🏁 Script executed:

# Look for any platform-specific issues or CI configs
find . -name ".github" -o -name ".gitlab-ci*" -o -name ".travis*" -o -name "appveyor*" 2>/dev/null | head -20

Repository: softhsm/SoftHSMv2

Length of output: 84


🏁 Script executed:

# Check if there's any documentation about supported platforms
cat README.md 2>/dev/null | head -50

Repository: softhsm/SoftHSMv2

Length of output: 2281


🏁 Script executed:

# Check if clock_gettime is conditionally compiled elsewhere or if POSIX is assumed
rg -n "localtime_r|strftime|struct tm|timespec" src/lib/common/ -B 2 -A 2

Repository: softhsm/SoftHSMv2

Length of output: 713


🏁 Script executed:

# Final check: verify the exact linking and compilation context
cat src/lib/common/CMakeLists.txt | grep -A 20 "add_library\|target_link"

Repository: softhsm/SoftHSMv2

Length of output: 930


🏁 Script executed:

# Verify if any platform-specific handling exists for clock functions
rg -n "rt\|clock\|time" src/lib/common/CMakeLists.txt

Repository: softhsm/SoftHSMv2

Length of output: 43


Consider adding null-guards and verify clock_gettime build support.

Two low-cost hardening points:

  • writeLogToFile() is only called with a guard check (if (logFile != nullptr) at line 205), making an internal null-check redundant. However, a defensive guard could protect against future refactoring. Note: the suggested syslog() fallback already exists at the higher call level (line 211).
  • clock_gettime(CLOCK_REALTIME, ...) (line 157) is POSIX.1-2001 standard and used unconditionally on non-Windows targets. The build system (configure.ac) lacks an explicit AC_CHECK_FUNCS([clock_gettime]) check. Confirm this function is available on all non-Windows platforms your project targets, or add a build-time portability check.


void softHSMLog(const int loglevel, const char* functionName, const char* fileName, const int lineNo, const char* format, ...)
{
if (loglevel > softLogLevel) return;
Expand Down Expand Up @@ -98,12 +197,22 @@ void softHSMLog(const int loglevel, const char* functionName, const char* fileNa
vsnprintf(&logMessage[0], 4096, format, args);
va_end(args);

// And log it
syslog(loglevel, "%s%s", prepend.str().c_str(), &logMessage[0]);
const char* msgText = &logMessage[0];
std::string prependStr = prepend.str();
const char* prependText = prependStr.c_str();

// Log to file if configured, otherwise use syslog
if (logFile != nullptr)
{
writeLogToFile(loglevel, prependText, msgText);
}
else
{
syslog(loglevel, "%s%s", prependText, msgText);
}

#ifdef DEBUG_LOG_STDERR
fprintf(stderr, "%s%s\n", prepend.str().c_str(), &logMessage[0]);
fprintf(stderr, "%s%s\n", prependText, msgText);
fflush(stderr);
#endif // DEBUG_LOG_STDERR
#endif
}

2 changes: 2 additions & 0 deletions src/lib/common/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@

/* Function definitions */
bool setLogLevel(const std::string &loglevel);
bool setLogFile(const std::string &logFilePath);
void closeLogFile();
void softHSMLog(const int loglevel, const char* functionName, const char* fileName, const int lineNo, const char* format, ...);

#endif /* !_SOFTHSM_V2_LOG_H */
Expand Down
Loading