Passphrase should not be stored in LocalStorage

[multiwebinc]

I see you've integrated a lot of my code. I just have one suggestion, instead of having the passphrase saved in LocalStorage (where it is available in plain text), it should be in the background page and the user should be prompted with a pop-up asking for the passphrase when starting the app for the first time.

[Sneezry]

Yeah, it's not a good idea to store passphrase in localStorage, however, the extension only has event page, but not background page, so we cannot store phrase in background. Maybe we can let users to choose store phrase in localStorage or input manually each time starting the extension.

Changing event page to background page is not a good idea, either. It may make Chrome slow and consume more memory.

[multiwebinc]

I see that you have LOTS of event pages, but most seem like they are just libraries. They can most likely be moved to just a regular <script> tags so that they will only be loaded when the popup is opened, which I would assume is just as efficient as an event page. From here:

A common need for extensions is to have a single long-running script to manage some task or state. Background pages to the rescue.

If the scripts are not managing some task or state, they should not be background/event pages. Even the background.js file looks like these functions could easily be moved to regular javascript since doesn't look like they are saving any sort of state (except chrome storage, which is also available in the regular JS).

Hope this makes sense.

[Sneezry]

We only have one event page - all scripts are included in one page. Our event page is watching request from content scripts for recognizing qr code. Perhaps we can encode phrase with base64 or something else to improve the security, however, security and convenience are always a contradiction.

[multiwebinc]

Since we are using Chrome storage, when moving to another computer the localStorage variables aren't available, so none of the accounts would be available, so it's not only a security issue, but also a functionality issue as well since there is currently no way to input a passphrase.

And, from your manifest file, all of these files are event pages, so you do have LOTS, not just one:

"background": {
    "scripts": [
        "assist/aes.js",
        "assist/md5.js",
        "jsqrcode/grid.js",
        "jsqrcode/version.js",
        "jsqrcode/detector.js",
        "jsqrcode/formatinf.js",
        "jsqrcode/errorlevel.js",
        "jsqrcode/bitmat.js",
        "jsqrcode/datablock.js",
        "jsqrcode/bmparser.js",
        "jsqrcode/datamask.js",
        "jsqrcode/rsdecoder.js",
        "jsqrcode/gf256poly.js",
        "jsqrcode/gf256.js",
        "jsqrcode/decoder.js",
        "jsqrcode/qrcode.js",
        "jsqrcode/findpat.js",
        "jsqrcode/alignpat.js",
        "jsqrcode/databr.js",
        "javascript/background.js"
    ],
    "persistent": false
},

I have edited it so that it is a background page instead of an event page and all of these files are loaded dynamically only when needed. I hope this should be a good compromise. This way these scripts will only be loaded when a user is scanning a QR code. I also made a thing to prompt the user for the passphrase.

Pull request to come in a few minutes.

[multiwebinc]

Also, base64 encoding the passphrase does absolutely nothing to increase security. It is an encoding, so all a hacker would have to do to obtain the original passphrase would be to just decode it. Decoding the string is trivial. At best, this is security through obscurity.

[Sneezry]

Hi, multiwebinc

We do have only ONE event page, scripts are included in ONE page automatically by Chrome

and we do give a way to input passphrase - click Encrypted text

and store plain text in memory is not safer than store in localstorage, if a hacker can read your data in your localstorage, I do believe he can read your memory (for our CSP, xss is impossible).

[multiwebinc]

Hi and thanks for your response. I think that the biggest problem with how it is now is the user interface. It wasn't at all obvious to me that I should click on the text "Encrypted" and even after I click it and the pop-up appears, it is not obvious that I am supposed to enter the existing passphrase because it is asking me to confirm it. Any time I have had to confirm a passphrase (i.e. enter it in twice) it's because I'm entering it for the first time. I think that a pop-up should be displayed automatically if the passphrase is not present and the accounts are encrypted and it should not ask for verification. The only time the user should need to verify the passphrase is when entering it for the first time or changing it.

And as for whether to use localstorage or memory to store the passphrase, my main problem with localStorage is that it is not cleared when the browser is closed. If it's a shared computer (e.g. shared with the family), the passphrase is easily available in localStorage even weeks in the future. If it's stored in memory, all it takes is a browser close and it needs to be re-entered. It is also significantly easier to retrieve something from localStorage than it is from javascript memory because Chrome extensions have a security feature that if any of the files are modified, they will automatically be re-downloaded from the server. I'm not saying it's impossible, but I do feel that storing it in memory is safer.

[Sneezry]

Thank you multiwebinc

I admit the way I handle asking passphrase for decrypt secret is terrible, and I will give an interface in later version :)

And I admit the way I handle storing passphrase is not good, I will test how much affect it may take changing from event page to background page.

Thanks again :)

Li Zhe

[multiwebinc]

Check out the pull request that I created. It loads all of the other scripts on demand, so unless someone needs to scan a QR code, the usage of the background page will be next to nothing. Let me know if you want me to help you with something and I will be glad to.