rnpgp / rnp

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

Consider to use shorter armor lines #1099

Closed kaie closed 4 years ago

kaie commented 4 years ago

When exporting armored blocks, e.g. for public keys, RNP uses a fixed line length of 76 chars. I believe this is set in the following line:

third_party/rnp/src/librepgp/stream-armor.cpp:    param->llen = 76; /* must be multiple of 4 */

When attaching these in email, Thunderbird wraps it at position 73.

(Armored blocks produced by GnuPG wrap at position 64, and didn't cause Enigmail to wrap.)

It's not a big deal, it simply looks slightly ugly when looking at the message source (with alternating lines of 73 and 3 characters).

Not sure if I should ask you to change it to 64, too, or make it configurable. What do you think?

Low priority, because from the functional perspective its working fine.

ronaldtse commented 4 years ago

@kaie a shorter line length seems reasonable. No preference between 64 or 72...

Thoughts @ni4 @dewyatt ?

ni4 commented 4 years ago

@kaie @ronaldtse It can be easily changed, the only problem I see is propagating the line length parameter through the whole API. The easies solution I see now is adding this parameter to rnp_output_to_armor().

kaie commented 4 years ago

Just FYI, I tested and can confirm that changing the hardcoded value in the line mentioned above is sufficient to change the line length.

rrrooommmaaa commented 4 years ago

@ni4 Did you mean to configure it via rnp_cfg_t? And what value should be default?

ni4 commented 4 years ago

@rrrooommmaaa No, rnp_cfg_t is used only in CLI, while this should be changed on FFI layer. Thing which comes first in mind is to update rnp_output_to_armor() function so type parameter will be allowed to have appended line length after the semicolon, i.e. type = message:64. As per RFC 4880-bis, maximum (and currently used as default) value is 76.

Also init_armored_src() should be updated with line length parameter (defaulting to current 76, which should be moved to some #define).

kaie commented 4 years ago

Not sure if you like this indirection, but here's another potential approach:

rnp_output_armor_set_line_length(rnp_output_t out_armor, size_t len);

You could define that calling rnp_output_armor_set_line_length is allowed, only, if parameter out_armor has been obtained from rnp_output_to_armor.

kaie commented 4 years ago

And maybe allow it to be set for the output object given to rnp_enarmor

ni4 commented 4 years ago

@kaie Agree, rnp_output_armor_set_line_length() could be also a good solution. It would also allow to extend functionality later on with other settings (like, in case we'll need to add some custom header or so on).

However not sure whether this possible for rnp_enarmor, since there output may be of any type, and all work is done in one pass. Probably, this should be better replaced with combination of rnp_output_to_armor() and function like rnp_input_to_output().