Clarify intent for static analysis

lib/libpcsc-cpp/README.md

@@ -11,10 +11,10 @@ reader and transmit an APDU:
 
     auto readers = listReaders();
     auto card = readers[0].connectToCard();
-    auto command = CommandApdu::fromBytes({0x2, 0x1, 0x3, 0x4});
+    auto command = CommandApdu(0x2, 0x1, 0x3, 0x4);
 
-    auto transactionGuard = card->beginTransaction();
-    auto response = card->transmit(command);
+    auto session = card.beginSession();
+    auto response = session.transmit(command);
 
 See more examples in [tests](tests).
 

lib/libpcsc-cpp/src/SmartCard.cpp

@@ -127,6 +127,11 @@ class CardImpl
         if (id_vendor == VENDOR_HID_GLOBAL
             && (id_product == OMNIKEY_3x21 || id_product == OMNIKEY_6121))
             return false;
+        // Debug/workaround override: forces host-side PIN entry instead of the secure
+        // PIN-pad keypad. Moving PIN entry to the host process means the PIN passes
+        // through host memory and is no longer isolated on the reader hardware. Use only
+        // for testing or when a reader falsely reports PIN-pad capability but does not
+        // implement it correctly.
         if (getenv("SMARTCARDPP_NOPINPAD"))
             return false;
         return feature(FEATURE_VERIFY_PIN_START) != features.cend()

src/electronic-ids/TLV.hpp

@@ -34,6 +34,12 @@ namespace electronic_id
  * The constructor parses the tag and length from the provided byte range,
  * then adjusts its iterators so that `begin` and `end` reference only the value bytes.
  * If the TLV is empty, `operator bool()` returns false.
+ *
+ * Tag encoding: the `tag` field stores the raw BER-TLV tag bytes packed into a uint32
+ * (first byte in the most-significant position), including continuation-bit bytes for
+ * multi-byte tags. This is NOT the logical BER-TLV tag number from ISO 7816. All
+ * hardcoded tag literals used with `find()` follow the same packed-byte convention,
+ * e.g. a two-byte tag 0xBF 0x81 0x00 is stored as 0xBF8100.
  */
 struct TLV
 {
@@ -105,7 +111,7 @@ struct TLV
     PCSC_CPP_CONSTEXPR_VECTOR TLV find(uint32_t find) const
     {
         TLV tlv = *this;
-        for (; tlv && tlv.tag != find; ++tlv) {}
+        for (; tlv && tlv.tag != find; ++tlv) { }
         // Return the found TLV or an empty one if not found
         return tlv;
     }

src/electronic-ids/common.cpp

@@ -37,10 +37,12 @@ const std::set<SignatureAlgorithm>& ELLIPTIC_CURVE_SIGNATURE_ALGOS()
 
 const std::set<SignatureAlgorithm>& RSA_SIGNATURE_ALGOS()
 {
+    // SHA3+RSA (RS3_*, PS3_*) omitted: no card implementation (PCSC or PKCS#11) supports it.
     const static std::set<SignatureAlgorithm> RS_ALGOS = {
-        SignatureAlgorithm::RS224,   SignatureAlgorithm::RS256,   SignatureAlgorithm::RS384,
-        SignatureAlgorithm::RS512,   SignatureAlgorithm::RS3_224, SignatureAlgorithm::RS3_256,
-        SignatureAlgorithm::RS3_384, SignatureAlgorithm::RS3_512,
+        SignatureAlgorithm::RS224,
+        SignatureAlgorithm::RS256,
+        SignatureAlgorithm::RS384,
+        SignatureAlgorithm::RS512,
     };
     return RS_ALGOS;
 }

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

@@ -76,23 +76,27 @@ ElectronicID::Signature MsCryptoApiElectronicID::sign(const byte_vector& hash,
 {
     bool isRSA = signatureAlgo != SignatureAlgorithm::ES;
     BCRYPT_PKCS1_PADDING_INFO padInfo {};
-    switch (hashAlgo) {
-    case HashAlgorithm::SHA224:
-        padInfo.pszAlgId = L"SHA224";
-        break;
-    case HashAlgorithm::SHA256:
-        padInfo.pszAlgId = NCRYPT_SHA256_ALGORITHM;
-        break;
-    case HashAlgorithm::SHA384:
-        padInfo.pszAlgId = NCRYPT_SHA384_ALGORITHM;
-        break;
-    case HashAlgorithm::SHA512:
-        padInfo.pszAlgId = NCRYPT_SHA512_ALGORITHM;
-        break;
-    default:
-        // FIXME: what about HashAlgorithm::SHA3_*?
-        THROW(ArgumentFatalError,
-              "Hash algorithm " + std::to_string(hashAlgo) + " is not supported");
+    if (isRSA) {
+        // EC signing passes raw hash bytes to NCryptSignHash without a padding descriptor,
+        // so any hash including SHA3 works. RSA requires a padding descriptor with a hash
+        // algorithm ID; SHA3+RSA is not supported by NCrypt (SHA3 is BCrypt-only).
+        switch (hashAlgo) {
+        case HashAlgorithm::SHA224:
+            padInfo.pszAlgId = L"SHA224";
+            break;
+        case HashAlgorithm::SHA256:
+            padInfo.pszAlgId = NCRYPT_SHA256_ALGORITHM;
+            break;
+        case HashAlgorithm::SHA384:
+            padInfo.pszAlgId = NCRYPT_SHA384_ALGORITHM;
+            break;
+        case HashAlgorithm::SHA512:
+            padInfo.pszAlgId = NCRYPT_SHA512_ALGORITHM;
+            break;
+        default:
+            THROW(ArgumentFatalError,
+                  "Hash algorithm " + std::to_string(hashAlgo) + " is not supported for RSA");
+        }
     }
 
     DWORD size = 0;
@@ -125,7 +129,7 @@ ElectronicID::Signature MsCryptoApiElectronicID::sign(const byte_vector& hash,
         THROW(MsCryptoApiError, "Signing failed with error: " + std::to_string(err));
     }
 
-    return {signature, {signatureAlgo, hashAlgo}};
+    return {std::move(signature), {signatureAlgo, hashAlgo}};
 }
 
 } // namespace electronic_id

src/electronic-ids/ms-cryptoapi/listMsCryptoApiElectronicIDs.cpp

@@ -29,6 +29,9 @@
 #include "../scope.hpp"
 #include "../x509.hpp"
 
+#include <array>
+#include <string_view>
+
 using namespace std::string_literals;
 
 namespace electronic_id
@@ -108,13 +111,15 @@ std::vector<ElectronicID::ptr> listMsCryptoApiElectronicIDs()
             continue; // TODO: log.
         }
 
-        std::wstring algo(5, 0);
-        err = NCryptGetProperty(key, NCRYPT_ALGORITHM_GROUP_PROPERTY, PBYTE(algo.data()),
-                                DWORD(algo.size() + 1) * 2, &size, 0);
+        // Fixed-size buffer is intentional: only "RSA" (3) and "ECDSA" (5) are supported.
+        // 6 wchars = 5 chars + null terminator, fits all supported algorithm group names.
+        std::array<wchar_t, 6> algoBuf {};
+        err = NCryptGetProperty(key, NCRYPT_ALGORITHM_GROUP_PROPERTY, PBYTE(algoBuf.data()),
+                                DWORD(algoBuf.size()) * 2, &size, 0);
         if (FAILED(err)) {
             continue; // TODO: log.
         }
-        algo.resize(size / 2 - 1);
+        std::wstring_view algo(algoBuf.data());
         if (algo != L"RSA" && !algo.starts_with(L"EC")) {
             // We only support RSA and ECC algorithms.
             continue; // TODO: log.

src/electronic-ids/pcsc/EIDIDEMIA.cpp

@@ -130,13 +130,17 @@ ElectronicID::Signature EIDIDEMIA::signWithSigningKeyImpl(const SmartCard::Sessi
               PIN_PADDING_CHAR);
     auto tmp = hash;
     if (isECC) {
-        // https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5.pdf 6.4
+        // FIPS 186-5 §6.4: when signing with an n-bit key, if the hash output is longer
+        // than n bits, take the leftmost n bits; if shorter, the hash integer value is
+        // used as-is (leading zeros are implicit). We normalise to 48 bytes to match the
+        // P-384 key used on these cards.
+        // https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5.pdf
         constexpr size_t ECDSA384_INPUT_LENGTH = 384 / 8;
         if (tmp.size() < ECDSA384_INPUT_LENGTH) {
-            // Zero-pad hashes that are shorter than SHA-384.
+            // Prepend zeros — mathematically equivalent to the original integer value.
             tmp.insert(tmp.cbegin(), ECDSA384_INPUT_LENGTH - tmp.size(), 0x00);
         } else if (tmp.size() > ECDSA384_INPUT_LENGTH) {
-            // Truncate hashes that are longer than SHA-384.
+            // Keep the leftmost 48 bytes per FIPS 186-5 §6.4.
             tmp.resize(ECDSA384_INPUT_LENGTH);
         }
     }

src/electronic-ids/pkcs11/PKCS11CardManager.hpp

@@ -80,8 +80,10 @@ class PKCS11CardManager
         static std::unordered_map<std::string, std::weak_ptr<PKCS11CardManager>> instances;
 
         // There is no std::hash for std::filesystem::path, use the string value.
-        // Note that two different path strings that refer to the same filesystem location
-        // will be treated as different keys (e.g. /path/to/module and /path/to/../to/module).
+        // Known limitation: two path strings that resolve to the same file (e.g.
+        // /path/to/module and /path/to/../to/module) are treated as different keys and
+        // would load the module twice. This is safe in practice because all paths come
+        // from hardcoded constants in Pkcs11ElectronicID.cpp and are never user-supplied.
         std::string moduleStr = module.string();
 
         std::lock_guard<std::mutex> lock(mutex);
@@ -191,6 +193,9 @@ class PKCS11CardManager
         // If the module provides an external PIN dialog, login is not required.
         if (!providesExternalPinDialog) {
             try {
+                // The PIN buffer is cleared by the caller after C_Login returns. Whether
+                // the PKCS#11 module retains a copy of the PIN internally after this call
+                // is outside our control and is the module vendor's responsibility.
                 C(Login, session, CKU_USER, CK_CHAR_PTR(pin), CK_ULONG(pinSize));
             } catch (const VerifyPinFailed& e) {
                 if (e.status() != VerifyPinFailed::Status::RETRY_ALLOWED)
@@ -247,6 +252,11 @@ class PKCS11CardManager
     }
 
 private:
+    // Trust model: the module loaded here is a third-party vendor library installed by the
+    // OS administrator or card middleware package. Its integrity is gated by OS filesystem
+    // permissions on the hardcoded paths — not by web-eid code-signing. Every vendor
+    // module loaded this way runs inside the web-eid process and has access to PIN material
+    // and signing operations. This is an explicit trust delegation to the middleware vendor.
     PKCS11CardManager(const std::filesystem::path& module)
     {
         CK_C_GetFunctionList C_GetFunctionList = nullptr;
@@ -364,9 +374,11 @@ class PKCS11CardManager
 
     static constexpr uint8_t pinRetryCount(CK_FLAGS flags) noexcept
     {
-        // As PKCS#11 does not provide an API for querying remaining PIN retries, we currently
-        // simply assume max retry count of 3, which is quite common. We might need to revisit this
-        // in the future once it becomes a problem.
+        // PKCS#11 does not expose the exact remaining retry count — only flag buckets
+        // (locked / final-try / count-low). We map these to 0/1/2 and assume a maximum
+        // of 3, which is the most common value. Cards with a higher retry budget (e.g.
+        // FinEID v3 allows 5) will show "3 attempts left" in the UI when the actual count
+        // is higher. This is a known PKCS#11 protocol limitation with no workaround.
         if (flags & CKF_USER_PIN_LOCKED) {
             return 0;
         }

src/electronic-ids/pkcs11/Pkcs11ElectronicID.cpp

@@ -224,6 +224,11 @@ Pkcs11ElectronicID::Pkcs11ElectronicID(ElectronicID::Type type) :
 {
     REQUIRE_NON_NULL(manager)
 
+    // Known limitation: if the PKCS#11 module exposes more than one authentication or
+    // signing token (e.g. multiple certificates on one card or multiple cards in multiple
+    // slots), only the last token of each type is used. Surfacing multiple tokens requires
+    // a larger refactor of the card-detection pipeline to return a vector of ElectronicIDs
+    // instead of a single one.
     for (const auto& token : manager->tokens()) {
         const auto certType = certificateType(token.cert);
         if (certType.isAuthentication()) {