Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
102 changes: 65 additions & 37 deletions docs/guides/testing-passkey.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,29 @@ Testing Passkeys
Passkey can be tested using passkey related methods from
:class:`sssd_test_framework.utils.sssctl.SSSCTLUtils` and
:meth:`sssd_test_framework.utils.authentication.SUAuthenticationUtils.passkey`.
It requires umockdev in order to correctly mock the passkey hardware token and
record and playback the communications that happens between SSSD and
the token.
It requires `virtual-fido <https://github.com/bulwarkid/virtual-fido>`_ in order to
simulate a virtual FIDO2 authenticator for testing passkey functionality.

Three umockdev files are required to mock the device and playback the communication:
**System Requirements**

* device description (can be shared with all tests)
* ioctl description (can be shared with all tests)
* script of communication (mostly unique for each test)
The ``vhci-hcd`` kernel module must be installed and loaded for virtual-fido
to function properly:

You can store the files in data directories that are returned by
:func:`sssd_test_framework.fixtures.moduledatadir` (device and ioctl) and
:func:`sssd_test_framework.fixtures.testdatadir` (script, passkey mapping)
fixtures.
.. code-block:: bash

# Install the kernel module
sudo dnf install -y kernel-modules-extra # On Fedora/RHEL
# or
sudo apt install -y linux-modules-extra-$(uname -r) # On Ubuntu/Debian

# Load the module
sudo modprobe vhci-hcd

# Verify the module is loaded
lsmod | grep vhci

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with the feature detection logic in sssd_test_framework/hosts/client.py, it would be better to grep for vhci_hcd instead of just vhci. This makes the verification step more specific and aligned with the actual kernel module name.

Suggested change
lsmod | grep vhci
lsmod | grep vhci_hcd


# Load the module automatically at boot
sudo sh -c 'echo "vhci-hcd" > /etc/modules-load.d/vhci-hcd.conf'

Test examples
=============
Expand All @@ -33,57 +42,76 @@ Test examples
from sssd_test_framework.topology import KnownTopology


@pytest.mark.tier(0)
@pytest.mark.topology(KnownTopology.Client)
def test_passkey__register__sssctl(client: Client, moduledatadir: str, testdatadir: str):
""" Test registration of the passkey token with sssctl passkey-register"""
@pytest.mark.builtwith(client=["passkey", "vfido"])
def test_passkey__register_sssctl(client: Client):
"""
Test registration of the passkey token with sssctl passkey-register
"""
client.vfido.reset()
client.vfido.pin_enable()
client.vfido.pin_set(123456)
client.vfido.start()

mapping = client.sssctl.passkey_register(
username="user1",
domain="ldap.test",
pin=123456,
device=f"{moduledatadir}/umockdev.device",
ioctl=f"{moduledatadir}/umockdev.ioctl",
script=f"{testdatadir}/umockdev.script",
virt_type="vfido"
)
with open(f"{testdatadir}/passkey-mapping") as f:
assert mapping == f.read().strip()

assert mapping.startswith("passkey:"), f"Invalid mapping prefix: {mapping}"


@pytest.mark.tier(0)
@pytest.mark.topology(KnownTopology.IPA)
def test_passkey__register__ipa(ipa: IPA, moduledatadir: str, testdatadir: str):
""" Test registration of the passkey token with ipa user-add-passkey --register"""
@pytest.mark.builtwith(client=["passkey", "vfido"], ipa="passkey")
def test_passkey__register_ipa(client: Client, ipa: IPA):
"""
Test registration of the passkey token with ipa user-add-passkey --register
"""
client.vfido.reset()
client.vfido.pin_enable()
client.vfido.pin_set(123456)
client.vfido.start()

mapping = (
ipa.user("user1")
.add()
.passkey_add_register(
client=client,
pin=123456,
device=f"{moduledatadir}/umockdev.device",
ioctl=f"{moduledatadir}/umockdev.ioctl",
script=f"{testdatadir}/umockdev.script",
virt_type="vfido"
)
)

with open(f"{testdatadir}/passkey-mapping") as f:
assert mapping == f.read().strip()
assert mapping.startswith("Passkey mapping: passkey:")


@pytest.mark.tier(0)
@pytest.mark.topology(KnownTopology.LDAP)
@pytest.mark.topology(KnownTopology.IPA)
def test_passkey__su(client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str):
""" Test passkey authentication with su"""
suffix = type(provider).__name__.lower()

with open(f"{testdatadir}/passkey-mapping.{suffix}") as f:
provider.user("user1").add().passkey_add(f.read().strip())
@pytest.mark.builtwith(client=["passkey", "vfido"], ipa="passkey")
def test_passkey__su_user(client: Client, provider: GenericProvider):
"""
Test passkey authentication with su
"""
client.vfido.reset()
client.vfido.pin_enable()
client.vfido.pin_set(123456)
client.vfido.start()

user = provider.user("user1").add()
if isinstance(provider, IPA):
user.passkey_add_register(client=client, pin=123456, virt_type="vfido")
else:
mapping = client.sssctl.passkey_register(
username="user1", domain=provider.domain, pin=123456, virt_type="vfido"
)
user.passkey_add(mapping)

client.sssd.start()

assert client.auth.su.passkey(
username="user1",
pin=123456,
device=f"{moduledatadir}/umockdev.device",
ioctl=f"{moduledatadir}/umockdev.ioctl",
script=f"{testdatadir}/umockdev.script.{suffix}",
virt_type="vfido",
)
3 changes: 2 additions & 1 deletion sssd_test_framework/hosts/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def features(self) -> dict[str, bool]:
echo "limited_enumeration" || :
[ -f "/usr/bin/vicc" ] && echo "virtualsmartcard" || :
[ -f "/usr/bin/umockdev-run" ] && echo "umockdev" || :
[ -f "/opt/test_venv/bin/vfido.py" ] && echo "vfido" || :
lsmod | grep -q "vhci_hcd" && [ -f "/opt/test_venv/bin/vfido.py" ] && echo "vfido" || :
""",
log_level=ProcessLogLevel.Error,
)
Expand All @@ -78,6 +78,7 @@ def features(self) -> dict[str, bool]:
"gdm": False,
"virtualsmartcard": False,
"umockdev": False,
"vfido": False,
}

self._features.update({k: True for k in result.stdout_lines})
Expand Down
2 changes: 1 addition & 1 deletion sssd_test_framework/roles/ipa.py
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@ def run_expect():
retry += 1
time.sleep(1)

return result.stdout_lines[-1].strip()
return result.stdout_lines[-2].strip()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The output of the ipa command includes a summary line after the passkey mapping. Accessing the second to last line (-2) is fragile. A more robust approach would be to iterate through the output lines and find the one that starts with Passkey mapping:. This avoids issues if the ipa command's output format changes in the future.

        for line in result.stdout_lines:
            if line.strip().startswith("Passkey mapping:"):
                return line.strip()

        raise AssertionError("Passkey mapping not found in ipa command output")


def iduseroverride(self) -> IDUserOverride:
"""
Expand Down
94 changes: 48 additions & 46 deletions sssd_test_framework/utils/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ def passkey_with_output(
) -> tuple[int, int, str, str]:
"""wrapper for passkey_with_output methods"""
if "virt_type" in kwargs and kwargs["virt_type"] == "vfido":
del kwargs["virt_type"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Deleting virt_type from kwargs modifies the dictionary that was passed in by the caller. This can lead to unexpected side effects if the caller reuses the kwargs dictionary. It's safer to create a copy of the dictionary before modifying it.

Suggested change
del kwargs["virt_type"]
kwargs_copy = kwargs.copy()
del kwargs_copy["virt_type"]

return self.vfido_passkey_with_output(**kwargs)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To avoid the side effect of modifying the original kwargs dictionary, you should pass the modified copy to the vfido_passkey_with_output method.

Suggested change
return self.vfido_passkey_with_output(**kwargs)
return self.vfido_passkey_with_output(**kwargs_copy)

else:
return self.umockdev_passkey_with_output(**kwargs)
Expand All @@ -427,6 +428,7 @@ def passkey(
) -> bool:
"""wrapper for passkey methods"""
if "virt_type" in kwargs and kwargs["virt_type"] == "vfido":
del kwargs["virt_type"]
return self.vfido_passkey(**kwargs)
Comment on lines +431 to 432

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Similar to the passkey_with_output method, deleting virt_type from kwargs here can cause unexpected side effects. It's better to work with a copy of the dictionary to ensure the caller's dictionary remains unchanged.

Suggested change
del kwargs["virt_type"]
return self.vfido_passkey(**kwargs)
kwargs_copy = kwargs.copy()
del kwargs_copy["virt_type"]
return self.vfido_passkey(**kwargs_copy)

else:
return self.umockdev_passkey(**kwargs)
Expand Down Expand Up @@ -690,12 +692,15 @@ def vfido_passkey_with_output(
"""

match auth_method:
case PasskeyAuthenticationUseCases.PASSKEY_PIN | PasskeyAuthenticationUseCases.PASSKEY_PIN_AND_PROMPTS:
case (
PasskeyAuthenticationUseCases.PASSKEY_PIN
| PasskeyAuthenticationUseCases.PASSKEY_PIN_AND_PROMPTS
| PasskeyAuthenticationUseCases.PASSKEY_FALLBACK_TO_PASSWORD
):
if pin is None:
raise ValueError(f"PIN is required for {str(auth_method)}")
case (
PasskeyAuthenticationUseCases.PASSKEY_PROMPTS_NO_PIN
| PasskeyAuthenticationUseCases.PASSKEY_FALLBACK_TO_PASSWORD
| PasskeyAuthenticationUseCases.PASSKEY_NO_PIN_NO_PROMPTS
):
if pin is not None:
Expand Down Expand Up @@ -738,71 +743,68 @@ def vfido_passkey_with_output(
spawn "{run_su}"
set ID_su $spawn_id

# If the authentication method set without entering the PIN, it will directly ask
# prompt, if we set prompting options in sssd.conf it will ask interactive and touch prompt.

if {{ ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_NO_PIN_NO_PROMPTS}")
|| ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_PROMPTS_NO_PIN}") }} {{
expect {{
-i $ID_su -re "{interactive_prompt}*" {{ send -i $ID_su "\n" }}
-i $ID_su timeout {{exitmsg "Unexpected output" 201 }}
-i $ID_su eof {{exitmsg "Unexpected end of file" 202 }}
}}
# If prompt options are set
if {{ ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_PROMPTS_NO_PIN}") }} {{
expect {{
-i $ID_su -re "{touch_prompt}*" {{ }}
-i $ID_su timeout {{exitmsg "Unexpected output" 203 }}
-i $ID_su eof {{exitmsg "Unexpected end of file" 204 }}
}}
}}
# Phase 1: all methods start with interactive prompt
expect {{
-i $ID_su -re "{interactive_prompt}*" {{ send -i $ID_su "\n" }}
-i $ID_su timeout {{exitmsg "Unexpected output" 201 }}
-i $ID_su eof {{exitmsg "Unexpected end of file" 202 }}
}}

# If authentication method set with PIN, after interactive prompt always ask to Enter the PIN.
# If PIN is correct with prompt options in sssd.conf it will ask interactive and touch prompt.
# If we press Enter key for PIN, sssd will fallback to next auth method, here it will ask
# for Password.

# Phase 2: PIN-based methods need PIN prompt
if {{ ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_PIN}")
|| ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_PIN_AND_PROMPTS}")
|| ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_FALLBACK_TO_PASSWORD}")}} {{
expect {{
-i $ID_su -re "{interactive_prompt}*" {{ send -i $ID_su "\n" }}
-i $ID_su timeout {{exitmsg "Unexpected output" 205 }}
-i $ID_su eof {{exitmsg "Unexpected end of file" 206 }}
}}
expect {{
-i $ID_su -re "Enter PIN:*" {{send -i $ID_su "{pin}\r"}}
-i $ID_su timeout {{exitmsg "Unexpected output" 207}}
-i $ID_su eof {{exitmsg "Unexpected end of file" 208}}
}}
if {{ ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_FALLBACK_TO_PASSWORD}") }} {{
expect {{
-i $ID_su -re "Password:*" {{send -i $ID_su "Secret123\r"}}
-i $ID_su timeout {{exitmsg "Unexpected output" 209}}
-i $ID_su eof {{exitmsg "Unexpected end of file" 210}}
}}
}}

# Password fallback method needs password prompt
if {{ ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_FALLBACK_TO_PASSWORD}") }} {{
expect {{
-i $ID_su -re "Password:*" {{send -i $ID_su "Secret123\r"}}
-i $ID_su timeout {{exitmsg "Unexpected output" 209}}
-i $ID_su eof {{exitmsg "Unexpected end of file" 210}}
}}
if {{ ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_PIN_AND_PROMPTS}") }} {{
expect {{
-i $ID_su -re "{touch_prompt}*" {{ }}
-i $ID_su timeout {{exitmsg "Unexpected output" 211 }}
-i $ID_su eof {{exitmsg "Unexpected end of file" 212 }}
}}

# Phase 3: handle touch prompts (for methods that show them)
if {{ ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_PROMPTS_NO_PIN}")
|| ($auth_method eq "{PasskeyAuthenticationUseCases.PASSKEY_PIN_AND_PROMPTS}") }} {{
# Wait for touch prompt to appear and acknowledge it
expect {{
-i $ID_su -re "{touch_prompt}*" {{
# Touch prompt appeared - send Enter to acknowledge it
send -i $ID_su "\n"
}}
-i $ID_su timeout {{exitmsg "Unexpected output" 203 }}
-i $ID_su eof {{exitmsg "Unexpected end of file" 204 }}
}}
}}

# Now simulate touch on vfido device
# Phase 4: Device touch
spawn {test_venv_bin}/vfido_touch
set ID_touch $spawn_id

expect {{
-i $ID_su -re "Authentication failure" {{exitmsg "Authentication failure" 1}}
-i $ID_su eof {{exitmsg "Passkey authentication successful" 0}}
-i $ID_su timeout {{exitmsg "Unexpected output" 213}}
-i $ID_touch eof {{}}
-i $ID_touch timeout {{}}
}}
Comment on lines 791 to 794

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The expect block for ID_touch can be simplified. Since you're just waiting for the vfido_touch process to finish and don't care about its output, a single expect eof is sufficient. This makes the script slightly cleaner.

Suggested change
expect {{
-i $ID_su -re "Authentication failure" {{exitmsg "Authentication failure" 1}}
-i $ID_su eof {{exitmsg "Passkey authentication successful" 0}}
-i $ID_su timeout {{exitmsg "Unexpected output" 213}}
-i $ID_touch eof {{}}
-i $ID_touch timeout {{}}
}}
expect -i $ID_touch eof


expect -i $ID_touch eof
# Phase 5: Wait for authentication completion
expect {{
-i $ID_su -re "Authentication failure" {{
exitmsg "Authentication failure" 1
}}
-i $ID_su eof {{
exitmsg "Passkey authentication successful" 0
}}
-i $ID_su timeout {{
exitmsg "Unexpected output" 213
}}
}}

exitmsg "Unexpected code path" 220
""",
Expand Down
Loading