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

Make version handling for dev keys more explicit #631

Merged
merged 2 commits into from
Sep 2, 2024
Merged
Changes from 1 commit
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
Next Next commit
Make version handling for dev keys more explicit
  • Loading branch information
dainnilsson committed Aug 30, 2024
commit 92af831ea6faa6d19819e44d3d15f0a50a905c1d
16 changes: 6 additions & 10 deletions tests/device/conftest.py
Original file line number Diff line number Diff line change
@@ -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

27 changes: 26 additions & 1 deletion ykman/_cli/__main__.py
Original file line number Diff line number Diff line change
@@ -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,13 +82,18 @@
import time
import sys
import re
import os

import logging


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

5 changes: 3 additions & 2 deletions ykman/_cli/oath.py
Original file line number Diff line number Diff line change
@@ -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.")

10 changes: 9 additions & 1 deletion ykman/_cli/util.py
Original file line number Diff line number Diff line change
@@ -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

26 changes: 24 additions & 2 deletions yubikit/core/__init__.py
Original file line number Diff line number Diff line change
@@ -41,6 +41,10 @@
)
import re
import abc
import logging


logger = logging.getLogger(__name__)


_VERSION_STRING_PATTERN = re.compile(r"\b(?P<major>\d+).(?P<minor>\d).(?P<patch>\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)
4 changes: 3 additions & 1 deletion yubikit/hsmauth.py
Original file line number Diff line number Diff line change
@@ -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)
)
34 changes: 13 additions & 21 deletions yubikit/piv.py
Original file line number Diff line number Diff line change
@@ -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):
6 changes: 4 additions & 2 deletions yubikit/support.py
Original file line number Diff line number Diff line change
@@ -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")