fix(security): overwrite key/PIN files with fsync before deleting them
Two secure-delete paths overwrote a sensitive file and then unlinked it, but neither reliably forced the overwrite to stable storage. The plaintext key export used a buffered stream with only a flush, and the PIN vault removal opened a second descriptor to fsync only on non- Windows. On a crash, or on Windows, the zeroing write could stay in the OS cache and the original bytes could survive on disk. Both now go through one shared best-effort secure-delete helper: - It overwrites the file in place in chunks, never truncating first, so the original blocks are the ones rewritten. - It flushes the data to the device before unlinking (full fsync on macOS, write-through on Windows), then removes the file. - It is safe on a missing file and reports whether the file is gone. This is still best-effort: SSD wear-levelling and copy-on-write or journaling filesystems may retain copies. Tests exercise the overwrite-and-remove helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -300,6 +300,58 @@ bool Platform::deleteFile(const std::string& path)
|
||||
#endif
|
||||
}
|
||||
|
||||
bool Platform::secureWipeAndRemove(const std::string& path)
|
||||
{
|
||||
// Best-effort; SSD wear-levelling and CoW/journaling filesystems may retain copies.
|
||||
if (path.empty()) return false;
|
||||
static constexpr size_t kChunk = 64 * 1024;
|
||||
const std::string buf(kChunk, static_cast<char>(0xAB));
|
||||
#ifdef _WIN32
|
||||
HANDLE h = CreateFileA(path.c_str(), GENERIC_WRITE, 0, nullptr, OPEN_EXISTING,
|
||||
FILE_ATTRIBUTE_NORMAL | FILE_FLAG_WRITE_THROUGH, nullptr);
|
||||
if (h != INVALID_HANDLE_VALUE) {
|
||||
LARGE_INTEGER sz;
|
||||
if (GetFileSizeEx(h, &sz) && sz.QuadPart > 0) {
|
||||
long long left = sz.QuadPart;
|
||||
while (left > 0) {
|
||||
DWORD n = static_cast<DWORD>(left < (long long)kChunk ? left : (long long)kChunk);
|
||||
DWORD written = 0;
|
||||
if (!WriteFile(h, buf.data(), n, &written, nullptr) || written == 0) break;
|
||||
left -= written;
|
||||
}
|
||||
FlushFileBuffers(h);
|
||||
}
|
||||
CloseHandle(h);
|
||||
}
|
||||
DeleteFileA(path.c_str());
|
||||
return GetFileAttributesA(path.c_str()) == INVALID_FILE_ATTRIBUTES;
|
||||
#else
|
||||
int fd = ::open(path.c_str(), O_WRONLY | O_NOFOLLOW | O_CLOEXEC);
|
||||
if (fd >= 0) {
|
||||
struct stat st;
|
||||
if (::fstat(fd, &st) == 0 && S_ISREG(st.st_mode) && st.st_size > 0) {
|
||||
off_t off = 0;
|
||||
const off_t total = st.st_size;
|
||||
while (off < total) {
|
||||
size_t n = static_cast<size_t>(total - off < (off_t)kChunk ? total - off : (off_t)kChunk);
|
||||
ssize_t w = ::pwrite(fd, buf.data(), n, off);
|
||||
if (w <= 0) break;
|
||||
off += w;
|
||||
}
|
||||
#ifdef __APPLE__
|
||||
if (::fcntl(fd, F_FULLFSYNC) != 0) ::fsync(fd);
|
||||
#else
|
||||
::fsync(fd);
|
||||
#endif
|
||||
}
|
||||
::close(fd);
|
||||
}
|
||||
::remove(path.c_str());
|
||||
struct stat st2;
|
||||
return ::lstat(path.c_str(), &st2) != 0 && errno == ENOENT;
|
||||
#endif
|
||||
}
|
||||
|
||||
bool Platform::writeFileAtomically(const std::string& path,
|
||||
const std::string& content,
|
||||
bool restrictPermissions)
|
||||
|
||||
@@ -84,6 +84,15 @@ public:
|
||||
*/
|
||||
static bool deleteFile(const std::string& path);
|
||||
|
||||
/**
|
||||
* @brief Best-effort secure delete: overwrite the file in place, flush, then unlink.
|
||||
*
|
||||
* Never truncates first (that would free the original blocks). Always attempts the unlink.
|
||||
* SSD wear-levelling and CoW/journaling filesystems may retain copies.
|
||||
* @return true when the file no longer exists
|
||||
*/
|
||||
static bool secureWipeAndRemove(const std::string& path);
|
||||
|
||||
/**
|
||||
* @brief Write a file atomically and durably.
|
||||
*
|
||||
|
||||
@@ -15,11 +15,6 @@
|
||||
#include <algorithm>
|
||||
#include "../util/logger.h"
|
||||
|
||||
#ifndef _WIN32
|
||||
#include <fcntl.h>
|
||||
#include <unistd.h>
|
||||
#endif
|
||||
|
||||
namespace fs = std::filesystem;
|
||||
|
||||
namespace dragonx {
|
||||
@@ -260,32 +255,10 @@ bool SecureVault::changePin(const std::string& oldPin, const std::string& newPin
|
||||
bool SecureVault::removeVault() {
|
||||
std::string vaultPath = getVaultPath();
|
||||
std::error_code ec;
|
||||
if (fs::exists(vaultPath, ec)) {
|
||||
// Overwrite with zeros before removing (best-effort secure delete)
|
||||
{
|
||||
std::ifstream probe(vaultPath, std::ios::binary | std::ios::ate);
|
||||
size_t sz = probe.is_open() ? (size_t)probe.tellg() : 0;
|
||||
probe.close();
|
||||
if (sz > 0) {
|
||||
std::vector<uint8_t> zeros(sz, 0);
|
||||
std::ofstream zap(vaultPath, std::ios::binary);
|
||||
if (zap.is_open()) {
|
||||
zap.write(reinterpret_cast<const char*>(zeros.data()), sz);
|
||||
zap.flush();
|
||||
zap.close();
|
||||
// Force the zeros to stable storage before unlinking — otherwise the
|
||||
// write may never leave the OS cache (best-effort; not a guarantee on
|
||||
// CoW/journaling filesystems or wear-leveling SSDs).
|
||||
#ifndef _WIN32
|
||||
int fd = ::open(vaultPath.c_str(), O_WRONLY);
|
||||
if (fd >= 0) { ::fsync(fd); ::close(fd); }
|
||||
#endif
|
||||
}
|
||||
}
|
||||
}
|
||||
return fs::remove(vaultPath, ec);
|
||||
}
|
||||
return true; // Didn't exist — that's fine
|
||||
if (!fs::exists(vaultPath, ec)) return true; // Didn't exist — that's fine
|
||||
// Overwrite in place + fsync, then unlink (best-effort secure delete).
|
||||
util::Platform::secureWipeAndRemove(vaultPath);
|
||||
return !fs::exists(vaultPath, ec);
|
||||
}
|
||||
|
||||
} // namespace util
|
||||
|
||||
Reference in New Issue
Block a user