SsKaTeX robustness improvements

[ccorn]

These commits address some shortcomings of the commit resulting from #455:

1. JS engine availability test(s) now deferred until configuration is available. This also prevents ExecJS from making unnecessary engine tests und possibly other side effects until those are requested per configuration.
2. The Rakefile task dev:test_katexjs was not good enough (and therefore not accepted). It only tested for presence of katex/katex.min.js and therefore could not catch many engine-related errors or compatibility issues. The replacement dev:test_sskatex_deps now makes an actual SsKaTeX test run and tests whether the output is KaTeX. The test for katex/katex.min.js has been retained for nicer diagnostics.
3. Similarly, the flag KATEX_AVAILABLE in test/test_files.rb now indicates that successful SsKaTeX compilations can be done.
4. Added a security note to the SsKaTeX documentation. Basically, options with js in their name are admin stuff and should not be left to unknown users unless the service using kramdown runs in a properly isolated environment.

May need some final tuning. Give this a day to settle down.

[ccorn]

The security issues regarding user-specified Javascript (locations) are bothering me. Until recently, I figured that there is no reasonable way for kramdown to know what parts of its configuration input are trusted, so the responsibility for securing the configuration has to be passed upwards anyway. But there may be better ways to do this. What about this:

1. Reject tainted *js* options unless there is an untainted option userjs: true. The tainting mechanism works even when $SAFE == 0, one just has to check explicitly. This is a reasonable way of moving the responsibility upwards: Whoever wants to reconfigure the JS engine needs to explicitly untaint the corresponding options.
2. Designate a location for a trusted default configuration file and untaint its contents after loading. We might even use the new kramdownrc for that, provided that it is documented that that file is considered to be trusted. The rationale is, if a user can write to config files, that user probably has write permissions to about everything in $HOME and can therefore install local ruby gems as well, thus gaining arbitrary command execution.
3. No adaptation of the kramdown executable. Unless enabled with an untainted userjs: true from the trusted configuration file, command line *js* options are rejected.

I'd prefer to make rejection of tainted *js* options loud, i.e. raising a SecurityError instead of silently discarding them.

[ccorn]

There are some problems with the above concept:

• All *js* defaults would need to be secure. Currently, this means that the katexjs default would need to change. Which makes me wonder how we could get the tests to work at all.
• Testing needs to be independent from everything outside the kramdown tree. No kramdownrc therefore. Besides, that file is meant to be auto-loaded by the kramdown executable only.

Still trying to come up with something practical and reasonable. Ideas?

[gettalong]

Thanks again for you changes!

I think the easy solution for the security issues would be to add an option to disable the application of the 'options' extensions. Then it is not possible for a user to change anything.

Another way would be to document the issue and explain how to overcome it, i.e. by separating the parsing and converting phases and applying the correct options in between:

doc = Kramdown::Document.new(content)
doc.options[:math_engine_opts] = ...
doc.to_html

What do you think?

[ccorn]

I think the easy solution for the security issues would be to add an option to disable the application of the 'options' extensions. Then it is not possible for a user to change anything.

I thought about that. However:

1. This only applies to the kramdown executable.
2. This is an opt-out approach because it requires admins to make changes in their kramdown usage just to stay safe.

Ideally, we'd have an opt-in mechanism so that simply upgrading kramdown should not open JS code execution possibilities, whereas enabling SsKaTeX (or its JS configuration bits) should require a bit of affirmative administrator action.

There is a sensible solution but it would take effort that I have shied away from so far:
Separate SsKaTeX from kramdown by making it a separate tex-to-html gem like e.g. ritex or itex2mml. Benefits:

• Installing the SsKaTeX gem would be the required opt-in. The gem's readme would state that its configuration opens a path to arbitrary JS code execution unless properly secured against malicious use, and whoever installs it has to know what they are doing.
• kramdown would just test for the gem and use it if present, like with the other plugins.
• The gem could be used by other projects as well.

Costs:

• Complete gem infrastructure required, including tests and a maintained public repository instead of just a temporary fork (I'd rather set up an own Gitblit instance instead of using Github). I'd like to avoid that effort. Having to add the extra bits might delay things until the end of 2017.

It hurts me writing this, but the separate-gem approach currently seems to be the truly best way of doing things.

Thoughts? Suggestions?

[ccorn]

Encouraging news:

• I have now factored out the actual TeXmath-to-HTML conversion to a separate minimal gem, unsurprisingly named sskatex.
• This has drastically simplified the code in the corresponding interface module ::Kramdown::Converter::MathEngine::SsKaTeX.
• The JS files below data/ have gone with the extra gem.
• Along with those restructurings have come some regularizations:
  • I have inserted underscores in the engine-specific option names where appropriate.
• The documentation and remaining infrastructure (Rakefile, .travis.yml) has been updated in accordance with the changes
• The CI tests pass, and I am somewhat relieved.

[ccorn]

This branch seems to be ready for merging now. Further work is supposed to focus on the external SsKaTeX gem without touching those parts of its API that are used by kramdown.

[gettalong]

@ccorn This looks great - once you rebase and squash your commits into one, I will incorporate the commit and release a new kramdown version (all other tasks for this release are now finally done...).

[ccorn]

OK, rebased and squashed.

[gettalong]

Thanks @ccorn - merged and done!