GateNLP / gate-core

The GATE Embedded core API and GATE Developer application
GNU Lesser General Public License v3.0
75 stars 29 forks source link

Handle XStream security warning message #121

Closed johann-petrak closed 3 years ago

johann-petrak commented 4 years ago

The 9.0-SNAPSHOT version of GATE shows the following message in the message pane:

Security framework of XStream not initialized, XStream is probably vulnerable.

We need to find a way to deal with this:

greenwoodma commented 4 years ago

The message has been present for years (not just in 9.0-SNAPSHOT) it's just that previously the message appeared on the console rather than in the message pane so most people just ignored it.

We had a long discussion about this on slack earlier this year, and the conclusion was that we would do nothing, because the warning is accurate and there is no good solution that both allow us to support any object as a feature (a fixed requirement for backwards compatibility etc.) and that wouldn't risk opening up a security hole in any application that embeds GATE.

What might be a good idea would be to add a short piece on this to the developer track training materials to show how you could lock XStream down when you embed GATE inside another (web) app.

greenwoodma commented 4 years ago
  • xstream apparently (hearsay, did not check myself) directly prints to System.err instead of using a logger

and yes this is true. I checked this in the xstream source code when we discussed it before so there is no way to suppress the warning via the GATE GUI log configuration.

johann-petrak commented 4 years ago

Added this mainly so people not on Slack can also find this. Also, there may be something that can be done, e.g. allowing only POJOs in the features initially, and suppressing the message by default by having the security enabled, and only with some extra config allowing everything, but then also accept that message.

What is really annoying is e.g. that when somebody populates a corpus with a few thousand documents, as many xstream errors appear in the message pane.

greenwoodma commented 4 years ago

What is really annoying is e.g. that when somebody populates a corpus with a few thousand documents, as many xstream errors appear in the message pane

Interesting. I'm sure it only ever used to display the message once. If it's now showing it for every document, that might mean something inside xstream is being recreated each time we use it. If so that might also explain https://github.com/GateNLP/gate-core/issues/122

johann-petrak commented 4 years ago
  • xstream apparently (hearsay, did not check myself) directly prints to System.err instead of using a logger

and yes this is true. I checked this in the xstream source code when we discussed it before so there is no way to suppress the warning via the GATE GUI log configuration.

Do you by any chance remember the location?

greenwoodma commented 4 years ago

It was on line 1485 of the main XStream class in the version I checked out locally (1.4.11.1). Looking at the master branch though and I now can't find the same bit of code. It might be worth checking out the master branch, building it and then using that as a GATE dependency to see what happens. Maybe this has been "fixed" in some way ready for the next release.

greenwoodma commented 4 years ago

although from a quick check of doing that the GATE tests now fail with security exceptions related to XStream, so it's not just a case of dropping in the newest version of XStream, and when it is released we'll need to revisit this problem.

I'd suggest that for now we close this issue because it's likely anything we do about it will only last as long as the next release of XStream anyway, so we might as well live with it for now, and then look at fixing things properly to conform to the new security model once XStream release 1.5. Does that seem like a sensible approach?

ianroberts commented 4 years ago

I suspect XStream devs would say that the fact that it's impossible to suppress the "YOU REALLY SHOULDN'T BE DOING THIS, IT'S INSECURE" warning is considered a feature rather than a bug...

johann-petrak commented 4 years ago

I think we could leave it open until we actually do something about it, but we do not actually have to do anything about it now :)

greenwoodma commented 3 years ago

In the latest version of XStream (1.4.15) the message (and approach) has changed to read "Security framework of XStream not explicitly initialized, using predefined black list on your own risk." which is slightly less scary.

greenwoodma commented 3 years ago

Closing this via 9532ab9 which makes the new minimal blacklist the default for both Developer and Embedded but allows API users to change it (prior to Gate.init()) should they wish to.