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

[JENKINS-49027] Better report JEP-200 violations in Remoting #247

Merged
merged 2 commits into from
Jan 30, 2018

Conversation

jglick
Copy link
Member

@jglick jglick commented Jan 18, 2018

Partial improvement for JENKINS-49027. For example, update jenkinsci/pipeline-aws-plugin#37 to use a snapshot of remoting and comment out the whitelist entry. Without this change, you get

java.lang.SecurityException: Rejected: java.lang.String$CaseInsensitiveComparator
	at hudson.remoting.ClassFilter.check(ClassFilter.java:75)
	at hudson.remoting.MultiClassLoaderSerializer$Input.resolveClass(MultiClassLoaderSerializer.java:129)
	at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1863)
	at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1746)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2037)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1568)
	at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2282)
	at java.io.ObjectInputStream.defaultReadObject(ObjectInputStream.java:558)
	at java.util.TreeMap.readObject(TreeMap.java:2449)
	at sun.reflect.GeneratedMethodAccessor3.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at java.io.ObjectStreamClass.invokeReadObject(ObjectStreamClass.java:1158)
	at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2173)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2064)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1568)
	at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2282)
	at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2206)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2064)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1568)
	at java.io.ObjectInputStream.readObject(ObjectInputStream.java:428)
	at hudson.remoting.UserRequest.deserialize(UserRequest.java:277)
	at hudson.remoting.UserResponse.retrieve(UserRequest.java:310)
	at hudson.remoting.Channel.call(Channel.java:909)
	at hudson.FilePath.act(FilePath.java:998)
	at hudson.FilePath.act(FilePath.java:987)
	at de.taimos.pipeline.aws.S3UploadStep$Execution$1.run(S3UploadStep.java:261)

which is pretty opaque. With it, you get

java.lang.SecurityException: Rejected: java.lang.String$CaseInsensitiveComparator; see https://jenkins.io/redirect/class-filter/
	at hudson.remoting.ClassFilter.check(ClassFilter.java:76)
	at hudson.remoting.MultiClassLoaderSerializer$Input.resolveClass(MultiClassLoaderSerializer.java:129)
	at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1863)
	at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1746)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2037)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1568)
	at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2282)
	at java.io.ObjectInputStream.defaultReadObject(ObjectInputStream.java:558)
	at java.util.TreeMap.readObject(TreeMap.java:2449)
	at sun.reflect.GeneratedMethodAccessor3.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at java.io.ObjectStreamClass.invokeReadObject(ObjectStreamClass.java:1158)
	at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2173)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2064)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1568)
	at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2282)
	at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2206)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2064)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1568)
	at java.io.ObjectInputStream.readObject(ObjectInputStream.java:428)
	at hudson.remoting.UserRequest.deserialize(UserRequest.java:277)
	at hudson.remoting.UserResponse.retrieve(UserRequest.java:310)
	at hudson.remoting.Channel.call(Channel.java:908)
Caused: java.io.IOException: Failed to deserialize response to UserRequest:hudson.FilePath$FileCallableWrapper@61b60c71
	at hudson.remoting.Channel.call(Channel.java:916)
	at hudson.FilePath.act(FilePath.java:998)
Caused: java.io.IOException: remote file operation failed: …/workspace/p/x at hudson.remoting.Channel@45b84a70:slave0
	at hudson.FilePath.act(FilePath.java:1005)
	at hudson.FilePath.act(FilePath.java:987)
	at de.taimos.pipeline.aws.S3UploadStep$Execution$1.run(S3UploadStep.java:261)

which is somewhat better: the problem has been upgraded to a checked exception that code is likely to check; the JEP-200 permalink is visible; and you can see the Callable.toString (though in this case jenkinsci/jenkins#3071 would be necessary to make that point you to the actual plugin MasterToSlaveFileCallable).

I am also looking into whether we can get details on the actual field causing the problem, but I think this is mergeable as it stands.

@reviewbybees

@ghost
Copy link

ghost commented Jan 18, 2018

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Test failure is real.

2:33:55 [linux] [ERROR] testBlacklisting(hudson.remoting.ChannelFilterTest)  Time elapsed: 0.009 s  <<< ERROR!
22:33:55 [linux] java.io.IOException: Failed to deserialize response to UserRequest:hudson.remoting.ChannelFilterTest$GunImporter@21ffe51e: java.lang.SecurityException: Rejecting hudson.remoting.ChannelFilterTest$GunImporter
22:33:55 [linux] 	at hudson.remoting.Channel.call(Channel.java:916)
22:33:55 [linux] 	at hudson.remoting.ChannelFilterTest$ReverseGunImporter.call(ChannelFilterTest.java:89)
22:33:55 [linux] 	at hudson.remoting.ChannelFilterTest$ReverseGunImporter.call(ChannelFilterTest.java:87)
22:33:55 [linux] 	at hudson.remoting.UserRequest.perform(UserRequest.java:210)
22:33:55 [linux] 	at hudson.remoting.UserRequest.perform(UserRequest.java:53)
22:33:55 [linux] 	at hudson.remoting.Request$2.run(Request.java:358)
22:33:55 [linux] 	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
22:33:55 [linux] 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
22:33:55 [linux] 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
22:33:55 [linux] 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
22:33:55 [linux] 	at java.lang.Thread.run(Thread.java:748)
22:33:55 [linux] 	Suppressed: hudson.remoting.Channel$CallSiteStackTrace: Remote call to north
22:33:55 [linux] 		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1693)
22:33:55 [linux] 		at hudson.remoting.UserResponse.retrieve(UserRequest.java:313)
22:33:55 [linux] 		at hudson.remoting.Channel.call(Channel.java:908)
22:33:55 [linux] 		at hudson.remoting.ChannelFilterTest.testBlacklisting(ChannelFilterTest.java:63)
22:33:55 [linux] 		at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
22:33:55 [linux] 		at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
22:33:55 [linux] 		at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
22:33:55 [linux] 		at java.lang.reflect.Method.invoke(Method.java:498)
22:33:55 [linux] 		at junit.framework.TestCase.runTest(TestCase.java:176)
22:33:55 [linux] 		at junit.framework.TestCase.runBare(TestCase.java:141)
22:33:55 [linux] 		at junit.framework.TestResult$1.protect(TestResult.java:122)
22:33:55 [linux] 		at junit.framework.TestResult.runProtected(TestResult.java:142)
22:33:55 [linux] 		at junit.framework.TestResult.run(TestResult.java:125)
22:33:55 [linux] 		at junit.framework.TestCase.run(TestCase.java:129)
22:33:55 [linux] 		at junit.framework.TestSuite.runTest(TestSuite.java:252)
22:33:55 [linux] 		at junit.framework.TestSuite.run(TestSuite.java:247)
22:33:55 [linux] 		at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:86)
22:33:55 [linux] 		at org.apache.maven.surefire.junit4.JUnit4Provider.executeFailedMethod(JUnit4Provider.java:379)
22:33:55 [linux] 		at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:290)
22:33:55 [linux] 		at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:236)
22:33:55 [linux] 		at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
22:33:55 [linux] 		at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:386)
22:33:55 [linux] 		at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:323)
22:33:55 [linux] 		at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:143)
22:33:55 [linux] Caused by: java.lang.SecurityException: Rejecting hudson.remoting.ChannelFilterTest$GunImporter
22:33:55 [linux] 	at hudson.remoting.ChannelFilterTest$3.userRequest(ChannelFilterTest.java:53)
22:33:55 [linux] 	at hudson.remoting.CallableDecoratorList.wrapUserRequest(CallableDecoratorList.java:30)
22:33:55 [linux] 	at hudson.remoting.UserRequest.perform(UserRequest.java:204)
22:33:55 [linux] 	at hudson.remoting.UserRequest.perform(UserRequest.java:53)
22:33:55 [linux] 	at hudson.remoting.Request$2.run(Request.java:358)
22:33:55 [linux] 	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
22:33:55 [linux] 	at org.jenkinsci.remoting.CallableDecorator.call(CallableDecorator.java:19)
22:33:55 [linux] 	at hudson.remoting.CallableDecoratorList$1.call(CallableDecoratorList.java:21)
22:33:55 [linux] 	... 4 more
22:33:55 [linux] 	Suppressed: hudson.remoting.Channel$CallSiteStackTrace: Remote call to south
22:33:55 [linux] 		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1693)
22:33:55 [linux] 		at hudson.remoting.UserResponse.retrieve(UserRequest.java:313)
22:33:55 [linux] 		at hudson.remoting.Channel.call(Channel.java:908)
22:33:55 [linux] 		at hudson.remoting.ChannelFilterTest$ReverseGunImporter.call(ChannelFilterTest.java:89)
22:33:55 [linux] 		at hudson.remoting.ChannelFilterTest$ReverseGunImporter.call(ChannelFilterTest.java:87)
22:33:55 [linux] 		at hudson.remoting.UserRequest.perform(UserRequest.java:210)
22:33:55 [linux] 		at hudson.remoting.UserRequest.perform(UserRequest.java:53)
22:33:55 [linux] 		at hudson.remoting.Request$2.run(Request.java:358)
22:33:55 [linux] 		at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
22:33:55 [linux] 		... 4 more
22:33:55 [linux] 

@jglick
Copy link
Member Author

jglick commented Jan 29, 2018

I will take a look.

@jglick
Copy link
Member Author

jglick commented Jan 30, 2018

Reproduced locally, investigating.

@jglick
Copy link
Member Author

jglick commented Jan 30, 2018

Builds failing; see #249.

@@ -71,8 +72,9 @@ public final String check(String name) {
* @throws SecurityException if it is blacklisted
*/
public final Class check(Class c) {
if (isBlacklisted(c))
throw new SecurityException("Rejected: " +c.getName());
if (isBlacklisted(c)) {
Copy link

Choose a reason for hiding this comment

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

Nit: indentation looks off here

Copy link
Member

Choose a reason for hiding this comment

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

Just tabs vs. spaces. I can live with that

Copy link
Member

Choose a reason for hiding this comment

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

Will refactor the entire class later

@oleg-nenashev
Copy link
Member

CI Says that Windows tests have passed at least... Not the level of confidence I would like to see, but I doubt we can achieve more right now

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

It's still not enough to close the ticket IMO, but it moves us closer to better diagnostics. Hence 🐝

@oleg-nenashev oleg-nenashev merged commit 9e6472f into jenkinsci:master Jan 30, 2018
@jglick jglick deleted the diag-JENKINS-49027 branch January 30, 2018 17:49
jglick added a commit to jglick/jenkins that referenced this pull request Jan 31, 2018
oleg-nenashev pushed a commit to jenkinsci/jenkins that referenced this pull request Feb 6, 2018
* Slave.JnlpJar.getURL did not work in some modes when core had a snapshot dependency on Remoting.

* Starting to implement Channel.Listener.read/write.

* Now obtaining response timing statistics.

* For now, avoiding timestamped snapshots, as it caused problems for #3120 which I have asked for help from @stephenc diagnosing.

* Simplified logging a bit.

* onResponse

* hudson.FilePath$Mkdirs is a lot more readable than hudson.FilePath$13.

* Specific snapshot.

* onJar

* LoggingChannelListener

* remoting.version=3.17

* Making a few test assertions more lenient to adapt to jenkinsci/remoting#247.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants