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

HSMSigner: Use pre-hashing #548

Merged
merged 1 commit into from
Mar 23, 2023
Merged

Conversation

jku
Copy link
Collaborator

@jku jku commented Mar 23, 2023

This PR was previously only for SoftHSM, I have extended it so all signing now uses pre-hashing


Two benefits:

  • It's faster: devices like Yubikeys don't like being fed massive
    payloads. signing time for 100MB payload goes from 29 secs to
    0.4 secs on my USB A hardware key
  • Testing becomes simpler: SoftHSM only supports pre-hashing so the
    tests needed a mock patch to handle that specifically. This is a
    bit of a pain for applications.

So:

  • Use PyKCS11.CKM_ECDSA mechanism for both supported schemes
  • Hash the payload according to scheme, and feed HW the hash instead
    of the full payload
  • Remove the test patching

@jku jku marked this pull request as draft March 23, 2023 13:10
@jku jku force-pushed the softhsm-automation branch 2 times, most recently from 85adbc3 to c1aff18 Compare March 23, 2023 13:28
@jku
Copy link
Collaborator Author

jku commented Mar 23, 2023

I just tested the speed differences and the pre-hashing is so much faster that I'll expand this PR: let's pre-hash everything instead of just softhsm payloads

@jku jku force-pushed the softhsm-automation branch from c1aff18 to 99506e1 Compare March 23, 2023 14:11
@jku jku changed the title HSMSigner: Move the SoftHSM special case into the signer HSMSigner: Use pre-hashing Mar 23, 2023
Two benefits:
* It's faster: devices like Yubikeys don't like being fed massive
  payloads. signing time for 100MB payload goes from 29 secs to
  0.4 secs on my USB A hardware key
* Testing becomes simpler: SoftHSM only supports pre-hashing so the
  tests needed a mock patch to handle that specifically. This is a
  bit of a pain for applications.

So:
* Use PyKCS11.CKM_ECDSA mechanism for both supported schemes
* Hash the payload according to scheme, and feed HW the hash instead
  of the full payload
* Remove the test patching
@jku jku force-pushed the softhsm-automation branch from 99506e1 to 159ab53 Compare March 23, 2023 14:17
@jku
Copy link
Collaborator Author

jku commented Mar 23, 2023

The potential impact of this is that we now expect hardware to support the pre-hashing mechanism whereas before we expected the hardware to support the two other mechanisms... I don't think we have a lot of insight into what actual devices out there support: Yubikey at least works both ways.

@jku jku marked this pull request as ready for review March 23, 2023 14:29
@jku jku requested a review from lukpueh March 23, 2023 14:29
@lukpueh
Copy link
Member

lukpueh commented Mar 23, 2023

The potential impact of this is that we now expect hardware to support the pre-hashing mechanism whereas before we expected the hardware to support the two other mechanisms... I don't think we have a lot of insight into what actual devices out there support: Yubikey at least works both ways.

Given the insight we have, this is is definitely an improvement. If we learn later that we need a fallback in either direction, we can start asking the devices for the mechanism they support and handle appropriately.

@lukpueh lukpueh merged commit 5e388f4 into secure-systems-lab:main Mar 23, 2023
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

Successfully merging this pull request may close these issues.

2 participants