rnpgp / rnp

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

Check partial packet length edge cases. #939

Closed ni4 closed 4 years ago

ni4 commented 4 years ago

Description

Following cases should be checked:

This would require to modify some existing packet data and attempt to parse those via FFI calls. Separate test file should be added to rnp_tests suite.

rrrooommmaaa commented 4 years ago

I can take this one (since I've already dealt with partial packets)

ronaldtse commented 4 years ago

@rrrooommmaaa feel free to take it!

rrrooommmaaa commented 4 years ago

@ni4 Regarding "less then 512 byte size first chunk (should not be allowed)" Turns out that both GPG and RNP allow smaller first chunk (I tested with 256 bytes).

ni4 commented 4 years ago

@rrrooommmaaa nice found, however not sure what to do with it at the moment. In RFC it is stated as MUST, so such message will not be compliant with RFC 4880. Probably at the moment just issue a warning, and add test which makes sure that we do not produce first chunk of such length (when, for instance, data is read from stdin so input size is not known in advance). @ronaldtse @dewyatt what do you think about this case?

rrrooommmaaa commented 4 years ago

As far as I know, in protocol implementations, MUST means that such data must not be produced, but it can be still consumed as this is normal practice to allow wider range of input than output.

ni4 commented 4 years ago

@rrrooommmaaa Yeah, code should be flexible on processing data and strict on producing. However, in security case, data which doesn't conform to standard may be the sign of some sort of attack.