I’ve resumed my zwave S2 work on main/4.2.0-SNAPSHOT and was having a strange jar signature error when trying to run my junit tests when they invoked the BC (bouncy castle) crypto classes. Turns out the root cause was my code picking us the boucy castle classes from the io.jetty.certificate (instead of the BC jar) which has the BC classes embedded in it.
io.jetty.certificate uses maven shaded plugin to build a “fat” jar. The shaded plugin attempts to strip out the signature files but doesn’t’ get all of them which results in the jar loader rejecting the jar due to an invalid signature. I was able to confirm the current 4.2.0 jars bad signature by running the jarsigner command
jarsigner -verify org.openhab.core.io.jetty.certificate-4.2.0-20240111.192810-16.jar
jarsigner: java.lang.SecurityException: Invalid signature file digest for Manifest main attributes
While I’m somewhat new to OH development, I am quite versed in java security/crypto as I worked with it for many years. I don’t know the history as to why this was made to be a fat jar in the first place, but I suggest removing the maven-shaded plugin as there are various other OH bundles which use the bouncy castle classes without bundling them.
Bundling JCE classes can cause problems for a variety of reasons, one of which is that the JRE requires crypto provider classes to be digitally signed when using them through the JCE (which I’m going to do for OH S2). As seen above, the BC JCE classes won’t have a valid signature if bundled into another jar.
I’m happy to take some time out from OH S2 work to test out a skinny io.jetty.certificate jar, then create a PR if it would help. Just wanted to look for any historical/general feedback before doing so.
I couldn’t find any issue for this and according to “blame” it’s been embedded for a long time.
The Karaf ssh feature also uses bouncycastle so it does not save any space either when the version is kept in sync.
Maybe a specific version was needed to workaround an issue and embedding it was a quick workaround?
So if it works without embedding bouncycastle that would be nice. If that does not work at least we will know what the issue is again.
The HomeKit binding is stuck on an old bouncycastle version. I tried to upgrade it but it caused issues. So if there is someone who does use it, feel free to recycle my PRs :
I would GREATLY appreciate that. The code is currently in use in the AndroidTV binding so I know it all works at this point. Crypto can be hard for those that don’t deal in it, the whole point is to provide a solid library to use across the platform.
I made a quick review of the PR (work as a Cyber Security Consultant and have studied cryptography, so couldn’t pass the challenge ). Haven’t worked with crypto in java, so not sure about best practices in using the APIs, but left some comments from a general perspective.
Thank you very much for the review! I think there are just a few small changes from what I can see. @wborn could I convince you to look over the review and provide any additional feedback so I make sure I hit everything on the changes?
). Haven’t worked with crypto in java, so not sure about best practices in using the APIs, but left some comments from a general perspective.