Show HN: Tiny password manager with all data stored encrypted on your machine
github.com
github.com
* Don't allow the user to supply their password via argv. The names and argument vectors of running processes aren't considered privileged information, and the password can be inadvertently leaked or cached by e.g. a process auditing system. Instead, have the user provide their password via standard input.
* Consider a different interface for the master password. An environment variable isn't the worst, but has some of the same leakage problems that argv does. "Best practice" here is to either use the system keychain or an agent-style helper program.
cli add user1 -passWord
cli add user2 passw*
etc
read -s might be a safer way.
hide add -n account -u user -p password123
And gets an error message saying "Sorry, you can't put the password into the command,it's a security problem in things like bash_history. Please use STDIN"
Then goes "Fuckit!":
echo 'password123' > hide add -n account -u user -p
Maybe they should be allowed to do that?
(I can almost imagine scenarios where someone might want to use a tool like this in some server automation thing where the security tradesoffs really do make using it like that a not awful idea?)
If the program is on a TTY, then reading with echo disabled (i.e., `read -s`) is definitely preferable. I usually have stdin configured as a fallback for when the program isn't on a TTY, for programmatic use.
This has the `echo` problem that you mentioned, but it's strictly better than either argv or the environment (both of which persist throughout the program's lifetime and appear in places like procfs).
For the system keychain, I think we're talking about different use cases, no? The system keychain should be used to store the master password, but using it to pre-store any password you intend to later store in another password manager feels a little silly.
HISTCONTROL=“ignorespace”Pass was built by Jason Donenfield (zx2c4) who is competent enough to have also built WireGuard and survived the resulting years of uh, review by the Linux kernel team.
The parts of pass I've actually read (which is admittedly not all of it) are designed in a way that feels like the author understands both the large problem (password management) and lots of little implementation details to get there correctly.
One thing I particularly like is how pass gets random passwords. Unlike this tool 'pass' will generate you a strong random password, optionally constrained however you please. The insight is that the CSPRNG is cheap, so you shouldn't put any work into conserving random numbers by doing fancy algorithmic stuff. Instead pass consumes bytes from the CSPRNG (/dev/urandom) and just discards all the ones that aren't allowed in your password. So e.g. if you ask for a numeric password and 'pass' reads the byte 0x8F from the CSPRNG, it doesn't try to massage that into a numeral, it just discards it and fetches another byte until it gets one in the range 0x30 to 0x39. This inefficiency is actually totally fine and makes it easy to verify that the code is correct, which is much more important than slightly reducing the workload of the CSPRNG.
tl;dr I would (and do) trust pass but not this program.
I had a look at your crypto code, and the usual advice would be not to roll out your own crypto. Of course, since we are here to learn, that does not apply, so instead here are a few pointers on things you might want to learn about:
- authenticated encryption
- key derivation functions
- secure hash algorithms
I found this pretty good and pretty accessible https://www.crypto101.io/
Good learning!
That's definitely a good thing in terms of the quality of the code now by the way, I don't see any changes that make it worse and many make it better. But it does mean comments may not refer to the source you looked at.
Too late to edit this after having slept but I think the word I wanted here was "better" not "bad" ?
The author is not rolling their own cryptography (which is good) but they are clearly using a home made cryptosystem (and not PGP, TLS, or libsodium for example). For example they forgot authenticating their encryption, and the with "password extension" getFilledSecret is more than doubvious. Rule of thumb: if you are using cryptographic primitives directly (such as AES) you are rolling out your own crypto.
- No master password is required - if you don't set one a default value of "hide" is used
- The description says it uses AES256 (it is the default) but the code lets you specify your own algorithm, and supports ECB mode and even Triple DES
- When an algorithm requires a (for instance) 32 byte key, and your master password is larger than 32 bytes, it will just take the first 32 bytes and ignore the rest. If your master password is less than 32 bytes, it will just keep concatenating your password with itself until it gets 32 bytes: https://github.com/whatl3y/hide/blob/master/src/libs/Encrypt...
- for your second point, if you want to fork this repo and use another algo, go for it. if you install the package from npm and use it or clone and install from source without changing it, it uses the default AES256
- this could be argued as a valid point, and maybe forcing the user to use 32 bytes as their password is the right way to do it, but no 3rd party password manager forces this limitation so i don’t believe I should? Maybe just forcing a password difficulty is the right way to go? I’m certainly open to ideas, this is just a small project i enjoyed building and feel like the minimum security requirements to meet my needs are there.
[0]: https://github.com/whatl3y/hide/blob/1c3aaca634f504c3c274bac...
- no, you accept the algorithm as an environment variable - someone can install your package as is and still set it themselves
- as kohtatsu mentioned you should be using a key derivation function.
[0]: https://github.com/whatl3y/hide/blob/1c3aaca634f504c3c274bac...
- Got it, yeah I guess my defense would be it still requires the user to make a conscious decision so it’s not as bad as it being ambiguous or unclear, although removing the ability to control this via env var might be the right move.
- I’ll look into and add something to do this soon :)
I simply append the password to itself to get 32 bytes, but SHA256 is almost certainly the better/right way. Will add a PR this weekend.
---
(I apologize in advance. I don't typically make comments like this on HN but I feel it really is warranted here.)
Key Derivative Functions (KDFs) have already been mentioned here in the comments.
By not using one -- and, perhaps more importantly, by 1) not even understanding why you should use one and 2) thinking that one round of SHA256 is what you "should" do -- well, to be blunt, your project just screams out "hacked together piece of shit made by someone who has absolutely zero business whatsoever making a password manager".
---
"Show HN"'s are to get feedback. As it stands, your project should not be used -- by pretty much anyone! It doesn't have to stay that way, though. Please take some of the good advice you've gotten in the comments here and improve your project so that it can safely be used.
Nonetheless I appreciate the last bullet as I definitely post to Show HN to learn and get better as a developer, and certainly expect nothing less than to receive a few “fuck your shitty project”s along the way :)
---
> ... I’ve made several changes to the code ... including using PBKDF2 ... actually well before jlgaddis’ comment here was posted ...
Yes, you're right. That's my fault, mostly because of how I tend to browse HN.
I had opened both the GitHub repo and the discussion thread in their own new tabs. I actually had browsed through the repo and briefly looked over some of your code although I will certainly admit that I didn't conduct an extensive, in depth review of it! In fact, I was still reading through it when SWMBO suddenly appeared in front of me, informed me of an urgent mission, and instructed me to begin working on it at once. By that point, I had already "seen enough" so I just closed the tab, tossed the iPad down on the couch, and headed out on my new mission.
When I picked up the iPad again a couple hours later, Safari reloaded the tab that was open (this discussion thread), I began reading the comments, and, after reading your comments upthread, felt compelled to leave one of my own.
---
Anyways...
The point that I had intended to make was that, sometimes, a little knowledge (or, alternatively, "a lot of ignorance") can be a very dangerous thing. I'm gonna digress real quick but I'll come right back to that.
I didn't intend for my comment to be a personal attack on you and I'm sorry that it came across that way. There really was a legitimate point that I was trying to get at; the "bigger picture" that I was attempting to paint, if you will.
I can, however, be (in the words of an old co-worker and more than one student) a bit of a "security nazi" at times. Unfortunately, this was one of them. My knee-jerk reaction to someone "doing something stupid" was a hastily written comment which resulted in a poor choice of words that failed to paint that "bigger picture".
First, it's certainly a good thing that you took the direct advice you were given (i.e., "use a KDF!") and quickly improved upon your new project. Props for that. Presumably, tt's now much more secure as a result (I haven't actually looked at your changes but I'll give you the benefit of the doubt)!
Now, back to that "a little knowledge can be a very dangerous thing" thing.
When you start getting into security-related areas, simple mistakes can have a devastating, outsized, and lasting impact. There's no doubt that a password manager provides huge benefits -- if it's designed and implemented correctly! The only thing that I can think of that's worse than a complete dumpster fire of a password manager is a complete dumpster fire of a password manager that 1) the user assumes is working as expected, 2) the user trusts with their personal/private data, and 3) lulls the user into a false sense of security -- since they might actually be worse off (i.e., "less safe") than they were before!
Ultimately, the "real problem" is that you were developing a tool that claimed to "securely" store secrets without actually knowing how to "securely" store secrets at all! There are several well-tested, industry-standard algorithms designed to do exactly this -- yet, for whatever reason, you instead devised something that sounded okay and seemed like it might kinda sorta work. It quickly became clear from reading your other comments that you had good intentions; it was simply ignorance, not malice.
Unfortunately, when it comes to security-related stuff, it doesn't really matter if it's ignorance or malice -- the end result is the same. It's like handing a loaded AR-15 to someone who has never handled a gun before and telling them to have fun, or letting a little kid set off the fireworks on the Fourth of July: you may get lucky and nothing bad happens; eventually, though, someone's probably going to end up getting hurt and/or missing a few fingers!
---
> * ... I’m going to blast this peion [sic] publically [sic] because it’ll make my Sunday a little bit better ...*
To be clear, I wasn't trying to be condescending or imply that you're a "peion", or stupid, or a shitty developer, or anything like that. If you're creating something that has major security implications, however, you really do need to know what you're doing. If it makes you feel any better, I'm certainly not qualified to develop a password manager capable of securely storing passwords (and I wrote my first program sometime in the late 80's)! That's not to say you won't ever be capable of creating one, of course -- but, if you're going to be developing software that has security implications (especially for other people!) if you get it wrong, please make sure you learn how to do it right.
Good luck!
If nothing else I’m smarter and know a little more about what I didn’t know (and still don’t) which I find a ton of value in. Hopefully I’m slowly ridding the world of my contribution to the “ignorance” category you mention above which I admit to have been in wrt developing a secure password manager, but either way I enjoyed learning along the way.
2. It might be useful to have an option for show to just show the password. Perhaps -P (upper case P). E.g.,
$ hide show -n facebook.com -P
my_password1
I'd expect that most of the time people use -p to retrieve the password they are going to copy/paste it into a browser. With a password-only option they could pipe to something that puts it on the clipboard. A Mac user, for exampled, could use $ hide show -n facebook.com -P | pbcopy
It would also be useful with commands line tools that take a password on the command line: $ some_tool --password=$(hide show -n some_tool -P)
or with commands that read the password from a file $ other_command --pwfile <(hide other_command -n some_tool -P)Edit: looks like you had bcrypt then decided to remove it?
https://github.com/whatl3y/hide/commit/0a8262d50929366bdd9ae...
You don't need to compare, just feed the resulting hash to AES and see if the output is any good.
You need to arrange to store the salt somewhere to take this (correct) approach to using a PBKDF. The nodeJS bcrypt implementation provides an all-in-one API which was used here, in which you let it handle salting and it "just works" by storing passwords in the style of Unix crypt - but without you understanding why. This probably results in fewer nodeJS apps where the passwords are stored as plaintext or something dumb, but it doesn't afford the understanding you'd need to get encryption keys the same each time a user runs the program.
My guess is that the author realised they didn't understand what was happening here, and so they ripped it out perhaps intending to reintroduce it later once they had other parts working to their satisfaction, and then they never did.
You are correct though that in the absence of a good PBKDF bad guys who get your encrypted password database can "just" brute force the master password relatively inexpensively and this makes the program unsafe for essentially all ordinary users.
https://www.passwordstore.org/
Otherwise https://keepassxc.org/
I plug it in, enter my master password and right click to copy password to paste it on websites.
Licenses are per platform and are more expensive than earlier versions of 1Password
I prefer the inconvenience of using an old version (which I have a license for) than paying the extorsion price of a paid only update, and with the nonsense of different platforms being sold separately.
I guess they're free to do whatever they want, and I guess sadly nowadays subscriptions do make sense from a SaaS/Developer POV, I just can't fathom it as consumer.
That... "thing"... is absolutely horrendous (mostly, I think, as a result of the bad choice of NodeJS itself)!