add OWASP Dependency-Check to scan for vulnable components

[gtudan]

Some of the dependencies used in the ext packages got a little dated and have known vulnerabilities.
This adds the OWASP Dependency Check as stage to Travis to help us monitor the issues. The plugin also generates a nice HTML report under target/ in the project base dir that helps to understand what's wrong and where the broken dependencies came from.

I started fixing some of the issues by updating the dependencies. The testsuite looks good (yay!) but I'm pretty sure we need to go through some paperwork to get the new versions approved by eclipse.

For comparison: here's the test before those fixes: https://travis-ci.org/gtudan/krazo/jobs/546367007

There are still some issues left in these extensions, so I filed upstream issues for them:

• Jtwig: Jtwig-web ships a guava version with known vulnabilities jtwig/jtwig#375
• Handlebars: Default handlebars.js (4.0.4) contains security issues jknack/handlebars.java#703

We could wait for those issues to get fixed and update, or mark the test as allowed to fail. I guess it's a matter or what happens first: the fixes or the eclipse paperwork ;-)

To summarize, these are the CQs that need to pass before we can merge:

• Pebble: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20263
• Aciidoctor: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20262
• the dependency check itself: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20283

[chkal]

Awesome! LGTM!

[gtudan]

I filed two CQs for Pebble and Asciidoctorj:

• https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20262
• https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20263

[chkal]

@gtudan Thanks a lot! The CQs look fine! Let's see how long it takes for the Eclipse IP team to resolve them.

BTW: The build is failing for some reason:

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /home/travis/build/eclipse-ee4j/krazo/ext/pebble/src/main/java/org/eclipse/krazo/ext/pebble/PebbleEngineProducer.java:[20,31] package com.google.common.cache does not exist
[ERROR] /home/travis/build/eclipse-ee4j/krazo/ext/pebble/src/main/java/org/eclipse/krazo/ext/pebble/PebbleEngineProducer.java:[111,31] cannot find symbol
  symbol:   variable CacheBuilder
  location: class org.eclipse.krazo.ext.pebble.PebbleEngineProducer
[ERROR] /home/travis/build/eclipse-ee4j/krazo/ext/pebble/src/main/java/org/eclipse/krazo/ext/pebble/PebbleEngineProducer.java:[114,36] cannot find symbol
  symbol:   variable CacheBuilder
  location: class org.eclipse.krazo.ext.pebble.PebbleEngineProducer
[INFO] 3 errors 

Any idea? Maybe it would also be a good idea to rebase it against master to get the fix for #72 in.

[gtudan]

Ooops, sorry. Pebble changed it's caching API and removed the dependency to guava. That's why the test broke. The new API does no longer offer direct access to the cache, so I had to remove some test cases (I didn't want to do nasty casting tricks).

Also since pebble uses a simple concurrent hash map as default cache implementation, one can no longer set a max size for it. I had the choice to add a direkt dependency to guava to get the original cache implementation back, or just drop this option from the engines configuration.

I went with the 2nd option for simplicity (performance tuning of the engine seems out of scope for this kind of extension). Feel free to veto if you think we shouldn't break the API here, or if this option seems important to you.

Also I updated the indentation of all files in the package - only updating the files I needed to touch would have left us with a pretty wild mix. Sorry for the big changeset!

[chkal]

Looks fine to me! Thanks a lot!

Just two things:

• I guess we have to wait for the CQs to get approved.
• Looks like there is some dependency check failing (again?)

[gtudan]

Yes, the handlebars vulnerability is quiet severe and fails the build. I filed an issue for it, but nothing happened.

I‘m not quiet sure on how to proceed with this. It is possible to use a newer handlebars.js version with the engine that we could ship with the extension, but there seem to be known issues.
The other option would be to temporarily whitelist this vulnerability and maybe add a warning to the readme?

[chkal]

@gtudan I just checked the handlebars.java repository, but it seems to be inactive. There are about 8 pull requests which were created this year and none has been reviewed or merged. 😞

Is this Java version really bundling a JavaScript version. They claim that they are a "Java port of Handlebars" and there seems to be some grammar related stuff in their repository.

[gtudan]

I was asking about this as well. Most of it runs in Java, but some features use the JavaScript through Nashorn. There‘s a discussion about this here: jknack/handlebars.java#696 It‘s hard to tell if those features are affected by the reported vulnerability. The Question that comes to mind is whether we should continue to support the library if it‘s no longer being actively maintained?

…

Am 26.06.2019 um 07:11 schrieb Christian Kaltepoth ***@***.***>: @gtudan I just checked the handlebars.java repository, but it seems to be inactive. There are about 8 pull requests which were created this year and none has been reviewed or merged. 😞 Is this Java version really bundling a JavaScript version. They claim that they are a "Java port of Handlebars" and there seems to be some grammar related stuff in their repository. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[chkal]

The Question that comes to mind is whether we should continue to support the library if it‘s no longer being actively maintained?

Good question.

@eclipse-ee4j/eclipse-krazo Any other thoughts about that?

[lefloh]

+1 for removing it or moving it to a krazo-experimental repo.
(In general I'd prefer less ViewEngines with good krazo integration (CDI injection etc.) than a bunch of POCs ;-))

[chkal]

In general I'd prefer less ViewEngines with good krazo integration (CDI injection etc.) than a bunch of POCs ;-)

+1000!

We should definitely spend some time on improving the existing view engines. Especially regarding the support for CDI models.

However, I'm not sure if we should really remove the Handlerbars version. Especially, because we are not completely sure, if the Java version is affected by the CVE.

Maybe we should whitelist the CVE for now? IMO it would be great to have the new plugin in the master branch and I'm not sure if this should be blocked by the Handlebars issue.

[gtudan]

In general I'd prefer less ViewEngines with good krazo integration (CDI injection etc.) than a bunch of POCs ;-)

Definitely! I like the idea of dropping some that integrate engines that haven't been updated in a while and moving the rest to an experimental repo until they have matured enough.

Properly implementing all those extensions (with CDI, engine specific features, configuration, tests and documentation) and keeping them up to date will require a lot of work and we would need more maintainers for them.

[chkal]

Definitely! I like the idea of dropping some that integrate engines that haven't been updated in a while

If libraries are not maintained anymore, I'm fine with removing the corresponding integration module. AFAIK this would only affect Handlebars ATM, correct?

Perhaps we should simply whitelist the corresponding vulnerability for now to get this PR merged and then remove Handlebars in a subsequent PR?

and moving the rest to an experimental repo until they have matured enough.

I'm not sure I agree on this one.

AFAIK most of the integrations are more or less feature complete with CDI models being the only feature that is missing for most. I spent some time on fixing encoding and content type negotiation issues in the past and IMO this is working great now. And I think that configuration of these engines is quite flexible because you can always specialize the default producer for the corresponding configuration/engine instance and therefore have full control.

CDI models are not support by most engines, because these engines do not support the hooks we need. Most of them just support a simple Map<String, Object> for the model and doesn't support something like VariableResolver.getValue(String key). However, I think we could build some generic lazy-evaluating map which basically translates all read operations to BeanManager lookups. I hope that such a solution would allow us to integrate CDI models into most of these engines in a very simple way.

To sum up: I think we can keep most of the existing engines with a few exceptions although they don't support CDI models yet, because in all other regards they are quite stable.

[gtudan]

I‘m fine with whitelisting handlebars for now. After digging around the code I‘m quiet confident that the vulnerability only affects the rendering itself, which was rewritten in Java. So I‘ll update the MR.

…

Am 30.06.2019 um 16:24 schrieb Christian Kaltepoth ***@***.***>: Definitely! I like the idea of dropping some that integrate engines that haven't been updated in a while If libraries are not maintained anymore, I'm fine with removing the corresponding integration module. AFAIK this would only affect Handlebars ATM, correct? Perhaps we should simply whitelist the corresponding vulnerability for now to get this PR merged and then remove Handlebars in a subsequent PR? and moving the rest to an experimental repo until they have matured enough. I'm not sure I agree on this one. AFAIK most of the integrations are more or less feature complete with CDI models being the only feature that is missing for most. I spent some time on fixing encoding and content type negotiation issues in the past and IMO this is working great now. And I think that configuration of these engines is quite flexible because you can always specialize the default producer for the corresponding configuration/engine instance and therefore have full control. CDI models are not support by most engines, because these engines do not support the hooks we need. Most of them just support a simple Map<String, Object> for the model and doesn't support something like VariableResolver.getValue(String key). However, I think we could build some generic lazy-evaluating map which basically translates all read operations to BeanManager lookups. I hope that such a solution would allow us to integrate CDI models into most of these engines in a very simple way. To sum up: I think we can keep most of the existing engines with a few exceptions although they don't support CDI models yet, because in all other regards they are quite stable. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[chkal]

@gtudan Awesome! Thanks!

[chkal]

Do we have all required CQs for this one? IPZilla seems to be down. So I cannot look it up at the moment.

[gtudan]

Hi Christian, Yes, all CQs have been accepted. We‘re ready to merge.

…

Am 06.07.2019 um 07:22 schrieb Christian Kaltepoth ***@***.***>: Do we have all required CQs for this one? IPZilla seems to be down. So I cannot look it up at the moment. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

[chkal]

I think Handlebars 4.1.2 is not there yet, or am I missing something?

[gtudan]

You're right, I did not file a CQ for this, as I was hoping they might ship an update fixing the root cause of the issue. Since the new version only contains a few minor fixes I downgraded back to the original version, so we can get this in.

If anyone uses handlebars and would like to update nonetheless I'm happy to provide a new PR (with CQ).

[chkal]

@gtudan Thanks for updating the PR! Merging this now!