fix: oath::Encoder base clase should not be used via pointers and not via value types.

This bug was caught by clazy, and prevents invalid password detection/error UX feature from landing.

See-Also: https://invent.kde.org/utilities/keysmith/-/merge_requests/71
master
Johan Ouwerkerk 2020-11-23 19:37:03 +01:00
parent c31aa8df52
commit 0a74440fe4
2 changed files with 16 additions and 13 deletions

View File

@ -91,10 +91,10 @@ namespace oath
return m_addChecksum;
}
bool Algorithm::validate(const Encoder &encoder)
bool Algorithm::validate(const Encoder *encoder)
{
// HOTP spec mandates a minimum token length of 6 digits
return encoder.tokenLength() >= 6;
return encoder && encoder->tokenLength() >= 6;
}
bool Algorithm::validate(QCryptographicHash::Algorithm algorithm, const std::optional<uint> &offset)
@ -114,14 +114,14 @@ namespace oath
return digestSize && *digestSize >= 4U && (*digestSize - 4U) >= truncateAt;
}
std::optional<Algorithm> Algorithm::create(QCryptographicHash::Algorithm algorithm, const std::optional<uint> &offset, const Encoder &encoder, bool requireSaneKeyLength)
std::optional<Algorithm> Algorithm::create(QCryptographicHash::Algorithm algorithm, const std::optional<uint> &offset, const QSharedPointer<const Encoder> &encoder, bool requireSaneKeyLength)
{
if(!validate(algorithm, offset)) {
qCDebug(logger) << "Invalid algorithm:" << algorithm << "or incompatible with truncation offset:" << (offset ? *offset : 16U);
return std::nullopt;
}
if (!validate(encoder)) {
if (!encoder || !validate(encoder.data())) {
qCDebug(logger) << "Invalid token encoder";
return std::nullopt;
}
@ -140,17 +140,17 @@ namespace oath
std::optional<Algorithm> Algorithm::totp(QCryptographicHash::Algorithm algorithm, uint tokenLength, bool requireSaneKeyLength)
{
const Encoder encoder(tokenLength, false);
const QSharedPointer<const Encoder> encoder(new Encoder(tokenLength, false));
return create(algorithm, std::nullopt, encoder, requireSaneKeyLength);
}
std::optional<Algorithm> Algorithm::hotp(const std::optional<uint> &offset, uint tokenLength, bool checksum, bool requireSaneKeyLength)
{
const Encoder encoder(tokenLength, checksum);
const QSharedPointer<const Encoder> encoder(new Encoder(tokenLength, checksum));
return create(QCryptographicHash::Sha1, offset, encoder, requireSaneKeyLength);
}
Algorithm::Algorithm(const Encoder &encoder, const std::function<quint32(QByteArray)> &truncation, QCryptographicHash::Algorithm algorithm, bool requireSaneKeyLength) :
Algorithm::Algorithm(const QSharedPointer<const Encoder> &encoder, const std::function<quint32(QByteArray)> &truncation, QCryptographicHash::Algorithm algorithm, bool requireSaneKeyLength) :
m_encoder(encoder), m_truncation(truncation), m_enforceKeyLength(requireSaneKeyLength), m_algorithm(algorithm)
{
}
@ -178,8 +178,8 @@ namespace oath
std::optional<QByteArray> digest = hmac::compute(m_algorithm, secretBuffer, length, message, m_enforceKeyLength);
if (digest) {
quint32 result = m_truncation(*digest);
result = Encoder::reduceMod10(result, m_encoder.tokenLength());
return std::optional<QString>(m_encoder.encode(result));
result = Encoder::reduceMod10(result, m_encoder->tokenLength());
return std::optional<QString>(m_encoder->encode(result));
}
qCDebug(logger) << "Failed to compute token";

View File

@ -8,6 +8,7 @@
#include <QByteArray>
#include <QCryptographicHash>
#include <QDateTime>
#include <QSharedPointer>
#include <functional>
#include <optional>
@ -23,6 +24,8 @@ namespace oath
uint tokenLength(void) const;
bool checksum(void) const;
static quint32 reduceMod10(quint32 value, uint tokenLength);
private:
Q_DISABLE_COPY_MOVE(Encoder)
private:
static constexpr const quint32 powerTable[10] = {
1, 10, 100, 1'000, 10'000, 100'000, 1'000'000, 10'000'000, 100'000'000, 1'000'000'000
@ -34,16 +37,16 @@ namespace oath
class Algorithm
{
public:
static bool validate(const Encoder& encoder);
static bool validate(const Encoder *encoder);
static bool validate(QCryptographicHash::Algorithm algorithm, const std::optional<uint> &offset);
static std::optional<Algorithm> create(QCryptographicHash::Algorithm algorithm, const std::optional<uint> &offset, const Encoder &encoder, bool requireSaneKeyLength = false);
static std::optional<Algorithm> create(QCryptographicHash::Algorithm algorithm, const std::optional<uint> &offset, const QSharedPointer<const Encoder> &encoder, bool requireSaneKeyLength = false);
static std::optional<Algorithm> totp(QCryptographicHash::Algorithm algorithm, uint tokenLength, bool requireSaneKeyLength = false);
static std::optional<Algorithm> hotp(const std::optional<uint> &offset, uint tokenLength, bool checksum, bool requireSaneKeyLength = false);
std::optional<QString> compute(quint64 counter, char * secretBuffer, int length) const;
private:
Algorithm(const Encoder &encoder, const std::function<quint32(QByteArray)> &truncation, QCryptographicHash::Algorithm algorithm, bool requireSaneKeyLength);
Algorithm(const QSharedPointer<const Encoder> &encoder, const std::function<quint32(QByteArray)> &truncation, QCryptographicHash::Algorithm algorithm, bool requireSaneKeyLength);
private:
const Encoder m_encoder;
const QSharedPointer<const Encoder> m_encoder;
const std::function<quint32(QByteArray)> m_truncation;
bool m_enforceKeyLength;
const QCryptographicHash::Algorithm m_algorithm;