[MAGNOLIA-5248] Memory-Leak/Classloader errors because BouncyCastleProvider is registered for all WARs Created: 21/Aug/13  Updated: 25/Oct/13  Resolved: 21/Oct/13

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 4.5.10, 5.0.2
Fix Version/s: 4.5.13, 5.1.1

Type: Bug Priority: Major
Reporter: Markus Grieder Assignee: Milan Divilek
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

all os


Attachments: Text File directUseBouncyCastle.patch    
Issue Links:
Cloners
is cloned by MGNLLIC-47 CLONE - Memory-Leak/Classloader error... Closed
Relates
relates to MAGNOLIA-5345 Doubled bouncycastle libraries Closed
Template:
Patch included:
Yes
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: 5.1.1

 Description   

The registration of BouncyCastleProvider in SecurityUtil (core) with "Security.addProvider(new BouncyCastleProvider());" is changing JVM-settings and therefore all WARs in an Appserver are seeing this registration and classes of BouncyCastle.
If another WAR wants to use another version of BouncyCastle this can lead to Classloader issues. The Provider is also not removed on WAR-undeploy -> Memory-Leak.

In a WAR "Security.addProvider()" should be never used, because of all side-effects for other WARs ->

A simple fix would be:
private static final BouncyCastleProvider PROVIDER = new BouncyCastleProvider();

and then instead of Cipher.getInstance(ALGORITHM, "BC") -> Cipher.getInstance(ALGORITHM, PROVIDER)
or for KeyFactory
KeyFactory.getInstance(ALGORITHM, PROVIDER)

------------------------------------
If nobody is depending on the BouncyCastleProvider registration this fix should give no problems.

BouncyCastleProvider was introduced in MAGNOLIA-3904 -> by the way, I think only the dependency "bcprov-jdk16" is needed. I don't see a reason for "bcprov-ext-jdk16" and "bcpg-jdk16" introduced from MAGNOLIA-3904: bcprov-ext-jdk16 contains all classes of bcprov-jdk16 + IDEA cipher -> duplicate. bcpg-jdk16 -> Current code in core is not using OpenPGP

Attached is a patch for 4.5.10 which goes a step further: Using directly BouncyCastle instead of JCE-Wrapper-Classes, but it needs >= 1.48 of BouncyCastle for the Keygeneration. Except some error-messages the behavior should be the same as before (tested: patched author instance could communicate with an old public-instance).



 Comments   
Comment by Tobias Mattsson [ 16/Oct/13 ]

Needs backport to 4.5.13, please also update fix-version.

Comment by Jan Haderka [ 18/Oct/13 ]

Test. The one you removed could be easily repurposed to check that there's no system wide registered BC provider before loading SecurityUtil and also that there is non after loading that class.

Comment by Daniel Lipp [ 22/Oct/13 ]

Removing the "ext" library was done by the above ticket.

Generated at Mon Feb 12 04:03:22 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.