Skip to content

Add support for openpgp#730

Open
pointlander wants to merge 28 commits intomicro-editor:masterfrom
pointlander:master
Open

Add support for openpgp#730
pointlander wants to merge 28 commits intomicro-editor:masterfrom
pointlander:master

Conversation

@pointlander
Copy link

No description provided.

@zyedidia
Copy link
Member

zyedidia commented Jul 18, 2017

If I create a new encrypted file from the command line (micro test.asc) it asks for a password but then thinks that the password was incorrect (because the file didn't exist before) and the file just shows No name and test.asc: EOF. I'm not completely sure what the issue is here but I haven't looked to closely into fixing it.

I also think that the password shouldn't be displayed when typing it in. You can use terminal.ReadPassword (from golang.org/x/crypto/ssh/terminal which this already depends on) to read a command line password without displaying it. For reading the password using messenger within the editor, we might have to create a new PasswordPrompt which doesn't display what is being typed in.

Also, could you perhaps provide some description of what the intended behavior is?

I think this needs some more work before it can be merged but this is a great addition to the editor. Thanks for taking the time to contribute!

@pointlander
Copy link
Author

pointlander commented Jul 18, 2017 via email

@zyedidia
Copy link
Member

zyedidia commented Sep 5, 2017

Sorry I've just been idling with this issue. I'll try to test it and hopefully merge it soon.

@zyedidia
Copy link
Member

Am I correct in my understanding that a file can only be encrypted if it has the extension .asc or .gpg?

var err error
reader, err = b.Decode(reader, path, password)
if err != nil {
return NewBufferFromString(fmt.Sprintf("%s: %v", path, err), "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of just opening an empty file with the error message, I think micro should print the message and then open the encrypted file (without decrypting it). So this line should probably be something like

TermMessage(fmt.Sprintf("Error: %s: %v", path, err))
return NewBuffer(freader, size, path)

I have renamed the argument reader to freader to avoid the reassignment of the variable.

@pointlander
Copy link
Author

Correct. Only files with .asc or .gpg are encrypted.

@zyedidia
Copy link
Member

zyedidia commented Oct 6, 2017

Thanks for keeping this up-to-date. I'll try review and merge soon. My only concern is the x/crypto dependency which adds another 55,000 lines of dependency code -- but there isn't much that can be done about it and it really isn't a big deal.

@pointlander
Copy link
Author

build tag 'phat' is now required for openpgp support; gzip support has also been added @zyedidia

@silverark
Copy link

I was just about to start building this feature. I'm presuming this was never merged in? @pointlander do you know if I can resurrect this merge or should I start a new one??

@pointlander
Copy link
Author

I will fix the conflicts @silverark

@pointlander
Copy link
Author

@pointlander
Copy link
Author

merge completed @silverark

@silverark
Copy link

Thank you @pointlander

That's brilliant. Thanks for your hard work

@zyedidia
Copy link
Member

zyedidia commented Apr 2, 2020

Thanks for updating this! I will try to take a look through it soon.

@J08nY
Copy link
Contributor

J08nY commented Apr 11, 2020

I don't think micro should support OpenPGP in any way. The general consensus in the security community is that PGP does what it does quite badly:

https://latacora.singles/2019/07/16/the-pgp-problem.html
https://blog.cryptographyengineering.com/2014/08/13/whats-matter-with-pgp/

Adding support for it at this point would not be wise. It might make sense to create a plugin that does this (and change micro where it needs changes for the plugin to work).

If users want OpenPGP support, just use the gpg on files micro produces. It is easy and works with the unix philosophy, e.g. gpg -d encrypted.asc | micro.

@pointlander
Copy link
Author

How would the user write the file to disk in encrypted form? This feature also adds compression support, and vi encryption format should be supported. Should be using this: https://github.com/awnumar/memguard

@zyedidia
Copy link
Member

With version 2.0.3 you can also pipe the contents of a file edited with micro to another unix tool. I am not familiar with the unix tools used for encryption but presumably one could do something like decompress file.txt | micro | compress > file.txt. In general I'm hesitant to implement features in micro that require heightened security because it's easy to get things wrong and the stakes are much higher.

Are you suggesting using memguard for keeping the file contents that are being edited in-memory by micro in a secure enclave? Correct me if I'm wrong but it doesn't seem like the implementation of this PR currently supports that.

@pointlander
Copy link
Author

This pull request doesn't have memguard, but it would be a good idea.

@pointlander
Copy link
Author

And what about multiple files?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants