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

Added re-signing of existing signed files. #108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions autoload/gnupg.vim
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,16 @@ function gnupg#decrypt(bufread)
end
let start = match(output, asymmPattern, start)
endwhile

" determine if the file was signed; if so, we will too
let cmd = { 'level': 3 }
let cmd.args = '--list-packets ' . s:shellescape(filename, { 'cygpath': 1 })
Copy link
Owner

Choose a reason for hiding this comment

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

I just noticed that the man page describes this option as such (emphasis added):

List only the sequence of packets. This command is only useful for debugging. When used with option --verbose the actual MPI values are dumped and not only their lengths. Note that the output of this command may change with new releases.

It'd be nice if there was a more stable way to get this information.

Copy link
Author

Choose a reason for hiding this comment

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

Ugh, yes that is unfortunate.

Hm, often "use --status-fd" is recommended. So as a quick test:

$ gpg --status-fd 3 -d test.pgp 3>&1 1>/dev/null 2>/dev/null | awk '/^\[GNUPG:\] VALIDSIG /{print $3}'
96063BF9B5934CBCE31AA3846200F6E3781E3DD7

I don't know how well that redirection would play with s:GPGSystem, might need a separate helper for it.

Copy link
Owner

Choose a reason for hiding this comment

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

This sounds like more motivation for me to finally move to using jobs/channels, loopback pinentry, etc. so I can get more fine-grained information out of gpg, as well as supporting nvim.

let output = s:GPGSystem(cmd)

if (matchstr(output, "^:\(onepass_sig\|signature\) packet:") >= 0)
let g:GPGPreferSign = 1
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
let g:GPGPreferSign = 1
call s:GPGDebug(1, 'this file is signed')
let b:GPGOptions += ['sign']

Copy link
Owner

Choose a reason for hiding this comment

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

This would ensure that we only change the option for this buffer, instead of overriding the user's choice for all buffers just because this one file is signed.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, per-buffer sure sounds better than my method.

I think at one or two places I wasn't sure if it was safe to edit b:GPGOptions yet, because gnupg might be invoked for something else prior to encrypt-to-save (i.e. if another gpg -d were done, including --sign would be a usage error).

If you are sure it's safe/appropriate to do there - then +1 for sure!

Copy link
Owner

Choose a reason for hiding this comment

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

I think at one or two places I wasn't sure if it was safe to edit b:GPGOptions yet, because gnupg might be invoked for something else prior to encrypt-to-save

b:GPGOptions is only used as part of the command-line for the encryption phase, so it's safe.

endif

else
" file is not encrypted
let b:GPGEncrypted = 0
Expand Down Expand Up @@ -509,12 +519,14 @@ function gnupg#encrypt()
if (preferArmor >= 0 && preferArmor) || filename =~ '\.asc$'
let b:GPGOptions += ["armor"]
endif
if (exists("g:GPGPreferSign") && g:GPGPreferSign == 1)
let b:GPGOptions += ["sign"]
endif
call s:GPGDebug(1, "no options set, so using default options: " . string(b:GPGOptions))
endif

" check if we should sign this file
Copy link
Owner

Choose a reason for hiding this comment

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

This change isn't necessary with the above suggestion.

Copy link
Author

Choose a reason for hiding this comment

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

Well...

As a user, I want a way to say that any file I edit should get signed when I save it, even if the original was not signed. When I read of the g:GPGPreferSign option, that's initially what I expected it to do. Perhaps there should be a g:GPGAlwaysSign that behaves that way? (In fact I think I started to add a g:GPGAlwaysSign, then thought it was redundant. But maybe not?)

Copy link
Owner

Choose a reason for hiding this comment

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

As a user, I want a way to say that any file I edit should get signed when I save it, even if the original was not signed.

Then you probably want to add something like this to your vimrc:

autocmd User GnuPG if index(b:GPGOptions, 'sign') == -1 | call add(b:GPGOptions, 'sign') | endif

Copy link
Author

Choose a reason for hiding this comment

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

Then you probably want to add something like this to your vimrc:

Oh, nice, I will try that! (And sorry this has veered into stupid user questions ;)

if (exists("g:GPGPreferSign") && g:GPGPreferSign == 1)
let b:GPGOptions += ["sign"]
endif

" built list of options
let options = ""
for option in b:GPGOptions
Expand Down