diff --git a/NEWS b/NEWS index ba8d0eee..ba35d9b8 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ * Version 5.2.0 (unreleased) * PIV Support for compressed certificates. + * OpenPGP: Use InvalidPinError for wrong PIN. * Version 5.1.1 (released 2023-04-27) ** Bugfix: PIV: string representation of SLOT caused infinite loop on Python <3.11. diff --git a/tests/device/test_openpgp.py b/tests/device/test_openpgp.py index 1354308f..80c77e58 100644 --- a/tests/device/test_openpgp.py +++ b/tests/device/test_openpgp.py @@ -1,7 +1,14 @@ from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives.asymmetric import ec, rsa, ed25519, x25519, padding from cryptography.hazmat.primitives import hashes -from yubikit.openpgp import OpenPgpSession, KEY_REF, RSA_SIZE, OID +from yubikit.openpgp import ( + OpenPgpSession, + KEY_REF, + RSA_SIZE, + OID, + KdfIterSaltedS2k, + KdfNone, +) from yubikit.management import CAPABILITY from yubikit.core.smartcard import ApduError from . import condition @@ -187,3 +194,34 @@ def test_generate_x25519(session): shared2 = session.decrypt(e_priv.public_key()) assert shared1 == shared2 + + +@condition.min_version(5, 2) +def test_kdf(session): + with pytest.raises(ApduError): + session.set_kdf(KdfIterSaltedS2k.create()) + + session.change_admin(DEFAULT_ADMIN_PIN, NON_DEFAULT_ADMIN_PIN) + session.verify_admin(NON_DEFAULT_ADMIN_PIN) + session.set_kdf(KdfIterSaltedS2k.create()) + session.verify_admin(DEFAULT_ADMIN_PIN) + session.verify_pin(DEFAULT_PIN) + + session.change_admin(DEFAULT_ADMIN_PIN, NON_DEFAULT_ADMIN_PIN) + session.change_pin(DEFAULT_PIN, NON_DEFAULT_PIN) + session.verify_pin(NON_DEFAULT_PIN) + + session.set_kdf(KdfNone()) + session.verify_admin(DEFAULT_ADMIN_PIN) + session.verify_pin(DEFAULT_PIN) + + +@condition.min_version(5, 2) +def test_attestation(session): + session.verify_admin(DEFAULT_ADMIN_PIN) + pub = session.generate_ec_key(KEY_REF.SIG, OID.SECP256R1) + + session.verify_pin(DEFAULT_PIN) + cert = session.attest_key(KEY_REF.SIG) + + assert cert.public_key() == pub diff --git a/tests/test_util.py b/tests/test_util.py index aa4fdbe3..9336a861 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -1,6 +1,7 @@ # vim: set fileencoding=utf-8 : -from yubikit.core import Tlv, bytes2int +from ykman import __version__ as version +from yubikit.core import Tlv, bytes2int, InvalidPinError from yubikit.core.otp import modhex_encode, modhex_decode from yubikit.management import FORM_FACTOR from ykman.util import is_pkcs12, is_pem, parse_private_key, parse_certificates @@ -11,6 +12,12 @@ import unittest +def test_invalid_pin_exception_value_error(): + # Fail if InvalidPinError still inherits ValueError in ykman 6.0 + if int(version.split(".")[0]) != 5: + assert not isinstance(InvalidPinError(3), ValueError) + + class TestUtilityFunctions(unittest.TestCase): def test_bytes2int(self): self.assertEqual(0x57, bytes2int(b"\x57")) diff --git a/yubikit/core/__init__.py b/yubikit/core/__init__.py index a8033edd..473af30d 100644 --- a/yubikit/core/__init__.py +++ b/yubikit/core/__init__.py @@ -218,6 +218,19 @@ class NotSupportedError(ValueError): """Attempting an action that is not supported on this YubiKey""" +class InvalidPinError(CommandError, ValueError): + """An incorrect PIN/PUK was used, with the number of attempts now remaining. + + WARNING: This exception currently inherits from ValueError for + backwards-compatibility reasons. This will no longer be the case with the next major + version of the library. + """ + + def __init__(self, attempts_remaining: int, message: Optional[str] = None): + super().__init__(message or f"Invalid PIN/PUK, {attempts_remaining} remaining") + self.attempts_remaining = attempts_remaining + + def require_version( my_version: Version, min_version: Tuple[int, int, int], message=None ): diff --git a/yubikit/openpgp.py b/yubikit/openpgp.py index f281ac19..833747c7 100644 --- a/yubikit/openpgp.py +++ b/yubikit/openpgp.py @@ -2,6 +2,7 @@ Tlv, Version, NotSupportedError, + InvalidPinError, require_version, int2bytes, bytes2int, @@ -1120,9 +1121,11 @@ def _verify(self, pw: PW, pin: str, mode: int = 0) -> None: pin_enc = self.get_kdf().process(pw, pin) try: self.protocol.send_apdu(0, INS.VERIFY, 0, pw + mode, pin_enc) - except ApduError: - attempts = self.get_pin_status().get_attempts(pw) - raise ValueError(f"Invalid PIN, {attempts} tries remaining.") + except ApduError as e: + if e.sw == SW.SECURITY_CONDITION_NOT_SATISFIED: + attempts = self.get_pin_status().get_attempts(pw) + raise InvalidPinError(attempts) + raise e def verify_pin(self, pin, extended: bool = False): """Verify the User PIN. @@ -1159,11 +1162,11 @@ def _change(self, pw: PW, pin: str, new_pin: str) -> None: kdf.process(pw, pin) + kdf.process(pw, new_pin), ) except ApduError as e: - if e.sw == SW.CONDITIONS_NOT_SATISFIED: - raise ValueError("Conditions of use not satisfied.") - else: - remaining = self.get_pin_status().get_attempts(pw) - raise ValueError(f"Invalid PIN, {remaining} tries remaining.") + if e.sw == SW.SECURITY_CONDITION_NOT_SATISFIED: + attempts = self.get_pin_status().get_attempts(pw) + raise InvalidPinError(attempts) + raise e + logger.info(f"New {pw.name} PIN set") def change_pin(self, pin: str, new_pin: str) -> None: @@ -1204,13 +1207,12 @@ def reset_pin(self, new_pin: str, reset_code: Optional[str] = None) -> None: try: self.protocol.send_apdu(0, INS.RESET_RETRY_COUNTER, p1, PW.USER, data) except ApduError as e: - if e.sw == SW.CONDITIONS_NOT_SATISFIED: - raise ValueError("Conditions of use not satisfied.") - else: - reset_remaining = self.get_pin_status().attempts_reset - raise ValueError( - f"Invalid Reset Code, {reset_remaining} tries remaining." + if e.sw == SW.SECURITY_CONDITION_NOT_SATISFIED and not reset_code: + attempts = self.get_pin_status().attempts_reset + raise InvalidPinError( + attempts, f"Invalid Reset Code, {attempts} remaining" ) + raise e logger.info("New User PIN has been set") def get_algorithm_attributes(self, key_ref: KEY_REF) -> AlgorithmAttributes: diff --git a/yubikit/piv.py b/yubikit/piv.py index 2d6a9fae..b5073c11 100755 --- a/yubikit/piv.py +++ b/yubikit/piv.py @@ -31,9 +31,9 @@ bytes2int, Version, Tlv, - CommandError, NotSupportedError, BadResponseError, + InvalidPinError, ) from .core.smartcard import ( SW, @@ -302,14 +302,6 @@ class TOUCH_POLICY(IntEnum): PUK_P2 = 0x81 -class InvalidPinError(CommandError): - def __init__(self, attempts_remaining): - super(InvalidPinError, self).__init__( - "Invalid PIN/PUK. Remaining attempts: %d" % attempts_remaining - ) - self.attempts_remaining = attempts_remaining - - def _pin_bytes(pin): pin = pin.encode() if len(pin) > PIN_LEN: