From 879c4c8ff087f78db90401627f855415f5c7779e Mon Sep 17 00:00:00 2001 From: Vytautas Mickus Date: Sun, 17 Mar 2024 15:29:42 +0200 Subject: [PATCH 1/3] Support for new Lithuanian ID cards. --- src/electronic-id.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/electronic-id.cpp b/src/electronic-id.cpp index 893f558..3c5de9e 100644 --- a/src/electronic-id.cpp +++ b/src/electronic-id.cpp @@ -85,6 +85,10 @@ const std::map SUPPORTED_ATRS { {{0x3B, 0x9D, 0x18, 0x81, 0x31, 0xFC, 0x35, 0x80, 0x31, 0xC0, 0x69, 0x4D, 0x54, 0x43, 0x4F, 0x53, 0x73, 0x02, 0x05, 0x05, 0xD3}, constructor}, + // LitEID v2.0 + {{0x3B, 0x9D, 0x18, 0x81, 0x31, 0xFC, 0x35, 0x80, 0x31, 0xC0, 0x69, + 0x4D, 0x54, 0x43, 0x4F, 0x53, 0x73, 0x02, 0x06, 0x04, 0xD1}, + constructor}, // HrvEID {{0x3b, 0xff, 0x13, 0x00, 0x00, 0x81, 0x31, 0xfe, 0x45, 0x00, 0x31, 0xb9, 0x64, 0x04, 0x44, 0xec, 0xc1, 0x73, 0x94, 0x01, 0x80, 0x82, 0x90, 0x00, 0x12}, From d8b914fad9530f9643c75e4fc8724af1561ad210 Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Mon, 1 Apr 2024 14:38:11 +0300 Subject: [PATCH 2/3] Fix coverity warnings IB-7930 Signed-off-by: Raul Metsma --- include/electronic-id/electronic-id.hpp | 6 +--- src/electronic-id.cpp | 2 +- .../ms-cryptoapi/MsCryptoApiElectronicID.hpp | 6 ++-- src/electronic-ids/pcsc/pcsc-common.hpp | 4 +-- .../pkcs11/PKCS11CardManager.hpp | 35 +++++++++---------- 5 files changed, 23 insertions(+), 30 deletions(-) diff --git a/include/electronic-id/electronic-id.hpp b/include/electronic-id/electronic-id.hpp index 481feda..0fabab9 100644 --- a/include/electronic-id/electronic-id.hpp +++ b/include/electronic-id/electronic-id.hpp @@ -24,10 +24,6 @@ #include "enums.hpp" -#include "pcsc-cpp/pcsc-cpp.hpp" - -#include - namespace electronic_id { @@ -37,7 +33,7 @@ class ElectronicID { public: using ptr = std::shared_ptr; - using PinMinMaxLength = std::pair; + using PinMinMaxLength = std::pair; using PinRetriesRemainingAndMax = std::pair; using byte_vector = pcsc_cpp::byte_vector; using byte_type = pcsc_cpp::byte_type; diff --git a/src/electronic-id.cpp b/src/electronic-id.cpp index 3c5de9e..85d67fa 100644 --- a/src/electronic-id.cpp +++ b/src/electronic-id.cpp @@ -85,7 +85,7 @@ const std::map SUPPORTED_ATRS { {{0x3B, 0x9D, 0x18, 0x81, 0x31, 0xFC, 0x35, 0x80, 0x31, 0xC0, 0x69, 0x4D, 0x54, 0x43, 0x4F, 0x53, 0x73, 0x02, 0x05, 0x05, 0xD3}, constructor}, - // LitEID v2.0 + // LitEID v2.0 {{0x3B, 0x9D, 0x18, 0x81, 0x31, 0xFC, 0x35, 0x80, 0x31, 0xC0, 0x69, 0x4D, 0x54, 0x43, 0x4F, 0x53, 0x73, 0x02, 0x06, 0x04, 0xD1}, constructor}, diff --git a/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp b/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp index 602dac9..ad7ebe1 100644 --- a/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp +++ b/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp @@ -38,8 +38,8 @@ class MsCryptoApiElectronicID : public ElectronicID MsCryptoApiElectronicID(PCCERT_CONTEXT certCtx, pcsc_cpp::byte_vector&& cert, CertificateType cType, bool isRsa, HCRYPTPROV_OR_NCRYPT_KEY_HANDLE k, bool freeK) : - ElectronicID {std::make_unique()}, - certContext {certCtx}, certData {cert}, certType {cType}, + ElectronicID {std::make_unique()}, certContext {certCtx}, + certData {cert}, certType {cType}, // TODO: SignatureAlgorithm::PS? signatureAlgo {isRsa ? SignatureAlgorithm::RS : SignatureAlgorithm::ES}, key {k}, freeKey {freeK} @@ -59,7 +59,7 @@ class MsCryptoApiElectronicID : public ElectronicID // The following placeholders are not used as the external PIN dialog manages PIN length // validation. static const int8_t PIN_RETRY_COUNT_PLACEHOLDER = -1; - static const size_t PIN_LENGTH_PLACEHOLDER = 0; + static const uint8_t PIN_LENGTH_PLACEHOLDER = 0; private: // Use the external dialog provided by the CryptoAPI cryptographic service provider. diff --git a/src/electronic-ids/pcsc/pcsc-common.hpp b/src/electronic-ids/pcsc/pcsc-common.hpp index caf5338..ce56e64 100644 --- a/src/electronic-ids/pcsc/pcsc-common.hpp +++ b/src/electronic-ids/pcsc/pcsc-common.hpp @@ -52,7 +52,7 @@ inline pcsc_cpp::byte_vector addPaddingToPin(const pcsc_cpp::byte_vector& pin, s } inline void verifyPin(pcsc_cpp::SmartCard& card, pcsc_cpp::byte_type p2, - const pcsc_cpp::byte_vector& pin, size_t pinMinLength, size_t paddingLength, + const pcsc_cpp::byte_vector& pin, uint8_t pinMinLength, size_t paddingLength, pcsc_cpp::byte_type paddingChar) { const pcsc_cpp::CommandApdu VERIFY_PIN {0x00, 0x20, 0x00, p2}; @@ -61,7 +61,7 @@ inline void verifyPin(pcsc_cpp::SmartCard& card, pcsc_cpp::byte_type p2, if (card.readerHasPinPad()) { const pcsc_cpp::CommandApdu verifyPin {VERIFY_PIN, addPaddingToPin({}, paddingLength, paddingChar)}; - response = card.transmitCTL(verifyPin, 0, uint8_t(pinMinLength)); + response = card.transmitCTL(verifyPin, 0, pinMinLength); } else { const pcsc_cpp::CommandApdu verifyPin {VERIFY_PIN, diff --git a/src/electronic-ids/pkcs11/PKCS11CardManager.hpp b/src/electronic-ids/pkcs11/PKCS11CardManager.hpp index 169c74c..8558ec6 100644 --- a/src/electronic-ids/pkcs11/PKCS11CardManager.hpp +++ b/src/electronic-ids/pkcs11/PKCS11CardManager.hpp @@ -124,7 +124,7 @@ class PKCS11CardManager std::vector cert, certID; int8_t retry; bool pinpad; - CK_ULONG minPinLen, maxPinLen; + uint8_t minPinLen, maxPinLen; }; std::vector tokens() const @@ -147,17 +147,15 @@ class PKCS11CardManager for (CK_OBJECT_HANDLE obj : findObject(session, CKO_CERTIFICATE)) { result.push_back({ - std::string(reinterpret_cast(tokenInfo.label), - sizeof(tokenInfo.label)), - std::string(reinterpret_cast(tokenInfo.serialNumber), - sizeof(tokenInfo.serialNumber)), + {std::begin(tokenInfo.label), std::end(tokenInfo.label)}, + {std::begin(tokenInfo.serialNumber), std::end(tokenInfo.serialNumber)}, slotID, attribute(session, obj, CKA_VALUE), attribute(session, obj, CKA_ID), pinRetryCount(tokenInfo.flags), (tokenInfo.flags & CKF_PROTECTED_AUTHENTICATION_PATH) > 0, - tokenInfo.ulMinPinLen, - tokenInfo.ulMaxPinLen, + uint8_t(tokenInfo.ulMinPinLen), + uint8_t(tokenInfo.ulMaxPinLen), }); } @@ -211,15 +209,15 @@ class PKCS11CardManager // token.certID.data()); CK_KEY_TYPE keyType = CKK_RSA; - CK_ATTRIBUTE attribute = {CKA_KEY_TYPE, &keyType, sizeof(keyType)}; - C(GetAttributeValue, session, privateKeyHandle[0], &attribute, 1ul); + CK_ATTRIBUTE attribute {CKA_KEY_TYPE, &keyType, sizeof(keyType)}; + C(GetAttributeValue, session, privateKeyHandle[0], &attribute, 1UL); - const electronic_id::SignatureAlgorithm signatureAlgorithm = { + const electronic_id::SignatureAlgorithm signatureAlgorithm { keyType == CKK_ECDSA ? electronic_id::SignatureAlgorithm::ES : electronic_id::SignatureAlgorithm::RS, hashAlgo}; - CK_MECHANISM mechanism = {keyType == CKK_ECDSA ? CKM_ECDSA : CKM_RSA_PKCS, nullptr, 0}; + CK_MECHANISM mechanism {keyType == CKK_ECDSA ? CKM_ECDSA : CKM_RSA_PKCS, nullptr, 0}; C(SignInit, session, &mechanism, privateKeyHandle[0]); std::vector hashWithPaddingOID = keyType == CKK_RSA ? addRSAOID(hashAlgo, hash) : hash; @@ -275,10 +273,9 @@ class PKCS11CardManager template static void Call(const char* function, const char* file, int line, const char* apiFunction, - Func func, Args... args) + Func&& func, Args... args) { - CK_RV rv = func(args...); - switch (rv) { + switch (CK_RV rv = func(args...)) { case CKR_OK: case CKR_CRYPTOKI_ALREADY_INITIALIZED: break; @@ -310,7 +307,7 @@ class PKCS11CardManager THROW_WITH_CALLER_INFO(Pkcs11Error, fn + " failed with return code " + pcsc_cpp::int2hexstr(rv), file, line, function); - }; + } break; } default: @@ -324,11 +321,11 @@ class PKCS11CardManager std::vector attribute(CK_SESSION_HANDLE session, CK_OBJECT_CLASS obj, CK_ATTRIBUTE_TYPE attr) const { - CK_ATTRIBUTE attribute = {attr, nullptr, 0}; - C(GetAttributeValue, session, obj, &attribute, 1ul); + CK_ATTRIBUTE attribute {attr, {}, 0}; + C(GetAttributeValue, session, obj, &attribute, 1UL); std::vector data(attribute.ulValueLen); attribute.pValue = data.data(); - C(GetAttributeValue, session, obj, &attribute, 1ul); + C(GetAttributeValue, session, obj, &attribute, 1UL); return data; } @@ -351,7 +348,7 @@ class PKCS11CardManager return objectHandle; } - static int8_t pinRetryCount(CK_FLAGS flags) + static constexpr int8_t pinRetryCount(CK_FLAGS flags) noexcept { // As PKCS#11 does not provide an API for querying remaining PIN retries, we currently // simply assume max retry count of 3, which is quite common. We might need to revisit this From fb3ea68a4fdd7a8f17474964a31fa757863433da Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Wed, 15 Nov 2023 16:33:47 +0200 Subject: [PATCH 3/3] Verify given signature validity WE2-818 Signed-off-by: Raul Metsma --- CMakeLists.txt | 4 + include/electronic-id/electronic-id.hpp | 6 +- include/electronic-id/enums.hpp | 7 ++ .../ms-cryptoapi/MsCryptoApiElectronicID.cpp | 28 +++-- .../ms-cryptoapi/MsCryptoApiElectronicID.hpp | 6 +- src/electronic-ids/pcsc/PcscElectronicID.hpp | 19 ++- .../pkcs11/Pkcs11ElectronicID.cpp | 14 ++- .../pkcs11/Pkcs11ElectronicID.hpp | 6 +- src/electronic-ids/x509.hpp | 112 ++++++++++++++++-- tests/common/verify.hpp | 80 ++----------- tests/integration/test-authenticate.cpp | 5 +- tests/integration/test-signing.cpp | 2 +- tests/mock/test-get-certificate.cpp | 25 ++-- 13 files changed, 198 insertions(+), 116 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index eb14cd3..2574278 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -85,6 +85,8 @@ add_executable(${MOCK_TEST_EXE} tests/mock/test-pkcs11-token.cpp ) +target_include_directories(${MOCK_TEST_EXE} PRIVATE src) + target_link_libraries(${MOCK_TEST_EXE} ${PROJECT_NAME} pcsc-mock # this is available via libpcsc-cpp @@ -105,6 +107,8 @@ add_executable(${INTEGRATION_TEST_EXE} tests/integration/test-signing.cpp ) +target_include_directories(${INTEGRATION_TEST_EXE} PRIVATE src) + target_link_libraries(${INTEGRATION_TEST_EXE} ${PROJECT_NAME} pcsc diff --git a/include/electronic-id/electronic-id.hpp b/include/electronic-id/electronic-id.hpp index 0fabab9..927c16b 100644 --- a/include/electronic-id/electronic-id.hpp +++ b/include/electronic-id/electronic-id.hpp @@ -66,7 +66,8 @@ class ElectronicID virtual PinRetriesRemainingAndMax authPinRetriesLeft() const = 0; - virtual pcsc_cpp::byte_vector signWithAuthKey(const byte_vector& pin, + virtual pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& cert, + const byte_vector& pin, const byte_vector& hash) const = 0; // Functions related to signing. @@ -78,7 +79,8 @@ class ElectronicID virtual PinRetriesRemainingAndMax signingPinRetriesLeft() const = 0; - virtual Signature signWithSigningKey(const byte_vector& pin, const byte_vector& hash, + virtual Signature signWithSigningKey(const byte_vector& cert, const byte_vector& pin, + const byte_vector& hash, const HashAlgorithm hashAlgo) const = 0; // General functions. diff --git a/include/electronic-id/enums.hpp b/include/electronic-id/enums.hpp index 91ecdbd..8dac581 100644 --- a/include/electronic-id/enums.hpp +++ b/include/electronic-id/enums.hpp @@ -197,6 +197,8 @@ class JsonWebSignatureAlgorithm operator std::string() const; + operator HashAlgorithm() const { return hashAlgorithm(); } + constexpr HashAlgorithm hashAlgorithm() const { switch (value) { @@ -223,6 +225,11 @@ class JsonWebSignatureAlgorithm return value == RS256 || value == RS384 || value == RS512; } + constexpr bool isRSAWithPSSPadding() + { + return value == PS256 || value == PS384 || value == PS512; + } + constexpr size_t hashByteLength() const { return hashAlgorithm().hashByteLength(); } private: diff --git a/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.cpp b/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.cpp index ac8abcb..c2b1b46 100644 --- a/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.cpp +++ b/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.cpp @@ -21,7 +21,7 @@ */ #include "MsCryptoApiElectronicID.hpp" -#include "../scope.hpp" +#include "../x509.hpp" #include #include @@ -35,12 +35,7 @@ using namespace electronic_id; JsonWebSignatureAlgorithm getESAlgorithmFromCert(const byte_vector& cert) { - const unsigned char* certPtr = cert.data(); - auto x509 = SCOPE_GUARD(X509, d2i_X509(nullptr, &certPtr, long(cert.size()))); - if (!x509) { - THROW(MsCryptoApiError, "Failed to create X509 object from certificate"); - } - + auto x509 = toX509(cert); EVP_PKEY* key = X509_get0_pubkey(x509.get()); if (EVP_PKEY_base_id(key) != EVP_PKEY_EC) { THROW(MsCryptoApiError, "EVP_PKEY_base_id() reports non-EC key where EC key expected"); @@ -61,8 +56,9 @@ JsonWebSignatureAlgorithm getESAlgorithmFromCert(const byte_vector& cert) } } -ElectronicID::Signature sign(const byte_vector& hash, HashAlgorithm hashAlgo, - const HCRYPTPROV_OR_NCRYPT_KEY_HANDLE key, const bool isRSA) +ElectronicID::Signature sign(const byte_vector& cert, const byte_vector& hash, + HashAlgorithm hashAlgo, const HCRYPTPROV_OR_NCRYPT_KEY_HANDLE key, + const bool isRSA) { BCRYPT_PKCS1_PADDING_INFO padInfo {}; switch (hashAlgo) { @@ -114,6 +110,10 @@ ElectronicID::Signature sign(const byte_vector& hash, HashAlgorithm hashAlgo, THROW(MsCryptoApiError, "Signing failed with error: " + std::to_string(err)); } + if (!verifyDigest(hashAlgo, cert, hash, signature)) { + THROW(SmartCardError, "Failed to validate given signature!"); + } + return {signature, SignatureAlgorithm {isRSA ? SignatureAlgorithm::RS : SignatureAlgorithm::ES, hashAlgo}}; } @@ -129,7 +129,8 @@ JsonWebSignatureAlgorithm MsCryptoApiElectronicID::authSignatureAlgorithm() cons return isRSA() ? JsonWebSignatureAlgorithm::RS256 : getESAlgorithmFromCert(certData); } -byte_vector MsCryptoApiElectronicID::signWithAuthKey(const byte_vector& /* pin */, +byte_vector MsCryptoApiElectronicID::signWithAuthKey(const byte_vector& cert, + const byte_vector& /* pin */, const byte_vector& hash) const { if (certType != CertificateType::AUTHENTICATION) { @@ -141,12 +142,13 @@ byte_vector MsCryptoApiElectronicID::signWithAuthKey(const byte_vector& /* pin * validateAuthHashLength(authSignatureAlgorithm(), name(), hash); - const auto signature = sign(hash, authSignatureAlgorithm().hashAlgorithm(), key, isRSA()); + const auto signature = sign(cert, hash, authSignatureAlgorithm().hashAlgorithm(), key, isRSA()); return signature.first; } ElectronicID::Signature -MsCryptoApiElectronicID::signWithSigningKey(const byte_vector& /* pin */, const byte_vector& hash, +MsCryptoApiElectronicID::signWithSigningKey(const byte_vector& cert, const byte_vector& /* pin */, + const byte_vector& hash, const HashAlgorithm hashAlgo) const { if (certType != CertificateType::SIGNING) { @@ -158,7 +160,7 @@ MsCryptoApiElectronicID::signWithSigningKey(const byte_vector& /* pin */, const validateSigningHash(*this, hashAlgo, hash); - return sign(hash, hashAlgo, key, isRSA()); + return sign(cert, hash, hashAlgo, key, isRSA()); } } // namespace electronic_id diff --git a/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp b/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp index ad7ebe1..76be90c 100644 --- a/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp +++ b/src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp @@ -86,7 +86,8 @@ class MsCryptoApiElectronicID : public ElectronicID return {uint8_t(PIN_RETRY_COUNT_PLACEHOLDER), PIN_RETRY_COUNT_PLACEHOLDER}; } - pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& pin, + pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& cert, + const pcsc_cpp::byte_vector& pin, const pcsc_cpp::byte_vector& hash) const override; const std::set& supportedSigningAlgorithms() const override @@ -104,7 +105,8 @@ class MsCryptoApiElectronicID : public ElectronicID return {uint8_t(PIN_RETRY_COUNT_PLACEHOLDER), PIN_RETRY_COUNT_PLACEHOLDER}; } - Signature signWithSigningKey(const pcsc_cpp::byte_vector& pin, + Signature signWithSigningKey(const pcsc_cpp::byte_vector& cert, + const pcsc_cpp::byte_vector& pin, const pcsc_cpp::byte_vector& hash, const HashAlgorithm hashAlgo) const override; diff --git a/src/electronic-ids/pcsc/PcscElectronicID.hpp b/src/electronic-ids/pcsc/PcscElectronicID.hpp index 07005c7..378edb4 100644 --- a/src/electronic-ids/pcsc/PcscElectronicID.hpp +++ b/src/electronic-ids/pcsc/PcscElectronicID.hpp @@ -25,6 +25,7 @@ #include "electronic-id/electronic-id.hpp" #include "../common.hpp" +#include "../x509.hpp" namespace electronic_id { @@ -41,23 +42,33 @@ class PcscElectronicID : public ElectronicID return getCertificateImpl(type); } - pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& pin, + pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& cert, + const pcsc_cpp::byte_vector& pin, const pcsc_cpp::byte_vector& hash) const override { validateAuthHashLength(authSignatureAlgorithm(), name(), hash); auto transactionGuard = card->beginTransaction(); - return signWithAuthKeyImpl(pin, hash); + auto signature = signWithAuthKeyImpl(pin, hash); + if (!verifyDigest(authSignatureAlgorithm(), cert, hash, signature)) { + THROW(SmartCardError, "Failed to validate given signature!"); + } + return signature; } - Signature signWithSigningKey(const pcsc_cpp::byte_vector& pin, + Signature signWithSigningKey(const pcsc_cpp::byte_vector& cert, + const pcsc_cpp::byte_vector& pin, const pcsc_cpp::byte_vector& hash, const HashAlgorithm hashAlgo) const override { validateSigningHash(*this, hashAlgo, hash); auto transactionGuard = card->beginTransaction(); - return signWithSigningKeyImpl(pin, hash, hashAlgo); + auto signature = signWithSigningKeyImpl(pin, hash, hashAlgo); + if (!verifyDigest(signature.second, cert, hash, signature.first)) { + THROW(SmartCardError, "Failed to validate given signature!"); + } + return signature; } PinRetriesRemainingAndMax signingPinRetriesLeft() const override diff --git a/src/electronic-ids/pkcs11/Pkcs11ElectronicID.cpp b/src/electronic-ids/pkcs11/Pkcs11ElectronicID.cpp index 7637dc5..7744fe4 100644 --- a/src/electronic-ids/pkcs11/Pkcs11ElectronicID.cpp +++ b/src/electronic-ids/pkcs11/Pkcs11ElectronicID.cpp @@ -232,7 +232,8 @@ ElectronicID::PinRetriesRemainingAndMax Pkcs11ElectronicID::authPinRetriesLeft() return {authToken.retry, module.retryMax}; } -pcsc_cpp::byte_vector Pkcs11ElectronicID::signWithAuthKey(const pcsc_cpp::byte_vector& pin, +pcsc_cpp::byte_vector Pkcs11ElectronicID::signWithAuthKey(const pcsc_cpp::byte_vector& cert, + const pcsc_cpp::byte_vector& pin, const pcsc_cpp::byte_vector& hash) const { try { @@ -242,6 +243,10 @@ pcsc_cpp::byte_vector Pkcs11ElectronicID::signWithAuthKey(const pcsc_cpp::byte_v manager->sign(authToken, hash, authSignatureAlgorithm().hashAlgorithm(), module.providesExternalPinDialog, reinterpret_cast(pin.data()), pin.size()); + if (!verifyDigest(authSignatureAlgorithm(), cert, hash, signature.first)) { + THROW(SmartCardError, "Failed to validate given signature!"); + } + return signature.first; } catch (const VerifyPinFailed& e) { // Catch and rethrow the VerifyPinFailed error with -1 to inform the caller of the special @@ -265,7 +270,8 @@ ElectronicID::PinRetriesRemainingAndMax Pkcs11ElectronicID::signingPinRetriesLef return {signingToken.retry, module.retryMax}; } -ElectronicID::Signature Pkcs11ElectronicID::signWithSigningKey(const pcsc_cpp::byte_vector& pin, +ElectronicID::Signature Pkcs11ElectronicID::signWithSigningKey(const pcsc_cpp::byte_vector& cert, + const pcsc_cpp::byte_vector& pin, const pcsc_cpp::byte_vector& hash, const HashAlgorithm hashAlgo) const { @@ -283,6 +289,10 @@ ElectronicID::Signature Pkcs11ElectronicID::signWithSigningKey(const pcsc_cpp::b + name()); } + if (!verifyDigest(signature.second, cert, hash, signature.first)) { + THROW(SmartCardError, "Failed to validate given signature!"); + } + return signature; } catch (const VerifyPinFailed& e) { // Same issue as in signWithAuthKey(). diff --git a/src/electronic-ids/pkcs11/Pkcs11ElectronicID.hpp b/src/electronic-ids/pkcs11/Pkcs11ElectronicID.hpp index 9c48f6c..968a527 100644 --- a/src/electronic-ids/pkcs11/Pkcs11ElectronicID.hpp +++ b/src/electronic-ids/pkcs11/Pkcs11ElectronicID.hpp @@ -73,7 +73,8 @@ class Pkcs11ElectronicID : public ElectronicID PinMinMaxLength authPinMinMaxLength() const override; PinRetriesRemainingAndMax authPinRetriesLeft() const override; - pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& pin, + pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& cert, + const pcsc_cpp::byte_vector& pin, const pcsc_cpp::byte_vector& hash) const override; const std::set& supportedSigningAlgorithms() const override @@ -83,7 +84,8 @@ class Pkcs11ElectronicID : public ElectronicID PinMinMaxLength signingPinMinMaxLength() const override; PinRetriesRemainingAndMax signingPinRetriesLeft() const override; - Signature signWithSigningKey(const pcsc_cpp::byte_vector& pin, + Signature signWithSigningKey(const pcsc_cpp::byte_vector& cert, + const pcsc_cpp::byte_vector& pin, const pcsc_cpp::byte_vector& hash, const HashAlgorithm hashAlgo) const override; diff --git a/src/electronic-ids/x509.hpp b/src/electronic-ids/x509.hpp index d0aa38a..31eaaa9 100644 --- a/src/electronic-ids/x509.hpp +++ b/src/electronic-ids/x509.hpp @@ -6,10 +6,24 @@ #include #include +#include namespace electronic_id { +inline auto toX509(const pcsc_cpp::byte_vector& cert) +{ + if (cert.empty()) { + throw std::logic_error("empty cert data"); + } + const unsigned char* certPtr = cert.data(); + auto x509 = SCOPE_GUARD(X509, d2i_X509(nullptr, &certPtr, long(cert.size()))); + if (!x509) { + throw std::runtime_error("Failed to create X509 object from certificate"); + } + return x509; +} + inline void* extension(X509* x509, int nid) { return X509_get_ext_d2i(x509, nid, nullptr, nullptr); @@ -18,8 +32,8 @@ inline void* extension(X509* x509, int nid) inline bool hasClientAuthExtendedKeyUsage(EXTENDED_KEY_USAGE* usage) { for (int i = 0; i < sk_ASN1_OBJECT_num(usage); ++i) { - ASN1_OBJECT* obj = sk_ASN1_OBJECT_value(usage, i); - if (OBJ_obj2nid(obj) == NID_client_auth) { + if (ASN1_OBJECT* obj = sk_ASN1_OBJECT_value(usage, i); + OBJ_obj2nid(obj) == NID_client_auth) { return true; } } @@ -28,11 +42,8 @@ inline bool hasClientAuthExtendedKeyUsage(EXTENDED_KEY_USAGE* usage) inline CertificateType certificateType(const pcsc_cpp::byte_vector& cert) { - const unsigned char* certPtr = cert.data(); - auto x509 = SCOPE_GUARD(X509, d2i_X509(nullptr, &certPtr, long(cert.size()))); - if (!x509) { - THROW(SmartCardChangeRequiredError, "Failed to create X509 object from certificate"); - } + auto x509 = toX509(cert); + auto keyUsage = SCOPE_GUARD(ASN1_BIT_STRING, extension(x509.get(), NID_key_usage)); if (!keyUsage) { return CertificateType::NONE; @@ -55,4 +66,91 @@ inline CertificateType certificateType(const pcsc_cpp::byte_vector& cert) return CertificateType::NONE; } +inline pcsc_cpp::byte_vector ECconcatToASN1(const pcsc_cpp::byte_vector& data) +{ + auto ecdsa = SCOPE_GUARD(ECDSA_SIG, ECDSA_SIG_new()); + if (ECDSA_SIG_set0(ecdsa.get(), BN_bin2bn(data.data(), int(data.size() / 2), nullptr), + BN_bin2bn(&data[data.size() / 2], int(data.size() / 2), nullptr)) + != 1) { + throw std::runtime_error("ECconcatToASN1: ECDSA_SIG_set0() failed"); + } + int size = i2d_ECDSA_SIG(ecdsa.get(), nullptr); + if (size < 1) { + throw std::runtime_error("ECconcatToASN1: i2d_ECDSA_SIG() failed"); + } + pcsc_cpp::byte_vector result(size_t(size), 0); + unsigned char* p = result.data(); + if (i2d_ECDSA_SIG(ecdsa.get(), &p) != size) { + throw std::runtime_error( + "ECconcatToASN1: i2d_ECDSA_SIG() result does not match expected size"); + } + return result; +} + +inline const EVP_MD* hashToMD(electronic_id::HashAlgorithm hashAlgo) +{ + switch (hashAlgo) { + case electronic_id::HashAlgorithm::SHA224: + return EVP_sha224(); + case electronic_id::HashAlgorithm::SHA256: + return EVP_sha256(); + case electronic_id::HashAlgorithm::SHA384: + return EVP_sha384(); + case electronic_id::HashAlgorithm::SHA512: + return EVP_sha512(); +#if OPENSSL_VERSION_NUMBER >= 0x10100000L + case electronic_id::HashAlgorithm::SHA3_224: + return EVP_sha3_224(); + case electronic_id::HashAlgorithm::SHA3_256: + return EVP_sha3_256(); + case electronic_id::HashAlgorithm::SHA3_384: + return EVP_sha3_384(); + case electronic_id::HashAlgorithm::SHA3_512: + return EVP_sha3_512(); +#endif + default: + throw std::logic_error("hashToMD: unknown hash algorithm"); + } +} + +template +inline bool verifyDigest(T signAlgo, const pcsc_cpp::byte_vector& der, + const pcsc_cpp::byte_vector& digest, + const pcsc_cpp::byte_vector& signature) +{ + if (digest.empty() || signature.empty()) { + throw std::logic_error("verify: digest or signature"); + } + auto cert = electronic_id::toX509(der); + EVP_PKEY* key = X509_get0_pubkey(cert.get()); + auto ctx = SCOPE_GUARD(EVP_PKEY_CTX, EVP_PKEY_CTX_new(key, nullptr)); + if (!ctx || EVP_PKEY_verify_init(ctx.get()) != 1) { + throw std::runtime_error("EVP CTX object creation failed"); + } + if (EVP_PKEY_base_id(key) == EVP_PKEY_EC) { + pcsc_cpp::byte_vector sig = ECconcatToASN1(signature); + return ctx && EVP_PKEY_verify_init(ctx.get()) == 1 + && EVP_PKEY_verify(ctx.get(), sig.data(), sig.size(), digest.data(), digest.size()) + == 1; + } + + bool isPSS = false; + if constexpr (std::is_same_v) { + isPSS = signAlgo.isRSAWithPSSPadding(); + } else { + isPSS = (signAlgo & electronic_id::SignatureAlgorithm::PS) > 0; + } + if (isPSS) { + EVP_PKEY_CTX_set_rsa_padding(ctx.get(), RSA_PKCS1_PSS_PADDING); + EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx.get(), RSA_PSS_SALTLEN_AUTO); + } else { + EVP_PKEY_CTX_set_rsa_padding(ctx.get(), RSA_PKCS1_PADDING); + } + + EVP_PKEY_CTX_set_signature_md(ctx.get(), hashToMD(signAlgo)); + return 1 + == EVP_PKEY_verify(ctx.get(), signature.data(), signature.size(), digest.data(), + digest.size()); +} + } // namespace electronic_id diff --git a/tests/common/verify.hpp b/tests/common/verify.hpp index 65164e9..20fba64 100644 --- a/tests/common/verify.hpp +++ b/tests/common/verify.hpp @@ -23,70 +23,17 @@ #pragma once #include "electronic-id/electronic-id.hpp" - -#include -#include - -#define SCOPE_GUARD_EX(TYPE, DATA, FREE) std::unique_ptr(DATA, FREE) -#define SCOPE_GUARD(TYPE, DATA) SCOPE_GUARD_EX(TYPE, DATA, TYPE##_free) - -inline pcsc_cpp::byte_vector ECconcatToASN1(const pcsc_cpp::byte_vector& data) -{ - auto ecdsa = SCOPE_GUARD(ECDSA_SIG, ECDSA_SIG_new()); - if (ECDSA_SIG_set0(ecdsa.get(), BN_bin2bn(data.data(), int(data.size() / 2), nullptr), - BN_bin2bn(&data[data.size() / 2], int(data.size() / 2), nullptr)) - != 1) { - throw std::runtime_error("ECconcatToASN1: ECDSA_SIG_set0() failed"); - } - int size = i2d_ECDSA_SIG(ecdsa.get(), nullptr); - if (size < 1) { - throw std::runtime_error("ECconcatToASN1: i2d_ECDSA_SIG() failed"); - } - pcsc_cpp::byte_vector result(size_t(size), 0); - unsigned char* p = result.data(); - if (i2d_ECDSA_SIG(ecdsa.get(), &p) != size) { - throw std::runtime_error( - "ECconcatToASN1: i2d_ECDSA_SIG() result does not match expected size"); - } - return result; -} - -inline const EVP_MD* hashToMD(electronic_id::HashAlgorithm hashAlgo) -{ - switch (hashAlgo) { - case electronic_id::HashAlgorithm::SHA224: - return EVP_sha224(); - case electronic_id::HashAlgorithm::SHA256: - return EVP_sha256(); - case electronic_id::HashAlgorithm::SHA384: - return EVP_sha384(); - case electronic_id::HashAlgorithm::SHA512: - return EVP_sha512(); -#if OPENSSL_VERSION_NUMBER >= 0x10100000L - case electronic_id::HashAlgorithm::SHA3_224: - return EVP_sha3_224(); - case electronic_id::HashAlgorithm::SHA3_256: - return EVP_sha3_256(); - case electronic_id::HashAlgorithm::SHA3_384: - return EVP_sha3_384(); - case electronic_id::HashAlgorithm::SHA3_512: - return EVP_sha3_512(); -#endif - default: - throw std::logic_error("hashToMD: unknown hash algorithm"); - } -} +#include "electronic-ids/x509.hpp" inline pcsc_cpp::byte_vector calculateDigest(electronic_id::HashAlgorithm hashAlgo, const pcsc_cpp::byte_vector& data) { - pcsc_cpp::byte_vector digest(size_t(EVP_MAX_MD_SIZE)); - const EVP_MD* md = hashToMD(hashAlgo); - unsigned int size = 0; - if (EVP_Digest(data.data(), data.size(), digest.data(), &size, md, nullptr) != 1) { + const EVP_MD* md = electronic_id::hashToMD(hashAlgo); + pcsc_cpp::byte_vector digest(size_t(EVP_MD_size(md))); + auto size = unsigned(digest.size()); + if (EVP_Digest(data.data(), data.size(), digest.data(), &size, md, nullptr) != 1 || digest.size() != size) { throw std::runtime_error("calculateDigest: EVP_Digest failed"); } - digest.resize(size); return digest; } @@ -95,22 +42,15 @@ inline bool verify(electronic_id::HashAlgorithm hashAlgo, const pcsc_cpp::byte_v bool isPSS) { if (der.empty() || data.empty() || signature.empty()) { - throw std::logic_error("verify: empty der, data or signature"); - } - const unsigned char* p = der.data(); - auto cert = SCOPE_GUARD(X509, d2i_X509(nullptr, &p, long(der.size()))); - if (!cert) { - throw std::runtime_error("verify: X509 object creation failed"); - } - auto key = SCOPE_GUARD(EVP_PKEY, X509_get_pubkey(cert.get())); - if (!key) { - throw std::runtime_error("verify: X509 public key object creation failed"); + throw std::logic_error("verify: data or signature"); } + auto cert = electronic_id::toX509(der); + EVP_PKEY *key = X509_get0_pubkey(cert.get()); pcsc_cpp::byte_vector sig = - EVP_PKEY_base_id(key.get()) == EVP_PKEY_EC ? ECconcatToASN1(signature) : signature; + EVP_PKEY_base_id(key) == EVP_PKEY_EC ? electronic_id::ECconcatToASN1(signature) : signature; auto ctx = SCOPE_GUARD(EVP_MD_CTX, EVP_MD_CTX_new()); EVP_PKEY_CTX* pkctx = nullptr; - if (EVP_DigestVerifyInit(ctx.get(), &pkctx, hashToMD(hashAlgo), nullptr, key.get()) != 1) { + if (EVP_DigestVerifyInit(ctx.get(), &pkctx, hashToMD(hashAlgo), nullptr, key) != 1) { ERR_print_errors_fp(stderr); throw std::runtime_error("verify: EVP_DigestVerifyInit() failed"); } diff --git a/tests/integration/test-authenticate.cpp b/tests/integration/test-authenticate.cpp index 7f16919..1ca7f33 100644 --- a/tests/integration/test-authenticate.cpp +++ b/tests/integration/test-authenticate.cpp @@ -59,13 +59,12 @@ TEST(electronic_id_test, authenticate) const byte_vector pin {'1', '2', '3', '4'}; - std::cout << "WARNING! Using hard-coded PIN " - << std::string(reinterpret_cast(pin.data()), pin.size()) << '\n'; + std::cout << "WARNING! Using hard-coded PIN " << std::string(pin.cbegin(), pin.cend()) << '\n'; const byte_vector dataToSign {'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!'}; const JsonWebSignatureAlgorithm hashAlgo = cardInfo->eid().authSignatureAlgorithm(); const byte_vector hash = calculateDigest(hashAlgo.hashAlgorithm(), dataToSign); - auto signature = cardInfo->eid().signWithAuthKey(pin, hash); + auto signature = cardInfo->eid().signWithAuthKey(cert, pin, hash); std::cout << "Authentication signature: " << signature << '\n'; diff --git a/tests/integration/test-signing.cpp b/tests/integration/test-signing.cpp index d013b08..d90aec2 100644 --- a/tests/integration/test-signing.cpp +++ b/tests/integration/test-signing.cpp @@ -65,7 +65,7 @@ static void signing(HashAlgorithm hashAlgo) const byte_vector dataToSign {'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!'}; const byte_vector hash = calculateDigest(hashAlgo, dataToSign); - auto signature = cardInfo->eid().signWithSigningKey(pin, hash, hashAlgo); + auto signature = cardInfo->eid().signWithSigningKey(cert, pin, hash, hashAlgo); std::cout << "Signing signature: " << signature.first << '\n'; diff --git a/tests/mock/test-get-certificate.cpp b/tests/mock/test-get-certificate.cpp index 816ad33..7187804 100644 --- a/tests/mock/test-get-certificate.cpp +++ b/tests/mock/test-get-certificate.cpp @@ -56,7 +56,7 @@ TEST(electronic_id_test, selectCertificateEstIDEMIA) const pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'}; const auto hash = calculateDigest(hashAlgo, dataToSign); - const auto authSignature = cardInfo->eid().signWithAuthKey(authPin, hash); + const auto authSignature = cardInfo->eid().signWithAuthKey(certificateAuth, authPin, hash); if (!verify(hashAlgo, certificateAuth, dataToSign, authSignature, false)) { throw std::runtime_error("Signature is invalid"); } @@ -71,7 +71,8 @@ TEST(electronic_id_test, selectCertificateEstIDEMIA) const pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5'}; EXPECT_EQ(cardInfo->eid().isSupportedSigningHashAlgorithm(hashAlgo), true); - const auto signSignature = cardInfo->eid().signWithSigningKey(signPin, hash, hashAlgo); + const auto signSignature = + cardInfo->eid().signWithSigningKey(certificateSign, signPin, hash, hashAlgo); EXPECT_EQ(signSignature.second, SignatureAlgorithm::ES384); if (!verify(hashAlgo, certificateSign, dataToSign, signSignature.first, false)) { throw std::runtime_error("Signature is invalid"); @@ -102,7 +103,7 @@ TEST(electronic_id_test, selectCertificateFinV3) const pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'}; const auto hash = calculateDigest(hashAlgo, dataToSign); - const auto authSignature = cardInfo->eid().signWithAuthKey(authPin, hash); + const auto authSignature = cardInfo->eid().signWithAuthKey(certificateAuth, authPin, hash); if (!verify(hashAlgo, certificateAuth, dataToSign, authSignature, true)) { throw std::runtime_error("Signature is invalid"); } @@ -117,7 +118,8 @@ TEST(electronic_id_test, selectCertificateFinV3) const pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'}; EXPECT_EQ(cardInfo->eid().isSupportedSigningHashAlgorithm(hashAlgo), true); - const auto signSignature = cardInfo->eid().signWithSigningKey(signPin, hash, hashAlgo); + const auto signSignature = + cardInfo->eid().signWithSigningKey(certificateSign, signPin, hash, hashAlgo); EXPECT_EQ(signSignature.second, SignatureAlgorithm::ES256); if (!verify(hashAlgo, certificateSign, dataToSign, signSignature.first, false)) { throw std::runtime_error("Signature is invalid"); @@ -148,7 +150,7 @@ TEST(electronic_id_test, selectCertificateFinV4) const pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'}; const auto hash = calculateDigest(hashAlgo, dataToSign); - const auto authSignature = cardInfo->eid().signWithAuthKey(authPin, hash); + const auto authSignature = cardInfo->eid().signWithAuthKey(certificateAuth, authPin, hash); if (!verify(hashAlgo, certificateAuth, dataToSign, authSignature, true)) { throw std::runtime_error("Signature is invalid"); } @@ -163,7 +165,8 @@ TEST(electronic_id_test, selectCertificateFinV4) const pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'}; EXPECT_EQ(cardInfo->eid().isSupportedSigningHashAlgorithm(hashAlgo), true); - const auto signSignature = cardInfo->eid().signWithSigningKey(signPin, hash, hashAlgo); + const auto signSignature = + cardInfo->eid().signWithSigningKey(certificateSign, signPin, hash, hashAlgo); EXPECT_EQ(signSignature.second, SignatureAlgorithm::ES384); if (!verify(hashAlgo, certificateSign, dataToSign, signSignature.first, false)) { throw std::runtime_error("Signature is invalid"); @@ -194,7 +197,7 @@ TEST(electronic_id_test, selectCertificateLat_V1) const pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'}; const auto hash = calculateDigest(hashAlgo, dataToSign); - const auto authSignature = cardInfo->eid().signWithAuthKey(authPin, hash); + const auto authSignature = cardInfo->eid().signWithAuthKey(certificateAuth, authPin, hash); if (!verify(hashAlgo, certificateAuth, dataToSign, authSignature, false)) { throw std::runtime_error("Signature is invalid"); } @@ -209,7 +212,8 @@ TEST(electronic_id_test, selectCertificateLat_V1) const pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'}; EXPECT_EQ(cardInfo->eid().isSupportedSigningHashAlgorithm(hashAlgo), true); - const auto signSignature = cardInfo->eid().signWithSigningKey(signPin, hash, hashAlgo); + const auto signSignature = + cardInfo->eid().signWithSigningKey(certificateSign, signPin, hash, hashAlgo); EXPECT_EQ(signSignature.second, SignatureAlgorithm::RS256); if (!verify(hashAlgo, certificateSign, dataToSign, signSignature.first, false)) { throw std::runtime_error("Signature is invalid"); @@ -240,7 +244,7 @@ TEST(electronic_id_test, selectCertificateLatV2) const pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'}; const auto hash = calculateDigest(hashAlgo, dataToSign); - const auto authSignature = cardInfo->eid().signWithAuthKey(authPin, hash); + const auto authSignature = cardInfo->eid().signWithAuthKey(certificateAuth, authPin, hash); if (!verify(hashAlgo, certificateAuth, dataToSign, authSignature, false)) { throw std::runtime_error("Signature is invalid"); } @@ -255,7 +259,8 @@ TEST(electronic_id_test, selectCertificateLatV2) const pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'}; EXPECT_EQ(cardInfo->eid().isSupportedSigningHashAlgorithm(hashAlgo), true); - const auto signSignature = cardInfo->eid().signWithSigningKey(signPin, hash, hashAlgo); + const auto signSignature = + cardInfo->eid().signWithSigningKey(certificateSign, signPin, hash, hashAlgo); EXPECT_EQ(signSignature.second, SignatureAlgorithm::RS256); if (!verify(hashAlgo, certificateSign, dataToSign, signSignature.first, false)) { throw std::runtime_error("Signature is invalid");