-
Notifications
You must be signed in to change notification settings - Fork 49
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
Add PKCS11-based HSM interface #229
Conversation
Snippets to test with YubiKey on PIV slot # Create key and sign it (the latter is important!)
yubico-piv-tool -a generate -s 9c -A ECCP256 -k -o public.pem
yubico-piv-tool -a verify-pin -a selfsign -s 9c -i public.pem -S /CN=a.b.c/OU=abc/O=b.c/ -o cert.pem
yubico-piv-tool -k -a import-certificate -s 9c -i cert.pem import securesystemslib.hsm
import securesystemslib.keys
sslib_key_id = \
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
scheme = securesystemslib.hsm.ECDSA_SHA2_NISTP256
data = b"data to be signed"
user_pin = "USER PIN" # TODO: Replace
pkcs11_so_path = "/path/to/pkcs11/shared/object/or/dynamic/lib" # TODO: replace
# E.g. /usr/local/Cellar/yubico-piv-tool/2.0.0/lib/libykcs11.dylib when
# installing with brew
# Init PKCS11
securesystemslib.hsm.load_pkcs11_lib(pkcs11_so_path)
# Get YubiKey assuming it is on the 1st slot
hsm_info = securesystemslib.hsm.get_hsms().pop()
key_infos = securesystemslib.hsm.get_keys_on_hsm(hsm_info)
# Grab keyid by label (NOTE: I promise better tooling ;)
hsm_key_id = [key_info["key_id"] for key_info in key_infos if key_info["label"] == "Public key for Digital Signature"].pop()
# Export public key
pub = securesystemslib.hsm.export_pubkey(
hsm_info, hsm_key_id, scheme, sslib_key_id)
# Create signature
sig = securesystemslib.hsm.create_signature(
hsm_info, hsm_key_id, user_pin, data, scheme, sslib_key_id)
# Verify signature
sig_valid = securesystemslib.keys.verify_signature(pub, sig, data)
assert sig_valid == True |
- pykcs11 to communicate with HSM (requires native swig and pkcs11 libraries, kept optional to allow pure Python sslib install) - asn1crypto to decode ecdsa public keys exported from HSM - unrelated: update enum34 TODO: Add installation docs (native dependencies, extras install)
Add public interface to - list available HSMs (get_hsms), and - keys on a given HSM (get_keys_on_hsm), - sign data with a private key on the HSM, exporting the signature into a securesystemslib-liek format (create_signature), and to - export a key from the HSM using a sslib-like format (export_pubkey) to be used for signature verification. Signature verification can be performed with the existing securesystemslib.keys.verify_signature() function. Currently the HSM interface supports ecdsa keys and the "ecdsa-sha2-nistp256" and "ecdsa-sha2-nistp384" signature schemes. The commit also adds a basic test loop (sign + export key -> verify) using SoftHSM, as well interface tests that check that the public functions error gracefully if optional dependencies are not installed. TODO: - error handling - PKCS11 seems brittle, make sure that there are no state problems (e.g. openSession on a slot requires prior call to get slots, etc.) and we always get back the values we expect, i.e. handle hsm return invalid / incomplete data (e.g. on getAttributeValue), and fail gracefully if not HINT: check `CKR_<RETURN VALUE TYPE>` constants - revise error messages and exception taxonomy - Add HSM_INFO_SCHEMA and HSM_INFO_SCHEMA, HSM_KEY_INFO_SCHEMA, HSM_KEY_ID_SCHEMA and check_match on them. - docs - flesh out function docstrings and add code comments - add links to PKCS11 specs - add installation and usage docs, e.g. replace test_hsm.TestECDSAOnLUKPUEHsYubiKey with some instructions for YubiKey in README.md - testing - check coverage - trigger edge cases (see error handling above) Co-authored-by: Tanishq Jasoria <[email protected]>
73cce96
to
2e20c56
Compare
slowly moving towards actually testing something... Documenting on the way: this is likely not useful to anyone else:
|
Continuing documenting my attempt at testing: Yubico does not document which tool should be used for this but I've now switched to yubico-piv-tool instead of ykman to reproduce Lukas's steps, I have reset the "management key" and...
Even full verbosity gives nothing useful. This UX is worse than pgp AND it also doesn't work. I am not sure if I can do anything more at the moment |
So I was finally able to run the snippet succesfully (thanks lukas):
So, using yubico-piv-tool 2.3.0 and Yubicos libykcs11.so as the pkcs library, the example code worked.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few years later, some review comments (I'm mostly looking at this from the POV of the Signer interface and #447):
I think we should not expose all of this as a new module -- especially things like get_keys_on_hsm()
, get_hsms()
seem like things that should not be public API here
HSMSigner
on the other hand sounds like a reasonable thing: The API would include
- key and device ids as constructor arguments (optional if PyKCS supports that)
- PKCS module loaded using environment variable (optional if there are reasonable defaults for some systems)
- PIN as constructor variable
This would be compatible with #447 with a URI like hsm:device/0/slot/c9
or hsm:?slot=c9&device=0
, and pin getting requested with secrets_handler()
After implementing HSMSigner, the remaining public functionality in this PR would then be export_pubkey()
. Interestingly this same issue comes up with KMS keys... Maybe public key export could actually be a method on the Signer as well: not all implementations would support it but some could?
Another thing that comes up with KMS keys as well is the need to prehash the payload. We should add that support somewhere -- figuring out the algorithm from the public key is currently very annoying. Can we have Key.get_payload_hash_algorithm()
?
Left also some code comments -- they may be obsolete so feel free to disregard
# If path is not passed PyKCS11 consults the PYKCS11LIB env var | ||
if path is None: | ||
PKCS11.load() | ||
|
||
else: | ||
securesystemslib.formats.PATH_SCHEMA.check_match(path) | ||
PKCS11.load(path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just PKCS11.load(path)
? docs seem to imply that None is perfectly vaid here
except PyKCS11.PyKCS11Error as e: | ||
PKCS11_DYN_LIB = False | ||
raise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you set PKCS11_DYN_LIB = False
before the try block, this except is not needed
import binascii | ||
|
||
if not six.PY2: | ||
import asn1crypto.keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asn1crypto is now a dependency for all securesystemslib users?
@@ -100,10 +100,11 @@ | |||
'Topic :: Software Development' | |||
], | |||
install_requires = ['six>=1.11.0', 'subprocess32; python_version < "3"', | |||
'python-dateutil>=2.8.0'], | |||
'python-dateutil>=2.8.0', 'asn1crypto; python_version > "3"'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mentioned this elsewhere but this seems wrong: it should be only needed for "pkcs11" extra, right?
Superseded by #472, which adds the relevant parts of this implementation to the new Signer/Key API. |
Fixes issue #:
Supersedes #170 and #221 (big kudos to @tanishqjasoria for excellent groundwork!)
Bases on / incorporates #228 (needs rebase here after #228 is merged)(rebased on #228)Description of the changes being introduced by the pull request:
Add public interface to:
get_hsms
), andget_keys_on_hsm
),create_signature
), and toexport_pubkey
) to be used for signature verification.Signature verification can be performed with the existing
securesystemslib.keys.verify_signature
function.Currently the HSM interface supports ecdsa keys and the
"ecdsa-sha2-nistp256"
and"ecdsa-sha2-nistp384"
signature schemes.Any interaction with the HSM interface require a one-time call to
load_pkcs11_lib
passing it the path to a PKCS11 dynamic library.The commit also adds a basic test loop (sign + export key -> verify) using SoftHSM, as well interface tests that check that the public functions error gracefully if optional dependencies are not installed.
TODO:
error handling
HINT: check
CKR_<RETURN VALUE TYPE>
constantsHSM_INFO_SCHEMA
,HSM_KEY_INFO_SCHEMA
,HSM_KEY_ID_SCHEMA
and check_match on them.docs
Please verify and check that the pull request fulfils the following
requirements: