Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read certificate file ref from DCOD file #121

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ jobs:
run: sudo apt update -qq && sudo apt install --no-install-recommends -y cmake libgtest-dev libpcsclite-dev

- name: Initialize CodeQL
uses: github/codeql-action/init@v2
uses: github/codeql-action/init@v3
with:
languages: cpp
queries: +security-and-quality

- name: Autobuild
uses: github/codeql-action/autobuild@v2
uses: github/codeql-action/autobuild@v3

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v2
uses: github/codeql-action/analyze@v3
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16.0)
cmake_minimum_required(VERSION 3.22)
if(POLICY CMP0092)
cmake_policy(SET CMP0092 NEW)
endif()
Expand All @@ -16,10 +16,10 @@ add_library(${PROJECT_NAME}
src/electronic-ids/common.hpp
src/electronic-ids/common.cpp
src/electronic-ids/scope.hpp
src/electronic-ids/TLV.hpp
src/electronic-ids/x509.hpp
src/electronic-ids/pcsc/EIDIDEMIA.cpp
src/electronic-ids/pcsc/EIDIDEMIA.hpp
src/electronic-ids/pcsc/EstEIDIDEMIA.cpp
src/electronic-ids/pcsc/EstEIDIDEMIA.hpp
src/electronic-ids/pcsc/FinEID.cpp
src/electronic-ids/pcsc/FinEID.hpp
Expand Down
2 changes: 1 addition & 1 deletion lib/libpcsc-cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.22)

project(pcsc-cpp)

Expand Down
2 changes: 1 addition & 1 deletion lib/libpcsc-cpp/tests/lib/libpcsc-mock/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.10.0)
cmake_minimum_required(VERSION 3.22)
if(POLICY CMP0092)
cmake_policy(SET CMP0092 NEW)
endif()
Expand Down
105 changes: 105 additions & 0 deletions src/electronic-ids/TLV.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright (c) 2025 Estonian Information System Authority
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/

#pragma once

#include "pcsc-cpp/pcsc-cpp.hpp"

namespace electronic_id
{

struct TLV
Copy link
Member

@mrts mrts Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a brief explanation that the constructor consumes the tag and length so that begin and end point to the value directly:

Suggested change
struct TLV
/**
* Represents a single Tag-Length-Value structure used in DER-encoded eID card files.
*
* 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.
*/
struct TLV

{
using byte_vector = pcsc_cpp::byte_vector;
uint16_t tag {};
uint32_t length {};
byte_vector::const_iterator begin;
byte_vector::const_iterator end;

PCSC_CPP_CONSTEXPR_VECTOR explicit TLV(const byte_vector& data) :
TLV(data.cbegin(), data.cend())
{
}

PCSC_CPP_CONSTEXPR_VECTOR TLV(byte_vector::const_iterator _begin,
byte_vector::const_iterator _end) : begin(_begin), end(_end)
{
if (!*this) {
return;
}

tag = *begin++;
if ((tag & 0x1F) == 0x1F) { // Multi-byte tag
if (!*this) {
throw std::invalid_argument("Invalid TLV: Unexpected end of tag");
Copy link
Member

@mrts mrts Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use THROW() to get context info into the log, also the error message could be a little more precise:

Suggested change
throw std::invalid_argument("Invalid TLV: Unexpected end of tag");
THROW(std::invalid_argument, "Invalid TLV: Multi-byte tag without second byte");

}
tag = (tag << 8) | (*begin++);
}

if (!*this) {
throw std::invalid_argument("Invalid TLV: Missing length field");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use THROW() here as well.

}

length = *begin++;
if (length & 0x80) { // Extended length encoding
auto num_bytes = uint8_t(length & 0x7F);
if (num_bytes == 0 || num_bytes > 4 || std::distance(begin, end) < num_bytes) {
throw std::invalid_argument("Invalid TLV: Incorrect extended length encoding");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use THROW() here as well.

}

length = 0;
for (uint8_t i = 0; i < num_bytes; ++i) {
length = (length << 8) | (*begin++);
}
}

if (std::distance(begin, end) < length) {
throw std::invalid_argument("Invalid TLV: Insufficient value data");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use THROW() here as well.

}
}

PCSC_CPP_CONSTEXPR_VECTOR TLV child() const { return {begin, begin + length}; }

PCSC_CPP_CONSTEXPR_VECTOR TLV& operator++() { return *this = {begin + length, end}; }

template <typename... Tags>
static PCSC_CPP_CONSTEXPR_VECTOR TLV path(TLV tlv, uint16_t tag, Tags... tags)
{
for (; tlv; ++tlv) {
if (tlv.tag == tag) {
if constexpr (sizeof...(tags) > 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our coding standard requires using curly braces with if:

Suggested change
if constexpr (sizeof...(tags) > 0)
if constexpr (sizeof...(tags) > 0) {

return path(tlv.child(), uint16_t(tags)...);
return tlv;
}
}
return TLV({});
}
template <typename... Tags>
static PCSC_CPP_CONSTEXPR_VECTOR TLV path(const byte_vector& data, uint16_t tag, Tags... tags)
{
return path(TLV(data), tag, tags...);
}

constexpr operator bool() const noexcept { return begin != end; }
};

} // namespace electronic_id
28 changes: 23 additions & 5 deletions src/electronic-ids/pcsc/EIDIDEMIA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ namespace
constexpr byte_type PIN_PADDING_CHAR = 0xFF;
constexpr byte_type AUTH_PIN_REFERENCE = 0x01;
constexpr byte_type SIGN_PIN_REFERENCE = 0x85;
constexpr byte_type DEFAULT_AUTH_KEY_ID = 0x81;
constexpr byte_type DEFAULT_SIGN_KEY_ID = 0x9F;

const auto MAIN_AID = CommandApdu::select(0x04,
{0xA0, 0x00, 0x00, 0x00, 0x77, 0x01, 0x08, 0x00, 0x07,
Expand All @@ -47,6 +49,11 @@ const auto SIGN_CERT = CommandApdu::select(0x09, {0xAD, 0xF2, 0x34, 0x1F});

} // namespace

void EIDIDEMIA::selectMain() const
{
transmitApduWithExpectedResponse(*card, MAIN_AID);
}

void EIDIDEMIA::selectADF1() const
{
transmitApduWithExpectedResponse(*card, ADF1_AID);
Expand All @@ -59,14 +66,20 @@ void EIDIDEMIA::selectADF2() const

byte_vector EIDIDEMIA::getCertificateImpl(const CertificateType type) const
{
transmitApduWithExpectedResponse(*card, MAIN_AID);
selectMain();
return electronic_id::getCertificate(*card, type.isAuthentication() ? AUTH_CERT : SIGN_CERT);
}

EIDIDEMIA::KeyInfo EIDIDEMIA::authKeyRef() const
{
return {DEFAULT_AUTH_KEY_ID, true};
}

byte_vector EIDIDEMIA::signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const
{
selectADF1();
selectAuthSecurityEnv();
auto [keyId, isECC] = authKeyRef();
selectSecurityEnv(*card, 0xA4, isECC ? 0x04 : 0x02, keyId, name());

verifyPin(*card, AUTH_PIN_REFERENCE, std::move(pin), authPinMinMaxLength().first,
authPinMinMaxLength().second, PIN_PADDING_CHAR);
Expand All @@ -80,18 +93,23 @@ byte_vector EIDIDEMIA::signWithAuthKeyImpl(byte_vector&& pin, const byte_vector&

ElectronicID::PinRetriesRemainingAndMax EIDIDEMIA::authPinRetriesLeftImpl() const
{
transmitApduWithExpectedResponse(*card, MAIN_AID);
selectMain();
return pinRetriesLeft(AUTH_PIN_REFERENCE);
}

EIDIDEMIA::KeyInfo EIDIDEMIA::signKeyRef() const
{
return {DEFAULT_SIGN_KEY_ID, true};
}

ElectronicID::Signature EIDIDEMIA::signWithSigningKeyImpl(byte_vector&& pin,
const byte_vector& hash,
const HashAlgorithm hashAlgo) const
{
selectADF2();
pcsc_cpp::byte_type algo = selectSignSecurityEnv();
auto [keyRef, isECC] = signKeyRef();
selectSecurityEnv(*card, 0xB6, isECC ? 0x54 : 0x42, keyRef, name());
auto tmp = hash;
bool isECC = algo == 0x54;
if (isECC) {
constexpr size_t ECDSA384_INPUT_LENGTH = 384 / 8;
if (tmp.size() < ECDSA384_INPUT_LENGTH) {
Expand Down
19 changes: 17 additions & 2 deletions src/electronic-ids/pcsc/EIDIDEMIA.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,37 @@ namespace electronic_id
class EIDIDEMIA : public PcscElectronicID
{
public:
struct KeyInfo
{
byte_type id;
bool isECC;
};

explicit EIDIDEMIA(pcsc_cpp::SmartCard::ptr _card) : PcscElectronicID(std::move(_card)) {}

protected:
byte_vector getCertificateImpl(const CertificateType type) const override;

PinRetriesRemainingAndMax authPinRetriesLeftImpl() const override;
virtual void selectAuthSecurityEnv() const = 0;
JsonWebSignatureAlgorithm authSignatureAlgorithm() const override
{
return JsonWebSignatureAlgorithm::ES384;
}
virtual KeyInfo authKeyRef() const;
byte_vector signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const override;

PinRetriesRemainingAndMax signingPinRetriesLeftImpl() const override;
virtual pcsc_cpp::byte_type selectSignSecurityEnv() const = 0;
const std::set<SignatureAlgorithm>& supportedSigningAlgorithms() const override
{
return ELLIPTIC_CURVE_SIGNATURE_ALGOS();
}
virtual KeyInfo signKeyRef() const;
Signature signWithSigningKeyImpl(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const override;

PinRetriesRemainingAndMax pinRetriesLeft(byte_type pinReference) const;

void selectMain() const;
void selectADF1() const;
void selectADF2() const;
};
Expand Down
45 changes: 0 additions & 45 deletions src/electronic-ids/pcsc/EstEIDIDEMIA.cpp

This file was deleted.

15 changes: 4 additions & 11 deletions src/electronic-ids/pcsc/EstEIDIDEMIA.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,22 @@

#include "EIDIDEMIA.hpp"

// ESTEID specification:
// https://installer.id.ee/media/id2019/TD-ID1-Chip-App.pdf

namespace electronic_id
{

class EstEIDIDEMIAV1 : public EIDIDEMIA
{
public:
explicit EstEIDIDEMIAV1(pcsc_cpp::SmartCard::ptr _card) : EIDIDEMIA(std::move(_card)) {}
using EIDIDEMIA::EIDIDEMIA;

private:
JsonWebSignatureAlgorithm authSignatureAlgorithm() const override
{
return JsonWebSignatureAlgorithm::ES384;
}
PinMinMaxLength authPinMinMaxLength() const override { return {4, 12}; }

const std::set<SignatureAlgorithm>& supportedSigningAlgorithms() const override;
PinMinMaxLength signingPinMinMaxLength() const override { return {5, 12}; }

std::string name() const override { return "EstEID IDEMIA v1"; }
Type type() const override { return EstEID; }

void selectAuthSecurityEnv() const override;
pcsc_cpp::byte_type selectSignSecurityEnv() const override;
};

} // namespace electronic_id
Loading