eclipse-cbi / org.eclipse.cbi

CBI Maven plugins and Webservices
https://eclipse-cbi.github.io/org.eclipse.cbi/
Eclipse Public License 2.0
1 stars 10 forks source link

Support signing using a Google KMS provider #487

Closed netomi closed 2 months ago

netomi commented 2 months ago

This is a draft PR as the deployment is not finished yet. Need to figure out how to provide the google cloud credentials file and cert chain to the docker image.

cc @mbarbero

mbarbero commented 2 months ago

Great progress already. Awesome work.

As we eventually package the dependencies into a single JAR, would it be worthwhile to add JSign as a dependency in the pom.xml file? This approach would make updates easier to manage compared to the current method of downloading the JAR in the Dockerfile.

netomi commented 2 months ago

yeah that would make sense, we just need to reference then the distributed runtime jar in the config file itself.

ebourg commented 2 months ago

You probably want to depend on jsign-core instead of the all-in-one jsign artifact. That will allow you to exclude some unnecessary dependencies such as Apache POI. Also once Jsign 7.0 is released you'll be able to replace jsign-core with jsign-crypto, that's a new artifact containing only the JCA provider and not the Authenticode signing stuff.

netomi commented 2 months ago

@ebourg ty for the heads up. Currently we use osslsigncode for signing windows executables, but will most likely replace that by using jsign. We just started with the jar signer as this is more pressing for us.

btw. I looked into the reason why you have to add --add-module java.sql and figured its related to the json-io library. Do you have plans to replace that library with something else? While the library works well, its update strategy is a bit confusing, doing breaking API changes in bugfix releases ...

ebourg commented 2 months ago

I picked json-io because it's very compact and I try to keep the size of the all-in-one jar as small as possible. But the size of the latest releases has increased and it's now close to Gson. Since the needs of Jsign are very basic I'm considering replacing it with a custom JSON parser/formatter to save a few kilobytes.

netomi commented 2 months ago

fyi: when adding the jsign-core package I ran into an issue with the maven-shade-plugin. As we create an uber-jar for the service it collects all dependencies and packs them into a single jar. Now after I added the jsign-core dependency, some signatures ended up in the final jar that resulted that java thought that the jar is signed but failed validation due to missing hashes.

I needed to add an exclude filter for .DSA and .SF files to get it to work. Did not yet check which dependency is signed to trigger that.

netomi commented 2 months ago

so updated the deployment descriptors and added jsign-core as dependency.

What is missing is updating the keystore.sh preDeploy script, for my testing this uses hard-coded values but it should extract the data from pass instead.

netomi commented 2 months ago

I needed to split the configuration for jce and default mode as the config is quite different, maybe its possible to make that generic somehow but wanted to have something working asap.

netomi commented 2 months ago

btw. I switched to ghcr.io as repository for the docker image for the jar-signing-service such that I can also push an image and pull it from there when deploying it to the kubernetes cluster as I dont have the credentials for docker.io

ebourg commented 2 months ago

I needed to add an exclude filter for .DSA and .SF files to get it to work. Did not yet check which dependency is signed to trigger that.

It probably came from Bouncy Castle, the bcprov artifact has to be signed by a certificate issued by Oracle to expose a JCA provider able to perform encryption operations. Jsign doesn't use BC encryption, so it's fine if the signature is stripped from the all-in-one jar.

netomi commented 2 months ago

so the changes are complete, will squash all commits.

There are still the key details from the testing filled in, should be changed to the final ones once they are available. Also the certificate and credentials are not present yet in pass, would need to be added. So we could wait till the certificate is available, update the PR and then merge.

netomi commented 2 months ago

/request-license-review

github-actions[bot] commented 2 months ago

/request-license-review

:heavy_check_mark: All licenses already successfully vetted.

Workflow run (with attached summary files): https://github.com/eclipse-cbi/org.eclipse.cbi/actions/runs/9877011360

netomi commented 2 months ago

hm learned something new, when rebasing the PR using the UI button, my commits are not signed anymore and in vigilante mode show as unverified. So I squashed again and force pushed.