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

Key should expose pre hash algorithm #469

Closed
jku opened this issue Nov 29, 2022 · 4 comments
Closed

Key should expose pre hash algorithm #469

jku opened this issue Nov 29, 2022 · 4 comments

Comments

@jku
Copy link
Collaborator

jku commented Nov 29, 2022

This is after #456

There could be a optional Key.get_pre_hash_algorithm():

  • would return the pre-signing content hash algorithm that should be used
  • this would be useful for for signers that need to prehash (KMS and HSM I believe)
  • this does not apply to all keys (e.g. "ed25519" is likely the non-prehashing variant)
@jku
Copy link
Collaborator Author

jku commented Dec 7, 2022

I suppose this could be an actual digest() API as well... so you could do something like

hasher = key.hasher() # this is like securesystemslib.hash.digest() except no argument needed 
hasher.update(data)
digest = hasher.digest()

but maybe securesystemslib shouldn't be trying to do all these things?

@lukpueh
Copy link
Member

lukpueh commented Dec 7, 2022

FYI: The HSMSigner API does not need pre-hashing. I only pre-hash in tests as workaround for SoftHSM's limited capabilities:

_orig_sign = HSMSigner.sign
def _sign(self, data):
"""Wraps HSMSigner sign method for testing
HSMSigner supports CKM_ECDSA_SHA256 and CKM_ECDSA_SHA384 mechanisms. But SoftHSM
only supports CKM_ECDSA. For testing we patch the HSMSigner mechanism attribute and
pre-hash the data.
"""
self._mechanism = PyKCS11.Mechanism( # pylint: disable=protected-access
PyKCS11.CKM_ECDSA
)
hasher = digest(algorithm=f"sha{self.public_key.scheme[-3:]}")
hasher.update(data)
return _orig_sign(self, hasher.digest())

@jku
Copy link
Collaborator Author

jku commented Dec 7, 2022

oh interesting. If this is only needed by KMS, then we can hold off implementing anything public/generic

@jku
Copy link
Collaborator Author

jku commented Feb 10, 2023

I'll close this based on above comments

@jku jku closed this as completed Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants