Use variable length string for passwordSubstitute

[Mygod]

The current implementation is painfully redundant. For example in the sample app, one could just simply use ••••• is a secret number as app:pref_summaryHasText without using String.format to insert a fixed String.

[gregkorossy]

The substitute text's length is intentionally fix so that no one can guess the actual password by looking at the number of substitute characters when the user scrolls through the list.

[Mygod]

I know. And you still have the option to do that after this change.

[gregkorossy]

I understand that but IMO the direction is wrong. The default should be the fixed length thing and the variable length would have to be enabled by an explicit attribute / call. This needs to be done in this fashion since most users won't care about the security aspects and dangers of having a hidden password displayed with its actual length.

[Mygod]

Even in theoretical security, the length of password isn't considered secret in most settings (for example, see the security definition for encryption scheme). It's not really a big deal IMO.

[gregkorossy]

The length technically limits the "guess space". In most cases this would mean something like if the length is 4, it's probably the year of birth. Webpages usually do the same: showing a fixed length hidden pass instead of the actual length when asking the user to review the entered data before submitting it. This is because the users do not care about the password directly since they only see asterisks (or other non-letter chars) which has no meaning other than to display the field as "there was a password here". I think showing the actual length of the entered value does not add any value to the user but might reveal the pass to the attacker by just seeing the size of it. The math also supports this: testing all combinations starting with 1 letter through 6 (just an example) takes significantly more time than testing for only 6-long passwords. So either add an argument / call that explicitly enables this less-secure behavior or it won't be merged.

[Mygod]

Talking is cheap.

Assuming alphabet size sigma, testing all combinations starting with 1 to n takes: sigma + sigma^2 + sigma^3 + ... + sigma^n = (sigma^n - 1) sigma / (sigma - 1) which has entropy nlog(sigma) - O(1/sigma).

Testing all combinations with length n takes: sigma^n which has entropy n*log(sigma).

You see the difference in entropy is just at most a constant if n is sufficiently large. If n is small, the user should blame himself for using a weak password.

[gregkorossy]

Still, it won't get merged until the default behavior is changed. If it's not a big deal to display the actual length then it shouldn't be a big deal to not display it.

[Mygod]

In plain English:

1. If the password is weak, a brute-force attacker can attack it without length information;
2. If the password is strong, giving a brute-force attacker the length information doesn't make him suddenly able to attack something he can't attack before.

For the latter comment, I agree. From the security perspective, it really makes no difference. But there are two drawbacks for the current approach:

1. passwordSubstitute is painfully redundant;
2. The user interface isn't intuitive to user either.

Therefore I suggest either:

1. Merge this change or
2. Don't display a fake 5-digit password at all, use an empty string instead.

[zhanghai]

I think a fix library should not mess with this, and it does not have an agreed behavior as well. Simply let develeopers extend EditTextPreference and override getSummary() to obtain the behavior they see fit will be good enough. Or at least, please leave the default behavior unchanged and offer this new functionality in a different subclass.

[gregkorossy]

Superseded by the pref_summaryPasswordSubstituteLength attribute which can set either a fixed length or set the behavior to dynamic length of the substitute password in the summary. Also, the EditTextPreference has been reverted to its original state and the new behavior is implemented by AutoSummaryEditTextPreference available in the same package starting from v27.0.2.0.

[Mygod]

Sounds good. Why do we make that optional though? We can insert % by using %%.

[Mygod]

Also standard ListPreference uses this behavior so I think it wouldn't cause too much harm.

[gregkorossy]

Why do you want to insert %?

I personally do not like the ListPreference-like summary handling because there's no way to display a summary when there's no value set. Using a different attribute is not a big burden yet it keeps the possibility to choose either this way or the ListPreference one.