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

opgp: add support for KDF PINs #325

Merged
merged 1 commit into from Mar 31, 2020

Conversation

emilazy
Copy link
Contributor

@emilazy emilazy commented Mar 15, 2020

Resolves #279.

I've tested this on a YubiKey 5 Nano and NFC running firmware version 5.2.4, but more testing from others would be appreciated; in particular, I haven't actually verified that it still works when KDF is disabled, or on older firmware versions that don't support KDF at all...

@dagheyman
Copy link
Contributor

dagheyman commented Mar 16, 2020

Thanks for the contribution! Some initial testing with a 5.2.4 and a YubiKey 4 (not with KDF but just using the other commands as usual) seems to work well. Needs a bit more reviewing and testing before merging though.

Copy link
Member

@dainnilsson dainnilsson left a comment

Choose a reason for hiding this comment

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

LGTM

Resolves Yubico#279.

Signed-off-by: Dag Heyman <dag@yubico.com>
@dagheyman
Copy link
Contributor

Merging, thanks!

@dagheyman dagheyman merged commit 2bbab30 into Yubico:master Mar 31, 2020
@roconnor
Copy link

I have a yubikey nano 5 with firmware 5.2.6.

using gpg 2.2.23 I used --card-edit to enable kdf-setup. However my self._get_data(DO.KDF) command doesn't return tags for pw2_salt_bytes nor pw3_salt_bytes for some reason. Thus I'm not able to use the ykman openpgp set-touch command.

The assertion @ https://github.com/Yubico/yubikey-manager/pull/325/files#diff-da03da1a4f8bebc76d8400055f515dc06c481974caf7f96966fd0ee30810af43R259 fails, or at least it would have failed if it were not unceremoniously removed in c13a4c0#diff-da03da1a4f8bebc76d8400055f515dc06c481974caf7f96966fd0ee30810af43L259

@roconnor
Copy link

roconnor commented Feb 19, 2021

FWIW, I suspect that when pw2_salt_bytes and pw3_salt_bytes are omitted, the values ought to be set to the pw1_salt_bytes.

@roconnor
Copy link

The above issue seems specific to my nano. My YubiKey 5 NFC with 5.2.7 firmware doesn't seem to have this issue.

@emilazy
Copy link
Contributor Author

emilazy commented Feb 19, 2021

Yeah that assertion should definitely stay or at least be turned into an explicit UI error or something…

The latest OpenPGP Smart Card specification’s diagram on page 19 suggests to me that the KDF DOs have to contain all three salts. I’m a bit wary of silently reusing salts for different inputs in a cryptographic application in response to behaviour that seems to contradict the specification. Hopefully someone from Yubico can ask a hardware engineer about this to figure out what’s up and how it should be handled.

@emilazy
Copy link
Contributor Author

emilazy commented Feb 19, 2021

(As a general comment, I think this functionality is honestly of somewhat limited utility in general; a lot of applications seem to not have great support for it, as I’ve found, and the primary attack it defends against seems to be USB sniffing. As long as you’re using a unique passphrase for your PIN I don’t think there’s much reason to use kdf-setup… maybe it makes cracking things a little more painful for someone physically attacking your key?)

@roconnor
Copy link

I don't know how to turn kdf-setup off. :(

@emilazy
Copy link
Contributor Author

emilazy commented Feb 19, 2021

Sadly, you have to ykman openpgp reset and discard all the keys.

@dainnilsson
Copy link
Member

To be honest I don't know what the "proper" behavior is when the salts for pw2 and pw3 are absent. Really it's more of "whatever gpg does". I've pushed some changes to the main branch to fall back to the pw1_salt in this case, but I have no idea of if it will work or not since I've only seen the case where all 3 salts are present. @roconnor - Are you willing to test it out and report back if it works or not? Do note that if it doesn't it's going to decrement your admin PIN retries counter by one, so take care not to lock yourself out!

@roconnor
Copy link

roconnor commented Feb 19, 2021

I did some further investigation and I see that gpg has 2 modes for KDFs, on, single (and off) @ https://github.com/gpg/gnupg/blob/777019faf0b8f10a897c3ee477d35f9b29f02224/g10/card-util.c#L620-L627.

gpg tests this single mode by the absance of the 0x85 tag @ https://github.com/gpg/gnupg/blob/7f3ce66ec56a5aea6170b7eb1bda5626eb208c83/g10/call-agent.c#L736-L737 (pretty non-robustly I might add :/)

You can enable single mode by passing any parameter to kdf-setup in gpg --card-edit, which I think I did when trying to "disable KDF" at some point @ https://github.com/gpg/gnupg/blob/777019faf0b8f10a897c3ee477d35f9b29f02224/g10/card-util.c#L2125-L2126. ... not very intuitive command parameter.

I believe it is the case that I can no longer change the KDF mode after setting keys? I get gpg: error for setup KDF: Conditions of use not satisfied.

Anyhow this should be sufficient information that I think you should be able to reproduce my configuration yourself. If you are unable, I can do further testing.

Turns out this isn't a firmware issue with my nanos like I thought at first. This is entirely a gnupg thing.

@roconnor
Copy link

For completeness here is gnupg's tortured logic for finding the KDF salt @ https://github.com/gpg/gnupg/blob/7f3ce66ec56a5aea6170b7eb1bda5626eb208c83/scd/app-openpgp.c#L2402-L2405

@dainnilsson
Copy link
Member

Perfect, that gives me everything I need. I'll make sure ykman 4.0 works properly for all modes. Thank you for your investigation into this, it's very helpful!

@roconnor
Copy link

Thanks for the quick response. I've also filed another (much easier) openpgp issue @ #390 for another issue I encountered while investigating this.

@roconnor
Copy link

(As a general comment, I think this functionality is honestly of somewhat limited utility in general; a lot of applications seem to not have great support for it, as I’ve found, and the primary attack it defends against seems to be USB sniffing. As long as you’re using a unique passphrase for your PIN I don’t think there’s much reason to use kdf-setup… maybe it makes cracking things a little more painful for someone physically attacking your key?)

It would be closer to sensible if instead a secure communication channel was opened instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ykman missing support for openpgp configuration with kdf enabled pins
4 participants