[Tails-dev] Review of verification-extension

Dear Uzair,

so here is a more complete review of the extension.

As said, I think there are two urgent matters:

- manifest.json:
  -  "permissions": [
         "http://*/*",
     -> please deactivate HTTP. We only want to download over HTTPS.

- contentscript/verify.js:
- fetchConf(): please wrap regexps.

And here are some other points I realized. If you think that any of
these points are not applicable, please don't hesitate to comment. I'm
not an expert in webextensions.

- contentscript/conf.js:
- change the descriptor file to
https://tails.boum.org/install/v1/Tails/amd64/stable/latest.yml (I think
sajolida created a ticket on our bugtracker already. It's not urgent,
because the other files currently contains the same contents.)

- contentscript/verify.js:
  - are we sure that no other URL could be injected here: ajaxData.url=
this.conf.descriptor; ?
    - if not let's try to at least verify that the URL starts with
      https:// and comes from tails.boum.org
  - What kind of comments does this remove:
    44 data = data.replace(/^[^'"]*#.*/gm, ''); // remove most comments
    I don't see any in
https://tails.boum.org/install/v1/Tails/amd64/stable/latest.yml

  - setVerifyListener(){
    let self = this;
    this.$(this.document).on("change", this.conf.verifySelector, (e) => {
    self.calculateHash(e.target);
    });
    }
    -> So here we assume that the person clicks nicely on our button to
verify and that nobody will interfere.
       - Can we not verify automatically once the download is finished?
       - Also, can we have a listener for the hash in the URL?
         For example, if I closed the window but now I want to come back
and just do the verification without downloading again?

- manifest.json:
- "description": "Verify downloaded file", -> please make it clear
that this verifies a Tails ISO image using a SHA256 checksum. (I think
sajolida will handle this.)

- origin of files:
vendor/forge.js: please add a URL of origin for this script as well as
a version number so that we can update it in the future.

- JSLint http://www.jslint.com/ - this is a tool to write JS code which
is not error prone and I think it would be nice to follow the
requirements of JSLint.
  - Please use double quotes instead of single quotes.
  - convert tabs to spaces
  - fart operator =>
    -  example: this.fetchConf().done()=>{
       JSLint requires the parens around the parameters, and forbids a {
left brace after the => fart to avoid syntactic ambiguity. See:
http://www.jslint.com/help.html#function
 - replace for loops with foreach (low prio)

- Consider using strict-mode:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode
We want this code to be forwards compatible as much as possible as
well as as secure as possible.

- Integrated jquery library. Can we get rid of it and use only vanillaJS?

- Whishlist: please document how to test the extension locally in a
README file.
- Exclude this README file from the resulting XPI.
- See as example the HACKING file in tails
tails@???:download-and-verify-extension

Cheers!
u.