gui: Check the strength of an encryption password

emilengler commented at 4:52 pm on January 17, 2020: contributor

This PR checks the strength of a password when encrypting or changing the passphrase of a wallet. If someone has some suggestions in terms of password security let me so I can add them to the different levels of strength

in src/qt/askpassphrasedialog.cpp:244 in 2207a87c37 outdated

237+    std::string pass = lineedit->text().toUtf8().constData();
238+    bool mixedcase = (boost::to_upper_copy<std::string>(pass) != pass) && (boost::to_lower_copy<std::string>(pass) != pass);
239+    bool specialChars = pass.find_first_not_of("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxzy1234567890") != std::string::npos;
240+    // Do nothing if the PW was empty
241+    if (pass.size() == 0)
242+        return;

promag commented at 5:24 pm on January 17, 2020:

should clear the label?

emilengler commented at 8:46 pm on January 17, 2020:

Yes, thanks will add this

in src/qt/askpassphrasedialog.cpp:221 in 2207a87c37 outdated

217@@ -216,18 +218,54 @@ void AskPassphraseDialog::textChanged()
218     {
219     case Encrypt: // New passphrase x2
220         acceptable = !ui->passEdit2->text().isEmpty() && !ui->passEdit3->text().isEmpty();
221+        this->passwordSecurity(ui->passEdit2);

promag commented at 5:37 pm on January 17, 2020:

this should be updatePasswordStrengthIndicator or similar.

in src/qt/askpassphrasedialog.h:9 in 2207a87c37 outdated

5@@ -6,6 +6,7 @@
6 #define BITCOIN_QT_ASKPASSPHRASEDIALOG_H
7 
8 #include <QDialog>
9+#include <QLineEdit>

promag commented at 5:39 pm on January 17, 2020:

Move to .cpp and just forward declare QLineEdit.

emilengler commented at 8:50 pm on January 17, 2020:

It is required in this file because of QLineEdit is a parameter of updatePasswordStrengthIndicator

promag commented at 9:18 pm on January 17, 2020:

It’s a pointer to QLineEdit, the compiler doesn’t need the definition so a forward declaration is enough.

kristapsk commented at 11:24 am on January 18, 2020:

I agree with @promag here, it’s already done this way with WalletModel in the same files.

emilengler commented at 1:34 pm on January 18, 2020:

But the updatePasswordStrengthIndicator needs access to the ui variable. I could give it over a parameter the current solution is more elegant IMO

promag commented at 10:01 pm on January 18, 2020:

Right, so instead of this:

0this->updatePasswordStrengthIndicator(ui->passEdit2);

just do

0// receive password as const QString&
1this->updatePasswordStrengthIndicator(ui->passEdit2->text());

or

0// access ui->passEdit2 directly
1this->updatePasswordStrengthIndicator();

and in either cases you should add #include <QLineEdit> in src/qt/askpassphrasedialog.cpp.

promag commented at 5:41 pm on January 17, 2020: member

Concept ACK, it’s just an strength indicator.

DrahtBot added the label GUI on Jan 17, 2020

Empact commented at 7:36 pm on January 17, 2020: member

Concept ACK

emilengler force-pushed on Jan 17, 2020

kristapsk commented at 11:25 am on January 18, 2020: contributor

Concept ACK

sipa commented at 5:49 pm on January 18, 2020: member

Concept ACK, but I strongly disagree with the chosen policies about passwords. For example, a sufficiently long passphrase of chosen lowercase-only words can be extremely high in entropy, but would be classified as “weak” here. It’d would be highly unfortunate to discourage good practices.

in src/qt/askpassphrasedialog.cpp:239 in b771102e1f outdated

234 
235+void AskPassphraseDialog::updatePasswordStrengthIndicator(QLineEdit *lineedit)
236+{
237+    std::string pass = lineedit->text().toUtf8().constData();
238+    bool mixedcase = (boost::to_upper_copy<std::string>(pass) != pass) && (boost::to_lower_copy<std::string>(pass) != pass);
239+    bool specialChars = pass.find_first_not_of("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxzy1234567890") != std::string::npos;

paymog commented at 8:38 pm on January 18, 2020:

should we add digit checks too? Seems like right now we only consider lowercase, uppercase, special characters and length.

emilengler commented at 7:49 am on January 19, 2020:

Thanks will add this later

emilengler commented at 7:48 am on January 19, 2020: contributor

@sipa

Concept ACK, but I strongly disagree with the chosen policies about passwords. For example, a sufficiently long passphrase of chosen lowercase-only words can be extremely high in entropy, but would be classified as “weak” here. It’d would be highly unfortunate to discourage good practices.

Thanks for the feedback. I will re-think the strength measure the next days

fanquake added the label Waiting for author on Jan 19, 2020

emilengler force-pushed on Jan 19, 2020

jonasschnelli commented at 4:42 am on January 20, 2020: contributor

Concept ACK… ~~Expect bike-shedding for the actual strength-logic.~~

This actual solves #5278 (an issue from 2014).

in src/qt/askpassphrasedialog.cpp:221 in f94c70c533 outdated

217@@ -216,18 +218,57 @@ void AskPassphraseDialog::textChanged()
218     {
219     case Encrypt: // New passphrase x2
220         acceptable = !ui->passEdit2->text().isEmpty() && !ui->passEdit3->text().isEmpty();
221+        this->updatePasswordStrengthIndicator(ui->passEdit2);

hebasto commented at 5:48 am on January 20, 2020:

Here and after this-> seems redundant.

in src/qt/askpassphrasedialog.cpp:235 in f94c70c533 outdated

230         break;
231     }
232     ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(acceptable);
233 }
234 
235+void AskPassphraseDialog::updatePasswordStrengthIndicator(QLineEdit *lineedit)

hebasto commented at 5:53 am on January 20, 2020:

style nit: QLineEdit* lineedit

in src/qt/askpassphrasedialog.cpp:242 in f94c70c533 outdated

237+    std::string pass = lineedit->text().toUtf8().constData();
238+    bool mixedcase = (boost::to_upper_copy<std::string>(pass) != pass) && (boost::to_lower_copy<std::string>(pass) != pass);
239+    bool specialChars = pass.find_first_not_of("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxzy123456789") != std::string::npos;
240+    bool digits = pass.find_first_not_of("1234567890") != std::string::npos;
241+    // Do nothing if the PW was empty
242+    if (pass.size() == 0) {

hebasto commented at 6:00 am on January 20, 2020:

pass.empty() ?

in src/qt/askpassphrasedialog.cpp:247 in f94c70c533 outdated

242+    if (pass.size() == 0) {
243+        ui->labelSecurityLevel->setText("");
244+        return;
245+    }
246+    // Passwords smaller than 10 chars are generally very weak
247+    if (pass.size() < 10) {

hebasto commented at 6:02 am on January 20, 2020:

Could 10 be a named constant?

in src/qt/askpassphrasedialog.cpp:253 in f94c70c533 outdated

248+        ui->labelSecurityLevel->setText("Very weak");
249+        ui->labelSecurityLevel->setStyleSheet("QLabel {color: #ff0000; }");
250+        return;
251+    }
252+    // Passwords with no mixed case or special characters but >8 are weak
253+    if (pass.size() >= 10 && !mixedcase && !specialChars) {

hebasto commented at 6:03 am on January 20, 2020:

Comment is inconsistent with code.

hebasto commented at 6:04 am on January 20, 2020: member

Concept ACK f94c70c533e08122efe72e2d768ed9526e757be5

in src/qt/askpassphrasedialog.cpp:252 in f94c70c533 outdated

247+    if (pass.size() < 10) {
248+        ui->labelSecurityLevel->setText("Very weak");
249+        ui->labelSecurityLevel->setStyleSheet("QLabel {color: #ff0000; }");
250+        return;
251+    }
252+    // Passwords with no mixed case or special characters but >8 are weak

elichai commented at 9:41 am on January 20, 2020:

nit 10, not 8

emilengler commented at 11:08 am on January 20, 2020:

hebasto already mentioned it. Will update it

gui: Check the strength of an encryption password a5fa237004

emilengler force-pushed on Jan 20, 2020

luke-jr commented at 4:03 pm on January 27, 2020: member

Agreed with @sipa

There are dedicated libraries for this purpose, so I’m not sure it can be reasonably simplified such that we want to roll our own crypto here…

Maybe we should just link CrackLib?

practicalswift commented at 6:00 pm on January 27, 2020: contributor

Maybe we should just link CrackLib?

Like many C code bases born in the 1990’s CrackLib doesn’t really live up to today’s standards for secure or robust coding. See the “assumed length of password” discussions in the comments of CrackLib’s fascist.c(!) as a small example :)

luke-jr commented at 6:15 pm on January 27, 2020: member

@practicalswift I don’t see what discussions you’re referring to (or any discussions at all at the link). In any case, the input is being provided by the user himself, so this isn’t a real concern…?

emilengler commented at 6:25 pm on January 27, 2020: contributor

This topic needs discussion. I personally think we should only use a library with a permissive license + a very small one (only one file). I don’t think it would be worth maintaining another subtree just for a small feature in the GUI

luke-jr commented at 8:03 pm on January 27, 2020: member

Dependencies shouldn’t be bundled at all, just picked up by configure and linked to when available.

But yes, the LGPL impact on gitian builds might need discussion. I wonder if there are any alternatives to CrackLib.

emilengler commented at 8:15 pm on January 27, 2020: contributor

@luke-jr There’s a JS library called zxcvbn from Dropbox. There is a C-port of it here: https://github.com/tsyrogit/zxcvbn-c

It’s also MIT

practicalswift commented at 9:24 pm on January 27, 2020: contributor

@practicalswift I don’t see what discussions you’re referring to (or any discussions at all at the link). In any case, the input is being provided by the user himself, so this isn’t a real concern…?

Sorry for my sloppy wording: I should have said “reasoning about string length in fascist.c” instead of “discussions about string length in fascist.c” :)

My point is that CrackLib (1993) was written in an era when robustness was not a priority. We can’t really blame the authors: the code was written three years before the widely read seminal article on buffer overflows (“Smashing the stack for fun and profit” (1996)), and I guess most input was assumed to be local and thus somewhat trusted back then. So while we can’t blame the authors we can avoid depending on C code from this dark era when possible :)

And perhaps more importantly and regardless of code quality: what was consider a non-weak password in 1993 is not necessarily non-weak in 2020 :)

Meta point: I don’t think we should ever tell a use that their password is strong. The absence of proof of weakness do not necessarily imply strongness :)

luke-jr commented at 9:33 pm on January 27, 2020: member

Sorry for my sloppy wording: I should have said “reasoning about string length in fascist.c” instead of “discussions about string length in fascist.c” :)

You mean that it only looks at the first 1024 characters? tbh, that seems more than sufficient…?

I guess most input was assumed to be local and thus somewhat trusted back then.

In our case, it is local. And skimming the linked file, I don’t see anything alarming either… It’s doing sanity checks on string sizes and such just fine…?

And perhaps more importantly and regardless of code quality: what was consider a non-weak password in 1993 is not necessarily non-weak in 2020 :)

It’s not like it’s been unmaintained since 1993… Last update was in 2019 from the looks of it.

practicalswift commented at 10:53 pm on January 27, 2020: contributor

And perhaps more importantly and regardless of code quality: what was consider a non-weak password in 1993 is not necessarily non-weak in 2020 :)

It’s not like it’s been unmaintained since 1993… Last update was in 2019 from the looks of it.

As far as I can tell the fixes since 1993 have been of the type “plug this implementation flaw” rather than modernising the definition of what constitutes a weak/non-weak password. Let me know if you find any significant counterexample.

Aside:

The thread buffer overflows in cracklib?! (1997) is quite telling:

Instead of receiving a simple “thank you for taking your time to report this vulnerability and thus making our software more robust” the security researcher receives a reply containing words/phrases such as “clever-clogs” (security researchers?), “nitpicking” (security research?) and “shooting your mouth off to prove how very clever you are” (non-embargoed disclosure?):

Wow! We should be very glad we live in an era where security researchers are given bounties instead of bullying in exchange for vulnerability reports :)

luke-jr commented at 4:41 am on January 28, 2020: member

It doesn’t look like bullying in context… but we’re getting off on a tangent here

luke-jr commented at 4:47 am on January 28, 2020: member

On another note: wallet encryption is significantly different from other cases where one has a password. It mainly only protects against people sitting at your PC for a few minutes - not against malware or remote crackers. The adversary is real humans, not automated password-cracking software.

With that in mind, it makes more sense to use a simple, easy to remember password, NOT one with high entropy. So this kind of strength meter is giving people a false sense of security at best (they’re more likely to think encryption protects against crackers), and helping them lose their wallet at worst (by forgetting their passphrase)…

So I think it’s a Concept NACK from me.

luke-jr commented at 11:53 pm on February 3, 2020: member

As far as I can tell the fixes since 1993 have been of the type “plug this implementation flaw” rather than modernising the definition of what constitutes a weak/non-weak password.

In any case, 1993 standards are probably still better than any simple algorithm we throw together here.

Also stumbled across libpwquality, which is BSD-licensed and slightly newer (2011?).

(None of this overcomes the Concept NACK issues, though)

emilengler commented at 9:46 am on February 4, 2020: contributor

I will have a look at these libraries but I’m a bit curious about how we should include them.

Make a subtree and include it staticly into the source
Add a dependency and check for it with the build system

Anyway I still think that these two feature might be a bit to overkill for a small GUI feature though.

laanwj commented at 1:47 pm on February 5, 2020: member

Concept ACK, but I strongly disagree with the chosen policies about passwords. For example, a sufficiently long passphrase of chosen lowercase-only words can be extremely high in entropy, but would be classified as “weak” here. It’d would be highly unfortunate to discourage good practices.

Exactly. weak concept-ACK, but what exactly constitutes a strong password is a common topic of heated discussion and bound to be controversial.

I also do not like the idea of adding a dependency just for this.

emilengler commented at 3:41 pm on February 5, 2020: contributor

Exactly. weak concept-ACK, but what exactly constitutes a strong password is a common topic of heated discussion and bound to be controversial.

I agree on this point. I plan on re-doing the password strength measure this weekend with more advanced checks. I’m still curios if we should add a dict (using the OS dict file) to check for words in passwords

luke-jr commented at 5:40 pm on February 5, 2020: member

If we’re doing this, a library should be a dependency, but can be an optional feature omitted when not available.

It’s possible no library exists for the criteria we want, though, since we’re outside the norm in that regard.

An ideal wallet passphrase probably should use only dictionary words, for example.

jonasschnelli commented at 7:54 pm on February 6, 2020: contributor

Todays IRC meeting discussions on this topic:

  0[20:35:16]  <wumpus>	#topic the library for [#17950](/bitcoin-bitcoin/17950/) (emilengler)
  1[20:35:18]  <gribble>	[#17950](/bitcoin-bitcoin/17950/) | gui: Check the strength of an encryption password by emilengler · Pull Request [#17950](/bitcoin-bitcoin/17950/) · bitcoin/bitcoin · GitHub
  2[20:35:33]  <wumpus>	I *really* do not like introducing a dependency for this
  3[20:35:35]  <emilengler>	thanks
  4[20:35:42]  <emilengler>	I agree with wumpus 
  5[20:35:59]  <jonasschnelli>	Yes. Please no dependency for a gimmick feature
  6[20:36:05]  <wumpus>	it's somewhat nice to display a measure of password strength (if people can ever agree on one), but it's not worth large changes to our build process for
  7[20:36:09]  <sipa>	i feel that anything that self-written is going to be too ad-hoc to be useful
  8[20:36:09]  <jonasschnelli>	It is already handholding...
  9[20:36:09]  <wumpus>	exactly
 10[20:36:24]  <sipa>	so either it's depending on a well-maintained library, or we don't do it at all
 11[20:36:24]  <jeremyrubin>	I think i'd rather just *suggest* a strong password
 12[20:36:25]  <luke-jr>	there's conceptual problems in the first place
 13[20:36:48]  <wumpus>	maybe it's a bad idea in the first place, thinking of it, we don't want to encourage a specific kind of password scheme, this only reduces security
 14[20:36:52]  <luke-jr>	this shouldn't be a "strong" password, it should be a memorable one
 15[20:36:56]  <jeremyrubin>	e.g., here are 12 random words
 16[20:37:04]  <wumpus>	that, too
 17[20:37:13]  <luke-jr>	encrypted wallets won't stop malware, just little brother
 18[20:37:30]  <luke-jr>	the risk of losing access outweighs the benefits of a string passphrase here
 19[20:37:31]  <jonasschnelli>	Can we just have a short text to help people do the right thing? Or a link (less likely)?
 20[20:37:32]  <wumpus>	it's not a brainwallet, not the entire internet can attack it, the security needed depends on how secure the wallet file is kept
 21[20:37:46]  <gwillen>	it is very hard to make a password-strength indicator that is not at least sometimes very misleading
 22[20:37:56]  <wumpus>	making it too strong might cause people to forgt it
 23[20:38:01]  <MarcoFalke>	I think more people have lost coins due to forgetting too strong passwords than first getting their wallet stolen, but not their password, and then got their password cracked offline
 24[20:38:02]  <wumpus>	which is much worse
 25[20:38:11]  <jonasschnelli>	Indeed
 26[20:38:11]  <sipa>	gwillen: zxcvbn seems pretty sophisticated already
 27[20:38:15]  <sipa>	*actually
 28[20:38:16]  <wumpus>	MarcoFalke: agree
 29[20:38:30]  <sipa>	it's very hard because users probably don't have a good intuition for what the requirements are
 30[20:38:30]  <wumpus>	what would be nice is a feature that makes people write down their HD seed
 31[20:38:42]  <wumpus>	aid recovery, not make it worse
 32[20:38:45]  <jeremyrubin>	(I'm actually recovering a wallet for someone who forgot their password, so I agree)
 33[20:38:45]  <MarcoFalke>	yeah
 34[20:39:00]  <wumpus>	a lot of people lose their coins either by losing their wallet or paspphrase
 35[20:39:01]  <sipa>	if the wallet.dat file leaking is an attack vector you want to protect against, the password needs to be *far* stronger than common recommendations for website login passwords
 36[20:39:02]  <jonasschnelli>	wumpus: you mean adding BIP39 support?
 37[20:39:06]  <jeremyrubin>	* attempting that is, let's hope the fragments are good enough
 38[20:39:28]  <gwillen>	sipa: as such things go, it's pretty sophisticated, but it does not know your dog's name, or your mother's maiden name, or your birthday, or your favorite book you're quoting from, or any of a number of stupid things users do that lower effective entropy
 39[20:39:40]  <gwillen>	while not lowering apparent entropy relative to the tool's model
 40[20:39:41]  <luke-jr>	sipa: but if the wallet.dat file leaks, you probably have a keylogger on your PC anyway, so..
 41[20:39:45]  <wumpus>	jonasschnelli: yes I suppose
 42[20:39:53]  <sipa>	gwillen: of course it can only give an upper bound
 43[20:40:00]  <gwillen>	it's never presented that way, though
 44[20:40:12]  <sipa>	anyway
 45[20:40:25]  <sipa>	i'm in favor of just not pursuing that feature
 46[20:40:31]  <jeremyrubin>	luke-jr: disagree with those priors
 47[20:40:35]  <sipa>	it's too hard to do right
 48[20:40:35]  <luke-jr>	jeremyrubin: ?
 49[20:40:49]  <jeremyrubin>	[11:39] <luke-jr> sipa: but if the wallet.dat file leaks, you probably have a keylogger on your PC anyway, so..
 50[20:40:49]  <MarcoFalke>	Yeah, we should recommend users use a shorter password, if anything
 51[20:40:51]  <sipa>	luke-jr: wallet.dat files get backed up
 52[20:40:59]  <wumpus>	luke-jr: they might copy it to a cloud service or something
 53[20:41:00]  <sipa>	MarcoFalke: i disagree with that as well
 54[20:41:01]  <luke-jr>	sipa: hopefully encrypted!
 55[20:41:04]  <jeremyrubin>	I think it's relatively likely you leak your file but don't get a keylogger
 56[20:41:13]  <sipa>	luke-jr: right, but in that case, the passphrase needs to be strong
 57[20:41:24]  <luke-jr>	sipa: no, I mean encrpying the file itself
 58[20:41:26]  <jeremyrubin>	e.g., keeping backups on thumb drives
 59[20:41:36] 	timothy (~tredaelli@redhat/timothy) left IRC (Remote host closed the connection)
 60[20:42:01]  <sipa>	luke-jr: it's hard to assume that people will use a strong password for an encrypted backup, but then not one inside the file?
 61[20:42:15]  <luke-jr>	perhaps we should put a suggestion to that effect somewhere
 62[20:42:16]  <wumpus>	in any case, there's no disagreement about whether the wallet encryption is useful or not, that's not the topic
 63[20:42:17]  <sipa>	i disagree that in general we should advise weak passwords
 64[20:42:34]  <wumpus>	no, we shouldn't advise that
 65[20:42:46]  <MarcoFalke>	ok, we shouldn't advise on weak passwords, but we might want to explain the tradeoffs
 66[20:42:53]  <sipa>	MarcoFalke: yes
 67[20:42:59]  <wumpus>	that would make sense, yes
 68[20:43:02]  <wumpus>	add an explanation
 69[20:43:03]  <MarcoFalke>	I.e. what the password protects against and what it does not protect against
 70[20:43:07]  <sipa>	"Losing this password will make your funds irrecoverably lost"
 71[20:43:28]  <jeremyrubin>	I think also saying "writing down the password in a notebook is probably better than not having one"
 72[20:43:36]  <jonatack>	https://www.xkcd.com/936/
 73[20:43:37]  <jeremyrubin>	Or something to that effect
 74[20:43:37]  <jonasschnelli>	I'm all for informing (text based) rather then applying rules that only works for certain use cases
 75[20:43:48]  <luke-jr>	jeremyrubin: it really depends on your risk exposure
 76[20:43:53]  <jeremyrubin>	I think people don't know that the password is not a seed 
 77[20:44:14]  <jeremyrubin>	if you just write down the password but they don't have the wallet.dat it's fine
 78[20:44:45]  <wumpus>	jeremyrubin: yep, some people are confused by that
 79[20:44:45]  <jeremyrubin>	luke-jr: if someone remote compromises your computer but you have a sticky note with a long password on your screen you're "fine"
 80[20:45:06]  <wumpus>	because most wallets work with seeds nowadays
 81[20:45:07]  <jeremyrubin>	(until you type it in, but let's assume read only compromise)
 82[20:45:09]  <jonasschnelli>	The wallet encryption should be better explained. I would not wonder if some users encrypt their watch only wallets in the hope to not leak metadata to computer wide text pattern searches, etc.
 83[20:45:21]  <wumpus>	agree with jonasschnelli on adding explanation text
 84[20:45:37]  <luke-jr>	you can't encrypt watch-only I thought?
 85[20:45:44]  <emilengler>	I think it may be a good way to add a way to encrypt the wallet in the intro dialog
 86[20:45:44]  <jonasschnelli>	Can't you?
 87[20:45:46]  <hebasto>	explanation is good
 88[20:46:05]  <wumpus>	only private keys are encrypted, so encrypting watch-only would be a no-op
 89[20:46:07]  <luke-jr>	jonasschnelli: what would it even do?
 90[20:46:07]  <sipa>	jonasschnelli: of course not
 91[20:46:10]  <jonasschnelli>	luke-jr: I guess you can because its mostly a mixed situation
 92[20:46:13]  <sipa>	what would there be to encrypt?
 93[20:46:24]  <jonasschnelli>	sipa: thats exactly the problem
 94[20:46:31]  <jonasschnelli>	people expect things are encrypted
 95[20:46:40]  <jonasschnelli>	while we only encrypt the keys
 96[20:46:44]  <wumpus>	the metadata is never encrypted
 97[20:46:52]  <jonasschnelli>	Yes. But we don't tell that to users
 98[20:47:01]  <sipa>	jonasschnelli: a no-key wallet can't be encrypted, i think?
 99[20:47:04]  <luke-jr>	it's obvious?
100[20:47:11]  <jonasschnelli>	So,.. IRS grabs wallet.dat file and reads transaction comments
101[20:47:16]  <wumpus>	this is why software needs documentation I guess
102[20:47:22]  <sipa>	luke-jr: don't assume things are obvious
103[20:47:22]  <jeremyrubin>	luke-jr: how is it obvious?
104[20:47:31]  <jeremyrubin>	That they can open the wallet and see stuff and only pw to sign?
105[20:47:34]  <jonasschnelli>	It's not obvious to most users
106[20:47:37]  <luke-jr>	you open Bitcoin Core and see your metadata, without entry of passphrase
107[20:47:51]  <wumpus>	jonasschnelli: well it doesn't ask th password at startup, only when you send
108[20:47:51]  <jonasschnelli>	encryption means for most users data can't be read by someone no knowing the secret
109[20:47:53]  <jeremyrubin>	Actually that sounds like a 2 birds one stone thing
110[20:48:08]  <jonasschnelli>	wumpus: sure. But novice users don't understand that either
111[20:48:15]  <jeremyrubin>	If people have to put their password in more often maybe they're less likely to forget it ;)
112[20:48:23]  <luke-jr>	jeremyrubin: hmm!
113[20:48:59]  <jonasschnelli>	I think adding more explanations on how the encryption work would be great in general
114[20:49:04]  <jonasschnelli>	works
115[20:49:21]  <MarcoFalke>	Opened an issue [#18085](/bitcoin-bitcoin/18085/)
116[20:49:22]  <gribble>	[#18085](/bitcoin-bitcoin/18085/) | doc: Explain what the wallet password does · Issue [#18085](/bitcoin-bitcoin/18085/) · bitcoin/bitcoin · GitHub
117[20:49:27]  <jonasschnelli>	Nice
118[20:49:37]  <wumpus>	thanks
119[20:49:47]  <jeremyrubin>	I think there's also a lot of room for improvements in what users have available, e.g. shamir's secret shares
120[20:49:52]  <jonasschnelli>	We don't encrypt the wallet, we encrypt the keys
121[20:50:29]  <jonatack>	I sense new options/config args in our future here
122[20:50:32]  <jeremyrubin>	Even though we know multisig is better, user's are really struggling to do anything better than a plaintext wallet
123[20:51:12]  <luke-jr>	not sure it makes sense to put any effort into pre-Taproot multisig at this point?
124[20:51:16]  <sipa>	jeremyrubin: that's not really an option in a model where a wallet is a file and not a seed
125[20:51:17]  <wumpus>	we should be careful only to add features that are actually used and useful though, don't want to end up with some GPG-like tool that does a zillion things but with a lot of pitfalls
126[20:51:30]  <jeremyrubin>	Maybe organizing some discussion at coredev.tech would be good about conducting some user research to improve things.
127[20:51:52]  <kanzure>	i'll add that to the list then.
128[20:51:57]  <kanzure>	empirical user testing would be interesting
129[20:52:00]  <sipa>	luke-jr: multisig support at this point means improving PSBT integration, I think

DrahtBot commented at 5:01 am on March 3, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

laanwj added the label Feature on Mar 27, 2020

fanquake commented at 6:52 am on July 15, 2020: member

If this is followed-up on, I’m going to suggest re-opening in the new gui repo: https://github.com/bitcoin-core/gui.

I’m in agreement with others that adding a new dependency for this doesn’t seem like a great idea. As was also alluded too above, this is the kind of change that is open for plenty of bike-shedding in regards to password strength etc, so ideally we wouldn’t be spending lots of cycles discussing it here.

fanquake closed this on Jul 15, 2020

DrahtBot locked this on Feb 15, 2022

gui: Check the strength of an encryption password #17950

Conflicts