search

gustavomondron / twik

Twik is an Android application that makes it easier to generate secure and different passwords for each website.
GNU General Public License v3.0
35 stars 9 forks source link

Allow longer private keys #5

Closed greizgh closed 9 years ago

greizgh commented 9 years ago

It would be nice to allow users to enter longer private keys. Current limit is 48 chars and is fine for random keys but not for long sentence. Is there any technical reason to such a limitation?

daMihe commented 9 years ago

It does not seem to be a technical reason at the app side - as i took a quick view of the hasher function. But allowing longer private keys may break compatibility to the browser plugins (where this algorithm is adapted from).

But also a question is, wheter you need to remember your private key. For example - i wrote my key down on a paper. This is not very critical because you need also your passphrase to generate correct passwords. And that one passphrase should be protected and somehow to remember.

So for clarification: If you need a longer master password you're maybe right - it maybe should allow more than 48 symbols (have not looked up how much is allowed) but for the private key this should be enough - as you don't need to remember this and can write it down.

greizgh commented 9 years ago

Thanks for clarification. Concerning browser plug-in compatibility, I don't think a longer private key would break it as long as the HMAC implementation follows the specification. Firefox add-on rely on internal implementation and should behave correctly.

I'm okay with the fact of writing down my private key, but if there is no technical/practical limitation, then we probably shouldn't restrict the user?

Note that I'm satisfied with current Twik behaviour and I totally understand this feature can break things.

FYI: master key is limited to 25 symbols.

Edit: I changed constants.xml as follow and I'm quite happy with it:

<resources>
[...]
    <item name="text_private_key_max_length" format="integer" type="integer">255
    </item>
</resources>
daMihe commented 9 years ago

Yes you're right - it would be better not to limit. Instead a warning should be displayed if a key is set which is longer than 48 symbols.

Since i can't change the repo (i have no rights) and my last pull-request is now already over a month old (and not inserted nor rejected), i'm not sure when we can change this in an "official" way.