Comment 1 David Williams 2013-02-19 00:24:01 EST

Igor, or Thanh, I'm assuming this is a matter of how the "jar signer" plugin works. Is there a 'configuration' we need to specify, or something?

Comment 2 Igor Fedorenko 2013-02-19 00:39:55 EST

I am not sure I understand the question. Tycho does not sign nested jars, it is not possible to enable this and there are no plans to implement this. I thought we discussed this in details when I implemented jarsigning last year... or did I completely misunderstand the problem? (sorry, it's been a long day)

Comment 3 David Williams 2013-02-19 03:12:47 EST

Are you saying this is a CBI issue, not Tycho? If so, fine by me, as long as someone pays attention it it.

I do remember long "talks" in bugzilla (or, somewhere) that you could not pack200 inner jars but, I thought, could still sign as long as not pack200'd. 

Perhaps others will have even other memories, but, that's mine. And, I tend to trust Tom's, when it comes down to low level OSGi security issues. (i.e. that it is important in some scenarios).

Comment 4 Thomas Watson 2013-02-19 09:18:04 EST

(In reply to comment #3)
> Perhaps others will have even other memories, but, that's mine. And, I tend
> to trust Tom's, when it comes down to low level OSGi security issues. (i.e.
> that it is important in some scenarios).

I do not want to overstate the importants of this issue.  The signing of inner jars has always been a bit of a grey area for how an OSGi framework should handle it.  The OSGi specification is clear on the handling of signed root bundle file but makes very little or no instructions for how signing of inner jars are handled.  For equinox the signature of the inner jar is used for two purposes.

1) If runtime verification is enabled the framework will verify the signed entries of the jar file are not corrupted.

2) If class certificates is enabled the framework will use the signing certificates of the signed jar to construct a proper CodeSource that answers the CodeSource.getCertificates() (see bug 110846 and bug 115719

For 2) it probably would be valid to simply use the signing certificates of the root bundle file, this would require a behavior change to the framework.

For 1) it could prove more problematic if a runtime really wants runtime verification of the jar content.  Without the signing of the inner jar it will be difficult to verify the jar is not corrupted.  Difficult but perhaps not impossible, but it would require a complete reading and verifying of the whole jar file against the signed entry for the jar file from the root bundle.  This would be very costly and we would not be able to provide any details on why a jar is corrupt (what files inside of it etc.) only that the whole jar is corrupted.  I would opt to not do any jar content verification at runtime unless the jar is signed.

Comment 5 Igor Fedorenko 2013-02-19 09:27:49 EST

Fair enough. Signing of nested jars should not be hard to implemented as long as it is not used together with pack200. The relevant code can be found in [1]

[1] https://git.eclipse.org/c/cbi/org.eclipse.cbi.maven.plugins.git/tree/eclipse-jarsigner-plugin/src/main/java/org/eclipse/cbi/maven/plugins/jarsigner/SignMojo.java

Comment 6 Thomas Watson 2013-02-19 09:35:41 EST

(In reply to comment #5)
> Fair enough. Signing of nested jars should not be hard to implemented as
> long as it is not used together with pack200. The relevant code can be found
> in [1]

I think pack200 of inner jars causes pain for other reasons (build with Java 6 but provision on Java 7) and I would be for disabling pack200 of inner jars altogether.

> 
> [1]
> https://git.eclipse.org/c/cbi/org.eclipse.cbi.maven.plugins.git/tree/eclipse-
> jarsigner-plugin/src/main/java/org/eclipse/cbi/maven/plugins/jarsigner/
> SignMojo.java

I think it would be "good" to get this working, but if this proves to be a difficult thing to get correct then I am also willing to make the behavior change to the framework from comment 2 to provide a proper CodeSource certificates in the case where the root bundle file is signed but the inner jars are not.  But I will still not do any runtime content validation unless the inner jar is signed.

Comment 7 Igor Fedorenko 2013-02-19 09:42:32 EST

Changing cbi jarsigner to sign nested jars should not be hard, and this is what I would recommend. The signer is ~200 LOC now and pretty much any java developer should be able to add logic to extract, sign and repack nested jars to the signer.

Comment 8 David Williams 2013-05-24 23:46:25 EDT

On behalf of a Platform client, there's certain classes of "JUnit tests" which can no longer be ran, without having our own (Platform's) inner jars signed. 

The user (see bug 406942) actually thinks it is a "blocker", but I do not know, yet, if there are work arounds. But, may change it to blocker, if I find there are not.

Comment 9 Jan Sievers 2013-05-27 01:45:00 EDT

(In reply to comment #8)
> The user (see bug 406942) actually thinks it is a "blocker"

hm. seems bug 406942 is unrelated to signing. maybe a typo?

Comment 10 David Williams 2013-05-27 01:53:51 EDT

(In reply to comment #9)
> (In reply to comment #8)
> > The user (see bug 406942) actually thinks it is a "blocker"
> 
> hm. seems bug 406942 is unrelated to signing. maybe a typo?

Yes, sorry, meant bug 408901.

Comment 11 Thanh Ha 2013-06-03 10:26:06 EDT

I spent some time working on a patch for this and finally came up with a working solution. I will attach the patch for review on Gerrit once I clean it up.

I have a concern however after reading about pack200 normalize. Since we run the pack200a first before the signing plugin, if we sign inner jars we will have to extract the jar to get to the inner jars, sign them and pack a new outer jar. Which means the work done by the pack200a is now undone.

Should the signing of the inner jars really be done in this step? or maybe we need to add an execution before pack200a happens to sign the inner jars first?


Some other questions I had while working on this:

Should we sign all inner jars? or should the user provide a list of inner jars to sign?

Can there be inner, inner jars? how recursive does this need to be?

Comment 12 Thanh Ha 2013-06-03 11:35:53 EDT

(In reply to comment #11)
> Should the signing of the inner jars really be done in this step? or maybe
> we need to add an execution before pack200a happens to sign the inner jars
> first?

Went back and reread the previous comments, sounds like if we want to sign inner jars, we should not enable pack200 for the jar (does this mean the outer jar, or the inner jar?). Unless I misunderstood?

Comment 13 Thanh Ha 2013-06-03 14:00:55 EDT

(In reply to comment #11)
> I will attach the patch for review on Gerrit once I clean it up.

Pushed the patch to Gerrit:

https://git.eclipse.org/r/13514

Comment 14 David Williams 2013-06-03 14:32:59 EDT

(In reply to comment #12)
> (In reply to comment #11)
> > Should the signing of the inner jars really be done in this step? or maybe
> > we need to add an execution before pack200a happens to sign the inner jars
> > first?
> 
> Went back and reread the previous comments, sounds like if we want to sign
> inner jars, we should not enable pack200 for the jar (does this mean the
> outer jar, or the inner jar?). Unless I misunderstood?

It means the inner jar. In short, we never want to pack200 inner jars, so we do not need to "condition" them first. We just want them signed. 

The "outter jar", as a whole, still needs to be conditioned, as a whole, before doing any signing ... but, that doesn't need to be done to the inner jars.

Comment 15 David Williams 2013-06-03 14:47:20 EDT

(In reply to comment #11)

> 
> Should we sign all inner jars? or should the user provide a list of inner
> jars to sign?

I think the default should be to sign all inner jars. BUT, there is already a mechanism for users (bundle providers) to say "do not sign inner jars". It is part of the eclipse.inf file. The significant properties are, from 
http://wiki.eclipse.org/JarProcessor_Options, 

jarprocessor.exclude.children.sign
jarprocessor.exclude.children

And, the ones already looked at by that code
jarprocessor.exclude.sign
jarprocessor.exclude

imply "not children either". 

I have sometimes wondered if the deluxe solution is to also provide some "plugin configuration", but I don't think that's necessary since there is already a mechanism, and then you'd have to check if if they contradicted each other and give a warning if they did, etc. That's my advice, anyway, is just to use the existing eclipse.inf file. 

> 
> Can there be inner, inner jars? how recursive does this need to be?

This, I am not sure about. Assuming you mean a jar inside the jar that is already in the bundled jar ... As far as I know, that has not been done in the past, but am not sure and would have to study "jarprocessor" code to know for sure. [But, I'd suggest not to worry about it, in the "first draft" of the fix.]. 

HTH

Comment 16 Thanh Ha 2013-06-13 16:18:05 EDT

(In reply to comment #14)
> The "outter jar", as a whole, still needs to be conditioned, as a whole,
> before doing any signing ... but, that doesn't need to be done to the inner
> jars.

Hmm I'm not sure what the best solution here is now. Here's how the process today works (without signing inner jars).

1. run tycho-pack200a-plugin (normalize)
2. run eclipse-jarsigner-plugin (sign)
3. run tycho-pack200b-plugin (pack)

My current proposed implementation invalidates the work done by pack200a since to sign the inner jars I'm extracting the jar and and creating a new jar. If I reimplement what the pack200a plugin is doing inside jarsigner to normalize after signing the inner jars that seems redundant to me.

I feel like there should be a step before pack200a which signs the inner jars rather than having the jarsigner do that. Maybe either implementing inner jar signing as a new "goal" that's part of the jarsigner and run multiple executions, or implementing it as a new plugin. In either case this new step would be inserted before pack200a executes.

To recap I think these are the options for implementation:

1. Implement the pack200 normalize in jarsigner and use it after inner jars are signed (seems like redundant effort?)
2. Implement inner jarsigner as a new goal to run before (tycho-pack200a-plugin)
3. Implement inner jarsigner as a new plugin to run before (tycho-pack200a-plugin)

At this point I think I will try implementing option 3 to try it out and see what happens.


(In reply to comment #15)
> I have sometimes wondered if the deluxe solution is to also provide some
> "plugin configuration", but I don't think that's necessary since there is
> already a mechanism, and then you'd have to check if if they contradicted
> each other and give a warning if they did, etc. That's my advice, anyway, is
> just to use the existing eclipse.inf file. 

Agreed I think using existing mechanisms is best.

Comment 17 David Williams 2013-06-13 16:35:39 EDT

(In reply to comment #16)
> (In reply to comment #14)

> My current proposed implementation invalidates the work done by pack200a
> since to sign the inner jars I'm extracting the jar and and creating a new
> jar. 

No, I think that's fine, it does not invalidate it (I don't think). Conceptually, all the pack200a does (well, I'm assuming all it does :) is call pack200 and it "adjusts" the .class files it can find exactly like the pack200b step does but leaves the bundle jar in an un-gzipped form (and pretty sure it ignores inner jars, just like it would ignore .gif files or similar). Its the "adjustment" or normalization of the .class files that's important, so that then later, when it is done again during pack200b, there is really no change to those .class files, hence they do not invalidate the signing. Might take some code digging or testing to confirm, but pretty sure the first pack200a would not do anything "recursively into inner jars" and otherwise, its perfectly valid to change that "blob of data" inside the outer jar. 

That's my theory, anyway.

Comment 18 Thanh Ha 2013-06-13 16:55:45 EDT

(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #14)
> 
> > My current proposed implementation invalidates the work done by pack200a
> > since to sign the inner jars I'm extracting the jar and and creating a new
> > jar. 
> 
> No, I think that's fine, it does not invalidate it (I don't think).
> Conceptually, all the pack200a does (well, I'm assuming all it does :) is
> call pack200 and it "adjusts" the .class files it can find exactly like the
> pack200b step does but leaves the bundle jar in an un-gzipped form (and
> pretty sure it ignores inner jars, just like it would ignore .gif files or
> similar). Its the "adjustment" or normalization of the .class files that's
> important, so that then later, when it is done again during pack200b, there
> is really no change to those .class files, hence they do not invalidate the
> signing. Might take some code digging or testing to confirm, but pretty sure
> the first pack200a would not do anything "recursively into inner jars" and
> otherwise, its perfectly valid to change that "blob of data" inside the
> outer jar. 
> 

Just want to make sure I understand... I was worried about the outer jar because the way I'm doing it is I extract all the data in the outer jar, and create a brand new outer jar. I figure since it produces a brand new outer jar the work that pack200a did is no longer valid because it did processing on the old outer jar. The reason I did this was because I couldn't find a way in Java to update a jar, so I opted to extract, do what's necessary to sign inner jars and create a new jar from the extracted folder.

I guess my assumption here maybe incorrect?

Comment 19 David Williams 2013-06-13 17:35:04 EDT

(In reply to comment #18)
> (In reply to comment #17)
> > (In reply to comment #16)
> > > (In reply to comment #14)
> > 

> 
> I guess my assumption here maybe incorrect?

I think so. There's nothing really wrong with your option 3 either ... but, it has the same assumption ... that pack200a does not touch the "inner jars" ... if it did, that'd invalidate the signature. The initial "conditioning" doesn't do anything "sensitive", just "moves around" some .class data so it will compress better (once it is compressed in stepb) so as long as you use the same .class files when you "recreate" the jar, i think it'll be fine.

Comment 20 Thanh Ha 2013-06-13 17:55:35 EDT

(In reply to comment #19)
> I think so. There's nothing really wrong with your option 3 either ... but,
> it has the same assumption ... that pack200a does not touch the "inner jars"
> ... if it did, that'd invalidate the signature. The initial "conditioning"
> doesn't do anything "sensitive", just "moves around" some .class data so it
> will compress better (once it is compressed in stepb) so as long as you use
> the same .class files when you "recreate" the jar, i think it'll be fine.

Thanks, I think I have a better understanding of what pack200 is doing now (or should). I guess I need to run some tests to make sure the current proposed code works as we expect.

Comment 21 Thanh Ha 2013-06-18 14:41:28 EDT

I ran a full build with my patch from https://git.eclipse.org/r/13514 and it ran successfully. Unfortunately the final produced Eclipse SDK didn't include the jars with the signed inner jars which likely has to do with the baseline comparator getting in the way and replacing my jars.

Interestingly I was able to reproduce the exception:

    "org.eclipse.core.runtime.IExtension"'s signer information does not match signer


and saw it disappear when I manually copied my custom built org.eclipse.core.runtime.compatibility.registry with signed inner jar over and saw the exception not appear.
(I used Fabio's steps from bug 408901 comment 3)

I think next steps is to merge the code and test this plugin in a production build. I'll do this when the Luna branch is ready.

Comment 22 Thanh Ha 2013-07-11 10:26:11 EDT

As a consequence of my plan to push changes for bug 388878 changing CBI plugins version to 1.0.4-SNAPSHOT, this new jarsigner plugin will also get tested in the next build.

Comment 23 Thanh Ha 2013-07-11 13:48:39 EDT

Was running a local test build before I push the changes and noticed this snippet of output below. The tests bundles have numerous inner jars that will get signed during the build. I'm not sure if test inner jars need to be signed but it's adding quite a bit of time to the build.




[INFO] --- eclipse-jarsigner-plugin:1.0.4-SNAPSHOT:sign (sign) @ org.eclipse.equinox.p2.tests ---
[INFO] Searching org.eclipse.equinox.p2.tests-1.5.0-SNAPSHOT.jar for inner jars...
[INFO] Signed testData/directorywatcher2/org.junit_3.8.2.v200706111738/junit.jar in 2 seconds.
[INFO] Signed testData/directorywatcher2/org.eclipse.osgi.services_3.1.200.v20070605.jar in 1 seconds.
[INFO] Signed testData/directorywatcher2/org.eclipse.equinox.jsp.jasper.registry_1.0.0.v20070827.jar in 1 seconds.
[INFO] Signed testData/VerifierBundle35/org.eclipse.equinox.p2.tests.verifier_1.0.0.jar in 1 seconds.
[INFO] Signed testData/bug306279/repo/helios/content.jar in 7 seconds.
[INFO] Signed testData/bug306279/repo/riena/content.jar in 1 seconds.
[INFO] Signed testData/bug306279/repo/rienatoolbox-a/content.jar in 7 seconds.
[INFO] Signed testData/permissiveSlicer/content.jar in 2 seconds.
[INFO] Signed testData/testLargeConflict/repo2/content.jar in 2 seconds.
[INFO] Signed testData/testLargeConflict/repo1/content.jar in 2 seconds.
[INFO] Signed testData/updatesite/siteurl2/siteurl/plugins/test.fragment_1.0.0.jar in 1 seconds.
[INFO] Signed testData/updatesite/siteurl2/siteurl/plugins/test.bundle_1.0.0.jar in 1 seconds.
[INFO] Signed testData/updatesite/siteurl2/siteurl/features/test.feature_1.0.0.jar in 1 seconds.
[INFO] Signed testData/updatesite/missingUpdateURLFeature/features/test.featurewithmissingupdateurl_1.0.0.jar in 1 seconds.
[INFO] Signed testData/updatesite/baddigestbadsite/plugins/test.fragment_1.0.0.jar in 1 seconds.
[INFO] Signed testData/updatesite/baddigestbadsite/plugins/test.bundle_1.0.0.jar in 2 seconds.
[INFO] Signed testData/updatesite/baddigestbadsite/features/test.feature_1.0.0.jar in 2 seconds.
[INFO] Signed testData/updatesite/digesturl2/plugins/test.fragment_1.0.0.jar in 2 seconds.
[INFO] Signed testData/updatesite/digesturl2/plugins/test.bundle_1.0.0.jar in 2 seconds.
[INFO] Signed testData/updatesite/digesturl2/features-usedigestinstead/test.feature_1.0.0.jar in 3 seconds.
[INFO] Signed testData/updatesite/siteFeatureReferences/plugins/test.fragment_1.0.0.jar in 3 seconds.
[INFO] Signed testData/updatesite/siteFeatureReferences/plugins/test.bundle_1.0.0.jar in 1 seconds.
...

Comment 24 Thomas Watson 2013-07-11 14:48:29 EDT

(In reply to comment #23)
> Was running a local test build before I push the changes and noticed this
> snippet of output below. The tests bundles have numerous inner jars that
> will get signed during the build. I'm not sure if test inner jars need to be
> signed but it's adding quite a bit of time to the build.
> 
> 
> [INFO] Signed testData/testLargeConflict/repo2/content.jar in 2 seconds.
> [INFO] Signed testData/testLargeConflict/repo1/content.jar in 2 seconds.

It seems you are signing jars that are pre-built and in our source git repo, I would guess that in most all cases we did not intend for these pre-built test jars to be signed.  My initial reaction is that you should only sign jars that are built during the build process, but I'm not sure you know the difference at this point in the build.  I'm not sure what inner jars the old PDE-Build process decided to sign or not.

Comment 25 Thanh Ha 2013-07-11 15:10:53 EDT

The next Luna build should include the inner jarsigner feature:

http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=9a393d0206a08ce987fbbe043b4ad63303ecfd80



(In reply to comment #24)
> It seems you are signing jars that are pre-built and in our source git repo,
> I would guess that in most all cases we did not intend for these pre-built
> test jars to be signed.  My initial reaction is that you should only sign
> jars that are built during the build process, but I'm not sure you know the
> difference at this point in the build.  I'm not sure what inner jars the old
> PDE-Build process decided to sign or not.

Your right, my code at this point doesn't know the difference. Maybe we need to add eclipse.inf file options for test bundles that shouldn't be signed?

http://wiki.eclipse.org/JarProcessor_Options

Comment 26 David Williams 2013-07-11 15:40:54 EDT

(In reply to comment #25)
> The next Luna build should include the inner jarsigner feature:
> 
> ... Maybe we need
> to add eclipse.inf file options for test bundles that shouldn't be signed?
> 
> http://wiki.eclipse.org/JarProcessor_Options

General reminder, we don't sign nightlies, so won't be until I or M build before we "see in action" (right?) ... I assume the executable signing is controlled by same profile? I've not looked at aggregator code yet, but that's the way we would want it. 

Guess I could ... and will plan to ... change that "don't sign nightlies" switch for a week or so, to help flush out any issues before I-build. 

Agree we should not try to be clever about when to sign and when not ... just go by what the eclipse.inf file says. In most cases, I can't imagine it hurting anything (and time doesn't seem that much extra) ... but an announcement to platform-releng-dev should be made for those that want/need to not sign inner jars in test bundles. This is "special" for test bundles, because with PDE build, we did not sign any of the test bundles. So code bundles should already be "marked" correctly in their eclipse.inf files.

Comment 27 David Williams 2013-07-11 15:58:44 EDT

Here's the commit that will cause nightlies to be signed: 

http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=d845cbe182aabed32fa561eae145832f4f43f2e3

It can be reverted after we confirm "all is well". 

(I'll send note to covering this, and the eclipse.inf issue).

Comment 28 David Williams 2013-07-11 16:00:49 EDT

I've marked as backport, since I think once we confirm all is working as expected, we'd want to put "signing" in maintenance branch too, for Kepler SR1. We can open a new bug for that ... but, just marking whiteboard to help not to forget.

Comment 29 Thanh Ha 2013-07-11 16:02:44 EDT

(In reply to comment #27)
> Here's the commit that will cause nightlies to be signed: 
> 
> http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/
> commit/?id=d845cbe182aabed32fa561eae145832f4f43f2e3
> 
> It can be reverted after we confirm "all is well". 
> 
> (I'll send note to covering this, and the eclipse.inf issue).

Thanks David, one other thought I just had is we might not see any inner jars signed in the final product for most bundles as well, unless their qualifiers are forced to update due to the comparator replacing issue. Would it be worth disabling the comparator or bumping a few bundles we know have inner jars?

Comment 30 David Williams 2013-07-11 16:23:39 EDT

(In reply to comment #29)
> (In reply to comment #27)
> > Here's the commit that will cause nightlies to be signed: 
> > 
> > http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/
> > commit/?id=d845cbe182aabed32fa561eae145832f4f43f2e3
> > 
> > It can be reverted after we confirm "all is well". 
> > 
> > (I'll send note to covering this, and the eclipse.inf issue).
> 
> Thanks David, one other thought I just had is we might not see any inner
> jars signed in the final product for most bundles as well, unless their
> qualifiers are forced to update due to the comparator replacing issue. Would
> it be worth disabling the comparator or bumping a few bundles we know have
> inner jars?

Good reminder. I think we'll let that "shake out" in comparator ... I think Tom is the only one that really "needs" this for one of his bundles so he might want to ... and eventually he may want to restore the test cases he changed that showed the issue in the first place. 

[And, glad I read this before sending my reminder note :) ]

Comment 31 Thomas Watson 2013-07-12 09:21:30 EDT

(In reply to comment #30)

> Good reminder. I think we'll let that "shake out" in comparator ... I think
> Tom is the only one that really "needs" this for one of his bundles so he
> might want to ... and eventually he may want to restore the test cases he
> changed that showed the issue in the first place. 

Done in bug 412852 for the org.eclipse.core.runtime.compatibility.registry bundle.

Comment 32 John Arthorne 2013-07-15 14:33:24 EDT

From bug 412850 it sounds like we don't currently have a viable way to exclude child jars from being signed in our CBI build? If I understand that correctly, should we revert this change until we sort that out? If the current choice is between signing all inner jars, or none, I think none is better.

Comment 33 Thanh Ha 2013-07-15 15:00:05 EDT

(In reply to comment #32)
> From bug 412850 it sounds like we don't currently have a viable way to
> exclude child jars from being signed in our CBI build? If I understand that
> correctly, should we revert this change until we sort that out? If the
> current choice is between signing all inner jars, or none, I think none is
> better.

I can back this patch out of CBI until we find a Tycho fix.

Comment 34 David Williams 2013-07-15 15:42:11 EDT

(In reply to comment #33)
> (In reply to comment #32)
> > From bug 412850 it sounds like we don't currently have a viable way to
> > exclude child jars from being signed in our CBI build? If I understand that
> > correctly, should we revert this change until we sort that out? If the
> > current choice is between signing all inner jars, or none, I think none is
> > better.
> 
> I can back this patch out of CBI until we find a Tycho fix.

I agree, best to back out for I and M builds (Tuesday and Wednesday). 

Feel free to add back Wednesday afternoon if you/we have figured out how to "exclude children" by then.

Comment 35 Thanh Ha 2013-07-15 16:38:25 EDT

I went ahead and reverted this patch, it should be removed in the next 1.0.4-SNAPSHOT build.

http://git.eclipse.org/c/cbi/org.eclipse.cbi.maven.plugins.git/commit/?id=5fc9caf7d49996134a53c0fb5f8be492174ef02d

Comment 36 Thanh Ha 2013-10-16 09:54:46 EDT

David, do we still need to backport this or can this bug closed?


I think we've been successfully signing inner jars for awhile now.

Comment 37 Dani Megert 2013-10-16 10:06:03 EDT

I can't speak for CBI, but in the SDK there were quite some changes involved and distributed over several repos to achieve the current state. So, unless there's a really critical bug we fix with this, I'll -1 the backport for the SDK.

Comment 38 David Williams 2013-10-16 10:17:10 EDT

(In reply to Dani Megert from comment #37)
> I can't speak for CBI, but in the SDK there were quite some changes involved
> and distributed over several repos to achieve the current state. So, unless
> there's a really critical bug we fix with this, I'll -1 the backport for the
> SDK.

And, I'll punt to Tom. I know there are some things that "just don't work", if some of the core inner jars are not signed (such as, if I recall, some test frameworks ... I don't recall which ... won't work with 4.3.x) ... but not sure how "main stream" those cases are. (I personally would like to see it fixed, since it's a regression compared to earlier releases where we used PDE to do builds ... but, again, don't recall use-cases right off ... hoping Tom knows off the top of his head).

Comment 39 Thomas Watson 2013-10-16 11:20:36 EDT

(In reply to David Williams from comment #38)
> ... but, again, don't recall use-cases right off ... hoping
> Tom knows off the top of his head).

See bug 408901 for the details.  The use-case is running the extension registry AND its compatibility fragment org.eclipse.core.runtime.compatibility.registry jars outside of OSGi.  For example, using a URLClassLoader.  In this case the URLClassLoader will fail to load classes from the same package which are not signed by the same certificate.

The org.eclipse.core.runtime.compatibility.registry fragment contains an inner jar that is not signed, but org.eclipse.equinox.registry jar is signed.

Comment 40 Dani Megert 2013-10-16 11:29:09 EDT

(In reply to Thomas Watson from comment #39)
> (In reply to David Williams from comment #38)
> > ... but, again, don't recall use-cases right off ... hoping
> > Tom knows off the top of his head).
> 
> See bug 408901 for the details.  The use-case is running the extension
> registry AND its compatibility fragment
> org.eclipse.core.runtime.compatibility.registry jars outside of OSGi.  For
> example, using a URLClassLoader.  In this case the URLClassLoader will fail
> to load classes from the same package which are not signed by the same
> certificate.
> 
> The org.eclipse.core.runtime.compatibility.registry fragment contains an
> inner jar that is not signed, but org.eclipse.equinox.registry jar is signed.

Tom, do you consider this critical enough for SR2? The problem is, that if we switch the policy, all other bundles also have to spend time and backport their changes.

Comment 41 Thomas Watson 2013-10-16 12:07:08 EDT

(In reply to Dani Megert from comment #40)
> Tom, do you consider this critical enough for SR2? The problem is, that if
> we switch the policy, all other bundles also have to spend time and backport
> their changes.

The only workaround for the use-case is to either stop using the compatibility fragment jar (that is not signed) or to strip the signatures from all the other jars that contribute to the org.eclipse.core.runtime package.

Its not really a super great work around, but I also fear that the level of work required to fix this is not well suited for SR2.  Especially because this is not really considered a main stream use-case.

Comment 42 Dani Megert 2013-10-17 03:42:21 EDT

(In reply to Thomas Watson from comment #41)
> (In reply to Dani Megert from comment #40)
> > Tom, do you consider this critical enough for SR2? The problem is, that if
> > we switch the policy, all other bundles also have to spend time and backport
> > their changes.
> 
> The only workaround for the use-case is to either stop using the
> compatibility fragment jar (that is not signed) or to strip the signatures
> from all the other jars that contribute to the org.eclipse.core.runtime
> package.
> 
> Its not really a super great work around, but I also fear that the level of
> work required to fix this is not well suited for SR2.  Especially because
> this is not really considered a main stream use-case.

David, maybe we could address this in a different way for SR2: instead of signing by default with opt-out (<defaultSigning-excludeInnerJars>), allow to opt-in in SR2. That would allow to only fix Tom's issue.

Comment 43 David Williams 2013-10-17 09:09:51 EDT

(In reply to Dani Megert from comment #42)
> (In reply to Thomas Watson from comment #41)
> > (In reply to Dani Megert from comment #40)
> > > Tom, do you consider this critical enough for SR2? The problem is, that if
> > > we switch the policy, all other bundles also have to spend time and backport
> > > their changes.
> > 
> > The only workaround for the use-case is to either stop using the
> > compatibility fragment jar (that is not signed) or to strip the signatures
> > from all the other jars that contribute to the org.eclipse.core.runtime
> > package.
> > 
> > Its not really a super great work around, but I also fear that the level of
> > work required to fix this is not well suited for SR2.  Especially because
> > this is not really considered a main stream use-case.
> 
> David, maybe we could address this in a different way for SR2: instead of
> signing by default with opt-out (<defaultSigning-excludeInnerJars>), allow
> to opt-in in SR2. That would allow to only fix Tom's issue.

I don't recall why we didn't do it that way in the first place, but I'd hesitate to "diverge" the streams ... that'll just get more confusing in the long run. Guess I was hoping we'd have a nice list of where it had been fixed in Luna stream, and it'd be simpler, second time around, to cherry pick those fixes back to Kepler. If even that is too much work for our committers, I'd skip it, unless/until there was a more definitive need requested by community or adopters. 

If/When there is any interest, I'd be glad to write a script to 'git log grep' to find where changed for Luna. 

Otherwise I'd count as fixed and not worry about the regression in Kepler.

Comment 44 Thanh Ha 2013-10-18 22:18:54 EDT

Thanks everyone. I guess I'll just consider this fixed. If there's something more to do lets do it in new bug.