olastor / clevis-pin-fido2

MIT License
4 stars 1 forks source link

`clevis-encrypt-fido2` contrary to the description does not store a specified `device` and `clevis-decrypt-fido2` therefore cannot reuse it #3

Closed m-ueberall closed 4 months ago

m-ueberall commented 4 months ago

At the moment, clevis-decrypt-fido2 cannot be used in scenarios where multiple FIDO2 tokens are required (e.g., different users have their own key and need to be able to decode the same encrypted information).

The attached (rather longish) patch device_naming_patch.txt

It might make sense to also reword the documentation to include the below examples/explanations; for the time being, only the additional parameter timeout is mentioned.

With the above patch, it is possible to use multiple fido2 pins when combining them using sss as long as the individual FIDO2 devices have distinct, unambiguous device names:

% echo "Hello, world." | clevis encrypt sss '{"t": 1, "pins": {"fido2": [{"device": "/dev/input/by-id/yubikey_12345678"}, {"device": "/dev/input/by-id/yubikey_87654321"}]}}' >test01.jwe
clevis-encrypt-fido2: Please insert your specified FIDO2 token /dev/input/by-id/yubikey_12345678
clevis-encrypt-fido2: Please insert your specified FIDO2 token /dev/input/by-id/yubikey_87654321
% echo "Hello, world." | clevis encrypt sss '{"t": 2, "pins": {"fido2": [{"device": "/dev/input/by-id/yubikey_12345678"}, {"device": "/dev/input/by-id/yubikey_87654321"}]}}' >test02.jwe
%

(Note that the second prompt in line three will only be shown after the first token has been used; since both key devices are already available during the second test, no prompts are shown.)

Now, if you unplug both devices in question and try to decrypt both payloads …

% clevis decrypt < test02.jwe
clevis-decrypt-fido2: Please insert your specified FIDO2 token /dev/input/by-id/yubikey_12345678 (if available)
clevis-decrypt-fido2: Please insert your specified FIDO2 token /dev/input/by-id/yubikey_87654321 (if available)
Hello, world.
% clevis decrypt < test01.jwe
Hello, world.
% fido2-assert: fido_dev_get_assert: FIDO_ERR_ACTION_TIMEOUT

(Here, the first two prompts in lines two and three will be displayed in parallel; since test01.jwe specifies that you only need (to touch) one of both keys, the message will be displayed and the second instance of clevis-decrypt-fido2 will run into a timeout. This means that the upstream latchset/clevis framework–as of release 20–does not yet ensure that all child processes are correctly terminated after decryption (whether it's successful or not).)

⚠ Since clevis-decrypt-sss will invoke multiple instances of clevis-decrypt-fido2 in parallel, it is currently not possible to use multiple FIDO2 devices without explicitly naming them–due to the fact that all available devices need to be queried in parallel (not a problem by itself) via multiple concurrent instances of clevis-decrypt-fido2 (i.e., multiple processes will try to access at least one device at the same time which does not work). In order to solve this, we'd need to introduce both a separate process (singleton) and a publisher/subscriber mechanism which ensures that only a single fido2-assert call addresses a specific device at any given time. (Simply retrying fido2-assert during the specified timeout does not work because this cannot ensure that the output of a specific device reaches the correct clevis-decrypt-fido2 instance.)

olastor commented 4 months ago

@m-ueberall Thank you for the detailed explanations and the contributions in the patch! This is quite a big of an improvement. I totally agree that currently the "device" option is not very well explained or even useful. When I wrote the pin I encountered the same issues you mentioned about choosing a specific device, but didn't focus on it too much.

I've pushed your patch into a branch and created a PR. I've made a few adjustments (summarized in this commit).

I changed the code where a "device" starting with "/dev/hid" would not be stored in the header to instead fail if that happens. That is to prevent users from thinking it is included when it's actually not. Instead I introduced an env variable FIDO2_TOKEN that can be used to temporarily choose the device to use (without storing it). So you can be sure that whatever set as the "device" option is also included in the header (or it fails loudly) and the env variable is dedicated for ephemeral device paths. The default timeout of 30 seconds might be a bit too short imho and I'd like having that a bit longer, but it could also be lowered again if there is a good reason.

Please let me know if you what you think about that and feel free to add review comments.

adds two files that allow to address a specific Yubikey based on its serial number even if ykpersonalize has not been used to ensure that the serial-usb-visible flag is set, 99-fido2.rules (an udev rule) and fido2-serial (used by said rule)

I haven't included that (yet), but I'd probably prefer adding this as an example in the man page because the script is about a specific brand/device model and uses new dependencies (ykman). A few comments about that in general:

With the above patch, it is possible to use multiple fido2 pins when combining them using sss as long as the individual FIDO2 devices have distinct, unambiguous device names:

Thanks! That makes this pin actually work nicely with the "sss" pin.

This means that the upstream latchset/clevis framework–as of release 20–does not yet ensure that all child processes are correctly terminated after decryption (whether it's successful or not).)

Interesting, I haven't noticed that before.

⚠ Since clevis-decrypt-sss will invoke multiple instances of clevis-decrypt-fido2 in parallel, it is currently not possible to use multiple FIDO2 devices without explicitly naming them–due to the fact that all available devices need to be queried in parallel (not a problem by itself) via multiple concurrent instances of clevis-decrypt-fido2 (i.e., multiple processes will try to access at least one device at the same time which does not work). In order to solve this, we'd need to introduce both a separate process (singleton) and a publisher/subscriber mechanism which ensures that only a single fido2-assert call addresses a specific device at any given time. (Simply retrying fido2-assert during the specified timeout does not work because this cannot ensure that the output of a specific device reaches the correct clevis-decrypt-fido2 instance.)

Yeah, I was considering the possibilty of doing a trial/error fashion, where each process simply tries to fido2-assert each available token until it found the right one. According to the CTAP 2.1 spec, I think the token should just return a "busy" error if it's called more than once. I never thought it'd be possible that the response of one assertion comes out at the wrong process, and I don't know if that could actually happen. If the credential doesn't work it also returns a "no credentials" error. These two errors could be used to try all devices if you'd want to find a way without using device naming. The problem is that tokens could require user presence (or even PIN) for assertions, which can lead to having to unnecessarily enter PINs / tap on tokens.

The solution with unique device names might indeed be the only one that works well here.

m-ueberall commented 4 months ago

I changed the code where a "device" starting with "/dev/hid" would not be stored in the header to instead fail if that happens. That is to prevent users from thinking it is included when it's actually not. Instead I introduced an env variable FIDO2_TOKEN that can be used to temporarily choose the device to use (without storing it). So you can be sure that whatever set as the "device" option is also included in the header (or it fails loudly) and the env variable is dedicated for ephemeral device paths. The default timeout of 30 seconds might be a bit too short imho and I'd like having that a bit longer, but it could also be lowered again if there is a good reason.

Agreed, specified devices should either be accepted or explicitly rejected, but user choices should not be silently barred from later reuse; a longer timeout is also more user-friendly (even about 120 seconds that are used in conjunction with other pins like tang should be fine).

adds two files that allow to address a specific Yubikey […]

I haven't included that (yet), but I'd probably prefer adding this as an example in the man page because the script is about a specific brand/device model and uses new dependencies (ykman). […]

Also agreed; for testing purposes, the above could be placed in an EXAMPLES section in the man page to help users in experimenting with these scripts, alongside a comment which briefly explains the constraints (not generally applicable, not advisable in production use).

  • The FIDO2-only Yubikeys actually don't have any serial number. So the script only works for the other Yubikey types that also support fido2.

This explains why I didn't find a suitable method to get hold of a serial number when browsing the API last week.

⚠ Since clevis-decrypt-sss will invoke multiple instances of clevis-decrypt-fido2 in parallel, it is currently not possible to use multiple FIDO2 devices without explicitly naming them–due to the fact that all available devices need to be queried in parallel (not a problem by itself) via multiple concurrent instances of clevis-decrypt-fido2 (i.e., multiple processes will try to access at least one device at the same time which does not work).

Yeah, I was considering the possibilty of doing a trial/error fashion, where each process simply tries to fido2-assert each available token until it found the right one. According to the CTAP 2.1 spec, I think the token should just return a "busy" error if it's called more than once.

Actually, if you do it like this in all instances of clevis-decrypt-fido2, it should eventually work, because those instances that don't encounter the FIDO_ERR_NO_CREDENTIALS error due to the right token choice will stop "occupying" all tokens and allow other instances to use fido2-assert without being blocked. Also, a token can be queried by all instances sequentially as long as an clevis-decrypt-fido2 instance never accesses a token that returned said error a second time (this is what I didn't think about when mentioning "Simply retrying fido2-assert during the specified timeout does not work").

The problem is that tokens could require user presence (or even PIN) for assertions, which can lead to having to unnecessarily enter PINs / tap on tokens.

Yes, this might be a bit confusing. OTOH, a short explanation (for this likely less common use case) in the manual should clarify this unavoidable behaviour. (Unless there is a reliable way to determine that the parent process of clevis-fido2-decrypt is clevis-sss-decrypt and there are more than one fido2 pins, outputting a scenario-specific warning/explanation would not be an option.)

olastor commented 4 months ago

Agreed, specified devices should either be accepted or explicitly rejected, but user choices should not be silently barred from later reuse; a longer timeout is also more user-friendly (even about 120 seconds that are used in conjunction with other pins like tang should be fine).

@m-ueberall Good idea, I increased it to 120 seconds.

Also agreed; for testing purposes, the above could be placed in an EXAMPLES section in the man page to help users in experimenting with these scripts, alongside a comment which briefly explains the constraints (not generally applicable, not advisable in production use).

I've added it in this commit, slightly adjusted so that it does not use the entire serial. Please feel free to review that.

Yes, this might be a bit confusing. OTOH, a short explanation (for this likely less common use case) in the manual should clarify this unavoidable behaviour. (Unless there is a reliable way to determine that the parent process of clevis-fido2-decrypt is clevis-sss-decrypt and there are more than one fido2 pins, outputting a scenario-specific warning/explanation would not be an option.)

In the man page there's a note I added for the "device" option ("This option is likely needed if you want to use the "sss" pin with multiple fido2 pins.") that hopefully makes it clear that for the "sss" pin this option is required (if it should work reliably).

m-ueberall commented 4 months ago

The above looks good!