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.

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

Update to bouncycastle jdk18on with version 1.78.1 #785

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pathob
Copy link

@pathob pathob commented May 29, 2024

The Jenkins update-center2 was still using a bouncycastle version from September 2008. Even though I exactly followed the instructions from the update-center2's README.adoc file, I was not able to generate an OpenSSL key (and certificate) that would work with that bouncycastle version. Debugging into the library revealed that the old bouncycastle version doesn't support reading private keys starting with the line -----BEGIN PRIVATE KEY-----.

Instead, bouncycastle only supported one of the following starting lines for private keys:

  • -----BEGIN RSA PRIVATE KEY-----
  • -----BEGIN DSA PRIVATE KEY-----
  • -----BEGIN EC PRIVATE KEY-----

In the case of a starting line of BEGIN PRIVATE KEY, the readObject() method of the PEMReader would just return null and cause the update-center2 to crash.

However, the documented command openssl genrsa -out demo.key 4096 also generates a key file starting with BEGIN PRIVATE KEY when using modern versions of OpenSSL (version 3.0.13 in my case).

For this reason, this change updates the used bouncycastle library to the most recent version, with which I was able to sign the files again.

@pathob
Copy link
Author

pathob commented May 29, 2024

Hi @daniel-beck , it would be great if you would consider accepting this PR, since this is the missing puzzle piece for generating a corporate update center for my client.

Unfortunately, the included bouncycastle dependencies are not available in https://repo.jenkins-ci.org/public so that I included Maven central for developing and testing purposes. If you could arrange for the library to be available in the Jenkins repository, I would remove Maven Central again.

The Jenkins update-center2 was still using a bouncycastle version from
September 2008. Even though I exactly followed the instructions from the
update-center2's README.adoc file, I was not able to generate an OpenSSL
key (and certificate) that would work with that bouncycastle version.
Debugging into the library revealed that the old bouncycastle version
doesn't support reading private keys starting with the line
`-----BEGIN PRIVATE KEY-----`.

Instead, bouncycastle only supported one of the following starting lines
for private keys:

- `-----BEGIN RSA PRIVATE KEY-----`
- `-----BEGIN DSA PRIVATE KEY-----`
- `-----BEGIN EC PRIVATE KEY-----`

In the case of a starting line of `BEGIN PRIVATE KEY`, the
`readObject()` method of the `PEMReader` would just return `null` and
cause the update-center2 to crash.

However, the documented command `openssl genrsa -out demo.key 4096` also
generates a key file starting with `BEGIN PRIVATE KEY` when using modern
versions of OpenSSL (version 3.0.13 in my case).

For this reason, this change updates the used bouncycastle library to
the most recent version, with which I was able to sign the files again.
@daniel-beck daniel-beck added enhancement This is an enhancement for the tool or wrapper scripts, typically adding features. dependencies This PR updates a dependency file and needs a new release to be effective. labels Jun 25, 2024
@daniel-beck daniel-beck self-assigned this Jun 25, 2024
@daniel-beck
Copy link
Contributor

@pathob Apologies for the delayed response here, I missed the notification. Thanks for this PR, I will try to review that in the next few weeks.

Meanwhile I think we encountered the same problem when we last created the new yearly cert, and the solution (per Gitter notes) was to pass the -traditional flag to openssl genrsa. Perhaps that's a suitable workaround for you if you don't want to deal with a fork.

@pathob
Copy link
Author

pathob commented Jul 16, 2024

@daniel-beck Thanks for the info about the -traditional flag. It would be great if you would find time to review this MR, otherwise I think it should be a good idea to add it to the README, because debugging this was quite frustrating. Thank you!

pom.xml Outdated Show resolved Hide resolved
@daniel-beck daniel-beck added the java This PR affects Java code and would need a new release to be effective label Sep 4, 2024
@daniel-beck
Copy link
Contributor

@pathob PTAL at the recent changes, do you see any potential problems?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies This PR updates a dependency file and needs a new release to be effective. enhancement This is an enhancement for the tool or wrapper scripts, typically adding features. java This PR affects Java code and would need a new release to be effective
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants