palant / pfp

A simple and secure browser extension to be used with KeePass databases.
https://pfp.works/
Mozilla Public License 2.0
113 stars 14 forks source link

Native host implementation for himitsu #147

Open apreiml opened 3 weeks ago

apreiml commented 3 weeks ago

I've started implementing a native host application to support the himitsu password storage. Besides a small thing, the protocol is compatible. I think it's a good fit and I'd really like to use PfP. This way we wouldn't need to maintain our specific one himitsu-firefox.

I'd like to also thank you very much for all your work you provide to the public. Not only this password manager is great, also your blog is a valuable learning resource.

palant commented 3 weeks ago

How's your take on accepting other native hosts implementations?

As in: maintained by you outside of my code? I guess I don’t mind. We’d need to see whether I should suggest your implementation as an alternative to mine then (there are various places where this would be a consideration, including one in the extension itself).

I hope you noticed however that this isn’t exactly a well-maintained browser extension. I haven’t really touched the code in a year. There is a bunch of minor issues that I probably won’t touch until something major breaks.

Oh, and something else: I recently decided to make it a policy that I don’t contribute to repositories using “master” as their default branch.

Would you be willing to have backwards support for protocol versions and/or announce protocol changes beforehand?

I generally prefer to keep backwards compatibility already, requiring all users to update php-native-host is quite intrusive. And it’s not like I’m implementing tons of new features – the extension is feature-complete as far as I’m concerned. But I can certainly give you an advance warning if protocol changes happen to come up.

Himitsu unlocks the store with a system prompt, hence the unlock action is not needed. Would you accept a MR that extends the protocol and the password manager to skip the main password prompt?

This should be ok if you can implement it in a backwards-compatible way – existing pfp-native-host instances should get the default behavior. It would also be necessary to disable locking in the extension as well as the corresponding configuration. But the bigger issue here is: the UI is not currently meant for long-running asynchronous actions. There will be no user feedback if some native API call locks up waiting on a prompt somewhere else. Have you considered this issue?

Something else for you to consider: I’ve also started out encrypting passwords only, back when PfP used its own storage format. I eventually realized that metadata is also valuable and that the only sensible way to deal with that is encrypting all of it. It definitely isn’t a good idea to ask users to decide what should and what shouldn’t be encrypted, they don’t have the knowledge.

apreiml commented 3 weeks ago

How's your take on accepting other native hosts implementations?

As in: maintained by you outside of my code? I guess I don’t mind.

Yes, it will be maintained outside of your code as an alternative. You wouldn't need to bother with it. A stable API using protocol versions sounds good and thanks for the offer to send a warning in advance on changes! I'll also try to watch the repository closely.

I hope you noticed however that this isn’t exactly a well-maintained browser extension. I haven’t really touched the code in a year. There is a bunch of minor issues that I probably won’t touch until something major breaks.

Alright. I'll try to contribute, if I stumble upon issues.

Oh, and something else: I recently decided to make it a policy that I don’t contribute to repositories using “master” as their default branch.

Acknowledged.

Himitsu unlocks the store with a system prompt, hence the unlock action is not needed. Would you accept a MR that extends the protocol and the password manager to skip the main password prompt?

But the bigger issue here is: the UI is not currently meant for long-running asynchronous actions. There will be no user feedback if some native API call locks up waiting on a prompt somewhere else. Have you considered this issue?

I'll experiment a bit. Though it shouldn't be a problem, since the current prompt is designed in a way that you need to give or deny consent in order to get rid of it.

Something else for you to consider: I’ve also started out encrypting passwords only, back when PfP used its own storage format. I eventually realized that metadata is also valuable and that the only sensible way to deal with that is encrypting all of it. It definitely isn’t a good idea to ask users to decide what should and what shouldn’t be encrypted, they don’t have the knowledge.

Agreed. Himitsu encrypts all data on rest. The metadata will be decrypted and loaded into memory when the store is unlocked. The private values will stay encrypted, even in the unlocked state and will be decrypted after consent. While unlocked the master key will be stored in the kernel keyring, if available.

Thanks! I'll get back when I've a proposal for skipping the unlock screens.