From 97d14a10cac909cf33b5f0ef3cc8904288604243 Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Mon, 5 Aug 2024 23:07:49 +0300 Subject: [PATCH] Take PIN ownership to minimze memory copy-s followup WE2-479 Signed-off-by: Raul Metsma --- lib/libelectronic-id | 2 +- .../command-handlers/authenticate.cpp | 22 +++++++------------ src/controller/command-handlers/sign.cpp | 19 ++++++---------- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/lib/libelectronic-id b/lib/libelectronic-id index 2f179731..9f10adab 160000 --- a/lib/libelectronic-id +++ b/lib/libelectronic-id @@ -1 +1 @@ -Subproject commit 2f179731676633c253a08cfe9956c0f4497e7ab7 +Subproject commit 9f10adab6bd8acb0d3aa30fcb742f8538f721c7c diff --git a/src/controller/command-handlers/authenticate.cpp b/src/controller/command-handlers/authenticate.cpp index 3af44db0..f711fa16 100644 --- a/src/controller/command-handlers/authenticate.cpp +++ b/src/controller/command-handlers/authenticate.cpp @@ -57,7 +57,7 @@ QVariantMap createAuthenticationToken(const QString& signatureAlgorithm, } QByteArray createSignature(const QString& origin, const QString& challengeNonce, - const ElectronicID& eid, const pcsc_cpp::byte_vector& pin) + const ElectronicID& eid, pcsc_cpp::byte_vector&& pin) { static const std::map SIGNATURE_ALGO_TO_HASH { @@ -86,7 +86,7 @@ QByteArray createSignature(const QString& origin, const QString& challengeNonce, const pcsc_cpp::byte_vector hashToBeSigned {hashToBeSignedQBytearray.cbegin(), hashToBeSignedQBytearray.cend()}; - const auto signature = eid.signWithAuthKey(pin, hashToBeSigned); + const auto signature = eid.signWithAuthKey(std::move(pin), hashToBeSigned); return QByteArray::fromRawData(reinterpret_cast(signature.data()), int(signature.size())) @@ -120,20 +120,14 @@ Authenticate::Authenticate(const CommandWithArguments& cmd) : CertificateReader( QVariantMap Authenticate::onConfirm(WebEidUI* window, const CardCertificateAndPinInfo& cardCertAndPin) { - const auto signatureAlgorithm = - QString::fromStdString(cardCertAndPin.cardInfo->eid().authSignatureAlgorithm()); - - pcsc_cpp::byte_vector pin; - getPin(pin, cardCertAndPin.cardInfo->eid(), window); - auto pin_cleanup = qScopeGuard([&pin] { - // Erase PIN memory. - std::fill(pin.begin(), pin.end(), '\0'); - }); - try { + const auto signatureAlgorithm = + QString::fromStdString(cardCertAndPin.cardInfo->eid().authSignatureAlgorithm()); + pcsc_cpp::byte_vector pin; + pin.reserve(5 + 16); // Avoid realloc: apdu + pin padding + getPin(pin, cardCertAndPin.cardInfo->eid(), window); const auto signature = - createSignature(origin.url(), challengeNonce, cardCertAndPin.cardInfo->eid(), pin); - + createSignature(origin.url(), challengeNonce, cardCertAndPin.cardInfo->eid(), std::move(pin)); return createAuthenticationToken(signatureAlgorithm, cardCertAndPin.certificateBytesInDer, signature); diff --git a/src/controller/command-handlers/sign.cpp b/src/controller/command-handlers/sign.cpp index 5b383aba..dc69a589 100644 --- a/src/controller/command-handlers/sign.cpp +++ b/src/controller/command-handlers/sign.cpp @@ -32,11 +32,11 @@ using namespace electronic_id; namespace { -QPair signHash(const ElectronicID& eid, const pcsc_cpp::byte_vector& pin, +QPair signHash(const ElectronicID& eid, pcsc_cpp::byte_vector&& pin, const QByteArray& docHash, const HashAlgorithm hashAlgo) { const auto hashBytes = pcsc_cpp::byte_vector {docHash.begin(), docHash.end()}; - const auto signature = eid.signWithSigningKey(pin, hashBytes, hashAlgo); + const auto signature = eid.signWithSigningKey(std::move(pin), hashBytes, hashAlgo); const auto signatureBase64 = QByteArray::fromRawData(reinterpret_cast(signature.first.data()), @@ -97,16 +97,11 @@ void Sign::emitCertificatesReady(const std::vector& c QVariantMap Sign::onConfirm(WebEidUI* window, const CardCertificateAndPinInfo& cardCertAndPin) { - pcsc_cpp::byte_vector pin; - getPin(pin, cardCertAndPin.cardInfo->eid(), window); - auto pin_cleanup = qScopeGuard([&pin] { - // Erase PIN memory. - std::fill(pin.begin(), pin.end(), '\0'); - }); - try { - const auto signature = signHash(cardCertAndPin.cardInfo->eid(), pin, docHash, hashAlgo); - + pcsc_cpp::byte_vector pin; + pin.reserve(5 + 16); // Avoid realloc: apdu + pin padding + getPin(pin, cardCertAndPin.cardInfo->eid(), window); + const auto signature = signHash(cardCertAndPin.cardInfo->eid(), std::move(pin), docHash, hashAlgo); return {{QStringLiteral("signature"), signature.first}, {QStringLiteral("signatureAlgorithm"), signature.second}}; @@ -144,7 +139,7 @@ void Sign::validateAndStoreDocHashAndHashAlgo(const QVariantMap& args) docHash = QByteArray::fromBase64(validateAndGetArgument(QStringLiteral("hash"), args)); - QString hashAlgoInput = validateAndGetArgument(QStringLiteral("hashFunction"), args); + auto hashAlgoInput = validateAndGetArgument(QStringLiteral("hashFunction"), args); if (hashAlgoInput.size() > 8) { THROW(CommandHandlerInputDataError, "hashFunction value is invalid"); }