From 92af831ea6faa6d19819e44d3d15f0a50a905c1d Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Fri, 30 Aug 2024 11:09:59 +0200 Subject: [PATCH 1/2] Make version handling for dev keys more explicit --- tests/device/conftest.py | 16 ++++++---------- ykman/_cli/__main__.py | 27 ++++++++++++++++++++++++++- ykman/_cli/oath.py | 5 +++-- ykman/_cli/util.py | 10 +++++++++- yubikit/core/__init__.py | 26 ++++++++++++++++++++++++-- yubikit/hsmauth.py | 4 +++- yubikit/piv.py | 34 +++++++++++++--------------------- yubikit/support.py | 6 ++++-- 8 files changed, 88 insertions(+), 40 deletions(-) diff --git a/tests/device/conftest.py b/tests/device/conftest.py index b10cb60a..9a8b318e 100644 --- a/tests/device/conftest.py +++ b/tests/device/conftest.py @@ -1,7 +1,7 @@ from ykman.device import list_all_devices, read_info from ykman.pcsc import list_devices from ykman._cli.util import find_scp11_params -from yubikit.core import TRANSPORT, Version +from yubikit.core import TRANSPORT, Version, _override_version from yubikit.core.otp import OtpConnection from yubikit.core.fido import FidoConnection from yubikit.core.smartcard import SmartCardConnection @@ -24,18 +24,12 @@ def _device(pytestconfig): else: pytest.skip("No serial specified for device tests") + version = None version_str = pytestconfig.getoption("use_version") if version_str: version = Version.from_string(version_str) - - # Monkey patch all parsing of Version to use the supplied value - # N.B. There are some instances where ideally we would replace the version, - # but we don't really care - def get_version(cls, data): - return version - - Version.from_bytes = classmethod(get_version) - Version.from_string = classmethod(get_version) + _override_version(version) + os.environ["_YK_OVERRIDE_VERSION"] = version_str reader = pytestconfig.getoption("reader") if reader: @@ -52,6 +46,8 @@ def get_version(cls, data): dev, info = devices[0] if info.serial != serial: pytest.exit("Device serial does not match: %d != %r" % (serial, info.serial)) + if version: + info.version = version return dev, info diff --git a/ykman/_cli/__main__.py b/ykman/_cli/__main__.py index 0ef721f0..a65a044a 100644 --- a/ykman/_cli/__main__.py +++ b/ykman/_cli/__main__.py @@ -25,7 +25,7 @@ # ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. -from yubikit.core import ApplicationNotAvailableError +from yubikit.core import ApplicationNotAvailableError, Version, _override_version from yubikit.core.otp import OtpConnection from yubikit.core.fido import FidoConnection from yubikit.core.smartcard import SmartCardConnection @@ -82,6 +82,7 @@ import time import sys import re +import os import logging @@ -89,6 +90,10 @@ logger = logging.getLogger(__name__) +# Development key builds are treated as having the following version +_OVERRIDE_VERSION = Version.from_string(os.environ.get("_YK_OVERRIDE_VERSION", "5.7.2")) + + CLICK_CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"], max_content_width=999) @@ -400,6 +405,26 @@ def resolve(): items = require_reader(connections, reader) else: items = require_device(connections, device) + + if items[1].version.major == 0: + logger.info( + "Debug key detected, " + f"overriding version with {_OVERRIDE_VERSION}" + ) + # Preview build, override version and get new DeviceInfo + _override_version(_OVERRIDE_VERSION) + for c in connections: + if items[0].supports_connection(c): + try: + with items[0].open_connection(c) as conn: + info = read_info(conn, items[0].pid) + items = (items[0], info) + except Exception: + logger.debug("Failed", exc_info=True) + continue + break + else: + raise CliFail("Failed to connect to YubiKey.") setattr(resolve, "items", items) return items diff --git a/ykman/_cli/oath.py b/ykman/_cli/oath.py index 818c3c57..ffb69f0e 100644 --- a/ykman/_cli/oath.py +++ b/ykman/_cli/oath.py @@ -48,6 +48,7 @@ prompt_timeout, EnumChoice, is_yk4_fips, + check_version, pretty_print, get_scp_params, ) @@ -569,14 +570,14 @@ def _add_cred(ctx, data, touch, force): if len(data.secret) < 2: raise CliFail("Secret must be at least 2 bytes.") - if touch and version < (4, 2, 6): + if touch and not check_version(version, (4, 2, 6)): raise CliFail("Require touch is not supported on this YubiKey.") if data.counter and data.oath_type != OATH_TYPE.HOTP: raise CliFail("Counter only supported for HOTP accounts.") if data.hash_algorithm == HASH_ALGORITHM.SHA512 and ( - version < (4, 3, 1) or is_yk4_fips(ctx.obj["info"]) + not check_version(version, (4, 3, 1)) or is_yk4_fips(ctx.obj["info"]) ): raise CliFail("Algorithm SHA512 not supported on this YubiKey.") diff --git a/ykman/_cli/util.py b/ykman/_cli/util.py index ea307713..77852156 100644 --- a/ykman/_cli/util.py +++ b/ykman/_cli/util.py @@ -26,7 +26,7 @@ # POSSIBILITY OF SUCH DAMAGE. from ..util import parse_certificates -from yubikit.core import TRANSPORT +from yubikit.core import TRANSPORT, Version, require_version, NotSupportedError from yubikit.core.smartcard import SmartCardConnection, ApduError from yubikit.core.smartcard.scp import ScpKid, KeyRef, ScpKeyParams, Scp11KeyParams from yubikit.management import DeviceInfo, CAPABILITY @@ -328,6 +328,14 @@ def pretty_print(value, level: int = 0) -> Sequence[str]: return lines +def check_version(version: Version, req: Tuple[int, int, int]) -> bool: + try: + require_version(version, req) + return True + except NotSupportedError: + return False + + def is_yk4_fips(info: DeviceInfo) -> bool: return info.version[0] == 4 and info.is_fips diff --git a/yubikit/core/__init__.py b/yubikit/core/__init__.py index a2117933..da9ff12b 100644 --- a/yubikit/core/__init__.py +++ b/yubikit/core/__init__.py @@ -41,6 +41,10 @@ ) import re import abc +import logging + + +logger = logging.getLogger(__name__) _VERSION_STRING_PATTERN = re.compile(r"\b(?P\d+).(?P\d).(?P\d)\b") @@ -234,12 +238,30 @@ def __init__(self, attempts_remaining: int, message: Optional[str] = None): self.attempts_remaining = attempts_remaining +class _OverrideVersion: + def __init__(self): + self._version: Optional[Version] = None + + def __call__(self, value): + logger.info("Overriding version check for development devices with {version}") + self._version = value + + +# Set this to override a version with major version == 0 in version checks +_override_version = _OverrideVersion() + + def require_version( my_version: Version, min_version: Tuple[int, int, int], message=None ): """Ensure a version is at least min_version.""" - # Skip version checks for major == 0, used for development builds. - if my_version < min_version and my_version[0] != 0: + # Allow overriding version checks for development devices + v = my_version[0] == 0 and _override_version._version + if v: + logger.debug("Overriding version check with {v}") + my_version = v + + if my_version < min_version: if not message: message = "This action requires YubiKey %d.%d.%d or later" % min_version raise NotSupportedError(message) diff --git a/yubikit/hsmauth.py b/yubikit/hsmauth.py index d101a498..9a430d39 100644 --- a/yubikit/hsmauth.py +++ b/yubikit/hsmauth.py @@ -630,7 +630,9 @@ def get_challenge( data: bytes = Tlv(TAG_LABEL, _parse_label(label)) - if credential_password is not None and self.version >= (5, 7, 1): + if credential_password is not None and ( + self.version >= (5, 7, 1) or self.version[0] == 0 + ): data += Tlv( TAG_CREDENTIAL_PASSWORD, _parse_credential_password(credential_password) ) diff --git a/yubikit/piv.py b/yubikit/piv.py index 243025bf..4fef71d9 100755 --- a/yubikit/piv.py +++ b/yubikit/piv.py @@ -26,7 +26,7 @@ # POSSIBILITY OF SUCH DAMAGE. from .core import ( - require_version as _require_version, + require_version, int2bytes, bytes2int, Version, @@ -94,13 +94,6 @@ class ALGORITHM(str, Enum): RSA = "rsa" -# Don't treat pre 1.0 versions as "developer builds". -def require_version(my_version: Version, *args, **kwargs): - if my_version <= (0, 1, 4): # Last pre 1.0 release of ykneo-piv - my_version = Version(1, 0, 0) - _require_version(my_version, *args, **kwargs) - - @unique class KEY_TYPE(IntEnum): RSA1024 = 0x06 @@ -599,17 +592,16 @@ def _do_check_key_support( generate: bool = True, fips_restrictions: bool = False, ) -> None: - if version[0] == 0 and version > (0, 1, 3): - return # Development build, skip version checks - - if version < (4, 0, 0): - if key_type == KEY_TYPE.ECCP384: - raise NotSupportedError("ECCP384 requires YubiKey 4 or later") - if touch_policy != TOUCH_POLICY.DEFAULT or pin_policy != PIN_POLICY.DEFAULT: - raise NotSupportedError("PIN/Touch policy requires YubiKey 4 or later") - - if version < (4, 3, 0) and touch_policy == TOUCH_POLICY.CACHED: - raise NotSupportedError("Cached touch policy requires YubiKey 4.3 or later") + if key_type == KEY_TYPE.ECCP384: + require_version(version, (4, 0, 0), "ECCP384 requires YubiKey 4 or later") + if touch_policy != TOUCH_POLICY.DEFAULT or pin_policy != PIN_POLICY.DEFAULT: + require_version( + version, (4, 0, 0), "PIN/Touch policy requires YubiKey 4 or later" + ) + if touch_policy == TOUCH_POLICY.CACHED: + require_version( + version, (4, 3, 0), "Cached touch policy requires YubiKey 4.3 or later" + ) # ROCA if (4, 2, 0) <= version < (4, 3, 5): @@ -624,13 +616,13 @@ def _do_check_key_support( raise NotSupportedError("PIN_POLICY.NEVER not allowed on YubiKey FIPS") # New key types - if version < (5, 7, 0) and key_type in ( + if key_type in ( KEY_TYPE.RSA3072, KEY_TYPE.RSA4096, KEY_TYPE.ED25519, KEY_TYPE.X25519, ): - raise NotSupportedError(f"{key_type} requires YubiKey 5.7 or later") + require_version(version, (5, 7, 0), f"{key_type} requires YubiKey 5.7 or later") def _parse_device_public_key(key_type, encoded): diff --git a/yubikit/support.py b/yubikit/support.py index 7d96b8ad..f758e6de 100644 --- a/yubikit/support.py +++ b/yubikit/support.py @@ -83,8 +83,10 @@ def _read_info_ccid(conn, key_type, interfaces): try: return mgmt.read_device_info() except NotSupportedError: - # Workaround to "de-select" the Management Applet needed for NEO - conn.send_and_receive(b"\xa4\x04\x00\x08") + if version.major == 3: + # Workaround to "de-select" the Management Applet needed for NEO + logger.debug("Send NEO de-select workaround...") + conn.send_and_receive(b"\xa4\x04\x00\x08") except ApplicationNotAvailableError: logger.debug("Couldn't select Management application, use fallback") From fad423c25c91a8caf368349a7d717af6b1284ca9 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Fri, 30 Aug 2024 11:12:01 +0200 Subject: [PATCH 2/2] Don't fail on ykneo-openpgp < 1.0.2 --- yubikit/openpgp.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/yubikit/openpgp.py b/yubikit/openpgp.py index 864753bb..4e36ba32 100644 --- a/yubikit/openpgp.py +++ b/yubikit/openpgp.py @@ -1023,8 +1023,14 @@ def __init__( def _read_version(self) -> Version: logger.debug("Getting version number") - bcd = self.protocol.send_apdu(0, INS.GET_VERSION, 0, 0) - return Version(*(_bcd(x) for x in bcd)) + try: + bcd = self.protocol.send_apdu(0, INS.GET_VERSION, 0, 0) + return Version(*(_bcd(x) for x in bcd)) + except ApduError as e: + # Pre 1.0.2 versions don't support reading the version + if e.sw == SW.CONDITIONS_NOT_SATISFIED: + return Version(1, 0, 0) + raise @property def aid(self) -> OpenPgpAid: