Skip to content

Commit 82eacfc

Browse files
metsmametsma
committed
CommandAPDU reuse data object to avoid PIN copy
WE2-1007 Signed-off-by: Raul Metsma <[email protected]>
1 parent 261c0d0 commit 82eacfc

File tree

5 files changed

+39
-76
lines changed

5 files changed

+39
-76
lines changed

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

Lines changed: 21 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -142,52 +142,37 @@ struct ResponseApdu
142142
/** Struct that wraps command APDUs. */
143143
struct CommandApdu
144144
{
145-
byte_type cla;
146-
byte_type ins;
147-
byte_type p1;
148-
byte_type p2;
149-
unsigned short le;
150-
// Lc is data.size()
151-
byte_vector data;
145+
static constexpr size_t MAX_DATA_SIZE = 255;
152146

153-
static const size_t MAX_DATA_SIZE = 255;
154-
static const unsigned short LE_UNUSED = std::numeric_limits<unsigned short>::max();
147+
// Case 1
148+
CommandApdu(byte_type cls, byte_type ins, byte_type p1, byte_type p2) : d {cls, ins, p1, p2} {}
155149

156-
CommandApdu(byte_type c, byte_type i, byte_type pp1, byte_type pp2, byte_vector d = {},
157-
unsigned short l = LE_UNUSED) :
158-
cla(c), ins(i), p1(pp1), p2(pp2), le(l), data(std::move(d))
150+
// Case 2
151+
CommandApdu(byte_type cls, byte_type ins, byte_type p1, byte_type p2, byte_type le) :
152+
d {cls, ins, p1, p2, le}
159153
{
160154
}
161155

162-
CommandApdu(const CommandApdu& other, byte_vector d) :
163-
CommandApdu(other.cla, other.ins, other.p1, other.p2, std::move(d), other.le)
156+
// Case 3
157+
CommandApdu(byte_type cls, byte_type ins, byte_type p1, byte_type p2, byte_vector data) :
158+
d {std::move(data)}
164159
{
160+
if (d.size() > MAX_DATA_SIZE) {
161+
throw std::invalid_argument("Command chaining and extended lenght not supported");
162+
}
163+
d.insert(d.begin(), {cls, ins, p1, p2, static_cast<byte_type>(d.size())});
165164
}
166165

167-
constexpr bool isLeSet() const noexcept { return le != LE_UNUSED; }
168-
169-
byte_vector toBytes() const
166+
// Case 4
167+
CommandApdu(byte_type cls, byte_type ins, byte_type p1, byte_type p2, byte_vector data,
168+
byte_type le) : CommandApdu {cls, ins, p1, p2, std::move(data)}
170169
{
171-
if (data.size() > MAX_DATA_SIZE) {
172-
throw std::invalid_argument("Command chaining not supported");
173-
}
174-
175-
auto bytes = byte_vector {cla, ins, p1, p2};
176-
177-
if (!data.empty()) {
178-
bytes.push_back(static_cast<byte_type>(data.size()));
179-
bytes.insert(bytes.end(), data.cbegin(), data.cend());
180-
}
170+
d.push_back(le);
171+
}
181172

182-
if (isLeSet()) {
183-
// TODO: EstEID spec: the maximum value of Le is 0xFE
184-
if (le > ResponseApdu::MAX_DATA_SIZE)
185-
throw std::invalid_argument("LE larger than response size");
186-
bytes.push_back(static_cast<byte_type>(le));
187-
}
173+
operator const byte_vector&() const { return d; }
188174

189-
return bytes;
190-
}
175+
byte_vector d;
191176
};
192177

193178
/** Opaque class that wraps the PC/SC smart card resources like card handle and I/O protocol. */
@@ -295,7 +280,7 @@ void transmitApduWithExpectedResponse(const SmartCard& card, const CommandApdu&
295280
size_t readDataLengthFromAsn1(const SmartCard& card);
296281

297282
/** Read lenght bytes from currently selected binary file in blockLength-sized chunks. */
298-
byte_vector readBinary(const SmartCard& card, const size_t length, const size_t blockLength);
283+
byte_vector readBinary(const SmartCard& card, const size_t length, byte_type blockLength);
299284

300285
// Errors.
301286

‎lib/libpcsc-cpp/src/SmartCard.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ ResponseApdu SmartCard::transmit(const CommandApdu& command) const
317317
THROW(std::logic_error, "Call SmartCard::transmit() inside a transaction");
318318
}
319319

320-
return card->transmitBytes(command.toBytes());
320+
return card->transmitBytes(command);
321321
}
322322

323323
ResponseApdu SmartCard::transmitCTL(const CommandApdu& command, uint16_t lang, uint8_t minlen) const
@@ -327,7 +327,7 @@ ResponseApdu SmartCard::transmitCTL(const CommandApdu& command, uint16_t lang, u
327327
THROW(std::logic_error, "Call SmartCard::transmit() inside a transaction");
328328
}
329329

330-
return card->transmitBytesCTL(command.toBytes(), lang, minlen);
330+
return card->transmitBytesCTL(command, lang, minlen);
331331
}
332332

333333
} // namespace pcsc_cpp

‎lib/libpcsc-cpp/src/utils.cpp‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ class UnexpectedResponseError : public Error
5959
const ResponseApdu& response, const char* file, const int line,
6060
const char* callerFunctionName) :
6161
Error("transmitApduWithExpectedResponse(): Unexpected response to command '"s
62-
+ bytes2hexstr(command.toBytes()) + "' - expected '"s
63-
+ bytes2hexstr(expectedResponseBytes) + "', got '"s + bytes2hexstr(response.toBytes())
64-
+ " in " + removeAbsolutePathPrefix(file) + ':' + std::to_string(line) + ':'
62+
+ bytes2hexstr(command) + "' - expected '"s + bytes2hexstr(expectedResponseBytes)
63+
+ "', got '"s + bytes2hexstr(response.toBytes()) + " in "
64+
+ removeAbsolutePathPrefix(file) + ':' + std::to_string(line) + ':'
6565
+ callerFunctionName)
6666
{
6767
}
@@ -101,7 +101,7 @@ size_t readDataLengthFromAsn1(const SmartCard& card)
101101
// p1 - offset size first byte, 0
102102
// p2 - offset size second byte, 0
103103
// le - number of bytes to read, need 4 bytes from start for length
104-
const auto readBinary4Bytes = CommandApdu {0x00, 0xb0, 0x00, 0x00, byte_vector(), 0x04};
104+
const CommandApdu readBinary4Bytes {0x00, 0xb0, 0x00, 0x00, 0x04};
105105

106106
auto response = card.transmit(readBinary4Bytes);
107107

@@ -135,24 +135,19 @@ size_t readDataLengthFromAsn1(const SmartCard& card)
135135
return length;
136136
}
137137

138-
byte_vector readBinary(const SmartCard& card, const size_t length, const size_t blockLength)
138+
byte_vector readBinary(const SmartCard& card, const size_t length, byte_type blockLength)
139139
{
140-
size_t blockLengthVar = blockLength;
141140
auto lengthCounter = length;
142141
auto resultBytes = byte_vector {};
143-
auto readBinary = CommandApdu {0x00, 0xb0, 0x00, 0x00};
144142

145143
for (size_t offset = 0; lengthCounter != 0;
146-
offset += blockLengthVar, lengthCounter -= blockLengthVar) {
144+
offset += blockLength, lengthCounter -= blockLength) {
147145

148-
if (blockLengthVar > lengthCounter) {
149-
blockLengthVar = lengthCounter;
146+
if (blockLength > lengthCounter) {
147+
blockLength = byte_type(lengthCounter);
150148
}
151149

152-
readBinary.p1 = HIBYTE(offset);
153-
readBinary.p2 = LOBYTE(offset);
154-
readBinary.le = static_cast<byte_type>(blockLengthVar);
155-
150+
CommandApdu readBinary {0x00, 0xb0, HIBYTE(offset), LOBYTE(offset), blockLength};
156151
auto response = card.transmit(readBinary);
157152

158153
resultBytes.insert(resultBytes.end(), response.data.cbegin(), response.data.cend());

‎src/electronic-ids/pcsc/FinEID.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ byte_vector FinEIDv3::sign(const HashAlgorithm hashAlgo, const byte_vector& hash
143143
byte_vector tlv {0x90, byte_type(hash.size())};
144144
tlv.insert(tlv.cend(), hash.cbegin(), hash.cend());
145145

146-
const CommandApdu computeSignature {{0x00, 0x2A, 0x90, 0xA0}, std::move(tlv)};
146+
const CommandApdu computeSignature {0x00, 0x2A, 0x90, 0xA0, std::move(tlv)};
147147
const auto response = card->transmit(computeSignature);
148148

149149
if (response.sw1 == ResponseApdu::WRONG_LENGTH) {
@@ -156,7 +156,7 @@ byte_vector FinEIDv3::sign(const HashAlgorithm hashAlgo, const byte_vector& hash
156156
"Command COMPUTE SIGNATURE failed with error " + bytes2hexstr(response.toBytes()));
157157
}
158158

159-
const CommandApdu getSignature {0x00, 0x2A, 0x9E, 0x9A, {}, LE};
159+
const CommandApdu getSignature {0x00, 0x2A, 0x9E, 0x9A, LE};
160160
const auto signature = card->transmit(getSignature);
161161

162162
if (signature.sw1 == ResponseApdu::WRONG_LENGTH) {

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

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace electronic_id
3434
inline pcsc_cpp::byte_vector getCertificate(pcsc_cpp::SmartCard& card,
3535
const pcsc_cpp::CommandApdu& selectCertFileCmd)
3636
{
37-
static const size_t MAX_LE_VALUE = 0xb5;
37+
static const pcsc_cpp::byte_type MAX_LE_VALUE = 0xb5;
3838

3939
transmitApduWithExpectedResponse(card, selectCertFileCmd);
4040

@@ -54,17 +54,16 @@ inline void verifyPin(pcsc_cpp::SmartCard& card, pcsc_cpp::byte_type p2,
5454
pcsc_cpp::byte_vector&& pin, uint8_t pinMinLength, size_t paddingLength,
5555
pcsc_cpp::byte_type paddingChar)
5656
{
57-
const pcsc_cpp::CommandApdu VERIFY_PIN {0x00, 0x20, 0x00, p2};
5857
pcsc_cpp::ResponseApdu response;
5958

6059
if (card.readerHasPinPad()) {
61-
const pcsc_cpp::CommandApdu verifyPin {VERIFY_PIN,
60+
const pcsc_cpp::CommandApdu verifyPin {0x00, 0x20, 0x00, p2,
6261
addPaddingToPin({}, paddingLength, paddingChar)};
6362
response = card.transmitCTL(verifyPin, 0, pinMinLength);
6463

6564
} else {
6665
const pcsc_cpp::CommandApdu verifyPin {
67-
VERIFY_PIN, addPaddingToPin(std::move(pin), paddingLength, paddingChar)};
66+
0x00, 0x20, 0x00, p2, addPaddingToPin(std::move(pin), paddingLength, paddingChar)};
6867

6968
response = card.transmit(verifyPin);
7069
}
@@ -119,15 +118,7 @@ inline pcsc_cpp::byte_vector internalAuthenticate(pcsc_cpp::SmartCard& card,
119118
const pcsc_cpp::byte_vector& hash,
120119
const std::string& cardType)
121120
{
122-
static const pcsc_cpp::CommandApdu INTERNAL_AUTHENTICATE {0x00, 0x88, 0x00, 0x00};
123-
124-
pcsc_cpp::CommandApdu internalAuth {INTERNAL_AUTHENTICATE, hash};
125-
// LE is needed in case of protocol T1.
126-
// TODO: Implement this in libpcsc-cpp.
127-
if (card.protocol() == pcsc_cpp::SmartCard::Protocol::T1) {
128-
internalAuth.le = 0;
129-
}
130-
121+
pcsc_cpp::CommandApdu internalAuth {0x00, 0x88, 0x00, 0x00, hash, 0};
131122
const auto response = card.transmit(internalAuth);
132123

133124
if (response.sw1 == pcsc_cpp::ResponseApdu::WRONG_LENGTH) {
@@ -148,15 +139,7 @@ inline pcsc_cpp::byte_vector computeSignature(pcsc_cpp::SmartCard& card,
148139
const pcsc_cpp::byte_vector& hash,
149140
const std::string& cardType)
150141
{
151-
static const pcsc_cpp::CommandApdu COMPUTE_SIGNATURE {0x00, 0x2A, 0x9E, 0x9A};
152-
153-
pcsc_cpp::CommandApdu computeSignature {COMPUTE_SIGNATURE, hash};
154-
// LE is needed in case of protocol T1.
155-
// TODO: Implement this in libpcsc-cpp.
156-
if (card.protocol() == pcsc_cpp::SmartCard::Protocol::T1) {
157-
computeSignature.le = 0;
158-
}
159-
142+
pcsc_cpp::CommandApdu computeSignature {0x00, 0x2A, 0x9E, 0x9A, hash, 0};
160143
const auto response = card.transmit(computeSignature);
161144

162145
if (response.sw1 == pcsc_cpp::ResponseApdu::WRONG_LENGTH) {

0 commit comments

Comments
 (0)