Skip to content

Commit

Permalink
Tidy credentials.??
Browse files Browse the repository at this point in the history
  • Loading branch information
@omoerbeek
omoerbeek committed Jan 20, 2025
1 parent 42e5155 commit 09239d5
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 34 deletions.
66 changes: 36 additions & 30 deletions pdns/credentials.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,11 @@ static std::string hashPasswordInternal([[maybe_unused]] const std::string& pass
}

// OpenSSL 3.0 changed the string arg to const unsigned char*, other versions use const char *
// NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast)
#if OPENSSL_VERSION_MAJOR >= 3
auto passwordData = reinterpret_cast<const char*>(password.data());
const auto* passwordData = reinterpret_cast<const char*>(password.data());
#else
auto passwordData = reinterpret_cast<const unsigned char*>(password.data());
const auto* passwordData = reinterpret_cast<const unsigned char*>(password.data());
#endif
if (EVP_PKEY_CTX_set1_pbe_pass(pctx.get(), passwordData, password.size()) <= 0) {
throw std::runtime_error("Error adding the password to the scrypt context to hash the supplied password");
Expand All @@ -118,6 +119,7 @@ static std::string hashPasswordInternal([[maybe_unused]] const std::string& pass
if (EVP_PKEY_CTX_set1_scrypt_salt(pctx.get(), reinterpret_cast<const unsigned char*>(salt.data()), salt.size()) <= 0) {
throw std::runtime_error("Error adding the salt to the scrypt context to hash the supplied password");
}
// NOLINTEND(cppcoreguidelines-pro-type-reinterpret-cast)

if (EVP_PKEY_CTX_set_scrypt_N(pctx.get(), workFactor) <= 0) {
throw std::runtime_error("Error setting the work factor to the scrypt context to hash the supplied password");
Expand All @@ -135,6 +137,7 @@ static std::string hashPasswordInternal([[maybe_unused]] const std::string& pass
out.resize(pwhash_output_size);
size_t outlen = out.size();

// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
if (EVP_PKEY_derive(pctx.get(), reinterpret_cast<unsigned char*>(out.data()), &outlen) <= 0 || outlen != pwhash_output_size) {
throw std::runtime_error("Error deriving the output from the scrypt context to hash the supplied password");
}
Expand All @@ -152,7 +155,8 @@ static std::string generateRandomSalt()
std::string salt;
salt.resize(pwhash_salt_size);

if (RAND_bytes(reinterpret_cast<unsigned char*>(salt.data()), salt.size()) != 1) {
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
if (RAND_bytes(reinterpret_cast<unsigned char*>(salt.data()), static_cast<int>(salt.size())) != 1) {
throw std::runtime_error("Error while generating a salt to hash the supplied password");
}

Expand Down Expand Up @@ -371,6 +375,7 @@ CredentialsHolder::CredentialsHolder(std::string&& password, bool hashPlaintext)

if (!d_isHashed) {
d_fallbackHashPerturb = dns_random_uint32();
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
d_fallbackHash = burtle(reinterpret_cast<const unsigned char*>(d_credentials.getString().data()), d_credentials.getString().size(), d_fallbackHashPerturb);
}
}
Expand All @@ -386,14 +391,13 @@ bool CredentialsHolder::matches(const std::string& password) const
if (d_isHashed) {
return verifyPassword(d_credentials.getString(), d_salt, d_workFactor, d_parallelFactor, d_blockSize, password);
}
else {
uint32_t fallback = burtle(reinterpret_cast<const unsigned char*>(password.data()), password.size(), d_fallbackHashPerturb);
if (fallback != d_fallbackHash) {
return false;
}

return constantTimeStringEquals(password, d_credentials.getString());
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
uint32_t fallback = burtle(reinterpret_cast<const unsigned char*>(password.data()), password.size(), d_fallbackHashPerturb);
if (fallback != d_fallbackHash) {
return false;
}

return constantTimeStringEquals(password, d_credentials.getString());
}

bool CredentialsHolder::isHashingAvailable()
Expand All @@ -410,8 +414,8 @@ bool CredentialsHolder::isHashingAvailable()

SensitiveData CredentialsHolder::readFromTerminal()
{
struct termios term;
struct termios oterm;
termios term{};
termios oterm{};
bool restoreTermSettings = false;
int termAction = TCSAFLUSH;
#ifdef TCSASOFT
Expand All @@ -438,30 +442,32 @@ SensitiveData CredentialsHolder::readFromTerminal()
}

struct std::map<int, struct sigaction> signals;
struct sigaction sa;
sigemptyset(&sa.sa_mask);
sa.sa_flags = 0;
sa.sa_handler = [](int /* s */) {};
sigaction(SIGALRM, &sa, &signals[SIGALRM]);
sigaction(SIGHUP, &sa, &signals[SIGHUP]);
sigaction(SIGINT, &sa, &signals[SIGINT]);
sigaction(SIGPIPE, &sa, &signals[SIGPIPE]);
sigaction(SIGQUIT, &sa, &signals[SIGQUIT]);
sigaction(SIGTERM, &sa, &signals[SIGTERM]);
sigaction(SIGTSTP, &sa, &signals[SIGTSTP]);
sigaction(SIGTTIN, &sa, &signals[SIGTTIN]);
sigaction(SIGTTOU, &sa, &signals[SIGTTOU]);
struct sigaction sigact // just sigaction does noty work, it clashes with sigaction(2)
{
};
sigemptyset(&sigact.sa_mask);
sigact.sa_flags = 0;
sigact.sa_handler = [](int /* s */) {};
sigaction(SIGALRM, &sigact, &signals[SIGALRM]);
sigaction(SIGHUP, &sigact, &signals[SIGHUP]);
sigaction(SIGINT, &sigact, &signals[SIGINT]);
sigaction(SIGPIPE, &sigact, &signals[SIGPIPE]);
sigaction(SIGQUIT, &sigact, &signals[SIGQUIT]);
sigaction(SIGTERM, &sigact, &signals[SIGTERM]);
sigaction(SIGTSTP, &sigact, &signals[SIGTSTP]);
sigaction(SIGTTIN, &sigact, &signals[SIGTTIN]);
sigaction(SIGTTOU, &sigact, &signals[SIGTTOU]);

std::string buffer;
/* let's allocate a huge buffer now to prevent reallocation,
which would leave parts of the buffer around */
buffer.reserve(512);

for (;;) {
char ch = '\0';
auto got = read(input, &ch, 1);
if (got == 1 && ch != '\n' && ch != '\r') {
buffer.push_back(ch);
char character = '\0';
auto got = read(input, &character, 1);
if (got == 1 && character != '\n' && character != '\r') {
buffer.push_back(character);
}
else {
break;
Expand All @@ -476,5 +482,5 @@ SensitiveData CredentialsHolder::readFromTerminal()
sigaction(sig.first, &sig.second, nullptr);
}

return SensitiveData(std::move(buffer));
return {std::move(buffer)};
}
13 changes: 9 additions & 4 deletions pdns/credentials.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@
class SensitiveData
{
public:
SensitiveData(const SensitiveData&) = delete;
SensitiveData(SensitiveData&&) = delete;
SensitiveData& operator=(const SensitiveData&) = delete;
SensitiveData(size_t bytes);
SensitiveData(std::string&& data);
SensitiveData& operator=(SensitiveData&&) noexcept;

~SensitiveData();
void clear();
const std::string& getString() const
[[nodiscard]] const std::string& getString() const
{
return d_data;
}
Expand All @@ -55,6 +58,8 @@ bool isPasswordHashed(const std::string& password);
class CredentialsHolder
{
public:
CredentialsHolder(CredentialsHolder&&) = delete;
CredentialsHolder& operator=(CredentialsHolder&&) = delete;
/* if hashPlaintext is true, the password is in cleartext and hashing is available,
the hashed form will be kept in memory.
Note that accepting hashed password from an untrusted source might open
Expand All @@ -66,14 +71,14 @@ public:
CredentialsHolder(const CredentialsHolder&) = delete;
CredentialsHolder& operator=(const CredentialsHolder&) = delete;

bool matches(const std::string& password) const;
[[nodiscard]] bool matches(const std::string& password) const;
/* whether it was constructed from a hashed and salted string */
bool wasHashed() const
[[nodiscard]] bool wasHashed() const
{
return d_wasHashed;
}
/* whether it is hashed in memory */
bool isHashed() const
[[nodiscard]] bool isHashed() const
{
return d_isHashed;
}
Expand Down

0 comments on commit 09239d5

Please sign in to comment.