Limit size of password sent to zxcvbn to prevent lock ups

[smason]

Overview

I enabled "secret service" integration, and some users of this service seem to store some very long JSON objects as their "password". this makes all interactions incredibly slow when the strength meter is enabled.

Steps to Reproduce

create a "password" containing an 8kib password

this causes the UI to hang for a minute or so which doesn't seem great!

Context

can reproduce under Linux (wayland) I've just built from develop branch. issue goes away with:

diff --git a/src/zxcvbn/zxcvbn.c b/src/zxcvbn/zxcvbn.c
index 23792b4d..0d2c9893 100644
--- a/src/zxcvbn/zxcvbn.c
+++ b/src/zxcvbn/zxcvbn.c
@@ -1577,6 +1577,11 @@ double ZxcvbnMatch(const char *Pwd, const char *UserDict[], ZxcMatch_t **Info)
     i = Cardinality(Passwd, Len);
     e = log((double)i);
 
+       if (Len > 64) {
+               fprintf(stderr, "not checking strength password, length=%i\n", Len);
+               return e;
+       }
+
     /* Do matching for all parts of the password */
     for(i = 0; i < Len; ++i)
     {

[droidmonkey]

Agree that we should just truncate that input to zxcvbn at some length (probably 999).

This requires explanation:

some users of this service seem to store some very long JSON objects as their "password"

[smason]

Agree that we should just truncate that input to zxcvbn at some length (probably 999).

why is it even worth bothering to calculate entropy of anything that long? i.e. how is any attacker going go about "guessing" 1 kilobyte of text? the authors of zxcvbn suggest 100 characters:

To bound runtime latency for really long passwords, consider sending zxcvbn() only the first 100 characters or so of user input.

not sure why it doesn't just bail itself and return entropy as something as log2(ndistinct) * Len, where ndistinct is the number of distinct code points seen in the password.

This requires explanation:

some users of this service seem to store some very long JSON objects as their "password"

not sure what explanation you want... some arbitrary program (I think it's minecraft) uses the freedesktop Secrets API to store multiple values in a manner as suggested by the spec:

Applications wishing to store multiple values as part of a single secret, may choose to use a textual format to combine these values into one. For example, the 'desktop' key file format, or XML or another form of markup.

in practice it's using JSON to encode these values, and maybe pushing the spec a bit far by wanting to store that much, but otherwise it seems OK

[droidmonkey]

why is it even worth bothering to calculate entropy of anything that long? i.e. how is any attacker going go about "guessing" 1 kilobyte of text? the authors of zxcvbn suggest 100 characters:

It's not worth it, but trust me when I say some of our users are... excessive... and VERY vocal about it. Why do you think we even allow passwords with length 999? That was a very dramatic conversation between multiple parties demanding passwords of length greater than 128 characters.

[smason]

you should certainly allow password entries of arbitrary length, not sure why you think I don't like that

it's just estimating entropy of long passwords via the zxcvbn library that takes a very long time. it's only the entropy estimation I'm talking about!

[phoerious]

you should certainly allow password entries of arbitrary length, not sure why you think I don't like that

Passwords of that length are useless. Anything that has a larger keyspace than the hash derived from it is a waste of space and computing time.

[smason]

@phoerious yup, wanting more than 256 bits of entropy in your secret seems to be considered meaningless, hence why it's the highest security target of sane cryptographic systems at the moment.

but these entries aren't passwords in any conventional sense, but they should be considered sensitive and hence it makes sense to look after them, just not calculate entropy

@droidmonkey I've have had a quick play, and the following Python code can be used generate a password that can take many seconds to calculate:

base64.b64encode(random.randbytes(num_bytes))

setting num_bytes to 5000 takes my laptop ~17 seconds, while 8000 unencoded bytes takes ~75 seconds.

estimating entropy via the code I gave above in Python, i.e.:

math.log2(len(set(pwd))) * len(pwd)

gives a good approximation to zxcvbn, for example:

input  entropy via
bytes python zxcvbn
8000   64246  59629
5000   40157  37374
3000   24000  22222
1000    8045   7426
 500    4022   3737
 200    1613   1478
 100     789    733

note that these are random bytes, so there will be some variance, but they track each other pretty well hence it seems reasonable to use this for "long" passwords

[phoerious]

If you store things that are not passwords in KeePassXC, I would suggest using appropriately named custom string fields instead.

[droidmonkey]

Or attachments or notes

[smason]

I'm not setting them myself, they're coming from other programs via your "secret service" integration.

I've had a look in your source code, and it looks like FdoSecrets::setEntrySecret is the method that gets called, maybe it could be doing something else when it gets passed a long "secret"?

[smason]

that said, the UI shouldn't lock up for a minute if the user accidentally copy/pastes a long value into a field...

[smason]

the upstream package tsyrogit/zxcvbn-c#28 made a change that returns results in milliseconds now, it might be easier to just pull in the latest version of their code