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

More webstart code-signing changes #2193

Merged
merged 3 commits into from Apr 1, 2014

Conversation

manics
Copy link
Member

@manics manics commented Mar 21, 2014

Add a Trusted-Only attribute to manifest. I think this helps avoids additional warnings in Insight webstart. When testing locally it didn't affect the running of the standard Insight client, but this should be tested more widely. Also disable automatic self-signing. See commit messages for more info.

Testing: check insight still fully works. Check the unsigned server works. Then we can try signing webstart with the proper certificate and redeploying.

@bpindelski Do you know anything about the impact (good or bad) of Trusted-Only?

We're going to be code-signing in a separate process, so this prevents jars being double-signed, once with the self-signed cert and once with a proper one.

The webstart-sign target is still there, so you can use `./build.py webstart-sign release-webstart`.
@manics manics changed the title More webstart codesigning tweaks More webstart codesigning changes Mar 21, 2014
@manics manics changed the title More webstart codesigning changes More webstart code-signing changes Mar 21, 2014
@jburel jburel added the dev_5_0 label Mar 21, 2014
@bpindelski
Copy link

I've not used that manifest field, so can't say much. Reading through the Java doc, it looks sensible to use it.

@bpindelski
Copy link

The (now) unsigned Insight from gretzky doesn't work - the addition of Trusted-Only causes the JVM to throw an exception on the unsigned Insight JAR it is trying to run. Looking again at the two commits - they seem to contradict each other. This isn't a good place to be and I anticipate some confusion from developers for whom webstart stopped working. Will our OME key be also used to sign artefacts deployed to our test servers (gretzky, trout etc.)?

@manics
Copy link
Member Author

manics commented Mar 24, 2014

I took webstart-sign out because it uses the self-signed cert included in the source tree: https://github.com/manics/openmicroscopy/blob/c6857e3d8e5ebc154e1bda81260582a37e33ab44/lib/keystore which I don't think is useful to anyone. I think the plan is to sign our releases in a separate manual step with the proper key which is kept under tight security, and to sign daily builds with a cert from a self-signed root CA (which can be distributed and installed by all OME developers), using an analagous but automated process.

Webstart hasn't worked for me for ages, I can revert Trusted-Only if you think it's going to cause too much trouble.

@bpindelski
Copy link

I fully understand why you took out the webstart-sign step out of the build, but I think we have to look again at the implications of that decision.

Today I cannot run webstart from gretzky at all (I can run webstart from trout, as I can add the URL to my Java security settings; that's probably the only positive side of the self-signed cert. This won't work for Trusted-Only though). In a hypothetical scenario where the team would need to work on an emergency release (which happened a couple of times in the past), testing webstart on gretzky would be blocked with this PR in.

I'd suggest coordinating with the team, so that downtime for the deployed systems is minimal and so that we don't slow down the daily testing process.

Summing up - changes here are OK and it's good to merge, but if I were in your place, I'd exclude this PR until the signing setup is in place and the switch can happen overnight.

@@ -1022,7 +1022,6 @@ omero.version=${omero.version}
</copy>
<copy file="components/insight/OUT/dist/omero.insight.jar" tofile="${dist.dir}/lib/insight/omero.insight.jar"/>
<copy file="components/insight/SRC/org/openmicroscopy/shoola/env/ui/graphx/omeabout-bk.png" tofile="${dist.dir}/lib/insight/ome.png"/>
<antcall target="webstart-sign" inheritAll="true" inheritRefs="true"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

release-all will likely need to be updated with this change. What's the reasoning? Is this no longer supported?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to do the signing in a separate process the jars will end up with two signatures (jarsign doesn't remove an existing one).

webstart-sign was only called from release-webstart, so I don't think release-all needs changing.

@manics
Copy link
Member Author

manics commented Mar 24, 2014

Another option would be to re-pack the jars into a single one during the external signing process, which would also get around the problems with timestamp server throttling.

@sbesson
Copy link
Member

sbesson commented Mar 24, 2014

Labelled as breaking.

@joshmoore
Copy link
Member

@manics: no real objection to an omero_webstart.jar or similar if that will solve further problems. Alternatives would be to call it omero_all.jar (maven-esque) or omero_package.jar (to match Bio-Formats) so that when we do proper shadowing, the jar is more widely usable than just webstart.

@manics
Copy link
Member Author

manics commented Mar 30, 2014

As discussed on Friday I'll make the signing script run as part of the standard merge build, can someone remove the breaking label?

@sbesson sbesson removed the breaking label Mar 30, 2014
@sbesson
Copy link
Member

sbesson commented Mar 30, 2014

Done

Automatically run this script with the existing self-signed certificate as part of the standard docs/hudson/OMERO.sh build
@sbesson
Copy link
Member

sbesson commented Mar 31, 2014

@manics
Copy link
Member Author

manics commented Mar 31, 2014

zip isn't installed on gretzky. I've been meaning to get the Python zipfile module working to reduce external dependencies, especially in omego, but not right now.

@manics
Copy link
Member Author

manics commented Mar 31, 2014

Unexclude and rebuild gretzky at some point?

@sbesson sbesson removed the exclude label Mar 31, 2014
@manics
Copy link
Member Author

manics commented Mar 31, 2014

Since Gretzky runs from dist (and not by deploying the OMERO.server-*.zip artifact) the deployed server isn't signed, so I've added
docs/hudson/omero_insight_sign.py lib/keystore omedev dist -kp omedev -cp omedev -ts no
to http://ci.openmicroscopy.org/job/OMERO-5.0-merge-daily/

@sbesson sbesson added this to the 5.0.1 milestone Apr 1, 2014
@manics
Copy link
Member Author

manics commented Apr 1, 2014

@bpindelski Do you mind having a quick check of today's OMERO-5.0-merge server build? webstart should be using the original self-signed cert, but with Trusted-Only: true in the manifest.

@bpindelski
Copy link

@manics Webstart works fine today on gretzky, thanks.

joshmoore added a commit that referenced this pull request Apr 1, 2014
More webstart code-signing changes (merging for `5.0.1`)
@joshmoore joshmoore merged commit 221236a into ome:dev_5_0 Apr 1, 2014
@manics manics mentioned this pull request Apr 1, 2014
@manics
Copy link
Member Author

manics commented Apr 1, 2014

--rebased-to #2238

@manics
Copy link
Member Author

manics commented Apr 16, 2014

--rebased-to #2321

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants