Skip to content

Commit

Permalink
Verify given signature validity
Browse files Browse the repository at this point in the history
WE2-818

Signed-off-by: Raul Metsma <[email protected]>
  • Loading branch information
metsma committed Mar 5, 2024
1 parent a89a2b1 commit 34acc7a
Show file tree
Hide file tree
Showing 13 changed files with 197 additions and 114 deletions.
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions include/electronic-id/electronic-id.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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.
Expand All @@ -82,7 +83,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.
Expand Down
7 changes: 7 additions & 0 deletions include/electronic-id/enums.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ class JsonWebSignatureAlgorithm

operator std::string() const;

operator HashAlgorithm() const { return hashAlgorithm(); }

constexpr HashAlgorithm hashAlgorithm() const
{
switch (value) {
Expand All @@ -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:
Expand Down
28 changes: 15 additions & 13 deletions src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*/

#include "MsCryptoApiElectronicID.hpp"
#include "../scope.hpp"
#include "../x509.hpp"

#include <openssl/x509v3.h>
#include <openssl/err.h>
Expand All @@ -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");
Expand All @@ -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) {
Expand Down Expand Up @@ -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}};
}
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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
6 changes: 4 additions & 2 deletions src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<SignatureAlgorithm>& supportedSigningAlgorithms() const override
Expand All @@ -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;

Expand Down
19 changes: 15 additions & 4 deletions src/electronic-ids/pcsc/PcscElectronicID.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "electronic-id/electronic-id.hpp"

#include "../common.hpp"
#include "../x509.hpp"

namespace electronic_id
{
Expand All @@ -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
Expand Down
14 changes: 12 additions & 2 deletions src/electronic-ids/pkcs11/Pkcs11ElectronicID.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<const char*>(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
Expand All @@ -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
{
Expand All @@ -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().
Expand Down
6 changes: 4 additions & 2 deletions src/electronic-ids/pkcs11/Pkcs11ElectronicID.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<SignatureAlgorithm>& supportedSigningAlgorithms() const override
Expand All @@ -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;

Expand Down

0 comments on commit 34acc7a

Please sign in to comment.