Skip to content

Commit fb3ea68

Browse files
metsmametsma
committed
Verify given signature validity
WE2-818 Signed-off-by: Raul Metsma <raul@metsma.ee>
1 parent d8b914f commit fb3ea68

13 files changed

Lines changed: 198 additions & 116 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ add_executable(${MOCK_TEST_EXE}
8585
tests/mock/test-pkcs11-token.cpp
8686
)
8787

88+
target_include_directories(${MOCK_TEST_EXE} PRIVATE src)
89+
8890
target_link_libraries(${MOCK_TEST_EXE}
8991
${PROJECT_NAME}
9092
pcsc-mock # this is available via libpcsc-cpp
@@ -105,6 +107,8 @@ add_executable(${INTEGRATION_TEST_EXE}
105107
tests/integration/test-signing.cpp
106108
)
107109

110+
target_include_directories(${INTEGRATION_TEST_EXE} PRIVATE src)
111+
108112
target_link_libraries(${INTEGRATION_TEST_EXE}
109113
${PROJECT_NAME}
110114
pcsc

‎include/electronic-id/electronic-id.hpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ class ElectronicID
6666

6767
virtual PinRetriesRemainingAndMax authPinRetriesLeft() const = 0;
6868

69-
virtual pcsc_cpp::byte_vector signWithAuthKey(const byte_vector& pin,
69+
virtual pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& cert,
70+
const byte_vector& pin,
7071
const byte_vector& hash) const = 0;
7172

7273
// Functions related to signing.
@@ -78,7 +79,8 @@ class ElectronicID
7879

7980
virtual PinRetriesRemainingAndMax signingPinRetriesLeft() const = 0;
8081

81-
virtual Signature signWithSigningKey(const byte_vector& pin, const byte_vector& hash,
82+
virtual Signature signWithSigningKey(const byte_vector& cert, const byte_vector& pin,
83+
const byte_vector& hash,
8284
const HashAlgorithm hashAlgo) const = 0;
8385

8486
// General functions.

‎include/electronic-id/enums.hpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ class JsonWebSignatureAlgorithm
197197

198198
operator std::string() const;
199199

200+
operator HashAlgorithm() const { return hashAlgorithm(); }
201+
200202
constexpr HashAlgorithm hashAlgorithm() const
201203
{
202204
switch (value) {
@@ -223,6 +225,11 @@ class JsonWebSignatureAlgorithm
223225
return value == RS256 || value == RS384 || value == RS512;
224226
}
225227

228+
constexpr bool isRSAWithPSSPadding()
229+
{
230+
return value == PS256 || value == PS384 || value == PS512;
231+
}
232+
226233
constexpr size_t hashByteLength() const { return hashAlgorithm().hashByteLength(); }
227234

228235
private:

‎src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.cpp‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
*/
2222

2323
#include "MsCryptoApiElectronicID.hpp"
24-
#include "../scope.hpp"
24+
#include "../x509.hpp"
2525

2626
#include <openssl/x509v3.h>
2727
#include <openssl/err.h>
@@ -35,12 +35,7 @@ using namespace electronic_id;
3535

3636
JsonWebSignatureAlgorithm getESAlgorithmFromCert(const byte_vector& cert)
3737
{
38-
const unsigned char* certPtr = cert.data();
39-
auto x509 = SCOPE_GUARD(X509, d2i_X509(nullptr, &certPtr, long(cert.size())));
40-
if (!x509) {
41-
THROW(MsCryptoApiError, "Failed to create X509 object from certificate");
42-
}
43-
38+
auto x509 = toX509(cert);
4439
EVP_PKEY* key = X509_get0_pubkey(x509.get());
4540
if (EVP_PKEY_base_id(key) != EVP_PKEY_EC) {
4641
THROW(MsCryptoApiError, "EVP_PKEY_base_id() reports non-EC key where EC key expected");
@@ -61,8 +56,9 @@ JsonWebSignatureAlgorithm getESAlgorithmFromCert(const byte_vector& cert)
6156
}
6257
}
6358

64-
ElectronicID::Signature sign(const byte_vector& hash, HashAlgorithm hashAlgo,
65-
const HCRYPTPROV_OR_NCRYPT_KEY_HANDLE key, const bool isRSA)
59+
ElectronicID::Signature sign(const byte_vector& cert, const byte_vector& hash,
60+
HashAlgorithm hashAlgo, const HCRYPTPROV_OR_NCRYPT_KEY_HANDLE key,
61+
const bool isRSA)
6662
{
6763
BCRYPT_PKCS1_PADDING_INFO padInfo {};
6864
switch (hashAlgo) {
@@ -114,6 +110,10 @@ ElectronicID::Signature sign(const byte_vector& hash, HashAlgorithm hashAlgo,
114110
THROW(MsCryptoApiError, "Signing failed with error: " + std::to_string(err));
115111
}
116112

113+
if (!verifyDigest(hashAlgo, cert, hash, signature)) {
114+
THROW(SmartCardError, "Failed to validate given signature!");
115+
}
116+
117117
return {signature,
118118
SignatureAlgorithm {isRSA ? SignatureAlgorithm::RS : SignatureAlgorithm::ES, hashAlgo}};
119119
}
@@ -129,7 +129,8 @@ JsonWebSignatureAlgorithm MsCryptoApiElectronicID::authSignatureAlgorithm() cons
129129
return isRSA() ? JsonWebSignatureAlgorithm::RS256 : getESAlgorithmFromCert(certData);
130130
}
131131

132-
byte_vector MsCryptoApiElectronicID::signWithAuthKey(const byte_vector& /* pin */,
132+
byte_vector MsCryptoApiElectronicID::signWithAuthKey(const byte_vector& cert,
133+
const byte_vector& /* pin */,
133134
const byte_vector& hash) const
134135
{
135136
if (certType != CertificateType::AUTHENTICATION) {
@@ -141,12 +142,13 @@ byte_vector MsCryptoApiElectronicID::signWithAuthKey(const byte_vector& /* pin *
141142

142143
validateAuthHashLength(authSignatureAlgorithm(), name(), hash);
143144

144-
const auto signature = sign(hash, authSignatureAlgorithm().hashAlgorithm(), key, isRSA());
145+
const auto signature = sign(cert, hash, authSignatureAlgorithm().hashAlgorithm(), key, isRSA());
145146
return signature.first;
146147
}
147148

148149
ElectronicID::Signature
149-
MsCryptoApiElectronicID::signWithSigningKey(const byte_vector& /* pin */, const byte_vector& hash,
150+
MsCryptoApiElectronicID::signWithSigningKey(const byte_vector& cert, const byte_vector& /* pin */,
151+
const byte_vector& hash,
150152
const HashAlgorithm hashAlgo) const
151153
{
152154
if (certType != CertificateType::SIGNING) {
@@ -158,7 +160,7 @@ MsCryptoApiElectronicID::signWithSigningKey(const byte_vector& /* pin */, const
158160

159161
validateSigningHash(*this, hashAlgo, hash);
160162

161-
return sign(hash, hashAlgo, key, isRSA());
163+
return sign(cert, hash, hashAlgo, key, isRSA());
162164
}
163165

164166
} // namespace electronic_id

‎src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ class MsCryptoApiElectronicID : public ElectronicID
8686
return {uint8_t(PIN_RETRY_COUNT_PLACEHOLDER), PIN_RETRY_COUNT_PLACEHOLDER};
8787
}
8888

89-
pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& pin,
89+
pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& cert,
90+
const pcsc_cpp::byte_vector& pin,
9091
const pcsc_cpp::byte_vector& hash) const override;
9192

9293
const std::set<SignatureAlgorithm>& supportedSigningAlgorithms() const override
@@ -104,7 +105,8 @@ class MsCryptoApiElectronicID : public ElectronicID
104105
return {uint8_t(PIN_RETRY_COUNT_PLACEHOLDER), PIN_RETRY_COUNT_PLACEHOLDER};
105106
}
106107

107-
Signature signWithSigningKey(const pcsc_cpp::byte_vector& pin,
108+
Signature signWithSigningKey(const pcsc_cpp::byte_vector& cert,
109+
const pcsc_cpp::byte_vector& pin,
108110
const pcsc_cpp::byte_vector& hash,
109111
const HashAlgorithm hashAlgo) const override;
110112

‎src/electronic-ids/pcsc/PcscElectronicID.hpp‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "electronic-id/electronic-id.hpp"
2626

2727
#include "../common.hpp"
28+
#include "../x509.hpp"
2829

2930
namespace electronic_id
3031
{
@@ -41,23 +42,33 @@ class PcscElectronicID : public ElectronicID
4142
return getCertificateImpl(type);
4243
}
4344

44-
pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& pin,
45+
pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& cert,
46+
const pcsc_cpp::byte_vector& pin,
4547
const pcsc_cpp::byte_vector& hash) const override
4648
{
4749
validateAuthHashLength(authSignatureAlgorithm(), name(), hash);
4850

4951
auto transactionGuard = card->beginTransaction();
50-
return signWithAuthKeyImpl(pin, hash);
52+
auto signature = signWithAuthKeyImpl(pin, hash);
53+
if (!verifyDigest(authSignatureAlgorithm(), cert, hash, signature)) {
54+
THROW(SmartCardError, "Failed to validate given signature!");
55+
}
56+
return signature;
5157
}
5258

53-
Signature signWithSigningKey(const pcsc_cpp::byte_vector& pin,
59+
Signature signWithSigningKey(const pcsc_cpp::byte_vector& cert,
60+
const pcsc_cpp::byte_vector& pin,
5461
const pcsc_cpp::byte_vector& hash,
5562
const HashAlgorithm hashAlgo) const override
5663
{
5764
validateSigningHash(*this, hashAlgo, hash);
5865

5966
auto transactionGuard = card->beginTransaction();
60-
return signWithSigningKeyImpl(pin, hash, hashAlgo);
67+
auto signature = signWithSigningKeyImpl(pin, hash, hashAlgo);
68+
if (!verifyDigest(signature.second, cert, hash, signature.first)) {
69+
THROW(SmartCardError, "Failed to validate given signature!");
70+
}
71+
return signature;
6172
}
6273

6374
PinRetriesRemainingAndMax signingPinRetriesLeft() const override

‎src/electronic-ids/pkcs11/Pkcs11ElectronicID.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,8 @@ ElectronicID::PinRetriesRemainingAndMax Pkcs11ElectronicID::authPinRetriesLeft()
232232
return {authToken.retry, module.retryMax};
233233
}
234234

235-
pcsc_cpp::byte_vector Pkcs11ElectronicID::signWithAuthKey(const pcsc_cpp::byte_vector& pin,
235+
pcsc_cpp::byte_vector Pkcs11ElectronicID::signWithAuthKey(const pcsc_cpp::byte_vector& cert,
236+
const pcsc_cpp::byte_vector& pin,
236237
const pcsc_cpp::byte_vector& hash) const
237238
{
238239
try {
@@ -242,6 +243,10 @@ pcsc_cpp::byte_vector Pkcs11ElectronicID::signWithAuthKey(const pcsc_cpp::byte_v
242243
manager->sign(authToken, hash, authSignatureAlgorithm().hashAlgorithm(),
243244
module.providesExternalPinDialog,
244245
reinterpret_cast<const char*>(pin.data()), pin.size());
246+
if (!verifyDigest(authSignatureAlgorithm(), cert, hash, signature.first)) {
247+
THROW(SmartCardError, "Failed to validate given signature!");
248+
}
249+
245250
return signature.first;
246251
} catch (const VerifyPinFailed& e) {
247252
// Catch and rethrow the VerifyPinFailed error with -1 to inform the caller of the special
@@ -265,7 +270,8 @@ ElectronicID::PinRetriesRemainingAndMax Pkcs11ElectronicID::signingPinRetriesLef
265270
return {signingToken.retry, module.retryMax};
266271
}
267272

268-
ElectronicID::Signature Pkcs11ElectronicID::signWithSigningKey(const pcsc_cpp::byte_vector& pin,
273+
ElectronicID::Signature Pkcs11ElectronicID::signWithSigningKey(const pcsc_cpp::byte_vector& cert,
274+
const pcsc_cpp::byte_vector& pin,
269275
const pcsc_cpp::byte_vector& hash,
270276
const HashAlgorithm hashAlgo) const
271277
{
@@ -283,6 +289,10 @@ ElectronicID::Signature Pkcs11ElectronicID::signWithSigningKey(const pcsc_cpp::b
283289
+ name());
284290
}
285291

292+
if (!verifyDigest(signature.second, cert, hash, signature.first)) {
293+
THROW(SmartCardError, "Failed to validate given signature!");
294+
}
295+
286296
return signature;
287297
} catch (const VerifyPinFailed& e) {
288298
// Same issue as in signWithAuthKey().

‎src/electronic-ids/pkcs11/Pkcs11ElectronicID.hpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ class Pkcs11ElectronicID : public ElectronicID
7373
PinMinMaxLength authPinMinMaxLength() const override;
7474

7575
PinRetriesRemainingAndMax authPinRetriesLeft() const override;
76-
pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& pin,
76+
pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& cert,
77+
const pcsc_cpp::byte_vector& pin,
7778
const pcsc_cpp::byte_vector& hash) const override;
7879

7980
const std::set<SignatureAlgorithm>& supportedSigningAlgorithms() const override
@@ -83,7 +84,8 @@ class Pkcs11ElectronicID : public ElectronicID
8384
PinMinMaxLength signingPinMinMaxLength() const override;
8485

8586
PinRetriesRemainingAndMax signingPinRetriesLeft() const override;
86-
Signature signWithSigningKey(const pcsc_cpp::byte_vector& pin,
87+
Signature signWithSigningKey(const pcsc_cpp::byte_vector& cert,
88+
const pcsc_cpp::byte_vector& pin,
8789
const pcsc_cpp::byte_vector& hash,
8890
const HashAlgorithm hashAlgo) const override;
8991

0 commit comments

Comments
 (0)