[JEP-200 Discussion] - Switch Remoting/XStream blacklist to a whitelist

[Oleg Nenashev]

Hi all,

I am starting this thread in order to collect extra feedback about JEP-200, which proposes switching Remoting/XStream implementations from a blacklist to a whitelist. The intention is to significantly reduce risks of class deserialization attacks, which was hitting Jenkins project seriously over last 2 years (e.g. SECUIRTY-429 this April). This JEP is accepted as a draft, and the current state is published here.

I am assigned as a BDFL Delegate who makes a decision about accepting/rejecting this Jenkins Enhancement Proposal (see JEP-1 for more info about the process). Over the next week I will be reviewing this JEP and providing feedback in this thread and in pull requests.

I also call other interested contributors to comment regarding this JEP. It is important, because the proposal implies a high risk of regressions in plugins and other Jenkins components. The JEP sponsor made a significant amount of testing, but there may be some gaps. Any feedback and extra testing of the reference implementation will be appreciated.

There are several ways to provide the feedback:

• Comment in this thread
• Create a pull request with document edits
  
• Ping me (oleg-nenashev) and Jesse Glick (jglick) in IRC
  

My current plan is to finalize the Draft reviews/edits by December 30 though it depends on the sponsor's availability during the Christmas break if there is a discussion needed. If you have any comments or interest to review the JEP deeper, please respond by this date.

Best regards,
Oleg Nenashev

[Oleg Nenashev]

While we are doing the JEP review in the pull requests, I would also want to start one topic here. The current JEP-200 design shares the whitelist between Remoting and XStream, and I am a bit aware of that.

The main purpose of whitelisting for Remoting is to allow particular communications between agent and master by saying the class is secure to be sent. For example, Jesse added several JGit classes in Jenkins PR #3120. Do we want these classes to be stored on the disk or submitted in config files? Hell no, it may cause behavior and performance issues in Jenkins. But currently there is no way to restrict classes for Remoting-only or XStream-only.

So, I would like to put the following proposal on the table:

• Allow passing the "SerializationType" (or context) to the CustomClassFilter. Modify the implementation to pass this context
• Remoting ClassFilter blacklist should stay global by default, I'd guess
  
• Create 2 new CustomClassFilter implementations, which operate only for Remoting or XStream
• Split whitelist resource files:
• Remoting: META-INF/hudson.remoting.ClassFilter 
• XStream: META-INF/ihudson.util.XStream2.ClassFilter (or so)

It will allow managing the whitelists individually in plugins if needed.

If we eventually have another type of serialization filtering (e.g. for JBoss Marshalling in Pipeline), we will be able to extend the engine easily as well.

SerializationType could be made an internal and Restricted extension point in such case.

WDYT?

BR, Oleg


понедельник, 18 декабря 2017 г., 13:41:20 UTC+1 пользователь Oleg Nenashev написал:

[Jesse Glick]

On Thu, Dec 21, 2017 at 1:38 PM, Oleg Nenashev <o.v.ne...@gmail.com> wrote:
> The main purpose of whitelisting for Remoting is to allow particular
> communications between agent and master by saying the class is secure to be
> sent.

Received. And/or loaded from XML files.

> For example, Jesse added several JGit classes in Jenkins PR #3120. Do
> we want these classes to be stored on the disk or submitted in config files?

IIRC some of these were in fact used in certain cases in `build.xml`
files, but I may not be remembering that right.

> Hell no, it may cause behavior and performance issues in Jenkins.

Which “behavior and performance issues” are you referring to? They
sound unrelated to security and are best addressed, if required, by
plugin developers on their own initiative.

The purpose of the JEP-200 whitelist is to protect the master JVM from
deserialization exploits (originally coming from CLI clients, now
coming from compromised agents or REST/CLI calls to upload XML). These
could come equally from Java Remoting or XStream, usually with the
same classes. (Sometimes only from XStream since there is no
requirement to implement `Serializable`; anyway is it harmless to
Remoting to include an entry which can be exploited only via XStream.)

If JBoss Marshalling were exposed to user actions, the class filter
would be applied to it as well. But `program.dat` is only ever written
by the master to begin with, and `script-security` should block
programmatic creation of payloads.

> currently there is no way to restrict classes for Remoting-only or
> XStream-only.
>
> So, I would like to put the following proposal on the table:
>
> Allow passing the "SerializationType" (or context) to the CustomClassFilter.
> Modify the implementation to pass this context

I can think of no reason to do that. It just adds complexity and
confuses the issue.

[Oleg Nenashev]

They sound unrelated to security and are best addressed, if required, by plugin developers on their own initiative.

Let's park this question for now. I am going to play with the current PR state, and then I will provide a response around Jan 02.

My IMHO is that it would be preferable to separate Remoting and XStream from very beginning, so that the plugin maintainers will think twice when they try to save custom classes on the disk or to send them via Remoting. But I agree it may be over-engineering. All contributors are welcome to comment.

BR, Oleg

 

понедельник, 18 декабря 2017 г., 13:41:20 UTC+1 пользователь Oleg Nenashev написал:

Hi all,

[Oleg Nenashev]

We have discussed the single whitelist concern with Jesse and agreed that there is no immediate need to implement it as a part of this JEP.
The testing concern has been also addressed last week, all recommended and other popular plugins have been tested by ATH/PCT.

There was no other feedback regarding JEP-200 in this thread and other channels, so I am going to accept it. I am going to continue testing the change in order to improve the coverage and maybe catch some missing whitelist entries. Tomorrow I will send a separate email with testing guidelines so that any plugin maintainer can test his/her plugin if needed.

The final pull-request to JEP-200 is here: https://github.com/jenkinsci/jep/pull/43
Once it is integrated, the JEP will be officially accepted. If you have any concerns, please shout about it ASAP

Best regards,
Oleg


вторник, 26 декабря 2017 г., 15:52:51 UTC+1 пользователь Oleg Nenashev написал:

[Oleg Nenashev]

Heads up, JEP-200 has been accepted. I am going to proceed with the current roll-out plan which targets delivery in weekly in 2.102 (next weekly) unless there is no major issues discovered.

My current plans:

• Jan 9 morning, EU TZ - Get Remoting 3.16 and all packaging (Docker/Swarm) released
• Jan 9 EoD, EU TZ - Send announcement to the mailing list with the testing guidelines so that others can try the patch if they want
  
• it's generally available for more than 1 month by now, but it will be easier to do testing with guidelines
  
• Jan 10..12 - More testing + feedback processing
  
• Jan 12 - Integrate https://github.com/jenkins-infra/jenkins.io/pull/1293 with the announcement to Jenkins users
  

Best regards,
Oleg Nenashev

понедельник, 8 января 2018 г., 18:24:40 UTC+1 пользователь Oleg Nenashev написал: