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.

Sign up for GitHub

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

Jump to bottom

Porting combined errors from mbedtls changes #372

Merged
merged 11 commits into from
Nov 14, 2024
Merged

Porting combined errors from mbedtls changes #372

merged 11 commits into from
Nov 14, 2024

Conversation

mridul-manohar
Copy link
Contributor

Due to downgrade back to v2.28 (from ~3.X), the enhancement in PR #271 was lost in the mbedtls 2.8 branch being used.
we need to port these combined errors from mbedtls changes back to the mbedtls 2.8 branch and build with the latest upgrade in rust toolchain version to apply the enhancement.
This PR ports said changes to a latest branch forked from mbedtls master and builds the same on latest rustc 1.83.0-nightly (26d8e9255 2024-10-11) version.
https://fortanix.atlassian.net/browse/PROD-9297

@github-actions GitHub Actions
Copy link

Benchmark for 1755351

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 279.6±12.79ns 283.6±18.31ns +1.43%
Cipher/AES CBC encrypt 221.8±9.72ns 210.1±9.41ns -5.28%
Cipher/AES CCM decrypt 412.4±9.18ns 410.9±15.84ns -0.36%
Cipher/AES CCM encrypt 401.3±24.26ns 377.7±19.20ns -5.88%
Cipher/AES GCM decrypt 572.7±16.08ns 573.3±23.65ns +0.10%
Cipher/AES GCM encrypt 538.1±14.68ns 541.6±19.36ns +0.65%
Cipher/AES KW decrypt 698.2±19.58ns 676.1±19.61ns -3.17%
Cipher/AES KW encrypt 641.7±21.30ns 627.4±22.44ns -2.23%
Cipher/AES KWP decrypt 712.8±23.54ns 704.4±9.90ns -1.18%
Cipher/AES KWP encrypt 648.8±21.04ns 654.1±23.07ns +0.82%

@Taowyoo
Copy link
Collaborator

Taowyoo commented Nov 11, 2024

From CI, it seems cargo test +stable --no-run --target=x86_64-fortanix-unknown-sgx under mbedtls folder is failing.

Taowyoo
Taowyoo reviewed Nov 11, 2024
View reviewed changes
mbedtls/src/bignum/mod.rs Outdated Show resolved Hide resolved
Taowyoo
Taowyoo reviewed Nov 11, 2024
View reviewed changes
Copy link
Collaborator

@Taowyoo Taowyoo left a comment

Choose a reason for hiding this comment

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

I saw some unaligned spaces. And Rustfmt CI is failed. Please run

  cargo fmt -p mbedtls
  cargo fmt -p mbedtls-platform-support
  cargo fmt -p mbedtls-sys-auto

to ensure everything is formated

mbedtls/src/bignum/mod.rs Outdated Show resolved Hide resolved
mbedtls/src/pk/mod.rs Show resolved Hide resolved
mbedtls/Cargo.toml Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

Benchmark for caeb784

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 280.7±10.53ns 294.0±12.88ns +4.74%
Cipher/AES CBC encrypt 222.3±12.05ns 209.9±7.65ns -5.58%
Cipher/AES CCM decrypt 414.8±19.92ns 412.5±14.99ns -0.55%
Cipher/AES CCM encrypt 395.8±14.45ns 389.2±20.33ns -1.67%
Cipher/AES GCM decrypt 579.7±16.24ns 546.6±16.50ns -5.71%
Cipher/AES GCM encrypt 545.4±14.18ns 557.9±78.06ns +2.29%
Cipher/AES KW decrypt 696.5±13.64ns 692.6±19.06ns -0.56%
Cipher/AES KW encrypt 642.8±18.68ns 636.2±11.30ns -1.03%
Cipher/AES KWP decrypt 713.9±25.28ns 702.6±20.82ns -1.58%
Cipher/AES KWP encrypt 637.3±19.02ns 642.4±22.31ns +0.80%

@github-actions GitHub Actions
Copy link

Benchmark for a24ebf5

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 284.1±22.34ns 292.8±18.86ns +3.06%
Cipher/AES CBC encrypt 221.8±8.27ns 211.6±5.47ns -4.60%
Cipher/AES CCM decrypt 415.3±20.63ns 413.6±15.16ns -0.41%
Cipher/AES CCM encrypt 385.6±14.69ns 384.8±23.50ns -0.21%
Cipher/AES GCM decrypt 567.0±35.09ns 548.2±23.13ns -3.32%
Cipher/AES GCM encrypt 543.7±58.49ns 556.7±16.99ns +2.39%
Cipher/AES KW decrypt 699.5±18.38ns 690.1±16.15ns -1.34%
Cipher/AES KW encrypt 644.7±17.37ns 629.8±24.44ns -2.31%
Cipher/AES KWP decrypt 712.9±18.78ns 702.2±33.67ns -1.50%
Cipher/AES KWP encrypt 650.0±16.09ns 633.3±16.05ns -2.57%

@github-actions GitHub Actions
Copy link

Benchmark for 92f7c2e

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 303.6±19.03ns 281.6±25.79ns -7.25%
Cipher/AES CBC encrypt 222.6±13.43ns 212.7±11.93ns -4.45%
Cipher/AES CCM decrypt 414.1±12.49ns 415.5±29.01ns +0.34%
Cipher/AES CCM encrypt 383.4±21.22ns 379.5±33.52ns -1.02%
Cipher/AES GCM decrypt 557.2±18.28ns 561.6±21.99ns +0.79%
Cipher/AES GCM encrypt 540.5±28.89ns 563.6±19.55ns +4.27%
Cipher/AES KW decrypt 683.1±12.88ns 693.1±37.75ns +1.46%
Cipher/AES KW encrypt 634.4±35.31ns 641.8±19.44ns +1.17%
Cipher/AES KWP decrypt 697.7±12.64ns 699.7±18.59ns +0.29%
Cipher/AES KWP encrypt 639.8±18.21ns 643.9±22.31ns +0.64%

@github-actions GitHub Actions
Copy link

Benchmark for 744d058

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 286.5±12.58ns 278.3±10.41ns -2.86%
Cipher/AES CBC encrypt 231.4±12.85ns 212.3±10.58ns -8.25%
Cipher/AES CCM decrypt 418.8±19.55ns 407.1±11.84ns -2.79%
Cipher/AES CCM encrypt 383.9±23.37ns 368.7±17.65ns -3.96%
Cipher/AES GCM decrypt 586.9±21.52ns 562.7±18.47ns -4.12%
Cipher/AES GCM encrypt 546.4±24.59ns 562.0±18.16ns +2.86%
Cipher/AES KW decrypt 699.3±15.72ns 691.9±18.19ns -1.06%
Cipher/AES KW encrypt 646.6±19.80ns 644.9±21.00ns -0.26%
Cipher/AES KWP decrypt 711.0±15.35ns 702.1±25.25ns -1.25%
Cipher/AES KWP encrypt 649.0±18.32ns 643.5±17.06ns -0.85%

@github-actions GitHub Actions
Copy link

Benchmark for c98f301

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 281.7±9.72ns 282.8±7.91ns +0.39%
Cipher/AES CBC encrypt 221.0±5.96ns 213.6±12.66ns -3.35%
Cipher/AES CCM decrypt 414.0±16.39ns 398.0±15.51ns -3.86%
Cipher/AES CCM encrypt 383.7±15.94ns 375.5±19.99ns -2.14%
Cipher/AES GCM decrypt 577.5±18.79ns 545.3±12.22ns -5.58%
Cipher/AES GCM encrypt 536.7±15.92ns 556.7±56.29ns +3.73%
Cipher/AES KW decrypt 699.2±12.45ns 675.6±13.45ns -3.38%
Cipher/AES KW encrypt 645.3±24.15ns 629.9±35.95ns -2.39%
Cipher/AES KWP decrypt 710.2±15.03ns 688.3±21.17ns -3.08%
Cipher/AES KWP encrypt 646.6±18.98ns 633.3±22.56ns -2.06%

@github-actions GitHub Actions
Copy link

Benchmark for 0171e72

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 290.0±6.08ns 283.1±10.84ns -2.38%
Cipher/AES CBC encrypt 221.7±7.18ns 209.7±5.53ns -5.41%
Cipher/AES CCM decrypt 413.2±16.47ns 411.5±23.68ns -0.41%
Cipher/AES CCM encrypt 388.4±19.26ns 388.4±26.71ns 0.00%
Cipher/AES GCM decrypt 575.3±12.82ns 570.4±24.04ns -0.85%
Cipher/AES GCM encrypt 549.2±15.85ns 566.5±26.06ns +3.15%
Cipher/AES KW decrypt 695.1±13.11ns 687.7±9.74ns -1.06%
Cipher/AES KW encrypt 644.0±20.62ns 627.7±15.76ns -2.53%
Cipher/AES KWP decrypt 715.6±17.85ns 698.4±11.63ns -2.40%
Cipher/AES KWP encrypt 650.9±17.90ns 642.2±20.51ns -1.34%

@github-actions GitHub Actions
Copy link

Benchmark for ac5f25f

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 289.5±8.70ns 278.4±8.65ns -3.83%
Cipher/AES CBC encrypt 221.3±5.72ns 209.7±8.69ns -5.24%
Cipher/AES CCM decrypt 411.5±15.76ns 411.8±16.51ns +0.07%
Cipher/AES CCM encrypt 397.1±14.65ns 388.0±18.11ns -2.29%
Cipher/AES GCM decrypt 563.5±26.90ns 580.5±18.37ns +3.02%
Cipher/AES GCM encrypt 564.2±26.47ns 542.5±14.04ns -3.85%
Cipher/AES KW decrypt 684.3±23.38ns 688.6±15.26ns +0.63%
Cipher/AES KW encrypt 633.2±40.80ns 642.1±15.97ns +1.41%
Cipher/AES KWP decrypt 699.4±15.93ns 699.9±11.02ns +0.07%
Cipher/AES KWP encrypt 637.4±21.59ns 643.3±16.33ns +0.93%

Taowyoo
Taowyoo reviewed Nov 12, 2024
View reviewed changes
mbedtls/src/cipher/raw/mod.rs Show resolved Hide resolved
mbedtls/src/hash/mod.rs Show resolved Hide resolved
mbedtls/src/pk/dsa/mod.rs Outdated Show resolved Hide resolved
mbedtls/src/ssl/config.rs Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

Benchmark for 051834e

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 294.4±12.69ns 289.1±11.56ns -1.80%
Cipher/AES CBC encrypt 221.0±4.95ns 212.5±7.75ns -3.85%
Cipher/AES CCM decrypt 415.7±20.38ns 411.8±18.34ns -0.94%
Cipher/AES CCM encrypt 371.1±38.66ns 392.1±29.92ns +5.66%
Cipher/AES GCM decrypt 560.2±29.03ns 563.4±11.54ns +0.57%
Cipher/AES GCM encrypt 546.2±18.95ns 562.3±22.34ns +2.95%
Cipher/AES KW decrypt 695.2±11.71ns 694.1±49.87ns -0.16%
Cipher/AES KW encrypt 647.8±53.06ns 637.6±18.32ns -1.57%
Cipher/AES KWP decrypt 708.4±17.36ns 698.0±16.28ns -1.47%
Cipher/AES KWP encrypt 651.3±15.97ns 642.8±20.57ns -1.31%

@Taowyoo Taowyoo self-requested a review November 13, 2024 19:51
Taowyoo
Taowyoo reviewed Nov 13, 2024
View reviewed changes
Copy link
Collaborator

@Taowyoo Taowyoo left a comment

Choose a reason for hiding this comment

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

LGTM,

@Taowyoo Taowyoo changed the title Porting combined errors from mbedtls changes [PROD-9297] Porting combined errors from mbedtls changes Nov 13, 2024
@Taowyoo Taowyoo changed the title [PROD-9297] Porting combined errors from mbedtls changes PROD-9297 Porting combined errors from mbedtls changes Nov 13, 2024
@Taowyoo Taowyoo changed the title PROD-9297 Porting combined errors from mbedtls changes Porting combined errors from mbedtls changes Nov 13, 2024
zugzwang
zugzwang reviewed Nov 14, 2024
View reviewed changes
mbedtls/src/error.rs Outdated Show resolved Hide resolved
zugzwang
zugzwang reviewed Nov 14, 2024
View reviewed changes
mbedtls/src/error.rs Outdated Show resolved Hide resolved
zugzwang
zugzwang reviewed Nov 14, 2024
View reviewed changes
mbedtls/src/lib.rs
@@ -27,7 +27,7 @@ mod wrapper_macros;
// API
// ==============
pub mod bignum;
mod error;
pub mod error;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not in favor of this module export, but also part of the original PR

zugzwang
zugzwang approved these changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@zugzwang zugzwang left a comment

Choose a reason for hiding this comment

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

Thanks for the PR

@github-actions GitHub Actions
Copy link

Benchmark for 16f6773

Click to view benchmark
Test Base PR %
Cipher/AES CBC decrypt 286.5±8.62ns 292.1±11.55ns +1.95%
Cipher/AES CBC encrypt 222.6±12.82ns 209.6±4.34ns -5.84%
Cipher/AES CCM decrypt 411.3±15.17ns 409.7±15.71ns -0.39%
Cipher/AES CCM encrypt 380.8±15.84ns 382.7±25.72ns +0.50%
Cipher/AES GCM decrypt 577.7±19.95ns 585.4±23.56ns +1.33%
Cipher/AES GCM encrypt 545.0±21.27ns 557.2±26.43ns +2.24%
Cipher/AES KW decrypt 683.1±13.82ns 688.9±18.90ns +0.85%
Cipher/AES KW encrypt 632.1±21.33ns 649.7±18.72ns +2.78%
Cipher/AES KWP decrypt 697.2±19.18ns 689.8±25.93ns -1.06%
Cipher/AES KWP encrypt 636.5±13.80ns 644.0±16.61ns +1.18%

@Taowyoo Taowyoo enabled auto-merge (squash) November 14, 2024 17:39
@Taowyoo Taowyoo merged commit 9b0dbdb into master Nov 14, 2024
11 checks passed
@Taowyoo Taowyoo deleted the mridul/port-combined-err-changes branch November 14, 2024 17:44
This was referenced Nov 19, 2024
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zugzwang zugzwang zugzwang approved these changes

@Taowyoo Taowyoo Taowyoo approved these changes

@s-arash s-arash Awaiting requested review from s-arash

@NicholasO-codes NicholasO-codes Awaiting requested review from NicholasO-codes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mridul-manohar @Taowyoo @zugzwang