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 ta...@git.tails.boum.org:download-and-verify-extension Cheers! u. _______________________________________________ Tails-dev mailing list Tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev To unsubscribe from this list, send an empty email to tails-dev-unsubscr...@boum.org.