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

Support for RSASSA-PKCS1-v1_5 with SHA1 digest #2188

Open
cocool97 opened this issue Dec 18, 2024 · 4 comments
Open

Support for RSASSA-PKCS1-v1_5 with SHA1 digest #2188

cocool97 opened this issue Dec 18, 2024 · 4 comments

Comments

@cocool97
Copy link

I'm maintaining adb_client, a rust crate used to communicate using Android devices over ADB protocol. I'd like to move from rsa crate to ring to handle authentication (as rsa currently has a known security vulnerability, and as I'm already depending on ring through rcgen(rustls))

When authenticating, devices need to sign random data using Pkcs1v1.5 with a SHA1 digest. rsa crate is providing this feature, but did not find any way to achieve this using ring crate .

Are you planning to provide a static RSA_PKCS1_SHA1 implementing RsaEncoding trait ? Or is there another way to achieve this ?

@briansmith
Copy link
Owner

Right now, we have ```RsaKeyPair::sign()that takes anRsaEncoding`. We intentionally don't provide an `RsaEncoding` for any SHA-1 encoding.

I am fine with accepting the addition of an RsaKeyPair::sign_with_pkcs1_sha1_for_legacy_use_only function that is basically a copy-paste of RsaKeyPair::sign() but hard-coded to use SHA-1 and PKCS1, so that we don't have to expose an RsaEncoding for SHA-1, if tests are added along with it. Many of our test vectors are from NIST and we just deleted/skipped the SHA-1-based ones, so we just need to "undelete" them.

@complexspaces
Copy link
Contributor

Would that be close to #1503 (which has the test vectors, etc already handled) but just with the different public API like you proposed above?

@cocool97
Copy link
Author

Indeed, except for a new public API as described above this PR does the trick :)

Maybe you could have a look at it @briansmith ? This would also fill other users needs !

@complexspaces
Copy link
Contributor

I could take on porting that PR's contents to the new proposed API.

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

No branches or pull requests

3 participants