rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
194 stars 55 forks source link

Redirected output can result in extra invalid bytes being appended #542

Closed dewyatt closed 4 years ago

dewyatt commented 6 years ago

Description

I think it's common to do something like echo message | rnp --sign > output.gpg and expect it to work. We do have a --output option as well, but *nix users are definitely going to expect that they can pipe/redir stdout in this case and it will work.

Unfortunately this doesn't work 100% correctly in rnp because of things like: https://github.com/riboseinc/rnp/blob/82fb956244d721fc6fa002922b36b9f3fe1887e6/src/lib/pass-provider.c#L124 https://github.com/riboseinc/rnp/blob/82fb956244d721fc6fa002922b36b9f3fe1887e6/src/lib/pass-provider.c#L132

A quick solution here might be to output to stderr (or use pgp_io_t).

Steps to Reproduce

  1. echo test | src/rnp/rnp --sign | pgpdump or echo test | src/rnp/rnp --sign | gpg --list-packets

Expected Behavior

There shouldn't be any issues with the PGP data.

Actual Behavior

At least one newline (0x0a) is appended to the data (more if password is incorrect). pgpdump and gpg --list-packets both spot this and complain in different ways (gpg --list-packets exits with a non-zero code, though --verify is good).

ni4 commented 6 years ago

I think that puts("\nPasswords do not match!"); should be considered as communication with user on the CLI layer, and we should have some callback/function/whatever else like show_user_message() which will check for output redirection and so on.

ronaldtse commented 6 years ago

Agree with @ni4 . We shouldn't mix lib and CLI responsibilities.

dewyatt commented 6 years ago

The rnp_password_provider_stdin provider is only meant to be used by the CLI utils, that's why it has print statements. It would be good to move it somewhere else though.

ni4 commented 6 years ago

@dewyatt I think we should create some 'clicommon' source file where it should be put in, among with other sources which may be shared between rnp, rnpkeys and rnp_tests