[CLOUDSTACK-9261] Upgrate jQuery-UI to 1.11 (JQuery UI 1.8.4 prone to XSS)

[rafaelweingartner]

Description

This PR addresses an old security issue regarding jQuery-UI. We are updating jQuery-UI to version 1.11, instead of 1.12, because the 1.12 version requires an update of jQuery.js as well. Therefore, to reduce the surface of changes, we are first only updating the jQuery-UI library.

Moreover, unnecessary CSS and image files from the jQuery-UI library were deleted.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

I tested the changes introduced here by generating the RMP packages, updating ACS in a test environment and then using ACS UI to see if I something is not working. So far I have tracked and fixed all of the problems I encountered. I would like to get some help on testing this as well. Therefore, if you have some minutes/hours to spare, please do help.

Checklist:

• I have read the CONTRIBUTING document.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
  Testing
• I have added tests to cover my changes.
• All relevant new and existing integration tests have passed.
• A full integration testsuite with all test that can run on my environment has passed.

@blueorangutan package

[blueorangutan]

@rafaelweingartner a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1870

[khos2ow]

Thanks Rafael for the PR, but keep in mind that there are lots of changes in these two versions (and considering some functionalities won't work with older JQuery itself, so that needs to be upgraded) and most of the time these are not backward-compatible. Going forward with this PR requires a lot of manual -in browser- testing.

[rafaelweingartner]

Yes, we need more manual testing. I already tested the most common features in ACS UI.

jQuery-UI 1.11 is compatible with our jQuery version. So, there should be no problems there. On the other hand, jQuery-UI changed quite considerably from 1.8 to 1.11. I believe I caught most of the problems, but there might be something I have not tested yet.

[khos2ow]

Cool, I didn't expect them to be compatible!!

[rafaelweingartner]

The 1.12 (the latest version) is not. However, the 1.11 is. The fix for the XSS vulnerability was first introduced in jQuery-UI 1.10.

[resmo]

Any reason not targeting 4.11.1 (4.11 branch) ?

[rafaelweingartner]

@resmo yes there is a reason. The possibility of getting conflicts and having to solve them when merging forward to 4.12.

It took me over 16 hours to track all of the problems after I updated the library. The Javascript part of ACS and the ad-hoc framework that we have to generate pages is pretty annoying to debug and track problems; especially when there is no error or stack trace. Sometimes information was not appearing, or windows were rendered in an odd way, and I needed to track what was going on under the hood debugging from the most basic method that I knew until the "framework" that is generating HTML elements at runtime.

[resmo]

@rafaelweingartner thanks for explanation.

[rohityadavcloud]

There are several changes that make break the UI, also if this is security related 4.11 can benefit from the change. Can you remove the new jquery lib with minified versions, as the added library is 16k+ loc.

[rafaelweingartner]

@rhtyd Changes break or may break? I have tested this. At least all of the functions I used are working just fine.

As I said here before, I know that this update can break some code. I executed a lot of testing so far, and I believe I caught and fixed most of the problems caused by this upgrade. That is why I asked for help, to get different eyes testing this.

Deploy VM, create zone, create VPC, create ACLs, create and edit ACL rules, create and edit LB rules, create and edit roles, create affinity groups, storage (create volume, edit, upload), networks (add l2, add guest, add isolated) and so on.

I can use the minified version of jQuery-UI, but bear in mind that our jquery.js is not minified. It looks like we have many other libraries that are not using minified version either. I only used the non-minified version to help during debugging.

I will not open a PR against 4.11, as I explained before to @resmo. This is a long-standing issue that I decided to address, and I intend to target master only.

I will wait for the community feedback on both.

• should I use minified version or non-minified ones?
• is the community ok with me opening this against master only?

[GabrielBrascher]

I spent some time testing it in a test environment and it seems to be all good. However, two buttons need some attention:
(i) 'About' button: here the user cannot get out of the info screen unless clicking on some button as the 'add instance' (or a respective button in another view, as the 'add network offering', 'add' a template, ...).

(ii) 'Add network offering' button on Home > Service Offerings - Network Offerings: in this case, the add network offering view requires a scroll down from the user.

It should be something like the 'Add compute offering':

Thanks for the PR @rafaelweingartner . So far these are the points that I could raise in my test environment.

[rafaelweingartner]

Thanks @GabrielBrascher! I fixed both of the issues you raised.
What do you guys think? Should I change the non-minified version of Jquery-Ui to the minified one?

Others (@khos2ow, @rhtyd, @DaanHoogland and so on), could you also help here?

[rohityadavcloud]

LGTM, subject to testing. This requires a lot of manual testing of UI to be merged.

[rohityadavcloud]

@rafaelweingartner we'll need to manually test this, I've approved this in theory. I will still prefer a minified jquery library instead of additing another 17k loc to the repo of dependency.

To allow this to be merged, we'll need someone other than the author to manually test the UI and its several views.

[rafaelweingartner]

@rhtyd I totally agree. That is why I asked for help.

Even though I tested all of the features we use here, on a second batch of test @GabrielBrascher caught some other problems.

I tested with an advanced setup. Whereas, I think, @GabrielBrascher tested with a basic setup.

[GabrielBrascher]

@rafaelweingartner @rhtyd I tested with a basic network. All points that I raised were fixed by @rafaelweingartner; however, I might have missed some use case when manually testing (e.g. an action such as configure IP range, start VM ...).

[rafaelweingartner]

@GabrielBrascher did you approve the PR?
I think everything is fine to merge then....

[GabrielBrascher]

LGTM based on building from the source and testing it.

[rafaelweingartner]

I will proceed and merge this one then. Thank you all for the help here.

[blueorangutan]

@rafaelweingartner a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2179