Remove Bouncy Castle dependency usage from PemUtils #1318
Conversation
- Added PEM format parsing in PemUtils - Added unit test for PemUtils - Removed Bouncy Castle Provider dependency from service common module - Removed Bouncy Castle Provider dependency from quarkus service module - Removed Bouncy Castle references from LICENSE and NOTICE files
exceptionfactory
requested review from
adutra,
ashvina,
dennishuo,
dimas-b,
eric-maynard,
jackye1995,
jbonofre,
vvcephei,
collado-mike and
snazy
as code owners
exceptionfactory
requested review from
RussellSpitzer,
takidau,
MonkeyCanCode,
flyrain and
ebyhr
as code owners
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
| @@ -46,9 +45,38 @@ private static byte[] parsePEMFile(Path pemPath) throws IOException { | |||
| throw new FileNotFoundException( | |||
| String.format("The file '%s' doesn't exist.", pemPath.toAbsolutePath())); | |||
| } | |||
| try (PemReader reader = new PemReader(Files.newBufferedReader(pemPath, UTF_8))) { | |||
| PemObject pemObject = reader.readPemObject(); | |||
There was a problem hiding this comment.
One general remark: the original readPemObject() method, in principle, only reads the first object in the PEM file:
With your changes, what happens when the PEM file contains more than one object? And what if it doesn't contain any? It might be good to add tests for these cases as well.
There was a problem hiding this comment.
Thanks for the feedback and noting the behavior of readPemObject(). I adjusted the new implementation to stop reading when finding a footer line, and added unit tests for that scenario, as well as an empty file, which throws an IOException.
quarkus/server/distribution/LICENSE
Outdated
| @@ -2050,12 +2050,6 @@ License (from POM): Apache License 2.0 - https://www.apache.org/licenses/LICENSE | |||
|
|
|||
| -------------------------------------------------------------------------------- | |||
|
|
|||
| Group: org.bouncycastle Name: bcprov-jdk18on Version: 1.80 | |||
There was a problem hiding this comment.
bouncycastle/bc-prov is still a dependency of polaris-quarkus-server (see dependencies of the runtimeClasspath Gradle configuration), so the removals in LICENSE/NOTICE are wrong.
There was a problem hiding this comment.
Thanks for noting the runtime transitive dependency on bcprov-jdk18on, I reverted the license and notice changes.
github-project-automation
bot
moved this from Ready to merge
to PRs In Progress
in Basic Kanban Board
|
I'm good here; @snazy do you have further remarks? |
This pull request removes use of the Bouncy Castle Provider library for the
PemUtilsclass for parsing public and private key files.The Bouncy Castle library includes a large number of cryptographic capabilities, some of which are duplicative of current capabilities in recent versions of Java. Parsing PEM-encoded files is the only direct use of Bouncy Castle in Polaris based on imported classes, so replacing this usage with an alternative solution removes the need for a significant direct dependency.
The
PemUtilsclass already uses standard Java Security components for parsing decoded public keys and private key material, so the changes are scoped to replacing the initial PEM content parsing and Base64 decoding.The changes include a new
PemUtilsTestclass that exercises parsing RSA public key and private key files.The
PemUtilsclass usage is limited to JWT asymmetric token support, narrowing the scope of changes to this specific token signing implementation.