From 27d6314b9598908cbb9c2a589fb62f3653fa9062 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 6 Jul 2021 20:48:36 +0200 Subject: [PATCH] QCryptographicHash: use a std::array to hold result (was: QByteArray) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The maximum size for a hash result is 64 atm. Even if, and esp when, we'll get to 128 and 256 bytes in the future, there's no reason to use dynamic memory, because the sizes will always be statically known. So use, essentially, a std::array to hold the result internally. Add a bit of convenience API on top to limit impact on the rest of the code and add a few static_asserts that ensure this is large enough. Then give users access to the internal buffer by adding QByteArrayView resultView() const noexcept. The documentation snippet is taken from QString::data(), suitably adjusted. Use resultView() in a few places instead of result(). [ChangeLog][QtCore][QCryptographicHash] Changed to use a statically-sized buffer internally. Added resultView() to access it. Change-Id: I96c35e55acacbe94529446d720c18325273ffd2f Reviewed-by: Edward Welbourne Reviewed-by: Mårten Nordheim --- src/corelib/tools/qcryptographichash.cpp | 91 ++++++++++++++----- src/corelib/tools/qcryptographichash.h | 3 +- .../tools/qmessageauthenticationcode.cpp | 2 +- src/network/kernel/qauthenticator.cpp | 3 +- src/plugins/tls/shared/qtlskey_generic.cpp | 4 +- .../tst_qcryptographichash.cpp | 12 +-- 6 files changed, 78 insertions(+), 37 deletions(-) diff --git a/src/corelib/tools/qcryptographichash.cpp b/src/corelib/tools/qcryptographichash.cpp index 512d38b02ba..f44dee89d42 100644 --- a/src/corelib/tools/qcryptographichash.cpp +++ b/src/corelib/tools/qcryptographichash.cpp @@ -138,47 +138,53 @@ static inline int SHA384_512AddLength(SHA512Context *context, unsigned int lengt QT_BEGIN_NAMESPACE +static constexpr qsizetype MaxHashLength = 64; + static constexpr int hashLengthInternal(QCryptographicHash::Algorithm method) noexcept { switch (method) { - case QCryptographicHash::Sha1: - return 20; +#define CASE(Enum, Size) \ + case QCryptographicHash:: Enum : \ + /* if this triggers, then increase MaxHashLength accordingly */ \ + static_assert(MaxHashLength >= qsizetype(Size) ); \ + return Size \ + /*end*/ + CASE(Sha1, 20); #ifndef QT_CRYPTOGRAPHICHASH_ONLY_SHA1 - case QCryptographicHash::Md4: - return 16; - case QCryptographicHash::Md5: - return 16; - case QCryptographicHash::Sha224: - return SHA224HashSize; - case QCryptographicHash::Sha256: - return SHA256HashSize; - case QCryptographicHash::Sha384: - return SHA384HashSize; - case QCryptographicHash::Sha512: - return SHA512HashSize; - case QCryptographicHash::Blake2s_128: - return 128 / 8; + CASE(Md4, 16); + CASE(Md5, 16); + CASE(Sha224, SHA224HashSize); + CASE(Sha256, SHA256HashSize); + CASE(Sha384, SHA384HashSize); + CASE(Sha512, SHA512HashSize); + CASE(Blake2s_128, 128 / 8); case QCryptographicHash::Blake2b_160: case QCryptographicHash::Blake2s_160: + static_assert(160 / 8 <= MaxHashLength); return 160 / 8; case QCryptographicHash::RealSha3_224: case QCryptographicHash::Keccak_224: case QCryptographicHash::Blake2s_224: + static_assert(224 / 8 <= MaxHashLength); return 224 / 8; case QCryptographicHash::RealSha3_256: case QCryptographicHash::Keccak_256: case QCryptographicHash::Blake2b_256: case QCryptographicHash::Blake2s_256: + static_assert(256 / 8 <= MaxHashLength); return 256 / 8; case QCryptographicHash::RealSha3_384: case QCryptographicHash::Keccak_384: case QCryptographicHash::Blake2b_384: + static_assert(384 / 8 <= MaxHashLength); return 384 / 8; case QCryptographicHash::RealSha3_512: case QCryptographicHash::Keccak_512: case QCryptographicHash::Blake2b_512: + static_assert(512 / 8 <= MaxHashLength); return 512 / 8; #endif +#undef CASE } return 0; } @@ -194,7 +200,8 @@ public: void reset() noexcept; void addData(QByteArrayView bytes) noexcept; - QByteArray finalize(); + void finalize() noexcept; + QByteArrayView resultView() const noexcept { return result.toByteArrayView(); } const QCryptographicHash::Algorithm method; union { @@ -219,7 +226,25 @@ public: }; void sha3Finish(int bitCount, Sha3Variant sha3Variant); #endif - QByteArray result; + class SmallByteArray { + std::array m_data; + static_assert(MaxHashLength <= std::numeric_limits::max()); + std::uint8_t m_size; + public: + char *data() noexcept { return m_data.data(); } + const char *data() const noexcept { return m_data.data(); } + qsizetype size() const noexcept { return qsizetype{m_size}; } + bool isEmpty() const noexcept { return size() == 0; } + void clear() noexcept { m_size = 0; } + void resize(qsizetype s) { + Q_ASSERT(s >= 0); + Q_ASSERT(s <= MaxHashLength); + m_size = std::uint8_t(s); + } + QByteArrayView toByteArrayView() const noexcept + { return QByteArrayView{data(), size()}; } + }; + SmallByteArray result; }; #ifndef QT_CRYPTOGRAPHICHASH_ONLY_SHA1 @@ -337,7 +362,7 @@ QCryptographicHash::~QCryptographicHash() /*! Resets the object. */ -void QCryptographicHash::reset() +void QCryptographicHash::reset() noexcept { d->reset(); } @@ -532,17 +557,33 @@ bool QCryptographicHash::addData(QIODevice *device) /*! Returns the final hash value. - \sa QByteArray::toHex() + \sa resultView(), QByteArray::toHex() */ QByteArray QCryptographicHash::result() const { - return d->finalize(); + return resultView().toByteArray(); } -QByteArray QCryptographicHashPrivate::finalize() +/*! + \since 6.3 + + Returns the final hash value. + + Note that the returned view remains valid only as long as the QCryptographicHash object is + not modified by other means. + + \sa result(), QByteArrayView::toHex() +*/ +QByteArrayView QCryptographicHash::resultView() const noexcept +{ + d->finalize(); + return d->resultView(); +} + +void QCryptographicHashPrivate::finalize() noexcept { if (!result.isEmpty()) - return result; + return; switch (method) { case QCryptographicHash::Sha1: { @@ -630,7 +671,6 @@ QByteArray QCryptographicHashPrivate::finalize() } #endif } - return result; } /*! @@ -643,7 +683,8 @@ QByteArray QCryptographicHash::hash(QByteArrayView data, Algorithm method) { QCryptographicHashPrivate hash(method); hash.addData(data); - return hash.finalize(); + hash.finalize(); + return hash.resultView().toByteArray(); } /*! diff --git a/src/corelib/tools/qcryptographichash.h b/src/corelib/tools/qcryptographichash.h index aecf8995d57..b3ab9c61266 100644 --- a/src/corelib/tools/qcryptographichash.h +++ b/src/corelib/tools/qcryptographichash.h @@ -102,7 +102,7 @@ public: explicit QCryptographicHash(Algorithm method); ~QCryptographicHash(); - void reset(); + void reset() noexcept; #if QT_DEPRECATED_SINCE(6, 4) QT_DEPRECATED_VERSION_X_6_4("Use the QByteArrayView overload instead") @@ -115,6 +115,7 @@ public: bool addData(QIODevice *device); QByteArray result() const; + QByteArrayView resultView() const noexcept; #ifdef QT_BUILD_FUNCTIONS_REMOVED_IN_6_3 static QByteArray hash(const QByteArray &data, Algorithm method); diff --git a/src/corelib/tools/qmessageauthenticationcode.cpp b/src/corelib/tools/qmessageauthenticationcode.cpp index 9580b024c2f..767a7a14d43 100644 --- a/src/corelib/tools/qmessageauthenticationcode.cpp +++ b/src/corelib/tools/qmessageauthenticationcode.cpp @@ -251,7 +251,7 @@ QByteArray QMessageAuthenticationCode::result() const const int blockSize = qt_hash_block_size(d->method); - QByteArray hashedMessage = d->messageHash.result(); + QByteArrayView hashedMessage = d->messageHash.resultView(); QVarLengthArray oKeyPad(blockSize); const char * const keyData = d->key.constData(); diff --git a/src/network/kernel/qauthenticator.cpp b/src/network/kernel/qauthenticator.cpp index 04698ddbfa9..6cfa1ff2ef1 100644 --- a/src/network/kernel/qauthenticator.cpp +++ b/src/network/kernel/qauthenticator.cpp @@ -1239,7 +1239,6 @@ QByteArray qEncodeHmacMd5(QByteArray &key, const QByteArray &message) Q_ASSERT_X(!(key.isEmpty()),"qEncodeHmacMd5", "Empty key check"); QCryptographicHash hash(QCryptographicHash::Md5); - QByteArray hMsg; QByteArray iKeyPad(blockSize, 0x36); QByteArray oKeyPad(blockSize, 0x5c); @@ -1272,7 +1271,7 @@ QByteArray qEncodeHmacMd5(QByteArray &key, const QByteArray &message) hash.reset(); hash.addData(iKeyPad); - hMsg = hash.result(); + QByteArrayView hMsg = hash.resultView(); //Digest gen after pass-1: H((K0 xor ipad)||text) QByteArray hmacDigest; diff --git a/src/plugins/tls/shared/qtlskey_generic.cpp b/src/plugins/tls/shared/qtlskey_generic.cpp index b9eaf3c1f6b..495ccec681f 100644 --- a/src/plugins/tls/shared/qtlskey_generic.cpp +++ b/src/plugins/tls/shared/qtlskey_generic.cpp @@ -424,9 +424,9 @@ QByteArray deriveAesKey(QSslKeyPrivate::Cipher cipher, const QByteArray &passPhr hash.addData(data); if (cipher == Cipher::Aes192Cbc) - return key.append(hash.result().constData(), 8); + return key.append(hash.resultView().first(8)); - return key.append(hash.result()); + return key.append(hash.resultView()); } QByteArray deriveKey(QSslKeyPrivate::Cipher cipher, const QByteArray &passPhrase, diff --git a/tests/auto/corelib/tools/qcryptographichash/tst_qcryptographichash.cpp b/tests/auto/corelib/tools/qcryptographichash/tst_qcryptographichash.cpp index 7fb9b69a03c..6b61349d01e 100644 --- a/tests/auto/corelib/tools/qcryptographichash/tst_qcryptographichash.cpp +++ b/tests/auto/corelib/tools/qcryptographichash/tst_qcryptographichash.cpp @@ -78,15 +78,17 @@ void tst_QCryptographicHash::repeated_result() hash.addData(first); QFETCH(QByteArray, hash_first); - QByteArray result = hash.result(); + QByteArrayView result = hash.resultView(); QCOMPARE(result, hash_first); + QCOMPARE(result, hash.resultView()); QCOMPARE(result, hash.result()); hash.reset(); hash.addData(first); - result = hash.result(); + result = hash.resultView(); QCOMPARE(result, hash_first); QCOMPARE(result, hash.result()); + QCOMPARE(result, hash.resultView()); } void tst_QCryptographicHash::intermediary_result_data() @@ -179,16 +181,14 @@ void tst_QCryptographicHash::intermediary_result() hash.addData(first); QFETCH(QByteArray, hash_first); - QByteArray result = hash.result(); - QCOMPARE(result, hash_first); + QCOMPARE(hash.resultView(), hash_first); // don't reset QFETCH(QByteArray, second); QFETCH(QByteArray, hash_firstsecond); hash.addData(second); - result = hash.result(); - QCOMPARE(result, hash_firstsecond); + QCOMPARE(hash.resultView(), hash_firstsecond); hash.reset(); }