Skip to content

Commit

Permalink
Take PIN ovnership to minimze memory copy-s
Browse files Browse the repository at this point in the history 
followup WE2-479

Signed-off-by: Raul Metsma <[email protected]>
  • Loading branch information
@metsma
metsma committed Aug 5, 2024
1 parent 7991e0e commit 14493a4
Show file tree
Hide file tree
Showing 14 changed files with 77 additions and 80 deletions.
4 changes: 2 additions & 2 deletions include/electronic-id/electronic-id.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class ElectronicID

virtual PinRetriesRemainingAndMax authPinRetriesLeft() const = 0;

virtual pcsc_cpp::byte_vector signWithAuthKey(const byte_vector& pin,
virtual pcsc_cpp::byte_vector signWithAuthKey(byte_vector&& pin,
const byte_vector& hash) const = 0;

// Functions related to signing.
Expand All @@ -77,7 +77,7 @@ class ElectronicID

virtual PinRetriesRemainingAndMax signingPinRetriesLeft() const = 0;

virtual Signature signWithSigningKey(const byte_vector& pin, const byte_vector& hash,
virtual Signature signWithSigningKey(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const = 0;

// General functions.
Expand Down
2 changes: 1 addition & 1 deletion lib/libpcsc-cpp/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ target_include_directories(${PROJECT_NAME}

target_compile_features(${PROJECT_NAME}
PUBLIC
cxx_std_17
cxx_std_20
)

target_compile_options(${PROJECT_NAME} PUBLIC
Expand Down
2 changes: 1 addition & 1 deletion lib/libpcsc-cpp/tests/lib/libpcsc-mock/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ target_include_directories(${PROJECT_NAME}

target_compile_features(${PROJECT_NAME}
PUBLIC
cxx_std_17
cxx_std_20
)

# PC/SC API dependencies.
Expand Down
12 changes: 6 additions & 6 deletions src/electronic-ids/pcsc/EIDIDEMIA.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ byte_vector EIDIDEMIA::getCertificateImpl(const CertificateType type) const
: selectCertificate().SIGN_CERT);
}

byte_vector EIDIDEMIA::signWithAuthKeyImpl(const byte_vector& pin, const byte_vector& hash) const
byte_vector EIDIDEMIA::signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const
{
// Select authentication application and authentication security environment.
transmitApduWithExpectedResponse(*card, selectApplicationID().MAIN_AID);
transmitApduWithExpectedResponse(*card, selectApplicationID().AUTH_AID);
selectAuthSecurityEnv();

verifyPin(*card, AUTH_PIN_REFERENCE, pin, authPinMinMaxLength().first, pinBlockLength(),
PIN_PADDING_CHAR);
verifyPin(*card, AUTH_PIN_REFERENCE, std::move(pin), authPinMinMaxLength().first,
pinBlockLength(), PIN_PADDING_CHAR);

return internalAuthenticate(*card,
authSignatureAlgorithm().isRSAWithPKCS1Padding()
Expand All @@ -69,7 +69,7 @@ ElectronicID::PinRetriesRemainingAndMax EIDIDEMIA::authPinRetriesLeftImpl() cons
return pinRetriesLeft(AUTH_PIN_REFERENCE);
}

ElectronicID::Signature EIDIDEMIA::signWithSigningKeyImpl(const byte_vector& pin,
ElectronicID::Signature EIDIDEMIA::signWithSigningKeyImpl(byte_vector&& pin,
const byte_vector& hash,
const HashAlgorithm hashAlgo) const
{
Expand All @@ -88,8 +88,8 @@ ElectronicID::Signature EIDIDEMIA::signWithSigningKeyImpl(const byte_vector& pin
}
}

verifyPin(*card, signingPinReference(), pin, signingPinMinMaxLength().first, pinBlockLength(),
PIN_PADDING_CHAR);
verifyPin(*card, signingPinReference(), std::move(pin), signingPinMinMaxLength().first,
pinBlockLength(), PIN_PADDING_CHAR);

return {useInternalAuthenticateAndRSAWithPKCS1PaddingDuringSigning()
? internalAuthenticate(*card, addRSAOID(hashAlgo, hash), name())
Expand Down
4 changes: 2 additions & 2 deletions src/electronic-ids/pcsc/EIDIDEMIA.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ class EIDIDEMIA : public PcscElectronicID
byte_vector getCertificateImpl(const CertificateType type) const override;

PinRetriesRemainingAndMax authPinRetriesLeftImpl() const override;
byte_vector signWithAuthKeyImpl(const byte_vector& pin, const byte_vector& hash) const override;
byte_vector signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const override;

PinRetriesRemainingAndMax signingPinRetriesLeftImpl() const override;
Signature signWithSigningKeyImpl(const byte_vector& pin, const byte_vector& hash,
Signature signWithSigningKeyImpl(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const override;

virtual const SelectApplicationIDCmds& selectApplicationID() const;
Expand Down
27 changes: 12 additions & 15 deletions src/electronic-ids/pcsc/FinEID.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ byte_vector FinEIDv3::getCertificateImpl(const CertificateType type) const
*card, type.isAuthentication() ? SELECT_AUTH_CERT_FILE : SELECT_SIGN_CERT_FILE_V3);
}

byte_vector FinEIDv3::signWithAuthKeyImpl(const byte_vector& pin, const byte_vector& hash) const
byte_vector FinEIDv3::signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const
{
return sign(authSignatureAlgorithm().hashAlgorithm(), hash, pin, AUTH_PIN_REFERENCE,
return sign(authSignatureAlgorithm().hashAlgorithm(), hash, std::move(pin), AUTH_PIN_REFERENCE,
authPinMinMaxLength(), AUTH_KEY_REFERENCE, RSA_PSS_ALGO, 0x00);
}

Expand All @@ -91,11 +91,10 @@ const std::set<SignatureAlgorithm>& FinEIDv3::supportedSigningAlgorithms() const
return ELLIPTIC_CURVE_SIGNATURE_ALGOS();
}

ElectronicID::Signature FinEIDv3::signWithSigningKeyImpl(const byte_vector& pin,
const byte_vector& hash,
ElectronicID::Signature FinEIDv3::signWithSigningKeyImpl(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const
{
return {sign(hashAlgo, hash, pin, SIGNING_PIN_REFERENCE, signingPinMinMaxLength(),
return {sign(hashAlgo, hash, std::move(pin), SIGNING_PIN_REFERENCE, signingPinMinMaxLength(),
SIGNING_KEY_REFERENCE_V3, ECDSA_ALGO, 0x40),
{SignatureAlgorithm::ES, hashAlgo}};
}
Expand All @@ -105,10 +104,9 @@ ElectronicID::PinRetriesRemainingAndMax FinEIDv3::signingPinRetriesLeftImpl() co
return pinRetriesLeft(SIGNING_PIN_REFERENCE);
}

byte_vector FinEIDv3::sign(const HashAlgorithm hashAlgo, const byte_vector& hash,
const byte_vector& pin, byte_type pinReference,
PinMinMaxLength pinMinMaxLength, byte_type keyReference,
byte_type signatureAlgo, byte_type LE) const
byte_vector FinEIDv3::sign(const HashAlgorithm hashAlgo, const byte_vector& hash, byte_vector&& pin,
byte_type pinReference, PinMinMaxLength pinMinMaxLength,
byte_type keyReference, byte_type signatureAlgo, byte_type LE) const
{
if (signatureAlgo != ECDSA_ALGO && hashAlgo.isSHA3()) {
THROW(ArgumentFatalError, "No OID for algorithm " + std::string(hashAlgo));
Expand Down Expand Up @@ -137,7 +135,7 @@ byte_vector FinEIDv3::sign(const HashAlgorithm hashAlgo, const byte_vector& hash

transmitApduWithExpectedResponse(*card, SELECT_MASTER_FILE);

verifyPin(*card, pinReference, pin, pinMinMaxLength.first, pinMinMaxLength.second,
verifyPin(*card, pinReference, std::move(pin), pinMinMaxLength.first, pinMinMaxLength.second,
PIN_PADDING_CHAR);
// Select security environment for COMPUTE SIGNATURE.
selectSecurityEnv(*card, 0xB6, signatureAlgo, keyReference, name());
Expand Down Expand Up @@ -198,17 +196,16 @@ byte_vector FinEIDv4::getCertificateImpl(const CertificateType type) const
*card, type.isAuthentication() ? SELECT_AUTH_CERT_FILE : SELECT_SIGN_CERT_FILE_V4);
}

byte_vector FinEIDv4::signWithAuthKeyImpl(const byte_vector& pin, const byte_vector& hash) const
byte_vector FinEIDv4::signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const
{
return sign(authSignatureAlgorithm().hashAlgorithm(), hash, pin, AUTH_PIN_REFERENCE,
return sign(authSignatureAlgorithm().hashAlgorithm(), hash, std::move(pin), AUTH_PIN_REFERENCE,
authPinMinMaxLength(), AUTH_KEY_REFERENCE, ECDSA_ALGO, 0x60);
}

ElectronicID::Signature FinEIDv4::signWithSigningKeyImpl(const byte_vector& pin,
const byte_vector& hash,
ElectronicID::Signature FinEIDv4::signWithSigningKeyImpl(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const
{
return {sign(hashAlgo, hash, pin, SIGNING_PIN_REFERENCE, signingPinMinMaxLength(),
return {sign(hashAlgo, hash, std::move(pin), SIGNING_PIN_REFERENCE, signingPinMinMaxLength(),
SIGNING_KEY_REFERENCE_V4, ECDSA_ALGO, 0x60),
{SignatureAlgorithm::ES, hashAlgo}};
}
Expand Down
10 changes: 5 additions & 5 deletions src/electronic-ids/pcsc/FinEID.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ class FinEIDv3 : public PcscElectronicID
std::string name() const override { return "FinEID v3"; }
Type type() const override { return FinEID; }

byte_vector signWithAuthKeyImpl(const byte_vector& pin, const byte_vector& hash) const override;
byte_vector signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const override;

Signature signWithSigningKeyImpl(const byte_vector& pin, const byte_vector& hash,
Signature signWithSigningKeyImpl(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const override;

byte_vector sign(const HashAlgorithm hashAlgo, const byte_vector& hash, const byte_vector& pin,
byte_vector sign(const HashAlgorithm hashAlgo, const byte_vector& hash, byte_vector&& pin,
byte_type pinReference, PinMinMaxLength pinMinMaxLength,
byte_type keyReference, byte_type signatureAlgo, byte_type LE) const;

Expand All @@ -76,9 +76,9 @@ class FinEIDv4 : public FinEIDv3

std::string name() const override { return "FinEID v4"; }

byte_vector signWithAuthKeyImpl(const byte_vector& pin, const byte_vector& hash) const override;
byte_vector signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const override;

Signature signWithSigningKeyImpl(const byte_vector& pin, const byte_vector& hash,
Signature signWithSigningKeyImpl(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const override;
};

Expand Down
20 changes: 8 additions & 12 deletions src/electronic-ids/pcsc/PcscElectronicID.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,27 @@ class PcscElectronicID : public ElectronicID
PcscElectronicID(pcsc_cpp::SmartCard::ptr _card) : ElectronicID(std::move(_card)) {}

protected:
pcsc_cpp::byte_vector getCertificate(const CertificateType type) const override
byte_vector getCertificate(const CertificateType type) const override
{
auto transactionGuard = card->beginTransaction();
return getCertificateImpl(type);
}

pcsc_cpp::byte_vector signWithAuthKey(const pcsc_cpp::byte_vector& pin,
const pcsc_cpp::byte_vector& hash) const override
byte_vector signWithAuthKey(byte_vector&& pin, const byte_vector& hash) const override
{
validateAuthHashLength(authSignatureAlgorithm(), name(), hash);

auto transactionGuard = card->beginTransaction();
return signWithAuthKeyImpl(pin, hash);
return signWithAuthKeyImpl(std::move(pin), hash);
}

Signature signWithSigningKey(const pcsc_cpp::byte_vector& pin,
const pcsc_cpp::byte_vector& hash,
Signature signWithSigningKey(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const override
{
validateSigningHash(*this, hashAlgo, hash);

auto transactionGuard = card->beginTransaction();
return signWithSigningKeyImpl(pin, hash, hashAlgo);
return signWithSigningKeyImpl(std::move(pin), hash, hashAlgo);
}

PinRetriesRemainingAndMax signingPinRetriesLeft() const override
Expand All @@ -77,15 +75,13 @@ class PcscElectronicID : public ElectronicID
// they have to be implemented when adding a new electronic ID.
// This design follows the non-virtual interface pattern.

virtual pcsc_cpp::byte_vector getCertificateImpl(const CertificateType type) const = 0;
virtual byte_vector getCertificateImpl(const CertificateType type) const = 0;

virtual pcsc_cpp::byte_vector signWithAuthKeyImpl(const pcsc_cpp::byte_vector& pin,
const pcsc_cpp::byte_vector& hash) const = 0;
virtual byte_vector signWithAuthKeyImpl(byte_vector&& pin, const byte_vector& hash) const = 0;

virtual PinRetriesRemainingAndMax authPinRetriesLeftImpl() const = 0;

virtual Signature signWithSigningKeyImpl(const pcsc_cpp::byte_vector& pin,
const pcsc_cpp::byte_vector& hash,
virtual Signature signWithSigningKeyImpl(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const = 0;

virtual PinRetriesRemainingAndMax signingPinRetriesLeftImpl() const = 0;
Expand Down
15 changes: 7 additions & 8 deletions src/electronic-ids/pcsc/pcsc-common.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,15 @@ inline pcsc_cpp::byte_vector getCertificate(pcsc_cpp::SmartCard& card,
return readBinary(card, length, MAX_LE_VALUE);
}

inline pcsc_cpp::byte_vector addPaddingToPin(const pcsc_cpp::byte_vector& pin, size_t paddingLength,
pcsc_cpp::byte_type paddingChar)
constexpr pcsc_cpp::byte_vector addPaddingToPin(pcsc_cpp::byte_vector&& pin, size_t paddingLength,
pcsc_cpp::byte_type paddingChar)
{
auto paddedPin = pin;
paddedPin.resize(std::max(pin.size(), paddingLength), paddingChar);
return paddedPin;
pin.resize(std::max(pin.size(), paddingLength), paddingChar);
return pin;
}

inline void verifyPin(pcsc_cpp::SmartCard& card, pcsc_cpp::byte_type p2,
const pcsc_cpp::byte_vector& pin, uint8_t pinMinLength, size_t paddingLength,
pcsc_cpp::byte_vector&& pin, uint8_t pinMinLength, size_t paddingLength,
pcsc_cpp::byte_type paddingChar)
{
const pcsc_cpp::CommandApdu VERIFY_PIN {0x00, 0x20, 0x00, p2};
Expand All @@ -64,8 +63,8 @@ inline void verifyPin(pcsc_cpp::SmartCard& card, pcsc_cpp::byte_type p2,
response = card.transmitCTL(verifyPin, 0, pinMinLength);

} else {
const pcsc_cpp::CommandApdu verifyPin {VERIFY_PIN,
addPaddingToPin(pin, paddingLength, paddingChar)};
const pcsc_cpp::CommandApdu verifyPin {
VERIFY_PIN, addPaddingToPin(std::move(pin), paddingLength, paddingChar)};

response = card.transmit(verifyPin);
}
Expand Down
4 changes: 2 additions & 2 deletions src/electronic-ids/pkcs11/Pkcs11ElectronicID.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ ElectronicID::PinRetriesRemainingAndMax Pkcs11ElectronicID::authPinRetriesLeft()
return {authToken.retry, module.retryMax};
}

pcsc_cpp::byte_vector Pkcs11ElectronicID::signWithAuthKey(const byte_vector& pin,
pcsc_cpp::byte_vector Pkcs11ElectronicID::signWithAuthKey(byte_vector&& pin,
const byte_vector& hash) const
{
REQUIRE_NON_NULL(manager)
Expand Down Expand Up @@ -254,7 +254,7 @@ ElectronicID::PinRetriesRemainingAndMax Pkcs11ElectronicID::signingPinRetriesLef
return {signingToken.retry, module.retryMax};
}

ElectronicID::Signature Pkcs11ElectronicID::signWithSigningKey(const byte_vector& pin,
ElectronicID::Signature Pkcs11ElectronicID::signWithSigningKey(byte_vector&& pin,
const byte_vector& hash,
const HashAlgorithm hashAlgo) const
{
Expand Down
4 changes: 2 additions & 2 deletions src/electronic-ids/pkcs11/Pkcs11ElectronicID.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ class Pkcs11ElectronicID : public ElectronicID
PinMinMaxLength authPinMinMaxLength() const override;

PinRetriesRemainingAndMax authPinRetriesLeft() const override;
byte_vector signWithAuthKey(const byte_vector& pin, const byte_vector& hash) const override;
byte_vector signWithAuthKey(byte_vector&& pin, const byte_vector& hash) const override;

const std::set<SignatureAlgorithm>& supportedSigningAlgorithms() const override;
PinMinMaxLength signingPinMinMaxLength() const override;

PinRetriesRemainingAndMax signingPinRetriesLeft() const override;
Signature signWithSigningKey(const byte_vector& pin, const byte_vector& hash,
Signature signWithSigningKey(byte_vector&& pin, const byte_vector& hash,
const HashAlgorithm hashAlgo) const override;

void release() const override;
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/test-authenticate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ TEST(electronic_id_test, authenticate)

GTEST_ASSERT_GE(cardInfo->eid().authPinRetriesLeft().first, 0U);

const byte_vector pin {'1', '2', '3', '4'};
byte_vector pin {'1', '2', '3', '4'};

std::cout << "WARNING! Using hard-coded PIN "
<< std::string(reinterpret_cast<const char*>(pin.data()), pin.size()) << '\n';
<< std::string_view(reinterpret_cast<const char*>(pin.data()), pin.size()) << '\n';

const byte_vector dataToSign {'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!'};
const JsonWebSignatureAlgorithm hashAlgo = cardInfo->eid().authSignatureAlgorithm();
const byte_vector hash = calculateDigest(hashAlgo.hashAlgorithm(), dataToSign);
auto signature = cardInfo->eid().signWithAuthKey(pin, hash);
auto signature = cardInfo->eid().signWithAuthKey(std::move(pin), hash);

std::cout << "Authentication signature: " << signature << '\n';

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test-signing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static void signing(HashAlgorithm hashAlgo)
const byte_vector dataToSign {'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!'};
const byte_vector hash = calculateDigest(hashAlgo, dataToSign);

auto signature = cardInfo->eid().signWithSigningKey(pin, hash, hashAlgo);
auto signature = cardInfo->eid().signWithSigningKey(std::move(pin), hash, hashAlgo);

std::cout << "Signing signature: " << signature.first << '\n';

Expand Down
Loading

0 comments on commit 14493a4

Please sign in to comment.