Skip to content

Commit

Permalink
core: add CSensitiveString using libsodium's memory functions
Browse files Browse the repository at this point in the history
Using sodium_malloc to create a buffer for our password input gives us
the following benefits:
- Password is not in the same region as other heap structures.
- Password contents do not show up in coredumps.
  • Loading branch information
PaideiaDilemma committed Dec 16, 2024
1 parent 9adc28c commit 0a8e8f0
Show file tree
Hide file tree
Showing 15 changed files with 295 additions and 33 deletions.
11 changes: 11 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,14 @@ install(
FILES ${CMAKE_SOURCE_DIR}/assets/example.conf
DESTINATION ${CMAKE_INSTALL_FULL_DATAROOTDIR}/hypr
RENAME hyprlock.conf)

# Tests
add_custom_target(tests)

add_executable(sensitive-string-test "tests/SensitiveString.cpp")
target_link_libraries(sensitive-string-test PRIVATE sodium)
add_test(
NAME sensitive-string-test
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests
COMMAND sensitive-string-test "SensitiveString")
add_dependencies(tests sensitive-string-test)
8 changes: 5 additions & 3 deletions setpwhash/main.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "../src/helpers/Log.hpp"
#include "../src/helpers/SensitiveString.hpp"

#include <filesystem>
#include <hyprutils/path/Path.hpp>
Expand All @@ -23,10 +24,11 @@ void setStdinEcho(bool enable = true) {
RASSERT(tcsetattr(STDIN_FILENO, TCSANOW, &tty) == 0, "Failed to set terminal attributes");
}

void readPW(std::string& pw) {
void readPW(CSensitiveString& pw) {
setStdinEcho(false);
std::string input;
std::getline(std::cin, pw);
std::getline(std::cin, input);
pw.set(input.c_str());
setStdinEcho(true);
}

Expand Down Expand Up @@ -125,7 +127,7 @@ int main(int argc, char** argv, char** envp) {
RASSERT(DOTDIR.has_value(), "Failed to find config directory!");
const auto DEST = DOTDIR.value() + "/hypr/hyprlock_pwhash.conf";

std::string pw;
CSensitiveString pw;
std::cout << "Enter password: ";

readPW(pw);
Expand Down
2 changes: 1 addition & 1 deletion src/auth/Auth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ void CAuth::start() {
}
}

void CAuth::submitInput(const std::string& input) {
void CAuth::submitInput(const CSensitiveString& input) {
for (const auto& i : m_vImpls) {
i->handleInput(input);
}
Expand Down
17 changes: 9 additions & 8 deletions src/auth/Auth.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include "../helpers/SensitiveString.hpp"
#include <memory>
#include <optional>
#include <vector>
Expand All @@ -14,13 +15,13 @@ class IAuthImplementation {
public:
virtual ~IAuthImplementation() = default;

virtual eAuthImplementations getImplType() = 0;
virtual void init() = 0;
virtual void handleInput(const std::string& input) = 0;
virtual bool checkWaiting() = 0;
virtual std::optional<std::string> getLastFailText() = 0;
virtual std::optional<std::string> getLastPrompt() = 0;
virtual void terminate() = 0;
virtual eAuthImplementations getImplType() = 0;
virtual void init() = 0;
virtual void handleInput(const CSensitiveString& input) = 0;
virtual bool checkWaiting() = 0;
virtual std::optional<std::string> getLastFailText() = 0;
virtual std::optional<std::string> getLastPrompt() = 0;
virtual void terminate() = 0;

friend class CAuth;
};
Expand All @@ -31,7 +32,7 @@ class CAuth {

void start();

void submitInput(const std::string& input);
void submitInput(const CSensitiveString& input);
bool checkWaiting();

// Used by the PasswordInput field. We are constraint to a single line for the authentication feedback there.
Expand Down
2 changes: 1 addition & 1 deletion src/auth/Fingerprint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void CFingerprint::init() {
});
}

void CFingerprint::handleInput(const std::string& input) {
void CFingerprint::handleInput(const CSensitiveString& input) {
;
}

Expand Down
2 changes: 1 addition & 1 deletion src/auth/Fingerprint.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class CFingerprint : public IAuthImplementation {
return AUTH_IMPL_FINGERPRINT;
}
virtual void init();
virtual void handleInput(const std::string& input);
virtual void handleInput(const CSensitiveString& input);
virtual bool checkWaiting();
virtual std::optional<std::string> getLastFailText();
virtual std::optional<std::string> getLastPrompt();
Expand Down
6 changes: 3 additions & 3 deletions src/auth/Pam.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,14 @@ void CPam::waitForInput() {
m_bBlockInput = true;
}

void CPam::handleInput(const std::string& input) {
void CPam::handleInput(const CSensitiveString& input) {
g_pAuth->postActivity(AUTH_IMPL_PAM);
std::unique_lock<std::mutex> lk(m_sConversationState.inputMutex);

if (!m_sConversationState.inputRequested)
Debug::log(ERR, "SubmitInput called, but the auth thread is not waiting for input!");

m_sConversationState.input = input;
m_sConversationState.input.set(input);
m_sConversationState.inputRequested = false;
m_sConversationState.waitingForPamAuth = true;
m_sConversationState.inputSubmittedCondition.notify_all();
Expand All @@ -186,7 +186,7 @@ void CPam::terminate() {
}

void CPam::resetConversation() {
m_sConversationState.input = "";
m_sConversationState.input.clear();
m_sConversationState.waitingForPamAuth = false;
m_sConversationState.inputRequested = false;
m_sConversationState.failTextFromPam = false;
Expand Down
4 changes: 2 additions & 2 deletions src/auth/Pam.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
class CPam : public IAuthImplementation {
public:
struct SPamConversationState {
std::string input = "";
CSensitiveString input = "";
std::string prompt = "";
std::string failText = "";

Expand All @@ -33,7 +33,7 @@ class CPam : public IAuthImplementation {
return AUTH_IMPL_PAM;
}
virtual void init();
virtual void handleInput(const std::string& input);
virtual void handleInput(const CSensitiveString& input);
virtual bool checkWaiting();
virtual std::optional<std::string> getLastFailText();
virtual std::optional<std::string> getLastPrompt();
Expand Down
4 changes: 2 additions & 2 deletions src/auth/SodiumPWHash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ void CSodiumPWHash::init() {
RASSERT(sodium_init() >= 0, "Failed to initialize libsodium");
}

void CSodiumPWHash::handleInput(const std::string& input) {
void CSodiumPWHash::handleInput(const CSensitiveString& input) {
std::lock_guard<std::mutex> lk(m_sCheckerState.requestMutex);

m_sCheckerState.input = input;
m_sCheckerState.input.set(input);
m_sCheckerState.requested = true;

m_sCheckerState.requestCV.notify_all();
Expand Down
4 changes: 2 additions & 2 deletions src/auth/SodiumPWHash.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class CSodiumPWHash : public IAuthImplementation {
return AUTH_IMPL_SODIUM;
}
void init() override;
void handleInput(const std::string& input) override;
void handleInput(const CSensitiveString& input) override;
bool checkWaiting() override;
std::optional<std::string> getLastFailText() override;
std::optional<std::string> getLastPrompt() override;
Expand All @@ -28,7 +28,7 @@ class CSodiumPWHash : public IAuthImplementation {

struct {
std::condition_variable requestCV;
std::string input;
CSensitiveString input;
bool requested = false;
std::mutex requestMutex;
std::string failText;
Expand Down
8 changes: 4 additions & 4 deletions src/core/hyprlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ void CHyprlock::clearPasswordBuffer() {
if (m_sPasswordState.passBuffer.empty())
return;

m_sPasswordState.passBuffer = "";
m_sPasswordState.passBuffer.clear();

renderAllOutputs();
}
Expand Down Expand Up @@ -931,7 +931,7 @@ void CHyprlock::handleKeySym(xkb_keysym_t sym, bool composed) {
if (SYM == XKB_KEY_Escape || (m_bCtrl && (SYM == XKB_KEY_u || SYM == XKB_KEY_BackSpace))) {
Debug::log(LOG, "Clearing password buffer");

m_sPasswordState.passBuffer = "";
m_sPasswordState.passBuffer.clear();
} else if (SYM == XKB_KEY_Return || SYM == XKB_KEY_KP_Enter) {
Debug::log(LOG, "Authenticating");

Expand All @@ -948,7 +948,7 @@ void CHyprlock::handleKeySym(xkb_keysym_t sym, bool composed) {
// handle utf-8
while ((m_sPasswordState.passBuffer.back() & 0xc0) == 0x80)
m_sPasswordState.passBuffer.pop_back();
m_sPasswordState.passBuffer = m_sPasswordState.passBuffer.substr(0, m_sPasswordState.passBuffer.length() - 1);
m_sPasswordState.passBuffer.pop_back();
}
} else if (SYM == XKB_KEY_Caps_Lock) {
m_bCapsLock = !m_bCapsLock;
Expand All @@ -960,7 +960,7 @@ void CHyprlock::handleKeySym(xkb_keysym_t sym, bool composed) {
xkb_keysym_to_utf8(SYM, buf, sizeof(buf)) /* already includes a nullbyte */;

if (len > 1)
m_sPasswordState.passBuffer += std::string{buf, len - 1};
m_sPasswordState.passBuffer.extend(buf, len - 1);
}
}

Expand Down
14 changes: 8 additions & 6 deletions src/core/hyprlock.hpp
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#pragma once

#include "../helpers/SensitiveString.hpp"
#include "Output.hpp"
#include "CursorShape.hpp"
#include "Timer.hpp"

#include <wayland-client.h>
#include "ext-session-lock-v1-protocol.h"
#include "fractional-scale-v1-protocol.h"
#include "wlr-screencopy-unstable-v1-protocol.h"
#include "viewporter-protocol.h"
#include "Output.hpp"
#include "CursorShape.hpp"
#include "Timer.hpp"
#include <memory>
#include <vector>
#include <condition_variable>
Expand Down Expand Up @@ -148,9 +150,9 @@ class CHyprlock {
} m_sLockState;

struct {
std::string passBuffer = "";
size_t failedAttempts = 0;
bool displayFailText = false;
CSensitiveString passBuffer = "";
size_t failedAttempts = 0;
bool displayFailText = false;
} m_sPasswordState;

struct {
Expand Down
117 changes: 117 additions & 0 deletions src/helpers/SensitiveString.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
#pragma once

#include "./Log.hpp"
#include <cstring>
#include <sodium.h>

class CSensitiveString {
public:
static constexpr size_t FIXED_BUFFER_SIZE = 4096;
static size_t capacity() {
return FIXED_BUFFER_SIZE;
}

CSensitiveString(const CSensitiveString&) = delete;
CSensitiveString& operator=(const CSensitiveString&) = delete;

CSensitiveString() {
RASSERT(sodium_init() >= 0, "sodium_init failed");
m_pData = (char*)sodium_malloc(FIXED_BUFFER_SIZE);
m_iLength = 0;
m_pData[0] = '\0';
}

CSensitiveString(const char* data) {
RASSERT(sodium_init() >= 0, "sodium_init failed");
m_pData = (char*)sodium_malloc(FIXED_BUFFER_SIZE);
set(data);
}

~CSensitiveString() {
sodium_free(m_pData);
m_pData = nullptr;
m_iLength = 0;
}

void set(const char* data) {
const auto LEN = strlen(data);
if (LEN >= FIXED_BUFFER_SIZE) {
Debug::log(ERR, "SensitiveString: data too large");
clear();
return;
}
memcpy(m_pData, data, LEN);
m_pData[LEN] = '\0';
m_iLength = LEN;
}

void set(const CSensitiveString& other) {
if (other.m_iLength >= FIXED_BUFFER_SIZE) {
Debug::log(ERR, "SensitiveString: data too large");
clear();
return;
}
memcpy(m_pData, other.m_pData, other.m_iLength);
m_pData[other.m_iLength] = '\0';
m_iLength = other.m_iLength;
}

void extend(char* buf, size_t len) {
if (m_iLength + len >= FIXED_BUFFER_SIZE) {
Debug::log(ERR, "SensitiveString: data too large");
clear();
return;
}
memcpy(m_pData + m_iLength, buf, len);
m_iLength += len;
m_pData[m_iLength] = '\0';
}

char pop_back() {
if (m_iLength == 0)
return '\0';
m_iLength--;
const auto C = m_pData[m_iLength];
m_pData[m_iLength] = '\0';
return C;
}

void clear() {
sodium_memzero(m_pData, FIXED_BUFFER_SIZE);
m_iLength = 0;
}

const char* c_str() const {
return m_pData;
}

size_t length() const {
return m_iLength;
}

size_t size() const {
return m_iLength;
}

char back() const {
if (m_iLength == 0)
return '\0';
return m_pData[m_iLength - 1];
}

bool empty() const {
return m_iLength == 0;
}

const char* begin() const {
return m_pData;
}

const char* end() const {
return m_pData + m_iLength;
}

private:
char* m_pData = nullptr;
size_t m_iLength = 0;
};
Loading

0 comments on commit 0a8e8f0

Please sign in to comment.