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

RSA key comparison: exit early after MODULUS / PUBLIC_EXPONENT are compared - no round-trip to HSM #346

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

space88man
Copy link
Contributor

@space88man space88man commented Feb 21, 2024

Addresses #345.

RSA keys: the match operator was falling-through to the generic comparator (look for associated objects) instead of exiting early after EXPONENT/MODULUS were verified

RSA keys should never need to use find_associated_obj: the private key has enough attributes to do a logical comparison with the public key.

This is the same behaviour as OpenSC/libp11

@simo5
Copy link
Member

simo5 commented Feb 21, 2024

It is probably better to just add a check for the modulus early on for CKK_RSA just where we check the public exponent, and then we can immediately return w/o even calling cmp_public_keys.

Alternatively, move the check for CKA_PUBLIC_EXPONENT into cmp_public_key_values(), and the initial check for CKK_RSA would simply call cmp_public_key_values() unconditionally (with a comment that Private RSA key always include the public attributes.

And again just return later on.
I think I prefer the latter option, because currently cmp_public_key_values() is misleading as not checking public exponent in there is kind of bad.

@space88man
Copy link
Contributor Author

And again just return later on. I think I prefer the latter option, because currently cmp_public_key_values() is misleading as not checking public exponent in there is kind of bad.

Ok - colocated both checks (MODULUS/PUBLIC_EXPONENT)

@space88man space88man changed the title For RSA keys we can exit early after MODULUS / EXPONENT are verified. RSA key comparision: exit early after MODULUS / EXPONENT are verified, no round-trip to HSM Feb 22, 2024
@space88man space88man changed the title RSA key comparision: exit early after MODULUS / EXPONENT are verified, no round-trip to HSM RSA key comparison: exit early after MODULUS / PUBLIC_EXPONENT are compared - no round-trip to HSM Feb 22, 2024
Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

Only a couple of style issues, then once CI is happy (ignore MacOS+softhsm if it fails, it if flaky) I'll merge.

src/objects.c Outdated Show resolved Hide resolved
src/objects.c Show resolved Hide resolved
@space88man
Copy link
Contributor Author

Only a couple of style issues, then once CI is happy (ignore MacOS+softhsm if it fails, it if flaky) I'll merge.

Done - please check

src/objects.c Outdated Show resolved Hide resolved
Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

LGTM

@simo5 simo5 merged commit 391cd32 into latchset:main Feb 22, 2024
20 checks passed
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