Skip to content

Commit 62e4f39

Browse files
metsmametsma
committed
Clear PIN from buffer on destruction
WE2-1055 Signed-off-by: Raul Metsma <[email protected]>
1 parent 2288a02 commit 62e4f39

File tree

3 files changed

+49
-27
lines changed

3 files changed

+49
-27
lines changed

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

Lines changed: 36 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
@@ -162,6 +165,8 @@ struct CommandApdu
162165
d.push_back(le);
163166
}
164167

168+
virtual ~CommandApdu() noexcept = default;
169+
165170
constexpr operator const byte_vector&() const { return d; }
166171

167172
/**
@@ -213,6 +218,37 @@ struct CommandApdu
213218
return {0x00, 0xb0, byte_type(pos >> 8), byte_type(pos), le};
214219
}
215220

221+
/**
222+
* A helper function to create a VERIFY command APDU.
223+
* The ISO 7816-4 Section 6.12 VERIFY command has the form:
224+
* CLA = 0x00
225+
* INS = 0x20
226+
* P1 = Only P1=’00’ is valid (other values are RFU)
227+
* P2 = Qualifier of the reference data
228+
* Lc and Data field = Empty or verification data
229+
* Le = Empty
230+
*/
231+
static PCSC_CPP_CONSTEXPR_VECTOR CommandApdu verify(byte_type p2, byte_vector&& pin,
232+
size_t paddingLength,
233+
pcsc_cpp::byte_type paddingChar)
234+
{
235+
if (!pin.empty() && pin.capacity() < paddingLength + 5) {
236+
throw std::invalid_argument(
237+
"PIN buffer does not have enough capacity to pad without reallocation");
238+
}
239+
if (pin.size() < paddingLength) {
240+
pin.insert(pin.end(), paddingLength - pin.size(), paddingChar);
241+
}
242+
struct VerifyApdu final : public CommandApdu
243+
{
244+
using CommandApdu::CommandApdu;
245+
PCSC_CPP_DISABLE_COPY(VerifyApdu);
246+
PCSC_CPP_DEFAULT_MOVE(VerifyApdu);
247+
constexpr ~VerifyApdu() noexcept final { std::fill(d.begin(), d.end(), byte_type(0)); }
248+
};
249+
return VerifyApdu {0x00, 0x20, 0x00, p2, std::move(pin)};
250+
}
251+
216252
byte_vector d;
217253
};
218254

‎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)