Skip to content

Replace Bouncy Castle AES-CTR implementation#5218

Closed
rubo wants to merge 18 commits into
masterfrom
feature/aes-ctr
Closed

Replace Bouncy Castle AES-CTR implementation#5218
rubo wants to merge 18 commits into
masterfrom
feature/aes-ctr

Conversation

@rubo

@rubo rubo commented Jan 29, 2023

Copy link
Copy Markdown
Contributor

See #5207

Changes

Implemented AES-CTR using .NET's own AES implementation replacing the one of Bouncy Castle

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Remarks

Triggered by #5196

@LukaszRozmej

Copy link
Copy Markdown
Member

Do you know what are performance implications?

@tanishqjasoria

Copy link
Copy Markdown
Contributor

Also is there a way we can benchmark on different machines - like with and without hardware acceleration enabled?

@rubo

rubo commented Jan 30, 2023

Copy link
Copy Markdown
Contributor Author

Do you know what are performance implications?

@LukaszRozmej I haven't done any benchmarks yet. Besides speed, native OS implementations are also considered more secure as they are usually FIPS-certified.

Also is there a way we can benchmark on different machines - like with and without hardware acceleration enabled?

@tanishqjasoria I'm not sure it's possible to make a fair comparison.

In general, this PR is a part of an effort of switching to native OS implementations from legacy code whenever possible.

@tanishqjasoria

Copy link
Copy Markdown
Contributor

do you mean .NET's implementation when you say native OS implementations?

@tanishqjasoria I'm not sure it's possible to make a fair comparison.

i meant we should also check that we are not moving to slower implementation when there are no hardware intrinsics used.

@rubo

rubo commented Jan 31, 2023

Copy link
Copy Markdown
Contributor Author

do you mean .NET's implementation when you say native OS implementations?

I do. .NET uses native OS implementation under the hood.

i meant we should also check that we are not moving to slower implementation when there are no hardware intrinsics used.

So far we haven't used any hardware acceleration for that and no one was concerned. Suddenly, there are concerns about the performance? That's weird given the fact that we're moving from a very old Bouncy Castle implementation to a modern one by Microsoft.

@LukaszRozmej

Copy link
Copy Markdown
Member

Can you do some basic x86 (ARM would be nice too) performance check? Nothing major just some simple sanity one.

@rubo rubo removed the wip Work in Progress label Mar 28, 2023
@rubo rubo closed this Mar 5, 2026
@rubo rubo deleted the feature/aes-ctr branch March 5, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants