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

Audit and/or remove encryption #694

Closed
tarcieri opened this Issue May 7, 2014 · 42 comments

Comments

Projects
None yet
10 participants
@tarcieri

tarcieri commented May 7, 2014

The encryption code (particularly in src/blowfish.c) is absolutely horrid, broken, unaudited, and written by people who don't know what they're doing.

This patch attempts to switch vim from one block cipher mode of operation (CFB) to what the author claims is OFB, because the author claims the code didn't match the documentation, so he changed the code rather than the documentation:

http://permalink.gmane.org/gmane.editors.vim.devel/44650

However, it's feeding the plaintext, not the ciphertext, into the next block. I'd step you through the maze of crazy macros that makes this happen, but then I'd probably have to stab my own eyes out with a fork.

At best, this is just broken. At worst it's a bugdoor. It's the kind of code that might make for a fun problem in a cryptanalysis challenge.

I would suggest burning all of the encryption code to the ground unless someone is going to step up and try to fix it.

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk May 7, 2014

Member

Is there a hardened C blowfish implementation that the editor could use instead? (I'm expecting the response: "a text editor has no business encrypting anything!")

A similar issue was also discussed to death on emacs-devel, which I linked to last time this was brought up: #309 (comment)

If a text editor could offer this in a non-broken way, wouldn't it would be useful? That's what I'm interested in. If that means calling a system service (libnettle, gnupg), then fine.

Member

justinmk commented May 7, 2014

Is there a hardened C blowfish implementation that the editor could use instead? (I'm expecting the response: "a text editor has no business encrypting anything!")

A similar issue was also discussed to death on emacs-devel, which I linked to last time this was brought up: #309 (comment)

If a text editor could offer this in a non-broken way, wouldn't it would be useful? That's what I'm interested in. If that means calling a system service (libnettle, gnupg), then fine.

@justinmk justinmk added this to the blowfish milestone May 7, 2014

@justinmk justinmk added the security label May 7, 2014

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 7, 2014

I'll start with: yes, it'd be cool if vim offered a good encryption option. It doesn't have one now. Due to what I can only assume are the random access requirements of a text editor, doing it correctly might be difficult.

Unless the goal is backwards compatibility (in which case I'd suggest removing the ability to save new ciphertexts and only allowing the old ones to be read) there's no reason to use Blowfish today. It's vulnerable to reflectively weak keys and all around inferior to other ciphers like AES, ChaCha20, or even Twofish/Threefish.

Quote Bruce Schneier, Blowfish's creator (circa 2007):

"At this point, though, I'm amazed it's still being used. If people ask, I recommend Twofish instead."

I could detail what sort of scheme I'd suggest (something like ChaCha20+Poly1305 on e.g. 1MB fragments of the input file, with a combination of a block sequence number and a random value for the nonce, and an empty sentinel block at the end), but really I wouldn't recommend supporting an encryption feature at all unless it's implemented by someone who knows what they're doing and then subsequently audited by someone else who knows what they're doing.

tarcieri commented May 7, 2014

I'll start with: yes, it'd be cool if vim offered a good encryption option. It doesn't have one now. Due to what I can only assume are the random access requirements of a text editor, doing it correctly might be difficult.

Unless the goal is backwards compatibility (in which case I'd suggest removing the ability to save new ciphertexts and only allowing the old ones to be read) there's no reason to use Blowfish today. It's vulnerable to reflectively weak keys and all around inferior to other ciphers like AES, ChaCha20, or even Twofish/Threefish.

Quote Bruce Schneier, Blowfish's creator (circa 2007):

"At this point, though, I'm amazed it's still being used. If people ask, I recommend Twofish instead."

I could detail what sort of scheme I'd suggest (something like ChaCha20+Poly1305 on e.g. 1MB fragments of the input file, with a combination of a block sequence number and a random value for the nonce, and an empty sentinel block at the end), but really I wouldn't recommend supporting an encryption feature at all unless it's implemented by someone who knows what they're doing and then subsequently audited by someone else who knows what they're doing.

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk May 7, 2014

Member

Well so much for that.

So I have a question. The alternative now for the command-line user who wants to decrypt a file, edit it, and then encrypt it again is to manage this process with, say, gpg and a shell script. Many users would

  1. decrypt the file using gpg, (plaintext is now exposed on the filesystem)
  2. edit it in, say, Vim, (which may shed artifacts like the .swp file, backup files, etc)
  3. then encrypt the file again.

Is this less broken than using Vim's potentially-vulnerable blowfish? If not, then why couldn't Vim do the same process, as well as keep the decrypted file contents off disk and avoid temp files, backups, .swp files, etc?

Basically what I'm asking is: is there any sane way to achieve this workflow at the command line using standard tools, and if so, why couldn't Vim wrap that workflow?

Thanks for your informed opinion.

Member

justinmk commented May 7, 2014

Well so much for that.

So I have a question. The alternative now for the command-line user who wants to decrypt a file, edit it, and then encrypt it again is to manage this process with, say, gpg and a shell script. Many users would

  1. decrypt the file using gpg, (plaintext is now exposed on the filesystem)
  2. edit it in, say, Vim, (which may shed artifacts like the .swp file, backup files, etc)
  3. then encrypt the file again.

Is this less broken than using Vim's potentially-vulnerable blowfish? If not, then why couldn't Vim do the same process, as well as keep the decrypted file contents off disk and avoid temp files, backups, .swp files, etc?

Basically what I'm asking is: is there any sane way to achieve this workflow at the command line using standard tools, and if so, why couldn't Vim wrap that workflow?

Thanks for your informed opinion.

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk May 7, 2014

Member

Also: if this feature (or the existing blowfish support) is important to users, maybe some will support it with a bounty.

  • would you be able to recommend capable auditors?
  • any idea how much would an audit reasonably cost? (assuming the "bounty" model is of any interest to those parties)
Member

justinmk commented May 7, 2014

Also: if this feature (or the existing blowfish support) is important to users, maybe some will support it with a bounty.

  • would you be able to recommend capable auditors?
  • any idea how much would an audit reasonably cost? (assuming the "bounty" model is of any interest to those parties)
@philix

This comment has been minimized.

Show comment
Hide comment
@philix

philix May 7, 2014

Member

There is probably a FEAT_ macro for the encryption feature. We could remove this feature code for Neovim as providing bad cryptography is worse than providing no cryptography at all. This should be done before the first release if we don't have any better plan for encryption until then.

I'm of the opinion that a text editor has nothing to do with encryption. Users should be encrypting their hard-drives, using gpg (or any other utility) for individual files... Calling a system service to "provide" encryption is fine though. I don't see that as "the Nevim editor supports encryption", but as "the Neovim editor makes system cryptography utilities easier to use".

What can work for a text editor like ssh works for git and other software that need a secure network channel of communication?

Member

philix commented May 7, 2014

There is probably a FEAT_ macro for the encryption feature. We could remove this feature code for Neovim as providing bad cryptography is worse than providing no cryptography at all. This should be done before the first release if we don't have any better plan for encryption until then.

I'm of the opinion that a text editor has nothing to do with encryption. Users should be encrypting their hard-drives, using gpg (or any other utility) for individual files... Calling a system service to "provide" encryption is fine though. I don't see that as "the Nevim editor supports encryption", but as "the Neovim editor makes system cryptography utilities easier to use".

What can work for a text editor like ssh works for git and other software that need a secure network channel of communication?

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 7, 2014

@justinmk you might be able to find crypto auditors who are fans of vim who would do it for free ;) Audits are going to vary in price depending on how involved the auditor gets in the code.

That said, my recommendation for the time being would be hitting the delete key on it

tarcieri commented May 7, 2014

@justinmk you might be able to find crypto auditors who are fans of vim who would do it for free ;) Audits are going to vary in price depending on how involved the auditor gets in the code.

That said, my recommendation for the time being would be hitting the delete key on it

@amtal

This comment has been minimized.

Show comment
Hide comment
@amtal

amtal May 7, 2014

Programs grow 'til they can send email and run Lisp; may as well grow to do encryption. :) There are realistic use cases where you have bells-and-whistles vim but not gpg/openssl/modern zip; keeping the existing interface but ditching both existing options for something safer (ish) seems best:

  • The "zip" cryptmethod is cute for historical reasons, but doesn't provide any practical security. The fact that it's the default option kind of sucks.
  • The "blowfish" cryptmethod is better than "zip", but has a major implementation fault. Fixing it would break backwards compatibility, at which point you may as well replace it.

My vision of a replacement would be pretty limited:

  • Most secure option as default, or even better simplify down to only the secure option.
  • Simple code. Macros like these are bad in correctness-critical areas, and reducing the layers of indirection on lengths used to construct critical values like the IV would be nice.
  • If padding out save files with an extra MAC is easy, then use an authenticated encryption mode to prevent evil maid stuff. If not, confidentiality alone would do; just mention the possibility of tampering in the manual.

The choice of (reasonably modern) primitives used seem a matter of personal preference if the above three things are covered, and basic symmetric gotchas checked for.

amtal commented May 7, 2014

Programs grow 'til they can send email and run Lisp; may as well grow to do encryption. :) There are realistic use cases where you have bells-and-whistles vim but not gpg/openssl/modern zip; keeping the existing interface but ditching both existing options for something safer (ish) seems best:

  • The "zip" cryptmethod is cute for historical reasons, but doesn't provide any practical security. The fact that it's the default option kind of sucks.
  • The "blowfish" cryptmethod is better than "zip", but has a major implementation fault. Fixing it would break backwards compatibility, at which point you may as well replace it.

My vision of a replacement would be pretty limited:

  • Most secure option as default, or even better simplify down to only the secure option.
  • Simple code. Macros like these are bad in correctness-critical areas, and reducing the layers of indirection on lengths used to construct critical values like the IV would be nice.
  • If padding out save files with an extra MAC is easy, then use an authenticated encryption mode to prevent evil maid stuff. If not, confidentiality alone would do; just mention the possibility of tampering in the manual.

The choice of (reasonably modern) primitives used seem a matter of personal preference if the above three things are covered, and basic symmetric gotchas checked for.

@kopischke

This comment has been minimized.

Show comment
Hide comment
@kopischke

kopischke May 7, 2014

Vim currently does not provide any secure option for encryption, at all. If you insist on speaking in relative terms, the options provided are, respectively, outrageously insecure and ridiculously insecure. Considering that even tech savvy people seem to take Vim’s pretence of encrypted security seriously enough to build password managers with it (I kid you not), burning that feature to the ground should actually improve the average Vim user’s security (because they will not even be tempted to trust Vim’s terminally broken encryption with confidential data).

Also, considering that Neovim tries to offload anything outside its core responsibilities to external utilities (which, as of this time, even includes interfacing with the system clipboard – #583), including a feature outside that core which even specialised teams get wrong (Heartbleed, anybody?) and which critically compromises user security when failing is a Bad Idea™.

kopischke commented May 7, 2014

Vim currently does not provide any secure option for encryption, at all. If you insist on speaking in relative terms, the options provided are, respectively, outrageously insecure and ridiculously insecure. Considering that even tech savvy people seem to take Vim’s pretence of encrypted security seriously enough to build password managers with it (I kid you not), burning that feature to the ground should actually improve the average Vim user’s security (because they will not even be tempted to trust Vim’s terminally broken encryption with confidential data).

Also, considering that Neovim tries to offload anything outside its core responsibilities to external utilities (which, as of this time, even includes interfacing with the system clipboard – #583), including a feature outside that core which even specialised teams get wrong (Heartbleed, anybody?) and which critically compromises user security when failing is a Bad Idea™.

@schmee

This comment has been minimized.

Show comment
Hide comment
@schmee

schmee May 7, 2014

Contributor

There is probably a FEAT_ macro for the encryption feature.

I checked and it seems that FEAT_CRYPT was removed in the initial import (it is only present in config.h.in). I definitely agree with removing all encryption for the reasons mentioned by @kopischke. I can take a look at the original vim code and see how FEAT_CRYPT is used there.

Contributor

schmee commented May 7, 2014

There is probably a FEAT_ macro for the encryption feature.

I checked and it seems that FEAT_CRYPT was removed in the initial import (it is only present in config.h.in). I definitely agree with removing all encryption for the reasons mentioned by @kopischke. I can take a look at the original vim code and see how FEAT_CRYPT is used there.

@jamessan

This comment has been minimized.

Show comment
Hide comment
@jamessan

jamessan May 7, 2014

Member

@justinmk You mean something like vim-gnupg? Or were you referring to having it baked into the editor?

Member

jamessan commented May 7, 2014

@justinmk You mean something like vim-gnupg? Or were you referring to having it baked into the editor?

@philix

This comment has been minimized.

Show comment
Hide comment
@philix

philix May 7, 2014

Member

@schmee yes, it was removed, but we can check old Vim to have a clue about
what to remove. And we will be able to say Neovim is like Vim compiled
without FEAT_MACRO.
On May 7, 2014 4:38 AM, "schmee" notifications@github.com wrote:

There is probably a FEAT_ macro for the encryption feature.

I checked and it seems that FEAT_CRYPT was removed in the initial import.
I definitely agree with removing all encryption for the reasons mentioned
by @kopischke https://github.com/kopischke.


Reply to this email directly or view it on GitHubhttps://github.com//issues/694#issuecomment-42397868
.

Member

philix commented May 7, 2014

@schmee yes, it was removed, but we can check old Vim to have a clue about
what to remove. And we will be able to say Neovim is like Vim compiled
without FEAT_MACRO.
On May 7, 2014 4:38 AM, "schmee" notifications@github.com wrote:

There is probably a FEAT_ macro for the encryption feature.

I checked and it seems that FEAT_CRYPT was removed in the initial import.
I definitely agree with removing all encryption for the reasons mentioned
by @kopischke https://github.com/kopischke.


Reply to this email directly or view it on GitHubhttps://github.com//issues/694#issuecomment-42397868
.

@philix

This comment has been minimized.

Show comment
Hide comment
@philix

philix May 7, 2014

Member

*FEAT_CRYPT
On May 7, 2014 8:18 AM, "Felipe Oliveira Carvalho" felipekde@gmail.com
wrote:

@schmee yes, it was removed, but we can check old Vim to have a clue
about what to remove. And we will be able to say Neovim is like Vim
compiled without FEAT_MACRO.
On May 7, 2014 4:38 AM, "schmee" notifications@github.com wrote:

There is probably a FEAT_ macro for the encryption feature.

I checked and it seems that FEAT_CRYPT was removed in the initial import.
I definitely agree with removing all encryption for the reasons mentioned
by @kopischke https://github.com/kopischke.


Reply to this email directly or view it on GitHubhttps://github.com//issues/694#issuecomment-42397868
.

Member

philix commented May 7, 2014

*FEAT_CRYPT
On May 7, 2014 8:18 AM, "Felipe Oliveira Carvalho" felipekde@gmail.com
wrote:

@schmee yes, it was removed, but we can check old Vim to have a clue
about what to remove. And we will be able to say Neovim is like Vim
compiled without FEAT_MACRO.
On May 7, 2014 4:38 AM, "schmee" notifications@github.com wrote:

There is probably a FEAT_ macro for the encryption feature.

I checked and it seems that FEAT_CRYPT was removed in the initial import.
I definitely agree with removing all encryption for the reasons mentioned
by @kopischke https://github.com/kopischke.


Reply to this email directly or view it on GitHubhttps://github.com//issues/694#issuecomment-42397868
.

@aktau

This comment has been minimized.

Show comment
Hide comment
@aktau

aktau May 7, 2014

Member

Err, yes, horrible business the lot of it, delete. We should just ensure that we have a plugin system powerful enough that a (perhaps sanctioned) lua plugin can dynamically interface with system tools/libraries and use something that is possibly actually secure. Let's not even pretend to offer builtin security or paper over it.

Member

aktau commented May 7, 2014

Err, yes, horrible business the lot of it, delete. We should just ensure that we have a plugin system powerful enough that a (perhaps sanctioned) lua plugin can dynamically interface with system tools/libraries and use something that is possibly actually secure. Let's not even pretend to offer builtin security or paper over it.

@schmee schmee referenced this issue May 7, 2014

Closed

[RDY] Remove cryptography #699

5 of 5 tasks complete
@schmee

This comment has been minimized.

Show comment
Hide comment
@schmee

schmee May 7, 2014

Contributor

I opened #699 to get rid of this.

Contributor

schmee commented May 7, 2014

I opened #699 to get rid of this.

@schmee

This comment has been minimized.

Show comment
Hide comment
@schmee

schmee May 7, 2014

Contributor

Also, what is the state of the sha256 algorithm in neovim, since it is also sort of encryption-related?

Contributor

schmee commented May 7, 2014

Also, what is the state of the sha256 algorithm in neovim, since it is also sort of encryption-related?

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 7, 2014

SHA256 is fine

tarcieri commented May 7, 2014

SHA256 is fine

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 7, 2014

Let me rephrase: SHA256 is a fine algorithm. I have no idea how (neo)vim uses it.

tarcieri commented May 7, 2014

Let me rephrase: SHA256 is a fine algorithm. I have no idea how (neo)vim uses it.

@schmee

This comment has been minimized.

Show comment
Hide comment
@schmee

schmee May 7, 2014

Contributor

Ahh, I was more concerned with the quality of the implementation, considering the state of the other cryptography code in vim.

Contributor

schmee commented May 7, 2014

Ahh, I was more concerned with the quality of the implementation, considering the state of the other cryptography code in vim.

@justinmk justinmk modified the milestones: first release, blowfish May 7, 2014

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk May 7, 2014

Member

It's settled to remove this feature. But it's annoying that no one is willing to unambiguously proffer an alternative way to accomplish the task (decrypt, edit, and re-encrypt a file on a non-encrypted filesystem). Or declare that it's impossible and a bad idea.

Considering that even tech savvy people seem to take Vim’s pretence of encrypted security seriously enough to build password managers with it (I kid you not)

@kopischke Yes, I'm one of them, if I may call myself savvy. I'm surprised at the breathless condemnations of this feature of Vim, since I never saw any criticism of it until the recent vim_dev post. The google results for "securely decrypt and edit file" suck (Vim and Ultraedit are suggested). The [top 10] google results for "vim encryption secure", "vim blowfish secure", and "vim blowfish criticism" all make absolutely zero mention of any security issues with Vim's blowfish implementation. So it's a bit disingenuous to act like Vim's blowfish implementation is obviously, totally, like insanely broken. I also didn't audit OpenSSL, and actually no one did (quoting Bruce Schneier).

I am being a tad defensive--but you must understand that I am admitting to the entire world that I managed my passwords with Vim's blowfish implementation. A fact that I may never live down.

@justinmk You mean something like vim-gnupg? Or were you referring to having it baked into the editor?

@jamessan Thank you, I will check that out! But does anyone claim that it is secure? Could we use it to provide an alternative to Vim's cryptmethod?

It's a cop-out to say that everything sucks, and then expect users to cobble together a hand-rolled solution.

Member

justinmk commented May 7, 2014

It's settled to remove this feature. But it's annoying that no one is willing to unambiguously proffer an alternative way to accomplish the task (decrypt, edit, and re-encrypt a file on a non-encrypted filesystem). Or declare that it's impossible and a bad idea.

Considering that even tech savvy people seem to take Vim’s pretence of encrypted security seriously enough to build password managers with it (I kid you not)

@kopischke Yes, I'm one of them, if I may call myself savvy. I'm surprised at the breathless condemnations of this feature of Vim, since I never saw any criticism of it until the recent vim_dev post. The google results for "securely decrypt and edit file" suck (Vim and Ultraedit are suggested). The [top 10] google results for "vim encryption secure", "vim blowfish secure", and "vim blowfish criticism" all make absolutely zero mention of any security issues with Vim's blowfish implementation. So it's a bit disingenuous to act like Vim's blowfish implementation is obviously, totally, like insanely broken. I also didn't audit OpenSSL, and actually no one did (quoting Bruce Schneier).

I am being a tad defensive--but you must understand that I am admitting to the entire world that I managed my passwords with Vim's blowfish implementation. A fact that I may never live down.

@justinmk You mean something like vim-gnupg? Or were you referring to having it baked into the editor?

@jamessan Thank you, I will check that out! But does anyone claim that it is secure? Could we use it to provide an alternative to Vim's cryptmethod?

It's a cop-out to say that everything sucks, and then expect users to cobble together a hand-rolled solution.

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 7, 2014

@justinmk I can't speak to the security of vim-gnupg, but if you're spawning GPG and passing all plaintexts and keys to it through pipes and/or files (i.e. NOT shell parameters or environment variables, and you've avoided shell escaping bugs), it sounds OK. GPG is at least a well-audited tool which is generally trusted by the security community.

tarcieri commented May 7, 2014

@justinmk I can't speak to the security of vim-gnupg, but if you're spawning GPG and passing all plaintexts and keys to it through pipes and/or files (i.e. NOT shell parameters or environment variables, and you've avoided shell escaping bugs), it sounds OK. GPG is at least a well-audited tool which is generally trusted by the security community.

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk May 7, 2014

Member

@tarcieri Ok, thanks.

Member

justinmk commented May 7, 2014

@tarcieri Ok, thanks.

@aktau

This comment has been minimized.

Show comment
Hide comment
@aktau

aktau May 7, 2014

Member

Trying to be constructive, we could look at NaCl (this lwn article, where alternatives are also suggested) or it's allegedly (even) easier to use cousin libsodium.

They say that NaCl/libsodium is a lot easier to use than OpenSSL, and it's made by the famous DJB (though one person, no matter how good, saying it's good shouldn't indicate goodness, let's see if we can find audits of it).

I remember researching it a bit when I was looking to use ZeroMQ and tried to code a security layer over it. What I read was impressive, but as a non-security researcher that shouldn't mean too much either.

What I really liked about NaCl/libsodium is also mentioned in the LWN article comments:

New formats and protocols should look at NaCl (and TweetNaCl) seriously. It's small, fast, unencumbered, uses modern algorithms with high security levels, and has removed all the API options and permutations that are easy to misuse in larger crypto libraries.

NOTE: I don't even know if this would be useful for neovim's usecase, just throwing it out there.

Member

aktau commented May 7, 2014

Trying to be constructive, we could look at NaCl (this lwn article, where alternatives are also suggested) or it's allegedly (even) easier to use cousin libsodium.

They say that NaCl/libsodium is a lot easier to use than OpenSSL, and it's made by the famous DJB (though one person, no matter how good, saying it's good shouldn't indicate goodness, let's see if we can find audits of it).

I remember researching it a bit when I was looking to use ZeroMQ and tried to code a security layer over it. What I read was impressive, but as a non-security researcher that shouldn't mean too much either.

What I really liked about NaCl/libsodium is also mentioned in the LWN article comments:

New formats and protocols should look at NaCl (and TweetNaCl) seriously. It's small, fast, unencumbered, uses modern algorithms with high security levels, and has removed all the API options and permutations that are easy to misuse in larger crypto libraries.

NOTE: I don't even know if this would be useful for neovim's usecase, just throwing it out there.

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 7, 2014

@ataku if you want random access properties, it's a bit more complex than just "use libsodium". If you just want to decrypt the entire file when it's opened, and don't mind re-encrypting the entire file when it's saved, using NaCl/libsodium's crypto_secretbox is an option.

tarcieri commented May 7, 2014

@ataku if you want random access properties, it's a bit more complex than just "use libsodium". If you just want to decrypt the entire file when it's opened, and don't mind re-encrypting the entire file when it's saved, using NaCl/libsodium's crypto_secretbox is an option.

@aktau

This comment has been minimized.

Show comment
Hide comment
@aktau

aktau May 7, 2014

Member

if you want random access properties, it's a bit more complex than just "use libsodium". If you just want to decrypt the entire file when it's opened, and don't mind re-encrypting the entire file when it's saved, using NaCl/libsodium's crypto_secretbox is an option.

Random access might be necessary for a large files yet perhaps we can make a basic implementation for smaller files to just do the decrypt/edit/encrypt cycle. It sounds easier and it would give us something to work with. It's also perhaps questionable why a user would want to edit a huge encrypted file...

(then I wanted to suggest that the user split that huge encrypted file up into several different files, which gave me the idea that we could possibly do some primitive chunking to band-aid it).

Member

aktau commented May 7, 2014

if you want random access properties, it's a bit more complex than just "use libsodium". If you just want to decrypt the entire file when it's opened, and don't mind re-encrypting the entire file when it's saved, using NaCl/libsodium's crypto_secretbox is an option.

Random access might be necessary for a large files yet perhaps we can make a basic implementation for smaller files to just do the decrypt/edit/encrypt cycle. It sounds easier and it would give us something to work with. It's also perhaps questionable why a user would want to edit a huge encrypted file...

(then I wanted to suggest that the user split that huge encrypted file up into several different files, which gave me the idea that we could possibly do some primitive chunking to band-aid it).

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 7, 2014

My suggested path forward would be:

  1. Delete the existing encryption code
  2. Make an issue about adding new encryption

I'd be happy to give some advice on #2. Using XSalsa20+Poly1305 (from NaCl) or ChaCha20+Poly1305 with a random nonce from /dev/urandom (or CryptoGenRandom on Windows) should be OK. I'd suggest avoiding any sort of random access mode and focusing entirely on something that decrypts the entire file in one fell swoop when it's opened, and rewrites a brand new ciphertext with a new random nonce each time the file is written.

tarcieri commented May 7, 2014

My suggested path forward would be:

  1. Delete the existing encryption code
  2. Make an issue about adding new encryption

I'd be happy to give some advice on #2. Using XSalsa20+Poly1305 (from NaCl) or ChaCha20+Poly1305 with a random nonce from /dev/urandom (or CryptoGenRandom on Windows) should be OK. I'd suggest avoiding any sort of random access mode and focusing entirely on something that decrypts the entire file in one fell swoop when it's opened, and rewrites a brand new ciphertext with a new random nonce each time the file is written.

@aktau

This comment has been minimized.

Show comment
Hide comment
@aktau

aktau May 7, 2014

Member

@tarcieri you seem to have some experience with both NaCl and libsodium (as evidenced by your repo's as well). What's your opinion, which is preferable? It seems libsodium provides the platform independent random bytes function you just talked about as well (randombytes_*()).

Member

aktau commented May 7, 2014

@tarcieri you seem to have some experience with both NaCl and libsodium (as evidenced by your repo's as well). What's your opinion, which is preferable? It seems libsodium provides the platform independent random bytes function you just talked about as well (randombytes_*()).

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk May 7, 2014

Member

If you just want to decrypt the entire file when it's opened, and don't mind re-encrypting the entire file when it's saved, using NaCl/libsodium's crypto_secretbox is an option.

That seems like a very reasonable "minimum viable product".

Delete the existing encryption code

@schmee is working on that right now.

I'd be happy to give some advice on 2

Thank you! #701

Thanks for the suggestions @aktau !

Member

justinmk commented May 7, 2014

If you just want to decrypt the entire file when it's opened, and don't mind re-encrypting the entire file when it's saved, using NaCl/libsodium's crypto_secretbox is an option.

That seems like a very reasonable "minimum viable product".

Delete the existing encryption code

@schmee is working on that right now.

I'd be happy to give some advice on 2

Thank you! #701

Thanks for the suggestions @aktau !

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 7, 2014

You could add libsodium as a dependency, but it's probably more than what you need. Alternatively you could just vendor the portable C implementation of ChaCha20+Poly1305.

tarcieri commented May 7, 2014

You could add libsodium as a dependency, but it's probably more than what you need. Alternatively you could just vendor the portable C implementation of ChaCha20+Poly1305.

@ManIVIctorious

This comment has been minimized.

Show comment
Hide comment
@ManIVIctorious

ManIVIctorious May 8, 2014

I am no expert about encryption but maybe it helps:
As linux user i use a container file in combination with LUKS, this avoids the problem of un-encrypted temporary files, as they are in the encrypted container.
Maybe EncFS is also interesting...

ManIVIctorious commented May 8, 2014

I am no expert about encryption but maybe it helps:
As linux user i use a container file in combination with LUKS, this avoids the problem of un-encrypted temporary files, as they are in the encrypted container.
Maybe EncFS is also interesting...

@jamessan

This comment has been minimized.

Show comment
Hide comment
@jamessan

jamessan May 8, 2014

Member

@jamessan Thank you, I will check that out! But does anyone claim that it is secure? Could we use it to provide an alternative to Vim's cryptmethod?

I've certainly taken steps to improve on its security since I took over maintenance, but I'm always open to another pair of eyes looking it over. I'd be foolish to claim it's definitively secure. :)

I delegate as much as possible to gpg itself, turn off options that would save information to disk, etc. However, it would be nice to have some more support in (neo)vim to provide extra protections for use cases like this. There's a relevant item in Vim's todo list:

8 Option to lock all used memory so that it doesn't get swapped to disk
(uncrypted). Patch by Jason Holt, 2003 May 23. Uses mlock.

Member

jamessan commented May 8, 2014

@jamessan Thank you, I will check that out! But does anyone claim that it is secure? Could we use it to provide an alternative to Vim's cryptmethod?

I've certainly taken steps to improve on its security since I took over maintenance, but I'm always open to another pair of eyes looking it over. I'd be foolish to claim it's definitively secure. :)

I delegate as much as possible to gpg itself, turn off options that would save information to disk, etc. However, it would be nice to have some more support in (neo)vim to provide extra protections for use cases like this. There's a relevant item in Vim's todo list:

8 Option to lock all used memory so that it doesn't get swapped to disk
(uncrypted). Patch by Jason Holt, 2003 May 23. Uses mlock.

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 8, 2014

@jamessan you probably want mlockall(), not mlock()

tarcieri commented May 8, 2014

@jamessan you probably want mlockall(), not mlock()

@jamessan

This comment has been minimized.

Show comment
Hide comment
@jamessan

jamessan May 8, 2014

Member

@tarcieri Possibly. I was just quoting the todo list item. I hadn't actually looked into the specifics.

Member

jamessan commented May 8, 2014

@tarcieri Possibly. I was just quoting the todo list item. I hadn't actually looked into the specifics.

@tarruda

This comment has been minimized.

Show comment
Hide comment
@tarruda

tarruda May 8, 2014

Member

but then I'd probably have to stab my own eyes out with a fork

Lately I've been having that urge :)

Member

tarruda commented May 8, 2014

but then I'd probably have to stab my own eyes out with a fork

Lately I've been having that urge :)

@amtal

This comment has been minimized.

Show comment
Hide comment
@amtal

amtal May 9, 2014

I'm surprised at the breathless condemnations of this feature of Vim, since I never saw any criticism of it until the recent vim_dev post. The google results for "securely decrypt and edit file" suck (Vim and Ultraedit are suggested). The [top 10] google results for "vim encryption secure", "vim blowfish secure", and "vim blowfish criticism" all make absolutely zero mention of any security issues with Vim's blowfish implementation. So it's a bit disingenuous to act like Vim's blowfish implementation is obviously, totally, like insanely broken.

Auditing the crypto is a pain, so few people did. Except Bram Moolenaar in Feb 2014, who correctly noticed the block cipher mode was CFB not OFB and fixed the name. That alone is an achievement, so refactoring bad code or looking deeper didn't happen. Between the macros (2010 mailing list mentioned "optimizations", which may explain them) the indirection and the closely coupled known-broken "zip" implementation, I can see why it got little attention.

Curiously, the swap file and undo buffer (I think?) got enough attention to encrypt them. That's neat. Case of familiarity with the codebase and working on the bike shed instead of the reactor?

As I understand the old implementation:

  • SHA2 seems to be from md5deep, has a standard KAT.
  • Don't know origin of Blowfish implementation, or where its KAT comes from.
  • Won't reason about the key derivation. Somewhere in there is a salt, 1000 iterations of something involving SHA256, and a Blowfish key... And also strings and binary data living together, a SHA256 self test whose result gets ignored, and code flow like a smoothly winding river.
  • Encryption uses CFB mode in 8 parallel 8 byte columns. The "seed" (initialization vector) is the same in each column: extracting enough plaintext to get a good idea of whether the contents are worth cracking is trivial.

On implementing alternatives, I guess the main constraint is not complicating the build process since this is a pretty obscure, low-use feature. That cuts out pretty much anything that's not directly embedable the way SHA256+Blowfish were, leaving us with...

  • KDF: scrypt with a config option for power users would be very nice, and even comes with the code to encrypt files - but - probably needs openssl and duct tape for Windows. PBKDF2 would be a significantly less shiny no-deps option.
  • Cipher+MAC: libsodium's an extra dependency; tweetnacl is completely standalone but idk if anyone takes it seriously. What other implementations would work without pulling in GMP/openssl?
  • Nonce/salt: may be able to get away with just a timestamp for this use case.

@tarcieri, others, thoughts on viable implementations? Are extra external deps doable?

amtal commented May 9, 2014

I'm surprised at the breathless condemnations of this feature of Vim, since I never saw any criticism of it until the recent vim_dev post. The google results for "securely decrypt and edit file" suck (Vim and Ultraedit are suggested). The [top 10] google results for "vim encryption secure", "vim blowfish secure", and "vim blowfish criticism" all make absolutely zero mention of any security issues with Vim's blowfish implementation. So it's a bit disingenuous to act like Vim's blowfish implementation is obviously, totally, like insanely broken.

Auditing the crypto is a pain, so few people did. Except Bram Moolenaar in Feb 2014, who correctly noticed the block cipher mode was CFB not OFB and fixed the name. That alone is an achievement, so refactoring bad code or looking deeper didn't happen. Between the macros (2010 mailing list mentioned "optimizations", which may explain them) the indirection and the closely coupled known-broken "zip" implementation, I can see why it got little attention.

Curiously, the swap file and undo buffer (I think?) got enough attention to encrypt them. That's neat. Case of familiarity with the codebase and working on the bike shed instead of the reactor?

As I understand the old implementation:

  • SHA2 seems to be from md5deep, has a standard KAT.
  • Don't know origin of Blowfish implementation, or where its KAT comes from.
  • Won't reason about the key derivation. Somewhere in there is a salt, 1000 iterations of something involving SHA256, and a Blowfish key... And also strings and binary data living together, a SHA256 self test whose result gets ignored, and code flow like a smoothly winding river.
  • Encryption uses CFB mode in 8 parallel 8 byte columns. The "seed" (initialization vector) is the same in each column: extracting enough plaintext to get a good idea of whether the contents are worth cracking is trivial.

On implementing alternatives, I guess the main constraint is not complicating the build process since this is a pretty obscure, low-use feature. That cuts out pretty much anything that's not directly embedable the way SHA256+Blowfish were, leaving us with...

  • KDF: scrypt with a config option for power users would be very nice, and even comes with the code to encrypt files - but - probably needs openssl and duct tape for Windows. PBKDF2 would be a significantly less shiny no-deps option.
  • Cipher+MAC: libsodium's an extra dependency; tweetnacl is completely standalone but idk if anyone takes it seriously. What other implementations would work without pulling in GMP/openssl?
  • Nonce/salt: may be able to get away with just a timestamp for this use case.

@tarcieri, others, thoughts on viable implementations? Are extra external deps doable?

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 9, 2014

@amtal I'd say vendor ChaCha20+Poly1305 and as you suggest use scrypt as a KDF.

Or you could use libsodium. It's an additional dependency, but it will provide scrypt in its next release and can provide XSalsa20+Poly1305 via the crypto_secretbox API, so it's at least a single dependency that can provide a complete solution for password-based symmetric encryption. It also has a decent portability story.

tarcieri commented May 9, 2014

@amtal I'd say vendor ChaCha20+Poly1305 and as you suggest use scrypt as a KDF.

Or you could use libsodium. It's an additional dependency, but it will provide scrypt in its next release and can provide XSalsa20+Poly1305 via the crypto_secretbox API, so it's at least a single dependency that can provide a complete solution for password-based symmetric encryption. It also has a decent portability story.

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk May 9, 2014

Member

@amtal Thank you, maybe soon a web search will show your analysis.

@amtal @tarcieri what is your opinion on the concern raised in #701 that asserts that any at-rest encryption on a non-encrypted filesystem is useless? I find it asymptotically convincing, but on the other hand I think that the risk of a stray swap write is better than no encryption at all.

Member

justinmk commented May 9, 2014

@amtal Thank you, maybe soon a web search will show your analysis.

@amtal @tarcieri what is your opinion on the concern raised in #701 that asserts that any at-rest encryption on a non-encrypted filesystem is useless? I find it asymptotically convincing, but on the other hand I think that the risk of a stray swap write is better than no encryption at all.

@aktau

This comment has been minimized.

Show comment
Hide comment
@aktau

aktau May 9, 2014

Member

@amtal I'd say vendor ChaCha20+Poly1305 and as you suggest use scrypt as a KDF.

Even though those are probably fine crypto primitives at this point, who's to say about the future (as you mentioned yourself). So I don't know how I feel about vendoring specific primitives like that. They could be broken at any point in the future and we'd be scrambling to look for an alternative.

Or you could use libsodium. It's an additional dependency, ...

That seem to me to be the most future-proof, hands-off thing we could do. If we judiciously update libsodium when they have a new release (which we already do for libuv and luajit), I have a feeling we'd be "safer". We wouldn't have to know exactly which primitives we're using, as libsodium/NaCl tries to make those choices for us. At least that's the general feeling/gist I get from those libraries. Or do I misunderstand what these libraries are all about?

Member

aktau commented May 9, 2014

@amtal I'd say vendor ChaCha20+Poly1305 and as you suggest use scrypt as a KDF.

Even though those are probably fine crypto primitives at this point, who's to say about the future (as you mentioned yourself). So I don't know how I feel about vendoring specific primitives like that. They could be broken at any point in the future and we'd be scrambling to look for an alternative.

Or you could use libsodium. It's an additional dependency, ...

That seem to me to be the most future-proof, hands-off thing we could do. If we judiciously update libsodium when they have a new release (which we already do for libuv and luajit), I have a feeling we'd be "safer". We wouldn't have to know exactly which primitives we're using, as libsodium/NaCl tries to make those choices for us. At least that's the general feeling/gist I get from those libraries. Or do I misunderstand what these libraries are all about?

@schmee

This comment has been minimized.

Show comment
Hide comment
@schmee

schmee May 9, 2014

Contributor

They could be broken at any point in the future and we'd be scrambling to look for an alternative.

This is why we shouldn't take on the responsibility of providing built-in cryptography. It incurs a high developer cost and very little gain for most users. As long as clipboard code is delegated I don't see how a case can be made for providing something as complex and error-prone as cryptography as a built-in.

Contributor

schmee commented May 9, 2014

They could be broken at any point in the future and we'd be scrambling to look for an alternative.

This is why we shouldn't take on the responsibility of providing built-in cryptography. It incurs a high developer cost and very little gain for most users. As long as clipboard code is delegated I don't see how a case can be made for providing something as complex and error-prone as cryptography as a built-in.

@amtal

This comment has been minimized.

Show comment
Hide comment
@amtal

amtal May 9, 2014

In regards to #701, I'd say that there is a certain elegance to having encryption in your editor, same as having it in your compressed archive tool. There are use cases like network shares, portable storage, and even already encrypted disks that could use an extra layer.

It's reasonable that something like vim or emacs, with their kitchen sink feature set, would have the feature. I don't know whether the same holds for neovim :)

As for dealing with breaks, those usually come from implementations rather than primitives. And really, encrypting a file at rest based on a user-provided password should be almost the simpler thing cryptography can do. If we can't do it with a fair degree of confidence with modern tools, what does that say about those tools?

amtal commented May 9, 2014

In regards to #701, I'd say that there is a certain elegance to having encryption in your editor, same as having it in your compressed archive tool. There are use cases like network shares, portable storage, and even already encrypted disks that could use an extra layer.

It's reasonable that something like vim or emacs, with their kitchen sink feature set, would have the feature. I don't know whether the same holds for neovim :)

As for dealing with breaks, those usually come from implementations rather than primitives. And really, encrypting a file at rest based on a user-provided password should be almost the simpler thing cryptography can do. If we can't do it with a fair degree of confidence with modern tools, what does that say about those tools?

@philix

This comment has been minimized.

Show comment
Hide comment
@philix

philix May 9, 2014

Member

@amtal 👍
On May 9, 2014 10:50 AM, "amtal" notifications@github.com wrote:

In regards to #701 #701, I'd say
that there is a certain elegance to having encryption in your editor, same
as having it in your compressed archive tool. There are use cases like
network shares, portable storage, and even already encrypted diskshttp://sockpuppet.org/blog/2014/04/30/you-dont-want-xts/that could use an extra layer.

It's reasonable that something like vim or emacs, with their kitchen sink
feature set, would have the feature. I don't know whether the same holds
for neovim :)

As for dealing with breaks, those usually come from implementations rather
than primitives. And really, encrypting a file at rest based on a
user-provided password should be almost the simpler thing cryptography can
do. If we can't do it with a fair degree of confidence with modern tools,
what does that say about those tools?


Reply to this email directly or view it on GitHubhttps://github.com//issues/694#issuecomment-42667570
.

Member

philix commented May 9, 2014

@amtal 👍
On May 9, 2014 10:50 AM, "amtal" notifications@github.com wrote:

In regards to #701 #701, I'd say
that there is a certain elegance to having encryption in your editor, same
as having it in your compressed archive tool. There are use cases like
network shares, portable storage, and even already encrypted diskshttp://sockpuppet.org/blog/2014/04/30/you-dont-want-xts/that could use an extra layer.

It's reasonable that something like vim or emacs, with their kitchen sink
feature set, would have the feature. I don't know whether the same holds
for neovim :)

As for dealing with breaks, those usually come from implementations rather
than primitives. And really, encrypting a file at rest based on a
user-provided password should be almost the simpler thing cryptography can
do. If we can't do it with a fair degree of confidence with modern tools,
what does that say about those tools?


Reply to this email directly or view it on GitHubhttps://github.com//issues/694#issuecomment-42667570
.

schmee added a commit to schmee/neovim that referenced this issue May 18, 2014

Remove cryptography
As discussed in neovim#694, vim encryption uses old,
obsolete algorithms that are poorly implemented.
Since insecure cryptography is worse than no
cryptgraphy, the community voted in favor of
removing all crypto.

Various alternatives to the old crypto is
being discussed in neovim#701.

Closes neovim#694.

@tarruda tarruda closed this in 85338fe May 20, 2014

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri May 20, 2014

🎉 🎉 🎉 👍 👍 👍

tarcieri commented May 20, 2014

🎉 🎉 🎉 👍 👍 👍

tarruda added a commit to tarruda/neovim that referenced this issue May 22, 2014

Remove cryptography
As discussed in neovim#694, vim encryption uses old,
obsolete algorithms that are poorly implemented.
Since insecure cryptography is worse than no
cryptgraphy, the community voted in favor of
removing all crypto.

Various alternatives to the old crypto is
being discussed in neovim#701.

Closes neovim#694.
@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri Oct 5, 2014

This seems to be making the rounds on various programming/crypto news sites:

https://dgl.cx/2014/10/vim-blowfish

tarcieri commented Oct 5, 2014

This seems to be making the rounds on various programming/crypto news sites:

https://dgl.cx/2014/10/vim-blowfish

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment