Skip to content

Commit 079c84d

Browse files
metsmametsma
committed
Clear PIN from buffer on destruction
WE2-1055 Signed-off-by: Raul Metsma <raul@metsma.ee>
1 parent affadb3 commit 079c84d

3 files changed

Lines changed: 55 additions & 27 deletions

File tree

‎lib/libpcsc-cpp/include/pcsc-cpp/pcsc-cpp.hpp‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
#include <vector>
3131

3232
// The rule of five (C++ Core guidelines C.21).
33+
#define PCSC_CPP_DEFAULT_MOVE(Class) \
34+
Class(Class&&) = default; \
35+
Class& operator=(Class&&) = default
3336
#define PCSC_CPP_DISABLE_COPY(Class) \
3437
Class(const Class&) = delete; \
3538
Class& operator=(const Class&) = delete
@@ -119,6 +122,7 @@ struct ResponseApdu
119122
}
120123
};
121124

125+
struct VerifyApdu;
122126
/**
123127
* Struct that wraps command APDUs.
124128
*
@@ -162,6 +166,8 @@ struct CommandApdu
162166
d.push_back(le);
163167
}
164168

169+
virtual ~CommandApdu() noexcept = default;
170+
165171
constexpr operator const byte_vector&() const { return d; }
166172

167173
/**
@@ -213,9 +219,45 @@ struct CommandApdu
213219
return {0x00, 0xb0, byte_type(pos >> 8), byte_type(pos), le};
214220
}
215221

222+
/**
223+
* A helper function to create a VERIFY command APDU.
224+
* The ISO 7816-4 Section 6.12 VERIFY command has the form:
225+
* CLA = 0x00
226+
* INS = 0x20
227+
* P1 = Only P1=’00’ is valid (other values are RFU)
228+
* P2 = Qualifier of the reference data
229+
* Lc and Data field = Empty or verification data
230+
* Le = Empty
231+
*/
232+
static PCSC_CPP_CONSTEXPR_VECTOR VerifyApdu verify(byte_type p2, byte_vector&& pin,
233+
size_t paddingLength,
234+
pcsc_cpp::byte_type paddingChar);
235+
216236
byte_vector d;
217237
};
218238

239+
struct VerifyApdu final : public CommandApdu
240+
{
241+
using CommandApdu::CommandApdu;
242+
PCSC_CPP_DISABLE_COPY(VerifyApdu);
243+
PCSC_CPP_DEFAULT_MOVE(VerifyApdu);
244+
constexpr ~VerifyApdu() noexcept final { std::fill(d.begin(), d.end(), byte_type(0)); }
245+
};
246+
247+
PCSC_CPP_CONSTEXPR_VECTOR VerifyApdu CommandApdu::verify(byte_type p2, byte_vector&& pin,
248+
size_t paddingLength,
249+
pcsc_cpp::byte_type paddingChar)
250+
{
251+
if (!pin.empty() && pin.capacity() < paddingLength + 5) {
252+
throw std::invalid_argument(
253+
"PIN buffer does not have enough capacity to pad without reallocation");
254+
}
255+
if (pin.size() < paddingLength) {
256+
pin.insert(pin.end(), paddingLength - pin.size(), paddingChar);
257+
}
258+
return {0x00, 0x20, 0x00, p2, std::move(pin)};
259+
}
260+
219261
/** Opaque class that wraps the PC/SC smart card resources like card handle and I/O protocol. */
220262
class CardImpl;
221263

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

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -87,33 +87,19 @@ inline pcsc_cpp::byte_vector readFile(const pcsc_cpp::SmartCard::Session& sessio
8787
return readBinary(session, pcsc_cpp::toSW(*size.begin, *(size.begin + 1)), blockLength);
8888
}
8989

90-
PCSC_CPP_CONSTEXPR_VECTOR inline pcsc_cpp::byte_vector
91-
addPaddingToPin(pcsc_cpp::byte_vector&& pin, size_t paddingLength, pcsc_cpp::byte_type paddingChar)
92-
{
93-
if (pin.capacity() < paddingLength) {
94-
THROW(ProgrammingError,
95-
"PIN buffer does not have enough capacity to pad without reallocation");
96-
}
97-
if (pin.size() < paddingLength) {
98-
pin.insert(pin.end(), paddingLength - pin.size(), paddingChar);
99-
}
100-
return std::move(pin);
101-
}
102-
10390
inline void verifyPin(const pcsc_cpp::SmartCard::Session& session, pcsc_cpp::byte_type p2,
10491
pcsc_cpp::byte_vector&& pin, ElectronicID::PinMinMaxLength pinMinMax,
10592
pcsc_cpp::byte_type paddingChar)
10693
{
10794
pcsc_cpp::ResponseApdu response;
10895

10996
if (session.readerHasPinPad()) {
110-
const pcsc_cpp::CommandApdu verifyPin {
111-
0x00, 0x20, 0x00, p2, pcsc_cpp::byte_vector(pinMinMax.second, paddingChar)};
112-
response = session.transmitCTL(verifyPin, 0, pinMinMax.first);
97+
response = session.transmitCTL(
98+
pcsc_cpp::CommandApdu::verify(p2, std::move(pin), pinMinMax.second, paddingChar), 0,
99+
pinMinMax.first);
113100
} else {
114-
const pcsc_cpp::CommandApdu verifyPin {
115-
0x00, 0x20, 0x00, p2, addPaddingToPin(std::move(pin), pinMinMax.second, paddingChar)};
116-
response = session.transmit(verifyPin);
101+
response = session.transmit(
102+
pcsc_cpp::CommandApdu::verify(p2, std::move(pin), pinMinMax.second, paddingChar));
117103
}
118104

119105
// NOTE: in case card-specific error handling logic is needed,

‎tests/mock/test-get-certificate.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ TEST(electronic_id_test, selectCertificateEstIDEMIA)
5555
const HashAlgorithm hashAlgo = authAlgo.hashAlgorithm();
5656

5757
pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'};
58-
authPin.reserve(12);
58+
authPin.reserve(17);
5959

6060
const auto hash = calculateDigest(hashAlgo, dataToSign);
6161
const auto authSignature = cardInfo->signWithAuthKey(std::move(authPin), hash);
@@ -72,7 +72,7 @@ TEST(electronic_id_test, selectCertificateEstIDEMIA)
7272
EXPECT_EQ(signingRetriesLeft.maxRetry, 3);
7373

7474
pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5'};
75-
signPin.reserve(12);
75+
signPin.reserve(17);
7676

7777
EXPECT_EQ(cardInfo->isSupportedSigningHashAlgorithm(hashAlgo), true);
7878
const auto signSignature = cardInfo->signWithSigningKey(std::move(signPin), hash, hashAlgo);
@@ -105,7 +105,7 @@ TEST(electronic_id_test, selectCertificateFinV3)
105105
const HashAlgorithm hashAlgo = authAlgo.hashAlgorithm();
106106

107107
pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'};
108-
authPin.reserve(12);
108+
authPin.reserve(17);
109109

110110
const auto hash = calculateDigest(hashAlgo, dataToSign);
111111
const auto authSignature = cardInfo->signWithAuthKey(std::move(authPin), hash);
@@ -122,7 +122,7 @@ TEST(electronic_id_test, selectCertificateFinV3)
122122
EXPECT_EQ(signingRetriesLeft.maxRetry, 5);
123123

124124
pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'};
125-
signPin.reserve(12);
125+
signPin.reserve(17);
126126

127127
EXPECT_EQ(cardInfo->isSupportedSigningHashAlgorithm(hashAlgo), true);
128128
const auto signSignature = cardInfo->signWithSigningKey(std::move(signPin), hash, hashAlgo);
@@ -155,7 +155,7 @@ TEST(electronic_id_test, selectCertificateFinV4)
155155
const HashAlgorithm hashAlgo = authAlgo.hashAlgorithm();
156156

157157
pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'};
158-
authPin.reserve(12);
158+
authPin.reserve(17);
159159

160160
const auto hash = calculateDigest(hashAlgo, dataToSign);
161161
const auto authSignature = cardInfo->signWithAuthKey(std::move(authPin), hash);
@@ -172,7 +172,7 @@ TEST(electronic_id_test, selectCertificateFinV4)
172172
EXPECT_EQ(signingRetriesLeft.maxRetry, 5);
173173

174174
pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'};
175-
signPin.reserve(12);
175+
signPin.reserve(17);
176176

177177
EXPECT_EQ(cardInfo->isSupportedSigningHashAlgorithm(hashAlgo), true);
178178
const auto signSignature = cardInfo->signWithSigningKey(std::move(signPin), hash, hashAlgo);
@@ -205,7 +205,7 @@ TEST(electronic_id_test, selectCertificateLatV2)
205205
const HashAlgorithm hashAlgo = authAlgo.hashAlgorithm();
206206

207207
pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'};
208-
authPin.reserve(12);
208+
authPin.reserve(17);
209209

210210
const auto hash = calculateDigest(hashAlgo, dataToSign);
211211
const auto authSignature = cardInfo->signWithAuthKey(std::move(authPin), hash);
@@ -222,7 +222,7 @@ TEST(electronic_id_test, selectCertificateLatV2)
222222
EXPECT_EQ(signingRetriesLeft.maxRetry, 3);
223223

224224
pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'};
225-
signPin.reserve(12);
225+
signPin.reserve(17);
226226

227227
EXPECT_EQ(cardInfo->isSupportedSigningHashAlgorithm(hashAlgo), true);
228228
const auto signSignature = cardInfo->signWithSigningKey(std::move(signPin), hash, hashAlgo);

0 commit comments

Comments
 (0)