v6 signature salt hashing

[TJ-91]

@ni4 I realized that we did not properly hash the v6 signature salt in all cases. For key signatures it was fine but not for messages. This commit fixes it: https://github.com/pqc-thunderbird/rnp/commit/fc367798a5c20f02da6c8713508379b4b852dfef (branch update-draft-05)

I have a few questions / comments.

1) When creating HashList elements, I introduce a new variant to create hashes and initialize it with the salt value. Are you ok with how this is done? I had to make sure that every time we start hashing signature stuff, we already have the salt hashed.

2) I also implemented v6 One Pass Signatures in the previous commit and finalized / fixed it in this commit.

3) As you can see, I have an outcommented test for RFC 9580's clear signed message test vector (test_ffi_verify_v2_seipd_cleartext_test_vector). This one doesn't work since I have to properly change the add_hash_for_sig() call in cleartext_parse_headers(). I'm not sure what the best way would be to implement that here. There is no easy way to get the salt at this point, I think. Do you have an idea?

[ni4]

@TJ-91 Looks like v6 brings more problems than I thought before. It completely disables possibility to process cleartext-signed messages in one pass, and I don't see an easy solution to this at the moment. Probably it should be postponed until we have practical need for this, or do you have it already?

Regarding HashList - I would suggest to add class HashListSalted: public HashList, and add all the required functionality via it (and field hashes_v6 in pgp_source_signed_param_t), protected by #ifdef ENABLE_CRYPTO_REFRESH, this should look cleaner.

P.S. Within current codebase I don't see salt comparison in matches_onepass(), which effectively disables the whole salt purpose :)

[TJ-91]

@ni4 thanks for the reply.

It completely disables possibility to process cleartext-signed messages in one pass

Yes, I fear that's true. If I remember correctly, it was discussed at one point to add a header for the salt. However, I think the WG does not like the CSF very much (for security reasons) and therefore would rather discontinue it. At least that's what I remember and it somewhat explains why they do not think one-pass processing is an important issue.

Probably it should be postponed until we have practical need for this, or do you have it already?

I have not worked this out yet. I will try to let RNP fail gracefully for v6 cleartext signatures.

Regarding HashList - I would suggest to add class HashListSalted: public HashList

Thanks for the idea.

P.S. Within current codebase I don't see salt comparison in matches_onepass(), which effectively disables the whole salt purpose :)

https://github.com/pqc-thunderbird/rnp/commit/fc367798a5c20f02da6c8713508379b4b852dfef#diff-d4350f37d2fcd50abb8a5df457b8cb831028d1ceaeddcd75897928142ecb53d9R1315

[ni4]

fc36779#diff-d4350f37d2fcd50abb8a5df457b8cb831028d1ceaeddcd75897928142ecb53d9R1315

Thanks for pointing at, was looking through already merged code.

[TJ-91]

I've made some improvements, see https://github.com/pqc-thunderbird/rnp/commit/4af70e5fc40df74d868521ea79e6a2ff7ac3ea9b (rebased commit).

• Fixed behaviour for v6 sigs and cleartext sigs: fail gracefully by skipping v6 cleartext, see the changes to signed_src_finish(): https://github.com/pqc-thunderbird/rnp/commit/4af70e5fc40df74d868521ea79e6a2ff7ac3ea9b#diff-bd4873e3a91db420f0ac0c6337edf1879800829af7c7ab5dfc16094906cdb621R1205-R1212
• Made the HashList stuff a bit clearer but did not create a subclass. I think this way it's cleaner.
• Most notably I now use a hash list of type std::vector<std::pair<std::unique_ptr<Hash>, std::vector<uint8_t>>> hashes; and for sigs <v6 the salt is simply an empty vector.
• I now store the salt in the pgp_dest_signer_info_t sinfo object where I think it makes more sense and is easily accessed in the OPS and non-OPS case. Also fixed an error here.

Feel free to leave comments. Otherwise I'll presumably create a new PR soon so all the changes will come up there anyway.