Clear PIN from buffer on destruction

WE2-1055

Signed-off-by: Raul Metsma <raul@metsma.ee>

Author: metsma

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

@@ -127,6 +127,7 @@ struct ResponseApdu
  */
 struct CommandApdu
 {
+    static constexpr size_t APDU_HEADER_AND_LC_SIZE = 5;
     static constexpr size_t MAX_DATA_SIZE = 255;
 
     // ISO 7816 part 4, Annex B.1, Case 1
@@ -162,6 +163,12 @@ struct CommandApdu
         d.push_back(le);
     }
 
+    PCSC_CPP_CONSTEXPR_VECTOR void clear() && noexcept
+    {
+        std::fill(d.begin(), d.end(), byte_type(0));
+        d.clear();
+    }
+
     constexpr operator const byte_vector&() const { return d; }
 
     /**
@@ -213,6 +220,44 @@ struct CommandApdu
         return {0x00, 0xb0, byte_type(pos >> 8), byte_type(pos), le};
     }
 
+    /**
+     * A helper function to create a VERIFY command APDU.
+     * The ISO 7816-4 Section 6.12 VERIFY command has the form:
+     *   CLA = 0x00
+     *   INS = 0x20
+     *   P1  = Only P1=’00’ is valid (other values are RFU)
+     *   P2  = Qualifier of the reference data
+     *   Lc and Data field = Empty or verification data
+     *   Le  = Empty
+     */
+    static PCSC_CPP_CONSTEXPR_VECTOR CommandApdu verify(byte_type p2, byte_vector&& pin,
+                                                        size_t paddingLength,
+                                                        pcsc_cpp::byte_type paddingChar)
+    {
+        if (!pin.empty() && pin.capacity() < paddingLength + APDU_HEADER_AND_LC_SIZE) {
+            throw std::invalid_argument(
+                "PIN buffer does not have enough capacity to pad without reallocation");
+        }
+        if (pin.size() > paddingLength) {
+            throw std::invalid_argument("PIN length exceeds maximum length");
+        }
+        if (pin.size() < paddingLength) {
+            pin.insert(pin.end(), paddingLength - pin.size(), paddingChar);
+        }
+        return {0x00, 0x20, 0x00, p2, std::move(pin)};
+    }
+
+    /**
+     * A helper function to create a VERIFY command APDU with empty data field and only padding.
+     */
+    static PCSC_CPP_CONSTEXPR_VECTOR CommandApdu verify(byte_type p2, size_t paddingLength,
+                                                        pcsc_cpp::byte_type paddingChar)
+    {
+        byte_vector emptyPin;
+        emptyPin.reserve(paddingLength + APDU_HEADER_AND_LC_SIZE);
+        return verify(p2, std::move(emptyPin), paddingLength, paddingChar);
+    }
+
     byte_vector d;
 };
 

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

@@ -24,6 +24,7 @@
 
 #include "electronic-id/electronic-id.hpp"
 #include "../TLV.hpp"
+#include "../scope.hpp"
 
 #include "pcsc-cpp/pcsc-cpp-utils.hpp"
 
@@ -87,33 +88,21 @@ inline pcsc_cpp::byte_vector readFile(const pcsc_cpp::SmartCard::Session& sessio
     return readBinary(session, pcsc_cpp::toSW(*size.begin, *(size.begin + 1)), blockLength);
 }
 
-PCSC_CPP_CONSTEXPR_VECTOR inline pcsc_cpp::byte_vector
-addPaddingToPin(pcsc_cpp::byte_vector&& pin, size_t paddingLength, pcsc_cpp::byte_type paddingChar)
-{
-    if (pin.capacity() < paddingLength) {
-        THROW(ProgrammingError,
-              "PIN buffer does not have enough capacity to pad without reallocation");
-    }
-    if (pin.size() < paddingLength) {
-        pin.insert(pin.end(), paddingLength - pin.size(), paddingChar);
-    }
-    return std::move(pin);
-}
-
 inline void verifyPin(const pcsc_cpp::SmartCard::Session& session, pcsc_cpp::byte_type p2,
                       pcsc_cpp::byte_vector&& pin, ElectronicID::PinMinMaxLength pinMinMax,
                       pcsc_cpp::byte_type paddingChar)
 {
     pcsc_cpp::ResponseApdu response;
 
     if (session.readerHasPinPad()) {
-        const pcsc_cpp::CommandApdu verifyPin {
-            0x00, 0x20, 0x00, p2, pcsc_cpp::byte_vector(pinMinMax.second, paddingChar)};
-        response = session.transmitCTL(verifyPin, 0, pinMinMax.first);
+        response = session.transmitCTL(
+            pcsc_cpp::CommandApdu::verify(p2, pinMinMax.second, paddingChar), 0, pinMinMax.first);
     } else {
-        const pcsc_cpp::CommandApdu verifyPin {
-            0x00, 0x20, 0x00, p2, addPaddingToPin(std::move(pin), pinMinMax.second, paddingChar)};
-        response = session.transmit(verifyPin);
+        auto verifyCommand =
+            pcsc_cpp::CommandApdu::verify(p2, std::move(pin), pinMinMax.second, paddingChar);
+        auto clearPinCommand = stdext::make_scope_exit(
+            [&verifyCommand]() noexcept { std::move(verifyCommand).clear(); });
+        response = session.transmit(verifyCommand);
     }
 
     // NOTE: in case card-specific error handling logic is needed,

tests/mock/test-get-certificate.cpp

@@ -55,7 +55,7 @@ TEST(electronic_id_test, selectCertificateEstIDEMIA)
     const HashAlgorithm hashAlgo = authAlgo.hashAlgorithm();
 
     pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'};
-    authPin.reserve(12);
+    authPin.reserve(17);
 
     const auto hash = calculateDigest(hashAlgo, dataToSign);
     const auto authSignature = cardInfo->signWithAuthKey(std::move(authPin), hash);
@@ -72,7 +72,7 @@ TEST(electronic_id_test, selectCertificateEstIDEMIA)
     EXPECT_EQ(signingRetriesLeft.maxRetry, 3);
 
     pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5'};
-    signPin.reserve(12);
+    signPin.reserve(17);
 
     EXPECT_EQ(cardInfo->isSupportedSigningHashAlgorithm(hashAlgo), true);
     const auto signSignature = cardInfo->signWithSigningKey(std::move(signPin), hash, hashAlgo);
@@ -105,7 +105,7 @@ TEST(electronic_id_test, selectCertificateFinV3)
     const HashAlgorithm hashAlgo = authAlgo.hashAlgorithm();
 
     pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'};
-    authPin.reserve(12);
+    authPin.reserve(17);
 
     const auto hash = calculateDigest(hashAlgo, dataToSign);
     const auto authSignature = cardInfo->signWithAuthKey(std::move(authPin), hash);
@@ -122,7 +122,7 @@ TEST(electronic_id_test, selectCertificateFinV3)
     EXPECT_EQ(signingRetriesLeft.maxRetry, 5);
 
     pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'};
-    signPin.reserve(12);
+    signPin.reserve(17);
 
     EXPECT_EQ(cardInfo->isSupportedSigningHashAlgorithm(hashAlgo), true);
     const auto signSignature = cardInfo->signWithSigningKey(std::move(signPin), hash, hashAlgo);
@@ -155,7 +155,7 @@ TEST(electronic_id_test, selectCertificateFinV4)
     const HashAlgorithm hashAlgo = authAlgo.hashAlgorithm();
 
     pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'};
-    authPin.reserve(12);
+    authPin.reserve(17);
 
     const auto hash = calculateDigest(hashAlgo, dataToSign);
     const auto authSignature = cardInfo->signWithAuthKey(std::move(authPin), hash);
@@ -172,7 +172,7 @@ TEST(electronic_id_test, selectCertificateFinV4)
     EXPECT_EQ(signingRetriesLeft.maxRetry, 5);
 
     pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'};
-    signPin.reserve(12);
+    signPin.reserve(17);
 
     EXPECT_EQ(cardInfo->isSupportedSigningHashAlgorithm(hashAlgo), true);
     const auto signSignature = cardInfo->signWithSigningKey(std::move(signPin), hash, hashAlgo);
@@ -205,7 +205,7 @@ TEST(electronic_id_test, selectCertificateLatV2)
     const HashAlgorithm hashAlgo = authAlgo.hashAlgorithm();
 
     pcsc_cpp::byte_vector authPin {'1', '2', '3', '4'};
-    authPin.reserve(12);
+    authPin.reserve(17);
 
     const auto hash = calculateDigest(hashAlgo, dataToSign);
     const auto authSignature = cardInfo->signWithAuthKey(std::move(authPin), hash);
@@ -222,7 +222,7 @@ TEST(electronic_id_test, selectCertificateLatV2)
     EXPECT_EQ(signingRetriesLeft.maxRetry, 3);
 
     pcsc_cpp::byte_vector signPin {'1', '2', '3', '4', '5', '6'};
-    signPin.reserve(12);
+    signPin.reserve(17);
 
     EXPECT_EQ(cardInfo->isSupportedSigningHashAlgorithm(hashAlgo), true);
     const auto signSignature = cardInfo->signWithSigningKey(std::move(signPin), hash, hashAlgo);