Skip to content

Support P521-SHA512 signatures with aws-lc-rs#1706

Merged
ctz merged 10 commits intomainfrom
jbp-ecdsa-sha512-signatures
Jan 4, 2024
Merged

Support P521-SHA512 signatures with aws-lc-rs#1706
ctz merged 10 commits intomainfrom
jbp-ecdsa-sha512-signatures

Conversation

@ctz
Copy link
Copy Markdown
Member

@ctz ctz commented Dec 22, 2023 •
edited
Loading

This will likely have some benchmark result noise, because the curves used in ECDSA benchmarks have changed.

Work remaining:

  • reinstate coverage of P384 in benchmarks
  • attend to testing gaps in crypto/*/sign.rs

closes #1666

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 22, 2023 •
edited
Loading

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (0d4b2df) 96.08% compared to head (839e95a) 96.16%.

Files Patch % Lines
rustls/src/crypto/aws_lc_rs/sign.rs 99.33% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1706      +/-   ##
==========================================
+ Coverage   96.08%   96.16%   +0.08%     
==========================================
  Files          79       80       +1     
  Lines       16911    17345     +434     
==========================================
+ Hits        16249    16680     +431     
- Misses        662      665       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rustls-benchmarking
Copy link
Copy Markdown

rustls-benchmarking bot commented Dec 22, 2023 •
edited
Loading

Benchmark results

Instruction counts

⚠️ Missing benchmarks

The following benchmark scenarios are present in the candidate but not in the baseline:

  • handshake_session_id_aws_lc_rs_1.3_ecdsap384_chacha_server

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap256_aes_client

  • handshake_tickets_ring_1.3_ecdsap384_chacha_client

  • handshake_session_id_ring_1.3_ecdsap256_chacha_client

  • transfer_no_resume_ring_1.3_ecdsap256_aes_server

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap256_aes_server

  • handshake_tickets_ring_1.3_ecdsap384_chacha_server

  • handshake_tickets_ring_1.3_ecdsap256_chacha_server

  • handshake_session_id_aws_lc_rs_1.3_ecdsap384_aes_server

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap384_aes_server

  • handshake_session_id_ring_1.3_ecdsap384_aes_client

  • handshake_no_resume_ring_1.3_ecdsap384_aes_client

  • transfer_no_resume_ring_1.3_ecdsap384_chacha_client

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap256_chacha_server

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap256_chacha_client

  • transfer_no_resume_ring_1.3_ecdsap384_chacha_server

  • handshake_tickets_aws_lc_rs_1.3_ecdsap256_chacha_server

  • handshake_tickets_aws_lc_rs_1.3_ecdsap256_chacha_client

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap256_aes_client

  • transfer_no_resume_ring_1.3_ecdsap384_aes_client

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap384_chacha_server

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap384_chacha_server

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap384_chacha_client

  • handshake_no_resume_ring_1.3_ecdsap256_chacha_server

  • handshake_tickets_ring_1.3_ecdsap384_aes_client

  • handshake_no_resume_ring_1.3_ecdsap384_aes_server

  • transfer_no_resume_ring_1.3_ecdsap256_chacha_client

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap384_aes_server

  • handshake_tickets_ring_1.3_ecdsap256_aes_server

  • handshake_no_resume_ring_1.3_ecdsap384_chacha_server

  • handshake_tickets_aws_lc_rs_1.3_ecdsap256_aes_server

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap256_chacha_server

  • transfer_no_resume_ring_1.3_ecdsap384_aes_server

  • handshake_session_id_aws_lc_rs_1.3_ecdsap256_chacha_client

  • handshake_tickets_ring_1.3_ecdsap384_aes_server

  • handshake_tickets_aws_lc_rs_1.3_ecdsap384_chacha_client

  • handshake_session_id_aws_lc_rs_1.3_ecdsap384_chacha_client

  • handshake_tickets_ring_1.3_ecdsap256_aes_client

  • handshake_session_id_ring_1.3_ecdsap384_chacha_client

  • handshake_session_id_aws_lc_rs_1.3_ecdsap256_aes_client

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap256_chacha_client

  • handshake_session_id_ring_1.3_ecdsap384_aes_server

  • handshake_no_resume_ring_1.3_ecdsap384_chacha_client

  • handshake_session_id_aws_lc_rs_1.3_ecdsap256_aes_server

  • handshake_no_resume_ring_1.3_ecdsap256_chacha_client

  • handshake_session_id_ring_1.3_ecdsap256_chacha_server

  • handshake_tickets_ring_1.3_ecdsap256_chacha_client

  • handshake_no_resume_ring_1.3_ecdsap256_aes_server

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap384_aes_client

  • handshake_session_id_ring_1.3_ecdsap256_aes_client

  • handshake_session_id_aws_lc_rs_1.3_ecdsap384_aes_client

  • handshake_tickets_aws_lc_rs_1.3_ecdsap384_aes_server

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap256_aes_server

  • transfer_no_resume_ring_1.3_ecdsap256_aes_client

  • handshake_tickets_aws_lc_rs_1.3_ecdsap256_aes_client

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap384_aes_client

  • handshake_tickets_aws_lc_rs_1.3_ecdsap384_aes_client

  • handshake_session_id_ring_1.3_ecdsap384_chacha_server

  • handshake_session_id_aws_lc_rs_1.3_ecdsap256_chacha_server

  • handshake_tickets_aws_lc_rs_1.3_ecdsap384_chacha_server

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap384_chacha_client

  • handshake_no_resume_ring_1.3_ecdsap256_aes_client

  • transfer_no_resume_ring_1.3_ecdsap256_chacha_server

  • handshake_session_id_ring_1.3_ecdsap256_aes_server

Significant differences

⚠️ There are significant instruction count differences

Click to expand
Scenario Baseline Candidate Diff Threshold
handshake_tickets_aws_lc_rs_1.2_rsa_aes_client 4507768 4525649 ⚠️ 17881 (0.40%) 0.20%
handshake_session_id_ring_1.2_rsa_aes_client 4442134 4455025 ⚠️ 12891 (0.29%) 0.20%
handshake_session_id_ring_1.2_rsa_aes_server 4429334 4417654 ✅ -11680 (-0.26%) 0.20%
handshake_tickets_ring_1.2_rsa_aes_client 4731640 4719970 ✅ -11670 (-0.25%) 0.20%
handshake_tickets_ring_1.3_rsa_chacha_server 43869444 43960183 ⚠️ 90739 (0.21%) 0.20%
handshake_tickets_ring_1.3_rsa_aes_server 43909165 43999548 ⚠️ 90383 (0.21%) 0.20%

Other differences

Click to expand
Scenario Baseline Candidate Diff Threshold
handshake_session_id_aws_lc_rs_1.2_rsa_aes_server 4152416 4129526 -22890 (-0.55%) 4.42%
handshake_session_id_aws_lc_rs_1.3_rsa_aes_server 33342953 33276914 -66039 (-0.20%) 0.42%
handshake_session_id_aws_lc_rs_1.3_rsa_chacha_server 33275087 33340268 65181 (0.20%) 0.70%
handshake_tickets_aws_lc_rs_1.3_rsa_aes_server 33625016 33560858 -64158 (-0.19%) 0.49%
handshake_no_resume_aws_lc_rs_1.3_rsa_chacha_server 12736683 12714138 -22545 (-0.18%) 1.34%
handshake_no_resume_aws_lc_rs_1.3_rsa_aes_server 12721623 12743828 22205 (0.17%) 0.89%
transfer_no_resume_aws_lc_rs_1.3_rsa_aes_server 57206975 57131546 -75429 (-0.13%) 0.33%
handshake_session_id_aws_lc_rs_1.3_rsa_chacha_client 31117313 31153091 35778 (0.11%) 0.20%
handshake_tickets_aws_lc_rs_1.3_rsa_chacha_server 33599004 33560686 -38318 (-0.11%) 0.49%
handshake_tickets_ring_1.2_rsa_aes_server 4869373 4864096 -5277 (-0.11%) 0.20%
transfer_no_resume_aws_lc_rs_1.2_rsa_aes_client 68413694 68451351 37657 (0.06%) 0.20%
handshake_session_id_aws_lc_rs_1.2_rsa_aes_client 4169109 4167006 -2103 (-0.05%) 0.20%
handshake_tickets_aws_lc_rs_1.2_rsa_aes_server 4647950 4650269 2319 (0.05%) 3.96%
transfer_no_resume_aws_lc_rs_1.2_rsa_aes_server 57100461 57073595 -26866 (-0.05%) 0.39%
handshake_no_resume_aws_lc_rs_1.2_rsa_aes_server 12329859 12335363 5504 (0.04%) 1.24%
transfer_no_resume_aws_lc_rs_1.3_rsa_chacha_client 92429391 92466929 37538 (0.04%) 0.20%
transfer_no_resume_ring_1.3_rsa_aes_server 57104071 57083227 -20844 (-0.04%) 0.20%
transfer_no_resume_aws_lc_rs_1.3_rsa_chacha_server 91362733 91389177 26444 (0.03%) 0.20%
handshake_session_id_ring_1.3_rsa_aes_client 42237705 42249204 11499 (0.03%) 0.20%
handshake_session_id_ring_1.3_rsa_chacha_client 42190938 42202333 11395 (0.03%) 0.20%
transfer_no_resume_ring_1.2_rsa_aes_server 56968231 56953887 -14344 (-0.03%) 0.20%
handshake_tickets_aws_lc_rs_1.3_rsa_chacha_client 31337245 31344968 7723 (0.02%) 0.20%
transfer_no_resume_ring_1.3_rsa_chacha_server 91268589 91247948 -20641 (-0.02%) 0.20%
handshake_session_id_ring_1.3_rsa_chacha_server 43619205 43627787 8582 (0.02%) 0.20%
handshake_session_id_aws_lc_rs_1.3_rsa_aes_client 31166133 31172143 6010 (0.02%) 0.20%
transfer_no_resume_ring_1.2_rsa_aes_client 57844945 57833931 -11014 (-0.02%) 0.20%
handshake_session_id_ring_1.3_rsa_aes_server 43685298 43693275 7977 (0.02%) 0.20%
handshake_tickets_ring_1.3_rsa_aes_client 42418396 42425176 6780 (0.02%) 0.20%
handshake_tickets_ring_1.3_rsa_chacha_client 42383385 42389915 6530 (0.02%) 0.20%
handshake_no_resume_ring_1.2_rsa_aes_client 4442883 4442204 -679 (-0.02%) 0.20%
handshake_no_resume_aws_lc_rs_1.3_rsa_chacha_client 3388857 3389340 483 (0.01%) 0.20%
handshake_no_resume_aws_lc_rs_1.2_rsa_aes_client 3174292 3173854 -438 (-0.01%) 0.20%
handshake_no_resume_aws_lc_rs_1.3_rsa_aes_client 3378237 3377887 -350 (-0.01%) 0.20%
handshake_no_resume_ring_1.2_rsa_aes_server 12045886 12047128 1242 (0.01%) 0.20%
transfer_no_resume_aws_lc_rs_1.3_rsa_aes_client 57978852 57974638 -4214 (-0.01%) 0.20%
handshake_tickets_aws_lc_rs_1.3_rsa_aes_client 31355333 31357478 2145 (0.01%) 0.20%
handshake_no_resume_ring_1.3_rsa_aes_client 4539369 4539099 -270 (-0.01%) 0.20%
handshake_no_resume_ring_1.3_rsa_aes_server 12241792 12242358 566 (0.00%) 0.20%
transfer_no_resume_ring_1.3_rsa_chacha_client 92384897 92380946 -3951 (-0.00%) 0.20%
handshake_no_resume_ring_1.3_rsa_chacha_client 4549267 4549079 -188 (-0.00%) 0.20%
transfer_no_resume_ring_1.3_rsa_aes_client 57944947 57943874 -1073 (-0.00%) 0.20%
handshake_no_resume_ring_1.3_rsa_chacha_server 12251795 12251954 159 (0.00%) 0.20%

Wall-time

⚠️ Missing benchmarks

The following benchmark scenarios are present in the candidate but not in the baseline:

  • handshake_tickets_ring_1.3_ecdsap384_aes

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap384_chacha

  • handshake_session_id_ring_1.3_ecdsap384_aes

  • handshake_no_resume_ring_1.3_ecdsap256_aes

  • transfer_no_resume_ring_1.3_ecdsap384_chacha

  • handshake_session_id_aws_lc_rs_1.3_ecdsap256_aes

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap256_aes

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap256_chacha

  • handshake_session_id_aws_lc_rs_1.3_ecdsap384_chacha

  • handshake_tickets_ring_1.3_ecdsap256_aes

  • handshake_tickets_aws_lc_rs_1.3_ecdsap384_chacha

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap256_aes

  • handshake_session_id_aws_lc_rs_1.3_ecdsap384_aes

  • handshake_session_id_ring_1.3_ecdsap256_aes

  • handshake_no_resume_ring_1.3_ecdsap256_chacha

  • handshake_no_resume_ring_1.3_ecdsap384_chacha

  • handshake_tickets_aws_lc_rs_1.3_ecdsap384_aes

  • transfer_no_resume_ring_1.3_ecdsap256_chacha

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap384_chacha

  • handshake_tickets_aws_lc_rs_1.3_ecdsap256_aes

  • handshake_tickets_ring_1.3_ecdsap384_chacha

  • handshake_no_resume_ring_1.3_ecdsap384_aes

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap384_aes

  • handshake_tickets_ring_1.3_ecdsap256_chacha

  • handshake_no_resume_aws_lc_rs_1.3_ecdsap256_chacha

  • handshake_session_id_aws_lc_rs_1.3_ecdsap256_chacha

  • handshake_tickets_aws_lc_rs_1.3_ecdsap256_chacha

  • transfer_no_resume_ring_1.3_ecdsap384_aes

  • handshake_session_id_ring_1.3_ecdsap384_chacha

  • transfer_no_resume_ring_1.3_ecdsap256_aes

  • transfer_no_resume_aws_lc_rs_1.3_ecdsap384_aes

  • handshake_session_id_ring_1.3_ecdsap256_chacha

Significant differences

There are no significant wall-time differences

Other differences

Click to expand
Scenario Baseline Candidate Diff Threshold
handshake_no_resume_ring_1.2_rsa_aes 1.07 ms 1.08 ms 0.00 ms (0.39%) 1.74%
handshake_session_id_ring_1.3_rsa_aes 7.48 ms 7.50 ms 0.03 ms (0.36%) 1.00%
handshake_tickets_ring_1.3_rsa_aes 7.50 ms 7.52 ms 0.03 ms (0.35%) 1.20%
handshake_no_resume_ring_1.3_rsa_chacha 1.08 ms 1.09 ms 0.00 ms (0.32%) 1.64%
transfer_no_resume_aws_lc_rs_1.2_rsa_aes 5.88 ms 5.90 ms 0.02 ms (0.31%) 5.05%
handshake_session_id_ring_1.3_rsa_chacha 7.45 ms 7.47 ms 0.02 ms (0.31%) 1.61%
handshake_no_resume_ring_1.3_rsa_aes 1.08 ms 1.08 ms 0.00 ms (0.28%) 1.55%
handshake_tickets_ring_1.3_rsa_chacha 7.46 ms 7.48 ms 0.02 ms (0.26%) 1.79%
handshake_session_id_ring_1.2_rsa_aes 1.72 ms 1.72 ms 0.00 ms (0.20%) 2.91%
transfer_no_resume_aws_lc_rs_1.3_rsa_aes 5.88 ms 5.89 ms 0.01 ms (0.18%) 6.04%
handshake_no_resume_aws_lc_rs_1.3_rsa_chacha 1.40 ms 1.40 ms -0.00 ms (-0.18%) 1.38%
transfer_no_resume_ring_1.3_rsa_aes 7.30 ms 7.31 ms 0.01 ms (0.17%) 4.21%
handshake_tickets_aws_lc_rs_1.2_rsa_aes 2.26 ms 2.26 ms -0.00 ms (-0.16%) 2.29%
handshake_session_id_aws_lc_rs_1.2_rsa_aes 2.09 ms 2.09 ms -0.00 ms (-0.15%) 1.76%
transfer_no_resume_ring_1.2_rsa_aes 7.22 ms 7.23 ms 0.01 ms (0.10%) 4.36%
transfer_no_resume_ring_1.3_rsa_chacha 14.01 ms 14.02 ms 0.01 ms (0.10%) 2.42%
handshake_tickets_ring_1.2_rsa_aes 1.80 ms 1.80 ms 0.00 ms (0.10%) 2.29%
handshake_tickets_aws_lc_rs_1.3_rsa_chacha 6.39 ms 6.39 ms -0.01 ms (-0.09%) 1.00%
handshake_session_id_aws_lc_rs_1.3_rsa_aes 6.37 ms 6.37 ms -0.01 ms (-0.08%) 1.23%
handshake_no_resume_aws_lc_rs_1.2_rsa_aes 1.35 ms 1.35 ms 0.00 ms (0.05%) 1.06%
transfer_no_resume_aws_lc_rs_1.3_rsa_chacha 14.37 ms 14.37 ms 0.01 ms (0.04%) 2.36%
handshake_session_id_aws_lc_rs_1.3_rsa_chacha 6.36 ms 6.36 ms -0.00 ms (-0.04%) 1.03%
handshake_no_resume_aws_lc_rs_1.3_rsa_aes 1.41 ms 1.41 ms -0.00 ms (-0.03%) 1.20%
handshake_tickets_aws_lc_rs_1.3_rsa_aes 6.40 ms 6.40 ms 0.00 ms (0.02%) 1.08%

Additional information

Historical results

Checkout details:

@ctz ctz force-pushed the jbp-ecdsa-sha512-signatures branch 2 times, most recently from 49e8bff to 9bf0275 Compare January 3, 2024 12:16
@ctz ctz marked this pull request as ready for review January 3, 2024 13:22
@ctz
Copy link
Copy Markdown
Member Author

ctz commented Jan 3, 2024

I think this is ready for review, though the first commit would need to be dropped before it can be merged.

cpu
cpu approved these changes Jan 3, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thanks!

That is achieved by -- with my sincere apologies -- applying the C preprocessor.

I laughed, but it seems like a reasonable choice in this instance.

rustls/tests/common/mod.rs
.unwrap()
}

pub fn ca_distinguished_name(&self) -> &'static [u8] {
Copy link
Copy Markdown
Member

@cpu cpu Jan 3, 2024

Choose a reason for hiding this comment

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

Thanks for tidying this up :-)

@ctz ctz force-pushed the jbp-ecdsa-sha512-signatures branch from 1988323 to 4d0ccf7 Compare January 3, 2024 16:10
ctz added 10 commits January 3, 2024 16:20
@ctz
Depend on rustls-webpki 0.102.1
6b3e432
@ctz
aws-lc-rs: support verifying with ECDSA_P521_SHA512
2086d48
@ctz
Split off crypto/aws_lc_rs/sign.rs
2b1ba0d
@ctz
aws_lc_rs::sign: note route to remove SEC1 hack
0632ede
@ctz
aws_lc_rs::sign: add support for NISTP521 ECDSA keys
aebf676
@ctz
Split test-ca ecdsa by curve; add p521
c033596 
This goes from being a single set of keys for ECDSA (with a
purposeful mix of curves) to a set of keys per curve.

That means we can avoid P521 chains in tests when it is not supported.

In those tests, reflect this as additional `KeyType` variants.
@ctz
Avoid extraenous .iter() in for loops
b12e44e 
clippy was complaining about manual `.into_iter()` calls, but actually
the manual `.iter()` calls are also not very idiomatic.
@ctz
De-duplicate knowledge of test-ca/ CA names
746e62b
@ctz
Test P521-SHA512 in bogo
71be83e 
This makes it possible for our bogo config.json to vary
between providers.  That is achieved by -- with my sincere apologies --
applying the C preprocessor.
@ctz
ci-bench: separately bench use of P384 curve
839e95a 
This renames the P256 cases, so will introduce a
discontinuity in results tracking.
@ctz ctz force-pushed the jbp-ecdsa-sha512-signatures branch from 4d0ccf7 to 839e95a Compare January 3, 2024 16:20
@ctz ctz added this pull request to the merge queue Jan 4, 2024
Merged via the queue into main with commit f8cd4e8 Jan 4, 2024
@ctz ctz deleted the jbp-ecdsa-sha512-signatures branch January 4, 2024 09:30
@djc
Copy link
Copy Markdown
Member

djc commented Jan 4, 2024

Can/should we backport this to 0.22?

@ctz ctz mentioned this pull request Jan 5, 2024
Merged
@cpu cpu mentioned this pull request Jan 5, 2024
Merged
@ctz ctz mentioned this pull request Jan 5, 2024
Merged
@cpu cpu mentioned this pull request Feb 28, 2024
Merged
@ctz ctz mentioned this pull request Mar 7, 2024
Closed
@kcon-stackav kcon-stackav mentioned this pull request Jun 25, 2024
Open
@ostrolucky ostrolucky mentioned this pull request Jul 11, 2024
Open
@dfunkt dfunkt mentioned this pull request Dec 15, 2025
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpu cpu cpu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

P-521 with aws-lc-rs

3 participants

@ctz @djc @cpu