Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Requesting code reviews from security engineers #3

Open
mvdan opened this issue Jun 23, 2019 · 3 comments
Open

Requesting code reviews from security engineers #3

mvdan opened this issue Jun 23, 2019 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@mvdan
Copy link
Owner

mvdan commented Jun 23, 2019

I'm not an expert at security by any means. I know enough to get this working, but I'd like some reviews and feedback before people start using this for their own passwords.

Current TODOs:

  • The password and decryption key are stored in memory for the lifetime of the process. Should we use https://github.com/awnumar/memguard?
  • The D-Bus service only implements the plaintext session encryption algorithm. Should we implement dh-ietf1024-sha256-aes128-cbc-pkcs7 and discourage the use of plain?
  • The encrypted sync data is stored on disk as-is. I assume this is fine because bitwarden-cli does the same, but I'm not 100% sure.
@mvdan mvdan added the help wanted Extra attention is needed label Jun 24, 2019
@Mic92
Copy link
Contributor

Mic92 commented May 19, 2020

At minimum bitw should use mlock(2) to prevent the password from being swapped.

@mvdan
Copy link
Owner Author

mvdan commented May 19, 2020

Yes, that's what libraries like memguard above do.

@mvdan
Copy link
Owner Author

mvdan commented Jan 25, 2021

D-Bus encryption to not use "plain" was implemented in #17.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants