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

Protect private minisign key with FIDO2 token #100

Closed
orolhawion opened this issue Apr 23, 2021 · 11 comments
Closed

Protect private minisign key with FIDO2 token #100

orolhawion opened this issue Apr 23, 2021 · 11 comments

Comments

@orolhawion
Copy link

As of v8.2 OpenSSH supports FIDO2 as a second factor to protect the private key. This makes resident keys on a token (yubikey etc.) avoidable. So you can locate the private key on the file system of your computer (as probably most people do anyway). In my opinion it even obsoletes passphrases for the private key, because it is imply not useable without the FIDO2 token.

I wonder if minisign keys could be protected the same way.

@jedisct1
Copy link
Owner

Yes!

The secret key can be stored and decrypted in any way. This doesn't affect how signatures are computed and verified.

I'm not planning to add FIDO2 support to this implementation (and would rather see keychain/touchid support) but feel free to do it!

Thank you!

@emlun
Copy link

emlun commented May 25, 2023

I think there's a misunderstanding here. It is true that minisign would support any kind of storage for the secret key as long you have full control of the data to be signed. But FIDO security keys are hard coded to always prepend a prefix before signing, a prefix that is not under the caller's control and cannot be omitted. See for example the definition of signature inputs in WebAuthn; the clientDataHash is controlled by the caller while the authenticatorData is controlled by the security key.

So generating or verifying FIDO signatures with minisign would require some small changes to minisign. Signers need to (1) record the authenticator data in the signature format; and (2) indicate that the signature includes this additional data, for example by using a new algorithm identifier. Verifiers need to (3) check for such an indicator; and (4) if present, prepend the authenticator data to the signed data before verifying the signature.

With that in mind, would that change the verdict on whether you'd want to support FIDO tokens in minisign? I'd be happy to help implement it if so, or help prototype it to see what it could look like before you make a decision.

@emlun
Copy link

emlun commented May 25, 2023

I should add that most of the benefit lies in supporting FIDO signatures for verification ((3) and (4) above). If only there is a suitable signature format with wide support in verifiers, the probably few signers that wish to use a FIDO token for signing ((1) and (2)) could use external tools to do so. 🙂

@jedisct1
Copy link
Owner

@emlun Thanks for clarifying!

As long as it doesn't complicate the spec too much, why not. If you could write a prototype, that would be awesome!

@emlun
Copy link

emlun commented May 25, 2023

Cool! I'm not sure when I'll get around to it, but I'll add it to my todo list.

@emlun
Copy link

emlun commented Jun 30, 2023

Alright, I have something to share! Please check out the wip/fido and wip/gh-pages-fido branches of YubicoLabs/minisign-fido for implementation prototype and documentation, respectively.

This adds a new signature_algorithm: FD (F as in FIDO, D as in the prehashed form of ED) and a corresponding extended signature format. The extended signature format is needed for two reasons:

  • To store the auth_data prefixes needed to reconstruct the signed data before verifying, and
  • to modify the verification algorithm to branch on the FD type and assemble the signed data differently.

As of right now, this implementation adds a dependency on OpenSSL for the SHA256 hash algorithm. This is for two reasons:

  • The clientDataHash parameter in the CTAP2 protocol is limited to 32 bytes, so a Blake2b-512 digest is too long, so I used SHA256 to "compress" the Blake2b digest down to 32 bytes.

    For this purpose, SHA256 could be avoided by using BLAKE2b-256 instead. Last time I looked, I thought libsodium only provided BLAKE2b-512, but it seems like it does provide BLAKE2b-256 too. python-cryptography doesn't, though, so I haven't been able to test it.

  • Part of the auth_data being signed is the RP ID hash, which is SHA256(rp_id), where rp_id is a free parameter. In WebAuthn this is set to the domain of the webpage requesting a signature, so for example example.org; in OpenSSH this is typically ssh:. It would make sense for rp_id to default to minisign: in Minisign, and for Minisign to validate this value to prevent cross-protocol attacks.

    The SHA256 hash is computed by the security key, so we cannot choose to use a different hash algorithm for this.

    SHA256 wouldn't be strictly needed if Minisign hard-codes that the RP ID must always be minisign: - we could just precompute SHA256("minisign:") and hard-code that as a constant. But it's worth considering a command line option to specify the RP ID, in which case SHA256 would be required in order to compute the hash of that RP ID.

    Right now the implementation reads the RP ID hash from the signature structure and does not validate it.

    Either way, both of these options - hardcoding rp_id: "minisign:" or adding a CLI option - would allow stripping the RP ID from the signature format, saving 32 bytes (pre-base64) per signature value. OpenSSH does this, and stores only the flags and counter parts of auth_data in the signature structure. This does however mean that the verifier must know the RP ID some other way (unless hard-coded in the protocol) in order to successfully verify a signature.

As for the C implementation, I tried to keep it minimally invasive. The implementation approach I ended up with was to over-allocate some of the signature struct buffers and use pointer arithmetic to separate the additional data (when present) from the base struct. I don't have much experience with plain C, so there may very well be some horrible crimes against conventions in there. I'm also aware that there are a few memory leaks that I haven't bothered fixing for now.

Of course, all aspects of the implementation prototype are open for discussion, from crypto architecture to code formatting. 😄

Other possibilities not currently implemented:

  • The C implementation only supports verifying the new format. Key generation and signing can be done by external tools, such as the Python script I used for testing (included in the repo root). Minisign could use the libfido2 (C) library in order to support these operations too, but it's substantially more work than just supporting signature verification.

  • FIDO signatures contain two bit flags of interest: user presence (UP) and user verified (UV). UP typically means pressing a button on the security key, UV typically means the security key verified a PIN or fingerprint before generating the signature.

    It's common for FIDO2 verifiers to require UP for most operations, unless unattended signing is acceptable. Conversely, UV is typially not required by default, but is sometimes required for sensitive operations.

    For example, OpenSSH by default requires UP=1 and allows UV=0, but can be configured to allow UP=0 and/or require UV=1, independently, on a per-key basis. These are both enforced on the server side (that would be verifier side in the case of Minisign).

    There is also a CTAP2 extension, credProtect: "userVerificationRequired", that can be used to configure the security key to require UV regardless of what the server (verifier) requires. This is enforced by the security key itself.

    Right now fido_minisign.py sets UP=1 in the content signature and UP=0 in the comment signature, in order to only require one button press per signature. The C verifier implementation currently does not validate the UP and UV flags, but it would make sense to require UP=1 by default add CLI options to opt out of UP and opt in to UV (for example, --fido-no-up-required and --fido-uv-required).

Anyway, here's how to try it all out if you have a CTAP2 security key available:

$ git checkout wip/fido

$ pip install fido2 PyNaCl
$ python fido_minisign.py generate
$ echo 'Hello, World!' > msg.txt
$ python fido_minisign.py sign msg.txt

$ (cd build && cmake .. && make)
$ ./build/minisign -V -m msg.txt

Note that the minisign.fidokey written by fido_minisign.py generate is not sensitive since it doesn't actually contain the private key. Instead it contains a key handle (the credential_id field) that the security key uses to unwrap the private key internally. This also means that these keys don't consume storage space on the security key, unlike PGP smart cards for example.

What do you think, now that there's something concrete to look into? Is this something you'd like to support?

@jedisct1
Copy link
Owner

In libsodium, the output of crypto_generichash (backed by BLAKE2B) can be anything from 1 to 64 bytes.

SHA256 is also available (crypto_hash_sha256), so we don't need OpenSSL for that.

@emlun
Copy link

emlun commented Jun 30, 2023

Oh! I just looked in the "generic hashing" section of the docs. It seemed weird that the library wouldn't support SHA at all, but it didn't occur to me to look anywhere else.

I've updated the prototype implementations and signature format documentation to use Blake2b-256 instead of SHA256 and SHA256(Blake2b-512). I also found now that I can just use pynacl instead of python-cryptography in the signer prototype, so I've also successfully tested that this works!

@emlun
Copy link

emlun commented Aug 1, 2023

Hi @jedisct1, have you had time to consider this? Would you like me to proceed with developing this proof of concept into a proper pull request?

@emlun
Copy link

emlun commented Feb 28, 2024

Related: w3c/webauthn#2026

@NicolasCARPi
Copy link

Hello everyone.

@emlun Your work is very interesting. I'm looking forward to being able to make FIDO2 signatures in my app. I hope this can make it into minisign!

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

No branches or pull requests

4 participants