Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 344691 - Invalid MD5 sums for signed jars in eclipse-repository
Summary: Invalid MD5 sums for signed jars in eclipse-repository
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Tycho (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P2 normal with 1 vote (vote)
Target Milestone: ---   Edit
Assignee: Tobias Oberlies CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-04 07:34 EDT by Hendrik Eeckhaut CLA
Modified: 2021-04-28 16:54 EDT (History)
9 users (show)

See Also:


Attachments
a project demostrating the issue (4.16 KB, application/zip)
2011-07-07 14:30 EDT, Rafal Krzewski CLA
no flags Details
A build log from the attached it project. (5.72 KB, text/x-log)
2011-07-07 14:33 EDT, Rafal Krzewski CLA
no flags Details
ant script to add the "second-generate-p2-metadata" step to your plugins and feature poms (2.10 KB, application/xml)
2011-07-28 11:19 EDT, Anthony Dahanne CLA
no flags Details
integration test for jar-signing (13.86 KB, patch)
2011-09-08 10:23 EDT, Matthias Gradl CLA
no flags Details | Diff
proposed patch (removes md5 sum generation from P2GeneratorImpl) (148.65 KB, patch)
2011-09-28 11:35 EDT, Holger Oehm CLA
t-oberlies: iplog+
Details | Diff
Report all issues not only the first issue detected. (3.66 KB, patch)
2011-10-11 09:16 EDT, Holger Oehm CLA
t-oberlies: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hendrik Eeckhaut CLA 2011-05-04 07:34:08 EDT
In the package phase the p2-metadata plugin runs before the jarsigner plugin. As a result the p2-metadata contains md5sums of the unsigned jars. If you generate an update site, the md5sums in artifacts.xml do not correspond to the actual (signed) jars and Eclipse refuses to perform the installation.
Comment 1 Tobias Oberlies CLA 2011-05-06 05:28:49 EDT
Please provide an example project that demostrates the problem. With so many details missing, I'm not sure I understand the problem.
Comment 2 Alex Blewitt CLA 2011-05-06 05:32:52 EDT
Use the RCP tycho project
Run mvn package sign 
Note how the update site jars are re-signed, changing their md5
Compare md5 of jar with md5 in artifacts.jar and note difference
Comment 3 Rafal Krzewski CLA 2011-07-07 14:30:40 EDT
Created attachment 199282 [details]
a project demostrating the issue

I've also run into this issue, while trying to generate signed plugins and features for Eclipse.

I've created a sample project demonstrating the problem.
Comment 4 Rafal Krzewski CLA 2011-07-07 14:33:50 EDT
Created attachment 199283 [details]
A build log from the attached it project.

Notice that tycho-p2-plugin:0.12.0:p2-metadata goal is executed before maven-jarsigner-plugin:1.2:sign goal.

Because of this md5 stored in p2artifacts.xml and actual md5 of the jar processed by jarsigner do not agree.
Comment 5 Alex Blewitt CLA 2011-07-12 07:36:45 EDT
You can just delete the entries with the md5 from the artifacts.jar/xml file and it works. I've found post-processing this is the easiest workaround in the meantime.
Comment 6 Rafal Krzewski CLA 2011-07-13 08:21:15 EDT
I've done some research and it appears we are hitting a caveat in Maven core here. 

maven-jarsigner-plugin needs to run in the package phase between

org.eclipse.tycho:tycho-packaging-plugin:${project.version}:package-plugin
and
org.eclipse.tycho:tycho-p2-plugin:${project.version}:p2-metadata

plugin executions in lifecycle mappings defined by Tycho for eclipse-plugin et al.

Maven, as of version 3.0.x does not provide the facilities that allow the user to insert, delete or replace lifecycle bindings into the default mapping provided for a packaging type.

See http://docs.codehaus.org/display/MAVEN/Deterministic+Lifecycle+Planning
http://docs.codehaus.org/display/MAVEN/Suppression%2C+Ordering%2C+and+Replacement+of+Plugins+and+Mojos+Bindings
Sadly, it seems that little have been done to implement it in the past 3 years.

Inserting maven-jarsigner-plugin into the packaging type definitions provided by Tycho is not an option because it would bind the plugin to the lifecycle for all builds with no means for the user to disable it. This is not acceptable, because maven-jarsigner-plugin should be executed only when relevant configuration is defined in the project POM hierarchy. Typically signing is only needed in a release build, controlled by a specific profile.

As things stand now in Maven core, one needs to define a custom packaging type to override the default lifecycle mapping. Providing signed-eclipse-plugin packaging type is IMO not practical, because packaging is not something you can control from a profile. On the other hand, one could create a dummy key for signing snapshot artifacts, so this might be workable, but really kludgy.
Comment 7 Igor Fedorenko CLA 2011-07-13 10:23:02 EDT
We are going to try to get rid of p2-artifacts.xml file and generate p2 artifact repository metadata using information available elsewhere. This will allow us to change default lifecycle to generate p2 installable unit metadata before packaging jar file and will also allow binding of signing and/or pack200 actions *after* default executions.
Comment 8 Tobias Oberlies CLA 2011-07-20 11:15:55 EDT
Just another idea for a workaround: Would it be possible to execute the p2-metadata goal of the tycho-p2-plugin again after the modification through signing (through manual configuration in the POM)?
Comment 9 Tobias Oberlies CLA 2011-07-20 11:51:45 EDT
(In reply to comment #8)
> Just another idea for a workaround: Would it be possible to execute the
> p2-metadata goal of the tycho-p2-plugin again after the modification through
> signing (through manual configuration in the POM)?

This workaround works. After the configuration signing in the POM, add the following config:

      <plugin>
      	<groupId>org.eclipse.tycho</groupId>
      	<artifactId>tycho-p2-plugin</artifactId>
      	<executions>
      		<execution>
      			<id>second-generate-p2-metadata</id>
      			<goals>
      				<goal>p2-metadata</goal>
      			</goals>
      			<phase>verify</phase>
      		</execution>
      	</executions>
      </plugin>
Comment 10 Anthony Dahanne CLA 2011-07-25 18:27:23 EDT
hello,
@tobias : I can not apply your workaround in my project; could you provide more detail ?
I have defined a sign profile in my parent pom, that gets executed when I invoke it explicitly (-Psign)
In this profile, I added your snippet, and as soon as my reactor is defined, the build fails on the parent module, with the following error : 

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-p2-plugin:0.12.0:p2-metadata (second-generate-p2-metadata) on project com.company.project.parent: Execution second-generate-p2-metadata of goal org.eclipse.tycho:tycho-p2-plugin:0.12.0:p2-metadata failed. IllegalStateException -> [Help 1]

I assume that it is because the tycho-p2-plugin expected to find a jar in the target folder; and that won't happen with a pom packaging ...

How do you run your workaround ? do you explicitly add this snippet to all your plugins' pom ? and if you do so, do you use a different snippet for the features (setting the goal feature-p2-metadata instead of p2-metadata ?)

I was wondering if I could bind this execution to a given packaging, but it does not seem to be possible in maven...

thanks in advance !
Comment 11 Anthony Dahanne CLA 2011-07-28 11:19:07 EDT
Created attachment 200536 [details]
ant script to add the "second-generate-p2-metadata" step to your plugins and feature poms

hello all,
I have found a way to add the proper "second-generate-p2-metadata" step (as described by Tobias https://bugs.eclipse.org/bugs/show_bug.cgi?id=344691#c9) to each of my eclipse-plugin and eclipse-feature only (ie not pom, eclipse-repository and other packaging artifacts).

So this is an ant script, you launch it in your workspace folder, and it will add the snippet described by Tobias in each of your eclipse-plugin and eclipse-feature modules.

This is far from being perfect ; since it forces you to use Ant as a preliminary step during your build and also if you have already some build configuration in your eclipse-plugin or eclipse-feature, it will mess your pom :-/ 

I wish I could tell maven to apply specific executions only to a given type of module types...
Comment 12 benson margulies CLA 2011-08-31 09:02:07 EDT
How do you deal with (or do you deal with) plugins coming straight out of the maven-bundle-plugin?
Comment 13 Igor Fedorenko CLA 2011-08-31 09:44:12 EDT
(In reply to comment #12)
> How do you deal with (or do you deal with) plugins coming straight out of the
> maven-bundle-plugin?

bundle providers are responsible for signing their plugins, so projects using maven-bundle-plugin are expected to sign their bundles.
Comment 14 Matthias Gradl CLA 2011-09-08 10:23:03 EDT
Created attachment 202996 [details]
integration test for jar-signing

Here is an integration test for jar-signing with Tycho. It is there to test the issue described in bug #340345, which causes build errors for products that include bundles with self-signed signatures. Until the fix in p2 is consumed in Tycho the test will fail because the signing certificate is not trusted.

Interestingly enough, the issue described in this bug does not accur during the test build, but the MD5 sums of the artifacts still mismatch. The test case should be enhanced to also check md5.
Comment 15 Tobias Oberlies CLA 2011-09-13 11:27:33 EDT
(In reply to comment #7)
> We are going to try to get rid of p2-artifacts.xml file and generate p2 artifact
> repository metadata using information available elsewhere.
Although this would be possible, an IArtifactRepository implementation (currently backed by the p2artifacts.xml) is the right output format of a module build:
* It already allows for multiple representations of the same artifact (through multiple IArtifactDescriptors for the same IArtifactKey)
* It makes it easy to create an eclipse-repository build result from the module IArtifactRepositories using raw artifact mirroring. 

A solution that is a lot easier to implement is to just remove the md5 sums and the sizes from the IArtifactDescriptors produced by the build. The md5 sums for external artifacts anyway get lost somewhere in Tycho (bug 357513), so we are not really making things worse. Btw, a signature (which will be feasible with this bug fixed and p2 version Indigo M2) also protects from corrupted artifacts.
Comment 16 Igor Fedorenko CLA 2011-09-13 12:12:15 EDT
Tycho only really needs a map between maven artifact GAVC and p2 artifact key so maintaining p2artifacts.xml seems like an overkill for such a simple problem. On top of that, Tycho needs to use p2 internal classes to generate p2artifacts.xml and, more importantly, the format of the file is not considered to be part of p2 API either so repository interoperability with future Tycho versions may become hard(er) to maintain.
Comment 17 Tobias Oberlies CLA 2011-09-15 07:50:39 EDT
Someone needs to produce IArtifactDescriptors, and since they are far from trivial to produce, for separation of concerns this needs to be done where the artifact is produced. The easiest way to store multiple IArtifactDescriptors is an IArtifactRepository, so this means that each module needs to produce an IArtifactRepository (as it is today). The p2artifacts.xml as persistence format for that IArtifactRepository is out there anyway, so I don't see the need to change anything there.
Comment 18 Holger Oehm CLA 2011-09-28 11:35:01 EDT
Created attachment 204198 [details]
proposed patch (removes md5 sum generation from P2GeneratorImpl)

Signatures are much better to verify that artifact have not been corrupted. This patch removes the (wrong) md5 sums to make way for signatures. (So the maven life cycle problem has not gone away, but it is no longer standing in the way for signatures, it only stands in the way for md5sums :-)).
Comment 19 Tobias Oberlies CLA 2011-10-11 04:50:56 EDT
Fixed with c283b91. Thank you for the great patch :-)

One minor question: The verify-repository goal (through the VerifierServiceImpl) only report the first inconsistency found. Wouldn't we rather want to see all problems - even if the list gets lengthy?

Renamed this bug from "Jarsigner should run before p2-metadata in package phase" to "Invalid MD5 sums for signed jars in eclipse-repository" to describe the symptom rather than the a solution we can't implement with the current Maven version.
Comment 20 Holger Oehm CLA 2011-10-11 05:53:34 EDT
(In reply to comment #19)
> One minor question: The verify-repository goal (through the
> VerifierServiceImpl) only report the first inconsistency found. Wouldn't we
> rather want to see all problems - even if the list gets lengthy?

Oups, thats what I wanted to achieve. (But this intention hasn't been covered by the tests, it seems that it proves that everything that lacks a test is broken:-)
Comment 21 Holger Oehm CLA 2011-10-11 09:16:26 EDT
Created attachment 204948 [details]
Report all issues not only the first issue detected.

Fix for the fix (to be applied on top of the first fix).
Comment 22 Tobias Oberlies CLA 2011-10-13 03:17:48 EDT
Comment on attachment 204948 [details]
Report all issues not only the first issue detected.

Thanks Holger for fixing and testing this :-)  Applied as 9b6a4d2
Comment 23 Meng Xin Zhu CLA 2011-11-09 01:28:50 EST
I tried 0.14.0-SNAPSHOT, but it doesn't work.

I added maven-jarsigner plugin in the parent pom,

		<profile>
			<id>jenkins</id>
			<properties>
				<skip.jar.signing>false</skip.jar.signing>
			</properties>
			<build>
				<plugins>
					<plugin>
						<groupId>org.apache.maven.plugins</groupId>
						<artifactId>maven-jarsigner-plugin</artifactId>
						<version>1.2</version>
						<configuration>
							<keystore>${keystore}</keystore>
							<alias>WindRiver</alias>
							<storepass>${storepass}</storepass>
							<keypass>${keypass}</keypass>
							<verify>true</verify>
							<skip>${skip.jar.signing}</skip>
							<arguments>
								<argument>-tsa</argument>
								<argument>https://timestamp.geotrust.com/tsa</argument>
							</arguments>
							<excludes>
								<exclude>*.zip</exclude>								
							</excludes>
						</configuration>
						<executions>
							<execution>
								<id>sign</id>
								<goals>
									<goal>sign</goal>
								</goals>
							</execution>
							<execution>
								<id>verify</id>
								<goals>
									<goal>verify</goal>
								</goals>
							</execution>
						</executions>
					</plugin>
				</plugins>
			</build>
		</profile>
		
And the newly built SNAPSHOT of my plug-ins has been signed after building it with 'clean install'. However the artifact.xml always has wrong MD5 value of my plug-ins, I have no idea which jar file is used by p2 publisher.

Need any additional configurations in the pom of eclipse-repository project?
Comment 24 Tobias Oberlies CLA 2011-11-14 09:27:25 EST
(In reply to comment #23)
> I tried 0.14.0-SNAPSHOT, but it doesn't work.
Note that we currently don't have a CI build for Tycho 0.14.0-SNAPSHOT (bug 363696), so I would suppose that your local snapshot is outdated.

The expected result of an eclipse-repository build is an artifact.xml without md5 sums.
Comment 25 Meng Xin Zhu CLA 2011-11-15 03:47:22 EST
(In reply to comment #24)
> Note that we currently don't have a CI build for Tycho 0.14.0-SNAPSHOT (bug
> 363696), so I would suppose that your local snapshot is outdated.
I see. I find the root cause after debugging tycho. There is a stale folder 'p2' under the local repository, it has higher priority when searching the artifact repository. I always use 'clean' to rebuild the project, however the stale 'p2' can't be removed. I manually removed it, then it works well.
> 
> The expected result of an eclipse-repository build is an artifact.xml without
> md5 sums.
I think the workaround mentioned in comment #9 is an acceptable way. Though users have to copy and paste the same configuration for all feature/plug-in projects. MD5sum is a good way to verify the integrity of the repository after transferring it among different machines.
Could you add a flag to keep the MD5 property when publishing the metadata repository if users would like to use the second metadata running as a workaround?
Comment 26 Holger Oehm CLA 2011-11-15 04:07:30 EST
(In reply to comment #25)
> Could you add a flag to keep the MD5 property when publishing the metadata
> repository if users would like to use the second metadata running as a
> workaround?

I dont understand why you need the md5 sums in this case: the content is signed, so it's integrity cannot be questioned, can it?
Comment 27 Meng Xin Zhu CLA 2011-11-16 02:55:11 EST
(In reply to comment #26)
> I dont understand why you need the md5 sums in this case: the content is signed,
> so it's integrity cannot be questioned, can it?
Got it. My original concern of missing MD5 sum is that we release the new build on build machine, then it will be synchronized to different ftps(they locate in different countries), the synchronized binaries might have problem due to the unstable network. Looks like P2 would fail installing it on manifest reading or signature checking if the artifact is damaged while synchronizing it.