Fixup HMAC key size sanity checking.

Key lengths should be checked against the output size of the hashing
algorithm (instead of block size, which was used previously).
master
Johan Ouwerkerk 2020-02-09 17:06:58 +01:00 committed by Bhushan Shah
parent eb99b2abf6
commit 6a06a2f5f0
9 changed files with 71 additions and 98 deletions

View File

@ -43,10 +43,10 @@ void HMacValidateKeySizeTest::testValidation_data(void)
{
define_test_data();
define_test_case("short keys for HMAC-SHA1 permitted", QCryptographicHash::Sha1, 42, false, true);
define_test_case("exact match for HMAC-SHA1 blocks", QCryptographicHash::Sha1, 64, true, true);
define_test_case("short keys for HMAC-SHA1 permitted", QCryptographicHash::Sha1, 19, false, true);
define_test_case("exact match for HMAC-SHA1 output", QCryptographicHash::Sha1, 20, true, true);
define_test_case("long keys for HMAC-SHA1", QCryptographicHash::Sha1, 500, true, true);
define_test_case("short keys for HMAC-SHA1 disallowed", QCryptographicHash::Sha1, 42, true, false);
define_test_case("short keys for HMAC-SHA1 disallowed", QCryptographicHash::Sha1, 19, true, false);
define_test_case("invalid key size: -1", QCryptographicHash::Sha1, -1, false, false);
define_test_case("invalid algorithm: -1", (QCryptographicHash::Algorithm) -1, 500, true, false);
}

View File

@ -2,7 +2,7 @@
* SPDX-License-Identifier: GPL-3.0-or-later
* SPDX-FileCopyrightText: 2020 Johan Ouwerkerk <jm.ouwerkerk@gmail.com>
*/
#include "oath/util.h"
#include "oath/oath.h"
#include <QTest>
#include <QtDebug>

View File

@ -55,10 +55,39 @@ namespace hmac
}
}
std::optional<uint> outputSize(QCryptographicHash::Algorithm algorithm)
{
switch (algorithm) {
case QCryptographicHash::Md4:
case QCryptographicHash::Md5:
return std::optional<uint>(16ULL);
case QCryptographicHash::Sha1:
return std::optional<uint>(20ULL);
case QCryptographicHash::Sha224:
case QCryptographicHash::RealSha3_224:
case QCryptographicHash::Keccak_224:
return std::optional<int>(28UL);
case QCryptographicHash::Sha256:
case QCryptographicHash::RealSha3_256:
case QCryptographicHash::Keccak_256:
return std::optional<int>(32UL);
case QCryptographicHash::Sha384:
case QCryptographicHash::RealSha3_384:
case QCryptographicHash::Keccak_384:
return std::optional<int>(48UL);
case QCryptographicHash::Sha512:
case QCryptographicHash::RealSha3_512:
case QCryptographicHash::Keccak_512:
return std::optional<uint>(64UL);
default:
return std::nullopt;
}
}
bool validateKeySize(QCryptographicHash::Algorithm algorithm, int ksize, bool requireSaneKeyLength)
{
std::optional<int> maybeBlockSize = blockSize(algorithm);
return maybeBlockSize && ksize >= 0 && (ksize >= (*maybeBlockSize) || !requireSaneKeyLength);
std::optional<int> maybeOutputSize = outputSize(algorithm);
return maybeOutputSize && ksize >= 0 && (ksize >= (*maybeOutputSize) || !requireSaneKeyLength);
}
std::optional<QByteArray> compute(QCryptographicHash::Algorithm algorithm, char * rawKey, int ksize, const QByteArray &message, bool requireSaneKeyLength)
@ -72,6 +101,7 @@ namespace hmac
// TODO warn
return std::nullopt;
}
if (!validateKeySize(algorithm, ksize, requireSaneKeyLength)) {
// TODO warn
return std::nullopt;

View File

@ -13,6 +13,7 @@
namespace hmac
{
std::optional<int> blockSize(QCryptographicHash::Algorithm algorithm);
std::optional<uint> outputSize(QCryptographicHash::Algorithm algorithm);
bool validateKeySize(QCryptographicHash::Algorithm algorithm, int ksize, bool requireSaneKeyLength = false);
std::optional<QByteArray> compute(QCryptographicHash::Algorithm algorithm, char * rawKey, int ksize, const QByteArray &message, bool requireSaneKeyLength = false);

View File

@ -4,7 +4,6 @@
#
set(oath_SRCS
oath.cpp
util.cpp
)
add_library(oath_lib STATIC ${oath_SRCS})

View File

@ -4,8 +4,6 @@
*/
#include "oath.h"
#include "util.h"
#include "../hmac/hmac.h"
static QString encodeDefaults(quint32 value, uint tokenLength)
@ -100,7 +98,7 @@ namespace oath
std::optional<Algorithm> Algorithm::usingDynamicTruncation(QCryptographicHash::Algorithm algorithm, const Encoder &encoder, bool requireSaneKeyLength)
{
std::optional<uint> digestSize = outputSize(algorithm);
std::optional<uint> digestSize = hmac::outputSize(algorithm);
if (!digestSize) {
// TODO warn about this
return std::nullopt;
@ -122,7 +120,7 @@ namespace oath
std::optional<Algorithm> Algorithm::usingTruncationOffset(QCryptographicHash::Algorithm algorithm, uint offset, const Encoder &encoder, bool requireSaneKeyLength)
{
std::optional<uint> digestSize = outputSize(algorithm);
std::optional<uint> digestSize = hmac::outputSize(algorithm);
if (!digestSize) {
// TODO warn about this
return std::nullopt;
@ -179,6 +177,37 @@ namespace oath
return std::nullopt;
}
uint luhnChecksum(quint32 value, uint digits)
{
static const uint lookupTable[10] = {
0, // 0 * 2
2, // 1 * 2
4, // 2 * 2
6, // 3 * 2
8, // 4 * 2
1, // 5 * 2 - 9
3, // 6 * 2 - 9
5, // 7 * 2 - 9
7, // 8 * 2 - 9
9, // 9 * 2 - 9
};
Q_ASSERT_X(digits > 0UL, Q_FUNC_INFO, "checksum cannot be computed over less than 1 digit");
uint sum = 0UL;
bool doubledMinus9 = true;
for (uint d = 0UL; d < digits && value != 0UL; ++d) {
uint position = value % 10UL;
sum += doubledMinus9 ? lookupTable[position] : position;
value /= 10UL;
doubledMinus9 = !doubledMinus9;
}
sum = sum % 10ULL;
return sum == 0UL ? 0UL : 10UL - sum;
}
std::optional<quint64> count(const QDateTime &epoch, uint timeStep, const std::function<qint64(void)> &clock)
{
qint64 epochMillis = epoch.toMSecsSinceEpoch();

View File

@ -48,6 +48,7 @@ namespace oath
const QCryptographicHash::Algorithm m_algorithm;
};
uint luhnChecksum(quint32 value, uint digits);
std::optional<quint64> count(const QDateTime &epoch, uint timeStep, const std::function<qint64(void)> &clock = &QDateTime::currentMSecsSinceEpoch);
}

View File

@ -1,68 +0,0 @@
/*
* SPDX-License-Identifier: GPL-3.0-or-later
* SPDX-FileCopyrightText: 2020 Johan Ouwerkerk <jm.ouwerkerk@gmail.com>
*/
#include "util.h"
namespace oath
{
std::optional<uint> outputSize(QCryptographicHash::Algorithm algorithm)
{
switch (algorithm) {
case QCryptographicHash::Md4:
case QCryptographicHash::Md5:
return std::optional<uint>(16ULL);
case QCryptographicHash::Sha1:
return std::optional<uint>(20ULL);
case QCryptographicHash::Sha224:
case QCryptographicHash::RealSha3_224:
case QCryptographicHash::Keccak_224:
return std::optional<int>(28UL);
case QCryptographicHash::Sha256:
case QCryptographicHash::RealSha3_256:
case QCryptographicHash::Keccak_256:
return std::optional<int>(32UL);
case QCryptographicHash::Sha384:
case QCryptographicHash::RealSha3_384:
case QCryptographicHash::Keccak_384:
return std::optional<int>(48UL);
case QCryptographicHash::Sha512:
case QCryptographicHash::RealSha3_512:
case QCryptographicHash::Keccak_512:
return std::optional<uint>(64UL);
default:
return std::nullopt;
}
}
uint luhnChecksum(quint32 value, uint digits)
{
static const uint lookupTable[10] = {
0, // 0 * 2
2, // 1 * 2
4, // 2 * 2
6, // 3 * 2
8, // 4 * 2
1, // 5 * 2 - 9
3, // 6 * 2 - 9
5, // 7 * 2 - 9
7, // 8 * 2 - 9
9, // 9 * 2 - 9
};
Q_ASSERT_X(digits > 0UL, Q_FUNC_INFO, "checksum cannot be computed over less than 1 digit");
uint sum = 0UL;
bool doubledMinus9 = true;
for (uint d = 0UL; d < digits && value != 0UL; ++d) {
uint position = value % 10UL;
sum += doubledMinus9 ? lookupTable[position] : position;
value /= 10UL;
doubledMinus9 = !doubledMinus9;
}
sum = sum % 10ULL;
return sum == 0UL ? 0UL : 10UL - sum;
}
}

View File

@ -1,19 +0,0 @@
/*
* SPDX-License-Identifier: GPL-3.0-or-later
* SPDX-FileCopyrightText: 2020 Johan Ouwerkerk <jm.ouwerkerk@gmail.com>
*/
#ifndef OATH_UTIL_H
#define OATH_UTIL_H
#include <QCryptographicHash>
#include <optional>
namespace oath
{
uint luhnChecksum(quint32 value, uint digits);
std::optional<uint> outputSize(QCryptographicHash::Algorithm algorithm);
}
#endif