From 02662c11e880d5083814fbdd9cb46aa86dd99f4c Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Wed, 15 Nov 2023 12:55:38 +0200 Subject: [PATCH 01/11] Do not allow PIN retry on unknown error, instead show "technical error" and cancel button Output {"invalid-argument":"message"} in stdin/out mode if json parsing fails --- src/controller/controller.cpp | 10 ++++++++++ src/controller/inputoutputmode.cpp | 3 --- src/ui/webeiddialog.cpp | 21 ++++++++++++++++++++- src/ui/webeiddialog.hpp | 1 + 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/controller/controller.cpp b/src/controller/controller.cpp index e82dfcac..a3331e82 100644 --- a/src/controller/controller.cpp +++ b/src/controller/controller.cpp @@ -103,6 +103,16 @@ void Controller::run() startCommandExecution(); + } catch (const std::invalid_argument& exc) { + if (isInStdinMode) { + // Pass invalid argument message to the caller just in case it may be interested + // The result will be {"invalid-argument" : message} + // Command parameter is only used if exception will be raised during json creation + writeResponseToStdOut(isInStdinMode, + {{QStringLiteral("invalid-argument"), exc.what()}}, + "invalid-argument"); + } + onCriticalFailure(exc.what()); } catch (const std::exception& error) { onCriticalFailure(error.what()); } diff --git a/src/controller/inputoutputmode.cpp b/src/controller/inputoutputmode.cpp index aa44e469..46871138 100644 --- a/src/controller/inputoutputmode.cpp +++ b/src/controller/inputoutputmode.cpp @@ -71,7 +71,6 @@ CommandWithArgumentsPtr readCommandFromStdin() const auto messageLength = readMessageLength(std::cin); if (messageLength < 5) { - // FIXME: Pass errors back up to caller in stdin mode. throw std::invalid_argument("readCommandFromStdin: Message length is " + std::to_string(messageLength) + ", at least 5 required"); } @@ -88,7 +87,6 @@ CommandWithArgumentsPtr readCommandFromStdin() const auto json = QJsonDocument::fromJson(message); if (!json.isObject()) { - // FIXME: Pass errors back up to caller in stdin mode. throw std::invalid_argument("readCommandFromStdin: Invalid JSON, not an object"); } @@ -97,7 +95,6 @@ CommandWithArgumentsPtr readCommandFromStdin() const auto arguments = jsonObject["arguments"]; if (!command.isString() || !arguments.isObject()) { - // FIXME: Pass errors back up to caller in stdin mode. throw std::invalid_argument("readCommandFromStdin: Invalid JSON, the main object does not " "contain a 'command' string and 'arguments' object"); } diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp index f41f010f..3dabf899 100644 --- a/src/ui/webeiddialog.cpp +++ b/src/ui/webeiddialog.cpp @@ -440,7 +440,6 @@ void WebEidDialog::onVerifyPinFailed(const VerifyPinFailed::Status status, const std::function message; - // FIXME: don't allow retry in case of UNKNOWN_ERROR switch (status) { case Status::RETRY_ALLOWED: message = [retriesLeft] { @@ -463,8 +462,13 @@ void WebEidDialog::onVerifyPinFailed(const VerifyPinFailed::Status status, const message = [] { return tr("PIN entry cancelled."); }; break; case Status::PIN_ENTRY_DISABLED: + message = [] { return tr("PIN entry disabled"); }; + break; case Status::UNKNOWN_ERROR: message = [] { return tr("Technical error"); }; + displayFatalError(message); + resizeHeight(); + return; break; } @@ -671,6 +675,21 @@ void WebEidDialog::displayPinBlockedError() ui->helpButton->show(); } +void WebEidDialog::displayFatalError(std::function message) +{ + ui->pinTitleLabel->hide(); + ui->pinInput->hide(); + ui->pinTimeoutTimer->stop(); + ui->pinTimeRemaining->hide(); + ui->pinEntryTimeoutProgressBar->hide(); + setTrText(ui->pinErrorLabel, message); + ui->pinErrorLabel->show(); + ui->okButton->hide(); + ui->cancelButton->setEnabled(true); + ui->cancelButton->show(); + //ui->helpButton->show(); +} + void WebEidDialog::showPinInputWarning(bool show) { style()->unpolish(ui->pinInput); diff --git a/src/ui/webeiddialog.hpp b/src/ui/webeiddialog.hpp index 15a2703c..426bcf05 100644 --- a/src/ui/webeiddialog.hpp +++ b/src/ui/webeiddialog.hpp @@ -104,6 +104,7 @@ class WebEidDialog final : public WebEidUI template void setupOK(Func&& func, const std::function& text = {}, bool enabled = false); void displayPinBlockedError(); + void displayFatalError(std::function message); void showPinInputWarning(bool show); void resizeHeight(); From fdc8b43b8ef1afe8ab5926297837499b4c7ae304 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Wed, 15 Nov 2023 15:24:13 +0200 Subject: [PATCH 02/11] Reorganized pin handling to be extra sure that copy remains in memory --- .../command-handlers/authenticate.cpp | 7 +++--- src/controller/command-handlers/sign.cpp | 9 +++++--- .../command-handlers/signauthutils.cpp | 23 +++++++++---------- .../command-handlers/signauthutils.hpp | 2 +- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/controller/command-handlers/authenticate.cpp b/src/controller/command-handlers/authenticate.cpp index e87787bd..99388552 100644 --- a/src/controller/command-handlers/authenticate.cpp +++ b/src/controller/command-handlers/authenticate.cpp @@ -119,21 +119,22 @@ QVariantMap Authenticate::onConfirm(WebEidUI* window, const auto signatureAlgorithm = QString::fromStdString(cardCertAndPin.cardInfo->eid().authSignatureAlgorithm()); - auto pin = getPin(cardCertAndPin.cardInfo->eid().smartcard(), window); + pcsc_cpp::byte_vector pin; + getPin(pin, cardCertAndPin.cardInfo->eid().smartcard(), window); try { const auto signature = createSignature(origin.url(), challengeNonce, cardCertAndPin.cardInfo->eid(), pin); // Erase the PIN memory. - // TODO: Use a scope guard. Verify that the buffers are actually zeroed and no copies - // remain. std::fill(pin.begin(), pin.end(), '\0'); return createAuthenticationToken(signatureAlgorithm, cardCertAndPin.certificateBytesInDer, signature); } catch (const VerifyPinFailed& failure) { + // Erase the PIN memory. + std::fill(pin.begin(), pin.end(), '\0'); switch (failure.status()) { case electronic_id::VerifyPinFailed::Status::PIN_ENTRY_CANCEL: case electronic_id::VerifyPinFailed::Status::PIN_ENTRY_TIMEOUT: diff --git a/src/controller/command-handlers/sign.cpp b/src/controller/command-handlers/sign.cpp index 42277b2d..93c4a948 100644 --- a/src/controller/command-handlers/sign.cpp +++ b/src/controller/command-handlers/sign.cpp @@ -95,20 +95,23 @@ void Sign::emitCertificatesReady(const std::vector& c QVariantMap Sign::onConfirm(WebEidUI* window, const CardCertificateAndPinInfo& cardCertAndPin) { - auto pin = getPin(cardCertAndPin.cardInfo->eid().smartcard(), window); + pcsc_cpp::byte_vector pin; + getPin(pin, cardCertAndPin.cardInfo->eid().smartcard(), window); try { const auto signature = signHash(cardCertAndPin.cardInfo->eid(), pin, docHash, hashAlgo); // Erase PIN memory. - // TODO: Use a scope guard. Verify that the buffers are actually zeroed - // and no copies remain. std::fill(pin.begin(), pin.end(), '\0'); return {{QStringLiteral("signature"), signature.first}, {QStringLiteral("signatureAlgorithm"), signature.second}}; } catch (const VerifyPinFailed& failure) { + + // Erase PIN memory. + std::fill(pin.begin(), pin.end(), '\0'); + switch (failure.status()) { case electronic_id::VerifyPinFailed::Status::PIN_ENTRY_CANCEL: case electronic_id::VerifyPinFailed::Status::PIN_ENTRY_TIMEOUT: diff --git a/src/controller/command-handlers/signauthutils.cpp b/src/controller/command-handlers/signauthutils.cpp index b2ec6bcf..ccb58488 100644 --- a/src/controller/command-handlers/signauthutils.cpp +++ b/src/controller/command-handlers/signauthutils.cpp @@ -78,29 +78,28 @@ inline void eraseData(T& data) } } -pcsc_cpp::byte_vector getPin(const pcsc_cpp::SmartCard& card, WebEidUI* window) +int getPin(pcsc_cpp::byte_vector& pin, const pcsc_cpp::SmartCard& card, WebEidUI* window) { // Doesn't apply to PIN pads. if (card.readerHasPinPad()) { - return {}; + return 0; } REQUIRE_NON_NULL(window) - auto pin = window->getPin(); - if (pin.isEmpty()) { + QString pinqs = window->getPin(); + if (pinqs.isEmpty()) { THROW(ProgrammingError, "Empty PIN"); } + int len = (int) pinqs.length(); - // TODO: Avoid making copies of the PIN in memory. - auto pinQByteArray = pin.toUtf8(); - auto pinBytes = pcsc_cpp::byte_vector {pinQByteArray.begin(), pinQByteArray.end()}; - - // TODO: Verify that the buffers are actually zeroed and no copies remain. - eraseData(pin); - eraseData(pinQByteArray); + pin.resize(len); + for (int i = 0; i < len; i++) { + pin[i] = pinqs[i].cell(); + } + eraseData(pinqs); - return pinBytes; + return len; } QVariantMap signatureAlgoToVariantMap(const SignatureAlgorithm signatureAlgo) diff --git a/src/controller/command-handlers/signauthutils.hpp b/src/controller/command-handlers/signauthutils.hpp index 2e808bc2..52f894ae 100644 --- a/src/controller/command-handlers/signauthutils.hpp +++ b/src/controller/command-handlers/signauthutils.hpp @@ -41,6 +41,6 @@ extern template QString validateAndGetArgument(const QString& argName, extern template QByteArray validateAndGetArgument(const QString& argName, const QVariantMap& args, bool allowNull); -pcsc_cpp::byte_vector getPin(const pcsc_cpp::SmartCard& card, WebEidUI* window); +int getPin(pcsc_cpp::byte_vector& pin, const pcsc_cpp::SmartCard& card, WebEidUI* window); QVariantMap signatureAlgoToVariantMap(const electronic_id::SignatureAlgorithm signatureAlgo); From 2ea88b0beb5b9eb5a341e5ff55ea4fda89f36d04 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Wed, 15 Nov 2023 16:09:03 +0200 Subject: [PATCH 03/11] Clear pin string from dialog after query --- src/ui/webeiddialog.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp index 3dabf899..312f65ed 100644 --- a/src/ui/webeiddialog.cpp +++ b/src/ui/webeiddialog.cpp @@ -282,7 +282,10 @@ QString WebEidDialog::getPin() { // getPin() is called from background threads and must be thread-safe. // QString uses QAtomicPointer internally and is thread-safe. - return pin; + // There should be only single reference and this is transferred to the caller for safety + QString ret = pin; + pin = ""; + return ret; } void WebEidDialog::onSmartCardStatusUpdate(const RetriableError status) From d743b24d4ef0ae40e1e8e723b3a4cfc197f44809 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Fri, 17 Nov 2023 14:42:13 +0200 Subject: [PATCH 04/11] Separated invalid input error from other technical erros --- src/controller/controller.cpp | 26 ++++++++++++++------------ src/controller/controller.hpp | 2 ++ src/ui/webeiddialog.cpp | 14 +++++++------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/controller/controller.cpp b/src/controller/controller.cpp index a3331e82..e0496962 100644 --- a/src/controller/controller.cpp +++ b/src/controller/controller.cpp @@ -39,9 +39,8 @@ using namespace electronic_id; namespace { -// TODO: Should we use more detailed error codes? E.g. report input data error back to the website -// etc. const QString RESP_TECH_ERROR = QStringLiteral("ERR_WEBEID_NATIVE_FATAL"); +const QString RESP_INVALID_INVOCATION = QStringLiteral("ERR_WEBEID_INVALID_INVOCATION"); const QString RESP_USER_CANCEL = QStringLiteral("ERR_WEBEID_USER_CANCELLED"); QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessage) @@ -103,16 +102,8 @@ void Controller::run() startCommandExecution(); - } catch (const std::invalid_argument& exc) { - if (isInStdinMode) { - // Pass invalid argument message to the caller just in case it may be interested - // The result will be {"invalid-argument" : message} - // Command parameter is only used if exception will be raised during json creation - writeResponseToStdOut(isInStdinMode, - {{QStringLiteral("invalid-argument"), exc.what()}}, - "invalid-argument"); - } - onCriticalFailure(exc.what()); + } catch (const std::invalid_argument& error) { + onInvalidInvocation(error.what()); } catch (const std::exception& error) { onCriticalFailure(error.what()); } @@ -371,6 +362,17 @@ void Controller::onCriticalFailure(const QString& error) exit(); } +void Controller::onInvalidInvocation(const QString& error) +{ + qCritical() << "Invalid arguments to command" << std::string(commandType()) + << "reason:" << error; + _result = makeErrorObject(RESP_INVALID_INVOCATION, error); + writeResponseToStdOut(isInStdinMode, _result, commandType()); + disposeUI(); + WebEidUI::showFatalError(); + exit(); +} + void Controller::exit() { if (window) { diff --git a/src/controller/controller.hpp b/src/controller/controller.hpp index ae973f7c..12ec9371 100644 --- a/src/controller/controller.hpp +++ b/src/controller/controller.hpp @@ -70,6 +70,8 @@ class Controller : public QObject // Failure handler, reports the error and quits the application. void onCriticalFailure(const QString& error); + // Invalid arguments, we keep it spearate to distinguish it from technical errors + void onInvalidInvocation(const QString& error); private: void startCommandExecution(); diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp index 312f65ed..054a61fa 100644 --- a/src/ui/webeiddialog.cpp +++ b/src/ui/webeiddialog.cpp @@ -553,10 +553,12 @@ void WebEidDialog::connectOkToCachePinAndEmitSelectedCertificate( // QString uses QAtomicPointer internally and is thread-safe. pin = ui->pinInput->text(); - // TODO: We need to erase the PIN in the widget buffer, this needs further work. - // Investigate if it is possible to keep the PIN in secure memory, e.g. with a - // custom Qt widget. - ui->pinInput->clear(); + // We use setText instead of clear, so undo/redo will be cleared as well + ui->pinInput->setText(""); + // TODO: To be sure that no copy of PIN text remains in memory, a custom + // widget should be implemented, that: + // - Stores PIN in locked byte vector + // - DOes not leak content through accessibility interface emit accepted(certAndPin); }); @@ -750,13 +752,11 @@ WebEidDialog::retriableErrorToTextTitleAndIcon(const RetriableError error) tr("Operation not supported"), pixmap("no-id-card"_L1)}; case RetriableError::SMART_CARD_COMMAND_ERROR: - return {tr("Error communicating with the card."), tr("Operation failed"), + return {tr("Error communicating with the card. Please try again."), tr("Operation failed"), pixmap("no-id-card"_L1)}; - // TODO: what action should the user take? Should this be fatal? case RetriableError::PKCS11_ERROR: return {tr("Card driver error. Please try again."), tr("Card driver error"), pixmap("no-id-card"_L1)}; - // TODO: what action should the user take? Should this be fatal? case RetriableError::SCARD_ERROR: return {tr("An error occurred in the Smart Card service required to use the ID-card. Make " "sure that the ID-card and the card reader are connected correctly or relaunch " From a6763ea55340c34366285e4e87349dbc2c293ead Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Fri, 17 Nov 2023 16:12:12 +0200 Subject: [PATCH 05/11] Clear PIN ui with setText("") if certificate is changed --- src/ui/webeiddialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp index 054a61fa..141fd326 100644 --- a/src/ui/webeiddialog.cpp +++ b/src/ui/webeiddialog.cpp @@ -334,7 +334,7 @@ void WebEidDialog::onMultipleCertificatesReady( ui->selectAnotherCertificate->setVisible(certificateAndPinInfos.size() > 1); connect(ui->selectAnotherCertificate, &QPushButton::clicked, this, [this, origin, certificateAndPinInfos] { - ui->pinInput->clear(); + ui->pinInput->setText(""); onMultipleCertificatesReady(origin, certificateAndPinInfos); }); setupOK([this, origin, certificateAndPinInfos] { From 3260c50e10e06811f6f4c2c8b38cacff982c14a1 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Wed, 22 Nov 2023 16:09:41 +0200 Subject: [PATCH 06/11] Revert "Separated invalid input error from other technical erros" This reverts commit d743b24d4ef0ae40e1e8e723b3a4cfc197f44809. --- src/controller/controller.cpp | 26 ++++++++++++-------------- src/controller/controller.hpp | 2 -- src/ui/webeiddialog.cpp | 14 +++++++------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/controller/controller.cpp b/src/controller/controller.cpp index e0496962..a3331e82 100644 --- a/src/controller/controller.cpp +++ b/src/controller/controller.cpp @@ -39,8 +39,9 @@ using namespace electronic_id; namespace { +// TODO: Should we use more detailed error codes? E.g. report input data error back to the website +// etc. const QString RESP_TECH_ERROR = QStringLiteral("ERR_WEBEID_NATIVE_FATAL"); -const QString RESP_INVALID_INVOCATION = QStringLiteral("ERR_WEBEID_INVALID_INVOCATION"); const QString RESP_USER_CANCEL = QStringLiteral("ERR_WEBEID_USER_CANCELLED"); QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessage) @@ -102,8 +103,16 @@ void Controller::run() startCommandExecution(); - } catch (const std::invalid_argument& error) { - onInvalidInvocation(error.what()); + } catch (const std::invalid_argument& exc) { + if (isInStdinMode) { + // Pass invalid argument message to the caller just in case it may be interested + // The result will be {"invalid-argument" : message} + // Command parameter is only used if exception will be raised during json creation + writeResponseToStdOut(isInStdinMode, + {{QStringLiteral("invalid-argument"), exc.what()}}, + "invalid-argument"); + } + onCriticalFailure(exc.what()); } catch (const std::exception& error) { onCriticalFailure(error.what()); } @@ -362,17 +371,6 @@ void Controller::onCriticalFailure(const QString& error) exit(); } -void Controller::onInvalidInvocation(const QString& error) -{ - qCritical() << "Invalid arguments to command" << std::string(commandType()) - << "reason:" << error; - _result = makeErrorObject(RESP_INVALID_INVOCATION, error); - writeResponseToStdOut(isInStdinMode, _result, commandType()); - disposeUI(); - WebEidUI::showFatalError(); - exit(); -} - void Controller::exit() { if (window) { diff --git a/src/controller/controller.hpp b/src/controller/controller.hpp index 12ec9371..ae973f7c 100644 --- a/src/controller/controller.hpp +++ b/src/controller/controller.hpp @@ -70,8 +70,6 @@ class Controller : public QObject // Failure handler, reports the error and quits the application. void onCriticalFailure(const QString& error); - // Invalid arguments, we keep it spearate to distinguish it from technical errors - void onInvalidInvocation(const QString& error); private: void startCommandExecution(); diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp index 141fd326..95e12def 100644 --- a/src/ui/webeiddialog.cpp +++ b/src/ui/webeiddialog.cpp @@ -553,12 +553,10 @@ void WebEidDialog::connectOkToCachePinAndEmitSelectedCertificate( // QString uses QAtomicPointer internally and is thread-safe. pin = ui->pinInput->text(); - // We use setText instead of clear, so undo/redo will be cleared as well - ui->pinInput->setText(""); - // TODO: To be sure that no copy of PIN text remains in memory, a custom - // widget should be implemented, that: - // - Stores PIN in locked byte vector - // - DOes not leak content through accessibility interface + // TODO: We need to erase the PIN in the widget buffer, this needs further work. + // Investigate if it is possible to keep the PIN in secure memory, e.g. with a + // custom Qt widget. + ui->pinInput->clear(); emit accepted(certAndPin); }); @@ -752,11 +750,13 @@ WebEidDialog::retriableErrorToTextTitleAndIcon(const RetriableError error) tr("Operation not supported"), pixmap("no-id-card"_L1)}; case RetriableError::SMART_CARD_COMMAND_ERROR: - return {tr("Error communicating with the card. Please try again."), tr("Operation failed"), + return {tr("Error communicating with the card."), tr("Operation failed"), pixmap("no-id-card"_L1)}; + // TODO: what action should the user take? Should this be fatal? case RetriableError::PKCS11_ERROR: return {tr("Card driver error. Please try again."), tr("Card driver error"), pixmap("no-id-card"_L1)}; + // TODO: what action should the user take? Should this be fatal? case RetriableError::SCARD_ERROR: return {tr("An error occurred in the Smart Card service required to use the ID-card. Make " "sure that the ID-card and the card reader are connected correctly or relaunch " From 7ed014b6a725d93ec1993d8b58940da46bd05550 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Mon, 27 Nov 2023 14:25:57 +0200 Subject: [PATCH 07/11] Use scope_exit for clearing pin, some minor cleanups --- src/controller/command-handlers/authenticate.cpp | 9 ++++----- src/controller/command-handlers/sign.cpp | 10 ++++------ src/controller/command-handlers/signauthutils.cpp | 2 +- src/controller/controller.cpp | 3 +-- src/ui/webeiddialog.cpp | 7 ++++--- 5 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/controller/command-handlers/authenticate.cpp b/src/controller/command-handlers/authenticate.cpp index 99388552..65c0198d 100644 --- a/src/controller/command-handlers/authenticate.cpp +++ b/src/controller/command-handlers/authenticate.cpp @@ -121,20 +121,19 @@ QVariantMap Authenticate::onConfirm(WebEidUI* window, pcsc_cpp::byte_vector pin; getPin(pin, cardCertAndPin.cardInfo->eid().smartcard(), window); + scope_exit([&pin] { + // Erase PIN memory. + std::fill(pin.begin(), pin.end(), '\0'); + }); try { const auto signature = createSignature(origin.url(), challengeNonce, cardCertAndPin.cardInfo->eid(), pin); - // Erase the PIN memory. - std::fill(pin.begin(), pin.end(), '\0'); - return createAuthenticationToken(signatureAlgorithm, cardCertAndPin.certificateBytesInDer, signature); } catch (const VerifyPinFailed& failure) { - // Erase the PIN memory. - std::fill(pin.begin(), pin.end(), '\0'); switch (failure.status()) { case electronic_id::VerifyPinFailed::Status::PIN_ENTRY_CANCEL: case electronic_id::VerifyPinFailed::Status::PIN_ENTRY_TIMEOUT: diff --git a/src/controller/command-handlers/sign.cpp b/src/controller/command-handlers/sign.cpp index 93c4a948..cb2c3b33 100644 --- a/src/controller/command-handlers/sign.cpp +++ b/src/controller/command-handlers/sign.cpp @@ -97,21 +97,19 @@ QVariantMap Sign::onConfirm(WebEidUI* window, const CardCertificateAndPinInfo& c { pcsc_cpp::byte_vector pin; getPin(pin, cardCertAndPin.cardInfo->eid().smartcard(), window); + scope_exit([&pin] { + // Erase PIN memory. + std::fill(pin.begin(), pin.end(), '\0'); + }); try { const auto signature = signHash(cardCertAndPin.cardInfo->eid(), pin, docHash, hashAlgo); - // Erase PIN memory. - std::fill(pin.begin(), pin.end(), '\0'); - return {{QStringLiteral("signature"), signature.first}, {QStringLiteral("signatureAlgorithm"), signature.second}}; } catch (const VerifyPinFailed& failure) { - // Erase PIN memory. - std::fill(pin.begin(), pin.end(), '\0'); - switch (failure.status()) { case electronic_id::VerifyPinFailed::Status::PIN_ENTRY_CANCEL: case electronic_id::VerifyPinFailed::Status::PIN_ENTRY_TIMEOUT: diff --git a/src/controller/command-handlers/signauthutils.cpp b/src/controller/command-handlers/signauthutils.cpp index ccb58488..da4e6a0f 100644 --- a/src/controller/command-handlers/signauthutils.cpp +++ b/src/controller/command-handlers/signauthutils.cpp @@ -91,7 +91,7 @@ int getPin(pcsc_cpp::byte_vector& pin, const pcsc_cpp::SmartCard& card, WebEidUI if (pinqs.isEmpty()) { THROW(ProgrammingError, "Empty PIN"); } - int len = (int) pinqs.length(); + int len = (int)pinqs.length(); pin.resize(len); for (int i = 0; i < len; i++) { diff --git a/src/controller/controller.cpp b/src/controller/controller.cpp index a3331e82..67a3d6d8 100644 --- a/src/controller/controller.cpp +++ b/src/controller/controller.cpp @@ -108,8 +108,7 @@ void Controller::run() // Pass invalid argument message to the caller just in case it may be interested // The result will be {"invalid-argument" : message} // Command parameter is only used if exception will be raised during json creation - writeResponseToStdOut(isInStdinMode, - {{QStringLiteral("invalid-argument"), exc.what()}}, + writeResponseToStdOut(isInStdinMode, {{QStringLiteral("invalid-argument"), exc.what()}}, "invalid-argument"); } onCriticalFailure(exc.what()); diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp index 95e12def..5a44e663 100644 --- a/src/ui/webeiddialog.cpp +++ b/src/ui/webeiddialog.cpp @@ -284,7 +284,7 @@ QString WebEidDialog::getPin() // QString uses QAtomicPointer internally and is thread-safe. // There should be only single reference and this is transferred to the caller for safety QString ret = pin; - pin = ""; + pin.clear(); return ret; } @@ -334,7 +334,8 @@ void WebEidDialog::onMultipleCertificatesReady( ui->selectAnotherCertificate->setVisible(certificateAndPinInfos.size() > 1); connect(ui->selectAnotherCertificate, &QPushButton::clicked, this, [this, origin, certificateAndPinInfos] { - ui->pinInput->setText(""); + // We set pinInput to empty text instead of clear() to also reset undo buffer + ui->pinInput->setText({}); onMultipleCertificatesReady(origin, certificateAndPinInfos); }); setupOK([this, origin, certificateAndPinInfos] { @@ -690,7 +691,7 @@ void WebEidDialog::displayFatalError(std::function message) ui->okButton->hide(); ui->cancelButton->setEnabled(true); ui->cancelButton->show(); - //ui->helpButton->show(); + // ui->helpButton->show(); } void WebEidDialog::showPinInputWarning(bool show) From e78f7762d2df52979d2be40f50e582d0fcee0527 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Mon, 27 Nov 2023 14:43:42 +0200 Subject: [PATCH 08/11] Use QScopeGuard instead of scope_exit --- src/controller/command-handlers/authenticate.cpp | 2 +- src/controller/command-handlers/sign.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controller/command-handlers/authenticate.cpp b/src/controller/command-handlers/authenticate.cpp index 65c0198d..0ed124d1 100644 --- a/src/controller/command-handlers/authenticate.cpp +++ b/src/controller/command-handlers/authenticate.cpp @@ -121,7 +121,7 @@ QVariantMap Authenticate::onConfirm(WebEidUI* window, pcsc_cpp::byte_vector pin; getPin(pin, cardCertAndPin.cardInfo->eid().smartcard(), window); - scope_exit([&pin] { + auto pin_cleanup = qScopeGuard([&pin] { // Erase PIN memory. std::fill(pin.begin(), pin.end(), '\0'); }); diff --git a/src/controller/command-handlers/sign.cpp b/src/controller/command-handlers/sign.cpp index cb2c3b33..e7dacb89 100644 --- a/src/controller/command-handlers/sign.cpp +++ b/src/controller/command-handlers/sign.cpp @@ -97,7 +97,7 @@ QVariantMap Sign::onConfirm(WebEidUI* window, const CardCertificateAndPinInfo& c { pcsc_cpp::byte_vector pin; getPin(pin, cardCertAndPin.cardInfo->eid().smartcard(), window); - scope_exit([&pin] { + auto pin_cleanup = qScopeGuard([&pin] { // Erase PIN memory. std::fill(pin.begin(), pin.end(), '\0'); }); From e7cdf632de45e322306b020a24974872cdf8f5a2 Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Tue, 28 Nov 2023 10:20:25 +0200 Subject: [PATCH 09/11] Included QScopeGuard headers --- src/controller/command-handlers/sign.cpp | 3 ++- src/controller/command-handlers/signauthutils.cpp | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/controller/command-handlers/sign.cpp b/src/controller/command-handlers/sign.cpp index e7dacb89..f808a353 100644 --- a/src/controller/command-handlers/sign.cpp +++ b/src/controller/command-handlers/sign.cpp @@ -23,7 +23,8 @@ #include "sign.hpp" #include "signauthutils.hpp" -#include "utils/utils.hpp" + +#include using namespace electronic_id; diff --git a/src/controller/command-handlers/signauthutils.cpp b/src/controller/command-handlers/signauthutils.cpp index da4e6a0f..f888088f 100644 --- a/src/controller/command-handlers/signauthutils.cpp +++ b/src/controller/command-handlers/signauthutils.cpp @@ -26,6 +26,8 @@ #include "commandhandler.hpp" #include "utils/utils.hpp" +#include + using namespace electronic_id; // Take argument names by copy/move as they will be modified. From 479a3a88f5a6199564b505e7b627c108935319ce Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Tue, 28 Nov 2023 12:14:43 +0200 Subject: [PATCH 10/11] Moved #include to the right place, removed commented out stuff etc. --- src/controller/command-handlers/authenticate.cpp | 1 + src/controller/command-handlers/signauthutils.cpp | 3 --- src/ui/webeiddialog.cpp | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/controller/command-handlers/authenticate.cpp b/src/controller/command-handlers/authenticate.cpp index 0ed124d1..5b6c8f8a 100644 --- a/src/controller/command-handlers/authenticate.cpp +++ b/src/controller/command-handlers/authenticate.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include diff --git a/src/controller/command-handlers/signauthutils.cpp b/src/controller/command-handlers/signauthutils.cpp index f888088f..0782513c 100644 --- a/src/controller/command-handlers/signauthutils.cpp +++ b/src/controller/command-handlers/signauthutils.cpp @@ -24,9 +24,6 @@ #include "ui.hpp" #include "commandhandler.hpp" -#include "utils/utils.hpp" - -#include using namespace electronic_id; diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp index 5a44e663..987fe4a1 100644 --- a/src/ui/webeiddialog.cpp +++ b/src/ui/webeiddialog.cpp @@ -691,7 +691,6 @@ void WebEidDialog::displayFatalError(std::function message) ui->okButton->hide(); ui->cancelButton->setEnabled(true); ui->cancelButton->show(); - // ui->helpButton->show(); } void WebEidDialog::showPinInputWarning(bool show) From f1bfe12a975e6cfbf4927bc01b42ef6e3436373b Mon Sep 17 00:00:00 2001 From: Lauris Kaplinski Date: Tue, 28 Nov 2023 12:47:21 +0200 Subject: [PATCH 11/11] Changed vcpkg git commit id --- .github/workflows/cmake-windows.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cmake-windows.yml b/.github/workflows/cmake-windows.yml index 18e6251a..12b720e8 100644 --- a/.github/workflows/cmake-windows.yml +++ b/.github/workflows/cmake-windows.yml @@ -21,7 +21,7 @@ jobs: with: vcpkgArguments: gtest openssl vcpkgTriplet: x64-windows - vcpkgGitCommitId: 9b9c2758ece1d8ac0de90589730bb5ccf45c0874 + vcpkgGitCommitId: 98a562a04cd03728f399e79e1b37bcccb5a69b37 - name: Install Qt uses: jurplel/install-qt-action@v3