Skip to content

feat: add ES512 algorithm support using P-521 elliptic curve - #532

Open
arsenin-kitsoft wants to merge 4 commits into
Keats:masterfrom
arsenin-kitsoft:feature/es512-p521-014
Open

feat: add ES512 algorithm support using P-521 elliptic curve#532
arsenin-kitsoft wants to merge 4 commits into
Keats:masterfrom
arsenin-kitsoft:feature/es512-p521-014

Conversation

@arsenin-kitsoft

Copy link
Copy Markdown

Suggested changes for #250

Replacement for #478 with updated dependencies.

@arsenin-kitsoft

Copy link
Copy Markdown
Author

@Keats @arckoor Local tests are green for both feature sets, but there are extensive dep bumps beyond p* libs. Please advise if this is okay. Specifically questionable is the bump of rsa to an rc version of 0.10 (the other libs have dragged it).

@arckoor

arckoor commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

rc versions are a no
is there no path at all in the deps to upgrade just p521? I assume it pulls a newer rand which is then too new for rsa?

Comment thread src/crypto/rust_crypto/ecdsa.rs Outdated
Comment on lines +92 to +93
// P-521 (ES512) has no `sign_recoverable()` support in RustCrypto, so it can't
// go through the `define_ecdsa_signer!` macro used for ES256/ES384.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not adjust the macro? We just discard the recovery ID do we not?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, that was easy 😅

@arsenin-kitsoft

Copy link
Copy Markdown
Author

@arckoor Eeh, there's a dep mess with P* 0.14 and not only with the 521 one 🤔
Should we wait for rc to release?
Because I don't think I'm equipped to untangle this one 😿

@atezet

atezet commented Jul 27, 2026

Copy link
Copy Markdown

Yeah, the rsa rc is very annoying. I think it should be possible to upgrade just the pxxx ones, but not just p521

@arsenin-kitsoft

Copy link
Copy Markdown
Author

Had to bump getrandom to 0.4 as well to clear the wasm tests.

@arsenin-kitsoft

Copy link
Copy Markdown
Author

@atezet didn't work for me, unfortunately - still breaks with dep clash.

@arsenin-kitsoft

Copy link
Copy Markdown
Author

RustCrypto/RSA#647 rsa 0.10 release track

@arckoor

arckoor commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

That's very unfortunate. I can still fully review this if you'd like, or we can pick it up once all the dependencies are sorted out

@arsenin-kitsoft

Copy link
Copy Markdown
Author

@arckoor I've addressed the issues with tests and with macros that you've highlighted above. But I think it's better to wait for the next release of RSA.

@atezet

atezet commented Jul 28, 2026

Copy link
Copy Markdown

I briefly looked at it and if I'm not mistaken there's a dependency clash on signature::{Signer, Verifier}, right? Why are these traits necessary even if the aws_lc_rs backend is used? Or is the problem also with getrandom and wasm?

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