Skip to content

Address BouncyCastle's deprecated AESFastEngine usage #16164

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
Merged
merged 3 commits into from
May 7, 2025
Merged

Address BouncyCastle's deprecated AESFastEngine usage #16164

merged 3 commits into from
May 7, 2025

Conversation

fjacobs
Copy link
Contributor

@fjacobs fjacobs commented Nov 25, 2024

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 25, 2024
@fjacobs fjacobs force-pushed the updateAESEngine branch 2 times, most recently from c1d94bf to 41a3736 Compare November 25, 2024 20:21
@sjohnr sjohnr self-assigned this Nov 26, 2024
@sjohnr sjohnr added in: crypto An issue in spring-security-crypto type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 26, 2024
@sjohnr
Copy link
Contributor

sjohnr commented Nov 26, 2024

@fjacobs thanks for opening this. I have reviewed the release notes you referenced and agree that the recommendation in certain cases is to use AESEngine. However, I think this could be at a minimum a performance impact on applications, so I am not certain we want to apply this without considering that. I'm unfamiliar with the runtime performance characteristics of the different engines other than what is stated in the docs about three levels of performance (slow, middle, fast). Do you have a sense of what the impact of this change might be in terms of performance, or an idea of how we might best gauge that?

@sjohnr sjohnr added the status: waiting-for-feedback We need additional information before we can continue label Nov 26, 2024
@sjohnr sjohnr removed the status: waiting-for-feedback We need additional information before we can continue label Dec 18, 2024
@sjohnr
Copy link
Contributor

sjohnr commented Dec 18, 2024

@fjacobs do you have any thoughts regarding the above comment?

@sjohnr sjohnr added the status: waiting-for-feedback We need additional information before we can continue label Dec 18, 2024
sjohnr added a commit to fjacobs/spring-security that referenced this pull request Feb 3, 2025
@sjohnr
Polish spring-projectsgh-16164
16fdfb1
@rwinch rwinch assigned rwinch and unassigned sjohnr May 7, 2025
@rwinch rwinch added type: breaks-passivity A change that breaks passivity with the previous release and removed status: waiting-for-feedback We need additional information before we can continue labels May 7, 2025
@rwinch rwinch added this to the 7.0.0-M1 milestone May 7, 2025
fjacobs and others added 3 commits May 7, 2025 10:56
@fjacobs @rwinch
Replace BouncyCastle's deprecated AESFastEngine with the default AESE…
ccc8386 
…ngine

- Update AESEngine to use the default AES engine, following BouncyCastle's recommendations
  (see release-1-56 of changelog: https://www.bouncycastle.org/download/bouncy-castle-java/?filter=java%3Drelease-1-56).
- Migrate to the latest API 'newInstance()' method to allow removal of @SuppressWarnings("deprecation")
- Remove @SuppressWarnings("deprecation")
@sjohnr @rwinch
Polish spring-projectsgh-16164
85c00e8
@rwinch
Remove Unnecessary Backwards Compatability
c9c064c 
Since this is going to be merged into Spring Security 7 (a major release) and AESFastEngine is deprecated,
we should no longer support it (as it will likely be removed from Bouncy Castle)
@rwinch rwinch force-pushed the updateAESEngine branch from 16fdfb1 to c9c064c Compare May 7, 2025 15:56
@rwinch rwinch enabled auto-merge (rebase) May 7, 2025 15:57
@rwinch rwinch disabled auto-merge May 7, 2025 15:57
@rwinch
Copy link
Member

rwinch commented May 7, 2025

Thanks for the PR @fjacobs! I've rebased this from main and removed the compatibility checks since this will be merged into Spring Security 7 and I'd prefer the deprecated usage to be removed entirely.

This will be merged into main as soon as the build passes. Thanks again for your contribution 😄

@rwinch rwinch merged commit d52289b into spring-projects:main May 7, 2025
4 checks passed
rwinch pushed a commit that referenced this pull request May 7, 2025
@sjohnr @rwinch
Polish gh-16164
5eb232c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@rwinch rwinch

Labels
in: crypto An issue in spring-security-crypto type: breaks-passivity A change that breaks passivity with the previous release type: enhancement A general enhancement
Projects
None yet
Milestone
7.0.0-M1
Development

Successfully merging this pull request may close these issues.
4 participants
@fjacobs @sjohnr @rwinch @spring-projects-issues