From bcbfebdeada9cb2888c80eec1c14cef8dedcafce Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Thu, 2 May 2024 10:40:19 +0300 Subject: [PATCH 1/3] Workaround when reading card delays and use presses sign again WE2-861, Fixes #311 Signed-off-by: Raul Metsma --- .../command-handlers/authenticate.cpp | 2 +- src/controller/command-handlers/sign.cpp | 2 +- .../command-handlers/signauthutils.cpp | 6 +- .../command-handlers/signauthutils.hpp | 12 ++-- src/ui/webeiddialog.cpp | 62 ++++++++++--------- src/ui/webeiddialog.hpp | 8 +-- 6 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/controller/command-handlers/authenticate.cpp b/src/controller/command-handlers/authenticate.cpp index c5171813..a5697386 100644 --- a/src/controller/command-handlers/authenticate.cpp +++ b/src/controller/command-handlers/authenticate.cpp @@ -122,7 +122,7 @@ QVariantMap Authenticate::onConfirm(WebEidUI* window, const auto signatureAlgorithm = QString::fromStdString(cardCertAndPin.cardInfo->eid().authSignatureAlgorithm()); - auto pin = getPin(cardCertAndPin.cardInfo->eid().smartcard(), window); + auto pin = getPin(cardCertAndPin.cardInfo->eid(), window); try { const auto signature = diff --git a/src/controller/command-handlers/sign.cpp b/src/controller/command-handlers/sign.cpp index 9c66c747..d2b7bc45 100644 --- a/src/controller/command-handlers/sign.cpp +++ b/src/controller/command-handlers/sign.cpp @@ -95,7 +95,7 @@ void Sign::emitCertificatesReady(const std::vector& c QVariantMap Sign::onConfirm(WebEidUI* window, const CardCertificateAndPinInfo& cardCertAndPin) { - auto pin = getPin(cardCertAndPin.cardInfo->eid().smartcard(), window); + auto pin = getPin(cardCertAndPin.cardInfo->eid(), window); try { const auto signature = signHash(cardCertAndPin.cardInfo->eid(), pin, docHash, hashAlgo); diff --git a/src/controller/command-handlers/signauthutils.cpp b/src/controller/command-handlers/signauthutils.cpp index 89d2429d..52615529 100644 --- a/src/controller/command-handlers/signauthutils.cpp +++ b/src/controller/command-handlers/signauthutils.cpp @@ -78,10 +78,10 @@ inline void eraseData(T& data) } } -pcsc_cpp::byte_vector getPin(const pcsc_cpp::SmartCard& card, WebEidUI* window) +pcsc_cpp::byte_vector getPin(const ElectronicID& card, WebEidUI* window) { // Doesn't apply to PIN pads. - if (card.readerHasPinPad()) { + if (card.smartcard().readerHasPinPad() || card.providesExternalPinDialog()) { return {}; } @@ -94,7 +94,7 @@ pcsc_cpp::byte_vector getPin(const pcsc_cpp::SmartCard& card, WebEidUI* window) // TODO: Avoid making copies of the PIN in memory. auto pinQByteArray = pin.toUtf8(); - auto pinBytes = pcsc_cpp::byte_vector {pinQByteArray.begin(), pinQByteArray.end()}; + pcsc_cpp::byte_vector pinBytes {pinQByteArray.begin(), pinQByteArray.end()}; // TODO: Verify that the buffers are actually zeroed and no copies remain. eraseData(pin); diff --git a/src/controller/command-handlers/signauthutils.hpp b/src/controller/command-handlers/signauthutils.hpp index 6019e785..344a20de 100644 --- a/src/controller/command-handlers/signauthutils.hpp +++ b/src/controller/command-handlers/signauthutils.hpp @@ -22,13 +22,17 @@ #pragma once -#include "electronic-id/enums.hpp" -#include "pcsc-cpp/pcsc-cpp-utils.hpp" +#include "pcsc-cpp/pcsc-cpp.hpp" #include class WebEidUI; -class QSslCertificate; + +namespace electronic_id +{ +class ElectronicID; +class SignatureAlgorithm; +} void requireArgumentsAndOptionalLang(QStringList argNames, const QVariantMap& args, const std::string& argDescriptions); @@ -41,6 +45,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); +pcsc_cpp::byte_vector getPin(const electronic_id::ElectronicID& card, WebEidUI* window); QVariantMap signatureAlgoToVariantMap(const electronic_id::SignatureAlgorithm signatureAlgo); diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp index f15db86b..7b8a53e0 100644 --- a/src/ui/webeiddialog.cpp +++ b/src/ui/webeiddialog.cpp @@ -296,7 +296,7 @@ void WebEidDialog::onSmartCardStatusUpdate(const RetriableError status) setTrText(ui->connectCardLabel, std::get<0>(retriableErrorToTextTitleAndIcon(status))); setTrText(ui->messagePageTitleLabel, std::get<1>(retriableErrorToTextTitleAndIcon(status))); - ui->cardChipIcon->setPixmap(std::get<2>(retriableErrorToTextTitleAndIcon(status))); + ui->cardChipIcon->setPixmap(pixmap(std::get<2>(retriableErrorToTextTitleAndIcon(status)))); // In case the insert card page is not shown, switch back to it. ui->helpButton->show(); @@ -320,6 +320,7 @@ void WebEidDialog::onMultipleCertificatesReady( switch (currentCommand) { case CommandType::GET_SIGNING_CERTIFICATE: setupOK([this] { + ui->okButton->setDisabled(true); if (auto* button = qobject_cast(ui->selectionGroup->checkedButton())) { emit accepted(button->certificateInfo()); @@ -338,6 +339,7 @@ void WebEidDialog::onMultipleCertificatesReady( onMultipleCertificatesReady(origin, certificateAndPinInfos); }); setupOK([this, origin] { + ui->okButton->setDisabled(true); // Authenticate continues with the selected certificate to onSingleCertificateReady(). if (auto* button = qobject_cast(ui->selectionGroup->checkedButton())) { @@ -372,7 +374,10 @@ void WebEidDialog::onSingleCertificateReady(const QUrl& origin, switch (currentCommand) { case CommandType::GET_SIGNING_CERTIFICATE: setupCertificateAndPinInfo({certAndPin}); - setupOK([this, certAndPin] { emit accepted(certAndPin); }); + setupOK([this, certAndPin] { + ui->okButton->setDisabled(true); + emit accepted(certAndPin); + }); ui->selectionGroup->buttons().at(0)->click(); ui->pageStack->setCurrentIndex(int(Page::SELECT_CERTIFICATE)); return; @@ -413,8 +418,7 @@ void WebEidDialog::onSingleCertificateReady(const QUrl& origin, ui->pinTitleLabel->hide(); } else if (useExternalPinDialog) { connectOkToCachePinAndEmitSelectedCertificate(certAndPin); - ui->pinInput->setText( - QString::fromLatin1("unused")); // Dummy value as empty PIN is not allowed. + ui->okButton->setEnabled(true); } else if (certAndPin.pinInfo.readerHasPinPad) { setupPinPadProgressBarAndEmitWait(certAndPin); } else { @@ -520,7 +524,7 @@ void WebEidDialog::onRetryImpl(Text text) setTrText(ui->connectCardLabel, std::forward(text)); setTrText(ui->messagePageTitleLabel, QT_TR_NOOP("Operation failed")); ui->cardChipIcon->setPixmap(pixmap("no-id-card"_L1)); - setupOK([this] { emit retry(); }, QT_TR_NOOP("Try again"), true); + setupOK(&WebEidDialog::retry, QT_TR_NOOP("Try again"), true); ui->pageStack->setCurrentIndex(int(Page::ALERT)); } @@ -581,7 +585,7 @@ void WebEidDialog::setupCertificateAndPinInfo( } } -void WebEidDialog::setupPinPrompt(const PinInfo& pinInfo) +void WebEidDialog::setupPinPrompt(PinInfo pinInfo) { ui->okButton->setHidden(pinInfo.readerHasPinPad); ui->cancelButton->setHidden(pinInfo.readerHasPinPad); @@ -593,9 +597,9 @@ void WebEidDialog::setupPinPrompt(const PinInfo& pinInfo) ui->pinErrorLabel->setVisible(showPinError); showPinInputWarning(showPinError); if (showPinError) { - setTrText(ui->pinErrorLabel, [pinInfo]() -> QString { + setTrText(ui->pinErrorLabel, [count = pinInfo.pinRetriesCount.first]() -> QString { return tr("The PIN has been entered incorrectly at least once. %n attempts left.", - nullptr, int(pinInfo.pinRetriesCount.first)); + nullptr, count); }); } } @@ -641,9 +645,9 @@ void WebEidDialog::setupPinInput(const CardCertificateAndPinInfo& certAndPin) certAndPin.cardInfo->eid().allowsUsingLettersAndSpecialCharactersInPin() ? QStringLiteral("[0-9 -/:-@[-`{-~\\p{L}]{%1,%2}") : QStringLiteral("[0-9]{%1,%2}"); - const auto numericMinMaxRegexp = - QRegularExpression(regexpWithOrWithoutLetters.arg(certAndPin.pinInfo.pinMinMaxLength.first) - .arg(certAndPin.pinInfo.pinMinMaxLength.second)); + const QRegularExpression numericMinMaxRegexp( + regexpWithOrWithoutLetters.arg(certAndPin.pinInfo.pinMinMaxLength.first) + .arg(certAndPin.pinInfo.pinMinMaxLength.second)); ui->pinInputValidator->setRegularExpression(numericMinMaxRegexp); ui->pinInput->setMaxLength(int(certAndPin.pinInfo.pinMinMaxLength.second)); ui->pinInput->setFocus(); @@ -651,7 +655,7 @@ void WebEidDialog::setupPinInput(const CardCertificateAndPinInfo& certAndPin) } template -void WebEidDialog::setupOK(Func&& func, const char* text, bool enabled) +void WebEidDialog::setupOK(Func func, const char* text, bool enabled) { ui->okButton->disconnect(); connect(ui->okButton, &QPushButton::clicked, this, std::forward(func)); @@ -697,78 +701,78 @@ QPixmap WebEidDialog::pixmap(QLatin1String name) .arg(name, Application::isDarkTheme() ? "_dark"_L1 : QLatin1String())}; } -std::tuple -WebEidDialog::retriableErrorToTextTitleAndIcon(const RetriableError error) +constexpr std::tuple +WebEidDialog::retriableErrorToTextTitleAndIcon(const RetriableError error) noexcept { switch (error) { case RetriableError::SMART_CARD_SERVICE_IS_NOT_RUNNING: return { QT_TR_NOOP("The smart card service required to use the ID-card is not running. Please " "start the smart card service and try again."), - QT_TR_NOOP("Launch the Smart Card service"), pixmap("cardreader"_L1)}; + QT_TR_NOOP("Launch the Smart Card service"), "cardreader"_L1}; case RetriableError::NO_SMART_CARD_READERS_FOUND: return {QT_TR_NOOP("Card reader not connected. Please connect the card reader to " "the computer."), - QT_TR_NOOP("Connect the card reader"), pixmap("cardreader"_L1)}; + QT_TR_NOOP("Connect the card reader"), "cardreader"_L1}; case RetriableError::NO_SMART_CARDS_FOUND: case RetriableError::PKCS11_TOKEN_NOT_PRESENT: return {QT_TR_NOOP("ID-card not found. Please insert the ID-card into the reader."), - QT_TR_NOOP("Insert the ID-card"), pixmap("no-id-card"_L1)}; + QT_TR_NOOP("Insert the ID-card"), "no-id-card"_L1}; case RetriableError::SMART_CARD_WAS_REMOVED: case RetriableError::PKCS11_TOKEN_REMOVED: return {QT_TR_NOOP( "The ID-card was removed from the reader. Please insert the ID-card into the " "reader."), - QT_TR_NOOP("Insert the ID-card"), pixmap("no-id-card"_L1)}; + QT_TR_NOOP("Insert the ID-card"), "no-id-card"_L1}; case RetriableError::SMART_CARD_TRANSACTION_FAILED: return { QT_TR_NOOP( "Operation failed. Make sure that the ID-card and the card reader are connected " "correctly."), - QT_TR_NOOP("Check the ID-card and the reader connection"), pixmap("no-id-card"_L1)}; + QT_TR_NOOP("Check the ID-card and the reader connection"), "no-id-card"_L1}; case RetriableError::FAILED_TO_COMMUNICATE_WITH_CARD_OR_READER: return { QT_TR_NOOP( "Connection to the ID-card or reader failed. Make sure that the ID-card and the " "card reader are connected correctly."), - QT_TR_NOOP("Check the ID-card and the reader connection"), pixmap("no-id-card"_L1)}; + QT_TR_NOOP("Check the ID-card and the reader connection"), "no-id-card"_L1}; case RetriableError::SMART_CARD_CHANGE_REQUIRED: return { QT_TR_NOOP( "The desired operation cannot be performed with the inserted ID-card. Make sure " "that the ID-card is supported by the Web eID application."), - QT_TR_NOOP("Operation not supported"), pixmap("no-id-card"_L1)}; + QT_TR_NOOP("Operation not supported"), "no-id-card"_L1}; case RetriableError::SMART_CARD_COMMAND_ERROR: return {QT_TR_NOOP("Error communicating with the card."), QT_TR_NOOP("Operation failed"), - pixmap("no-id-card"_L1)}; + "no-id-card"_L1}; // TODO: what action should the user take? Should this be fatal? case RetriableError::PKCS11_ERROR: return {QT_TR_NOOP("Card driver error. Please try again."), QT_TR_NOOP("Card driver error"), - pixmap("no-id-card"_L1)}; + "no-id-card"_L1}; // TODO: what action should the user take? Should this be fatal? case RetriableError::SCARD_ERROR: return {QT_TR_NOOP( "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 " "the Smart Card service."), - QT_TR_NOOP("Operation failed"), pixmap("no-id-card"_L1)}; + QT_TR_NOOP("Operation failed"), "no-id-card"_L1}; case RetriableError::UNSUPPORTED_CARD: return { QT_TR_NOOP( "The card in the reader is not supported. Make sure that the entered ID-card is " "supported by the Web eID application."), - QT_TR_NOOP("Operation not supported"), pixmap("no-id-card"_L1)}; + QT_TR_NOOP("Operation not supported"), "no-id-card"_L1}; case RetriableError::NO_VALID_CERTIFICATE_AVAILABLE: return {QT_TR_NOOP( "The inserted ID-card does not contain a certificate for the requested " "operation. Please insert an ID-card that supports the requested operation."), - QT_TR_NOOP("Operation not supported"), pixmap("no-id-card"_L1)}; + QT_TR_NOOP("Operation not supported"), "no-id-card"_L1}; case RetriableError::PIN_VERIFY_DISABLED: return { @@ -777,10 +781,10 @@ WebEidDialog::retriableErrorToTextTitleAndIcon(const RetriableError error) "used. Read more here."), - QT_TR_NOOP("Card driver error"), QStringLiteral(":/images/cardreader.svg")}; + QT_TR_NOOP("Card driver error"), "cardreader"_L1}; case RetriableError::UNKNOWN_ERROR: - return {QT_TR_NOOP("Unknown error"), QT_TR_NOOP("Unknown error"), pixmap("no-id-card"_L1)}; + return {QT_TR_NOOP("Unknown error"), QT_TR_NOOP("Unknown error"), "no-id-card"_L1}; } - return {QT_TR_NOOP("Unknown error"), QT_TR_NOOP("Unknown error"), pixmap("no-id-card"_L1)}; + return {QT_TR_NOOP("Unknown error"), QT_TR_NOOP("Unknown error"), "no-id-card"_L1}; } diff --git a/src/ui/webeiddialog.hpp b/src/ui/webeiddialog.hpp index 453a59de..cdbe2c0b 100644 --- a/src/ui/webeiddialog.hpp +++ b/src/ui/webeiddialog.hpp @@ -98,19 +98,19 @@ class WebEidDialog final : public WebEidUI void setTrText(QWidget* label, Text text) const; void setupCertificateAndPinInfo(const std::vector& cardCertAndPinInfos); - void setupPinPrompt(const PinInfo& pinInfo); + void setupPinPrompt(PinInfo pinInfo); void setupPinPadProgressBarAndEmitWait(const CardCertificateAndPinInfo& certAndPin); void setupPinInput(const CardCertificateAndPinInfo& certAndPin); template - void setupOK(Func&& func, const char* text = {}, bool enabled = false); + void setupOK(Func func, const char* text = {}, bool enabled = false); void displayPinBlockedError(); void showPinInputWarning(bool show); void resizeHeight(); static QPixmap pixmap(QLatin1String name); - static std::tuple - retriableErrorToTextTitleAndIcon(RetriableError error); + constexpr static std::tuple + retriableErrorToTextTitleAndIcon(RetriableError error) noexcept; class Private; Private* ui; From e056d2918ab721e03a73a9cffdeab7bc296d3ace Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Thu, 9 May 2024 10:17:08 +0300 Subject: [PATCH 2/3] And add additional BelEID ATR-s WE2-808 Signed-off-by: Raul Metsma --- lib/libelectronic-id | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libelectronic-id b/lib/libelectronic-id index d8b914fa..2d2ab20c 160000 --- a/lib/libelectronic-id +++ b/lib/libelectronic-id @@ -1 +1 @@ -Subproject commit d8b914fad9530f9643c75e4fc8724af1561ad210 +Subproject commit 2d2ab20cf92561533546c646eba4c4da64e17f5f From 65f83995eb0228ef748df8055962efe53ad38b80 Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Fri, 7 Jun 2024 12:24:45 +0300 Subject: [PATCH 3/3] Fix CodeQL warnings WE2-958, WE2-959, WE2-957 Signed-off-by: Raul Metsma --- .github/workflows/cmake-linux-codeql.yml | 8 +++--- .../command-handlers/signauthutils.hpp | 2 +- src/controller/controller.cpp | 3 +- tests/tests/changecertificatevaliduntil.hpp | 28 ++++++++----------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/.github/workflows/cmake-linux-codeql.yml b/.github/workflows/cmake-linux-codeql.yml index 72b3ec8f..dcd5ae23 100644 --- a/.github/workflows/cmake-linux-codeql.yml +++ b/.github/workflows/cmake-linux-codeql.yml @@ -20,14 +20,14 @@ jobs: with: submodules: recursive - - uses: github/codeql-action/init@v2 + - uses: github/codeql-action/init@v3 with: languages: cpp queries: +security-and-quality - - uses: github/codeql-action/autobuild@v2 + - uses: github/codeql-action/autobuild@v3 - - uses: github/codeql-action/analyze@v2 + - uses: github/codeql-action/analyze@v3 with: upload: False output: sarif-results @@ -39,6 +39,6 @@ jobs: input: sarif-results/cpp.sarif output: sarif-results/cpp.sarif - - uses: github/codeql-action/upload-sarif@v2 + - uses: github/codeql-action/upload-sarif@v3 with: sarif_file: sarif-results/cpp.sarif diff --git a/src/controller/command-handlers/signauthutils.hpp b/src/controller/command-handlers/signauthutils.hpp index 344a20de..b26cccbd 100644 --- a/src/controller/command-handlers/signauthutils.hpp +++ b/src/controller/command-handlers/signauthutils.hpp @@ -32,7 +32,7 @@ namespace electronic_id { class ElectronicID; class SignatureAlgorithm; -} +} // namespace electronic_id void requireArgumentsAndOptionalLang(QStringList argNames, const QVariantMap& args, const std::string& argDescriptions); diff --git a/src/controller/controller.cpp b/src/controller/controller.cpp index 0e71453b..57ddb8bc 100644 --- a/src/controller/controller.cpp +++ b/src/controller/controller.cpp @@ -354,7 +354,8 @@ void Controller::onCriticalFailure(const QString& error) { qCritical() << "Exiting due to command" << std::string(commandType()) << "fatal error:" << error; - _result = makeErrorObject(RESP_TECH_ERROR, QStringLiteral("Technical error, see application logs")); + _result = + makeErrorObject(RESP_TECH_ERROR, QStringLiteral("Technical error, see application logs")); writeResponseToStdOut(isInStdinMode, _result, commandType()); disposeUI(); WebEidUI::showFatalError(); diff --git a/tests/tests/changecertificatevaliduntil.hpp b/tests/tests/changecertificatevaliduntil.hpp index b6619dea..3b8e76cc 100644 --- a/tests/tests/changecertificatevaliduntil.hpp +++ b/tests/tests/changecertificatevaliduntil.hpp @@ -24,14 +24,14 @@ #include "pcsc-mock/pcsc-mock.hpp" -#include #include +#include -PcscMock::byte_vector::iterator findUTCDateTime(PcscMock::byte_vector::iterator first, - PcscMock::byte_vector::iterator last) +inline PcscMock::byte_vector::iterator findUTCDateTime(PcscMock::byte_vector::iterator first, + PcscMock::byte_vector::iterator last) { - static const unsigned char UTC_DATETIME_TAG = 0x17; - static const unsigned char LENGTH_TAG = 0x0d; + constexpr unsigned char UTC_DATETIME_TAG = 0x17; + constexpr unsigned char LENGTH_TAG = 0x0d; for (; first != last; ++first) { if (*first == UTC_DATETIME_TAG && first + 1 != last && *(first + 1) == LENGTH_TAG) { @@ -41,9 +41,9 @@ PcscMock::byte_vector::iterator findUTCDateTime(PcscMock::byte_vector::iterator return last; } -PcscMock::ApduScript replaceCertValidUntilYear(const PcscMock::ApduScript& script, - const size_t certBytesStartOffset, - const std::string& twoDigitYear) +inline PcscMock::ApduScript replaceCertValidUntilYear(const PcscMock::ApduScript& script, + const size_t certBytesStartOffset, + std::string_view twoDigitYear) { if (twoDigitYear.size() != 2) { throw std::invalid_argument("replaceCertValidUntilYear: twoDigitYear size must be 2, " @@ -85,17 +85,13 @@ PcscMock::ApduScript replaceCertValidUntilYear(const PcscMock::ApduScript& scrip return scriptCopy; } -PcscMock::ApduScript replaceCertValidUntilTo2010(const PcscMock::ApduScript& script) +inline PcscMock::ApduScript replaceCertValidUntilTo2010(const PcscMock::ApduScript& script) { return replaceCertValidUntilYear(script, 4, "10"); } -PcscMock::ApduScript replaceCertValidUntilToNextYear(const PcscMock::ApduScript& script) +inline PcscMock::ApduScript replaceCertValidUntilToNextYear(const PcscMock::ApduScript& script) { - const auto t = std::time(nullptr); - const auto now = std::localtime(&t); - // UTCDateTime needs 2-digit year since 2000, tm_year is years since 1900, add +1 for next year - const auto yearInt = now->tm_year + 1900 - 2000 + 1; - const auto yearStr = std::to_string(yearInt); - return replaceCertValidUntilYear(script, 4, yearStr); + // UTCDateTime needs 2-digit year since 2000, add +1 for next year + return replaceCertValidUntilYear(script, 4, std::to_string(QDate::currentDate().year() - 2000 + 1)); }