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.

Sign up for GitHub

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

Jump to bottom

Take PIN ownership to minimze memory copy-s #79

Merged
merged 1 commit into from
Sep 20, 2024
Merged

Take PIN ownership to minimze memory copy-s #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions include/electronic-id/electronic-id.hpp
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ JsonWebSignatureAlgorithm MsCryptoApiElectronicID::authSignatureAlgorithm() cons
return getAuthAlgorithmFromCert(certData);
}

byte_vector MsCryptoApiElectronicID::signWithAuthKey(const byte_vector& /* pin */,
byte_vector MsCryptoApiElectronicID::signWithAuthKey(byte_vector&& /* pin */,
const byte_vector& hash) const
{
if (certType != CertificateType::AUTHENTICATION) {
Expand All @@ -56,7 +56,7 @@ const std::set<SignatureAlgorithm>& MsCryptoApiElectronicID::supportedSigningAlg
}

ElectronicID::Signature
MsCryptoApiElectronicID::signWithSigningKey(const byte_vector& /* pin */, const byte_vector& hash,
MsCryptoApiElectronicID::signWithSigningKey(byte_vector&& /* pin */, const byte_vector& hash,
const HashAlgorithm hashAlgo) const
{
if (certType != CertificateType::SIGNING) {
Expand Down
4 changes: 2 additions & 2 deletions src/electronic-ids/ms-cryptoapi/MsCryptoApiElectronicID.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class MsCryptoApiElectronicID : public ElectronicID
return {uint8_t(PIN_RETRY_COUNT_PLACEHOLDER), PIN_RETRY_COUNT_PLACEHOLDER};
}

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;

Expand All @@ -98,7 +98,7 @@ class MsCryptoApiElectronicID : public ElectronicID
return {uint8_t(PIN_RETRY_COUNT_PLACEHOLDER), PIN_RETRY_COUNT_PLACEHOLDER};
}

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

std::string name() const override
Expand Down
12 changes: 6 additions & 6 deletions src/electronic-ids/pcsc/EIDIDEMIA.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
13 changes: 6 additions & 7 deletions src/electronic-ids/pcsc/pcsc-common.hpp
View file
Open in desktop
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,
inline 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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading