Skip to content

Make PrecomputedValues public#221

Merged
tarcieri merged 5 commits intoRustCrypto:masterfrom
BrettMayson:master
Nov 11, 2022
Merged

Make PrecomputedValues public#221
tarcieri merged 5 commits intoRustCrypto:masterfrom
BrettMayson:master

Conversation

@BrettMayson
Copy link
Copy Markdown
Contributor

I need access to these values in order to store a custom private key format, this is preventing me from switching away from openssl

@tarcieri
Copy link
Copy Markdown
Member

tarcieri commented Nov 8, 2022

Is there a particular reason why you want to store these values instead of computing them on-the-fly?

The trend in RSA private key format design has been to store fewer precomputed values rather than more. Modern key formats store only d and can compute everything else from that and the public key.

Also: are you actually using multiprime RSA? (re: crt_values)

@BrettMayson
Copy link
Copy Markdown
Contributor Author

I need to store a custom format that I have no control over, which requires these values to be stored

@tarcieri
Copy link
Copy Markdown
Member

tarcieri commented Nov 9, 2022

Do you actually need to support multiprime RSA?

It would probably be worth keeping that out of the public API, especially as it currently has an odd definition owing to PKCS#1 also needing the CRT coefficient for two primes, so the current vector stores "additional CRT coefficients" beyond that one IIUC.

@newpavlov
Copy link
Copy Markdown
Member

I don't think we should make the fields public. If you only need to read pre-computed values, then a bunch of getters should do the job. If you need to restore the values from serialized data, then we would need to think about validation.

@BrettMayson
Copy link
Copy Markdown
Contributor Author

BrettMayson commented Nov 10, 2022

Also: are you actually using multiprime RSA? (re: crt_values)

No, just dp, dq and qinv

I can modify this PR to have getters for those 3 values in a few days

@tarcieri
Copy link
Copy Markdown
Member

In that case it sounds like you should just add read-only getter methods for those three

@tarcieri tarcieri merged commit b06a5ce into RustCrypto:master Nov 11, 2022
@tarcieri tarcieri mentioned this pull request Nov 15, 2022
takumi-earth pushed a commit to earthlings-dev/RSA that referenced this pull request Jan 27, 2026
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.

4 participants