fix key hashing for v5 / v6 signatures

[TJ-91]

Justus pointed out that the hashing of keys is implemented incorrectly for RNP and some other implementations, see https://mailarchive.ietf.org/arch/msg/openpgp/PyP-XDv0VM5bYPX1Iq41-Oyytds/ This concerns computing v5 and v6 signatures over keys.

This PR fixes that. I separated signature and fingerprint computation logically, but the same code is used in the background. I also added a check that we don't accidently hash too large a key.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.27%. Comparing base (92adfd7) to head (c13ce03). Report is 28 commits behind head on main.

Files with missing lines	Patch %	Lines
src/librepgp/stream-sig.cpp	87.50%	1 Missing :warning:

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2261   +/-   ##
=======================================
  Coverage   84.27%   84.27%           
=======================================
  Files         114      114           
  Lines       23324    23324           
=======================================
  Hits        19656    19656           
  Misses       3668     3668           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[TJ-91]

@ni4 I'm not sure why a windows and Cirrus CI test fails. Does the problem still exists that tests need to be restarted occasionally?

Also codecov complains about some code that is not tested. Are we required to provide full test coverage for new code? It's not always trivial. For example, one failure case I added tests whether the length of a key is longer than what can be encoded in 4 length octets, i.e., 4 gigabyte.

[ni4]

@ni4 I'm not sure why a windows and Cirrus CI test fails. Does the problem still exists that tests need to be restarted occasionally?

Yeah, that could be caused by two reason: GnuPG is not designed to run in batch/multiple threads and it seem to fail occasionally in tests. Also there is issue with encryption to multiple password which in some cases not well handled by GnuPG (cannot find now, but we should have issue for that).

Also codecov complains about some code that is not tested. Are we required to provide full test coverage for new code? It's not always trivial. For example, one failure case I added tests whether the length of a key is longer than what can be encoded in 4 length octets, i.e., 4 gigabyte.

Codecov requires the patch coverage to be not smaller than overall project coverage, and sometimes that get tricky. In some cases which should never happen, like checks for memory failures, you may use LCOV_EXCL_LINE or LCOV_EXCL_START/LCOV_EXCL_END to disable check for certain code lines.

[TJ-91]

So what would you suggest how can I best satisfy codecov here? I agree with the general idea to ideally test every line but for most of the changes it'll be a bit tricky and I suppose a typical test case would fail somewhere earlier already.

Let's look for example at the codecov finding src/librepgp/stream-sig.cpp#L87-L89. I test for key version, and for this I use a switch-case statement. The default case will abort with some error message. If the switch-case statement is deep enough in the library, some earlier switch-case statement's default case will be triggered before I get to this code. For example, I can't see how a version "123" certificate would be getting that deep into the library code.

In summary, I'm not sure if it's reasonable to make the effort here and some more sophisticated testing utility framework to help write the specific test cases might be needed if that level of testing is desired.

Would you be fine with just using LCOV_EXCL_LINE for all reported lines?

[ni4]

@TJ-91 Please see the review comments. Let's just keep things simpler :)

[TJ-91]

@ni4 I hope now it's simple enough :) I think I need to restart tests for the macos test (fails cli_tests-Encryption) but I'm not sure for the codecov test again. Do I need to wait on your PRs here as well?

[ni4]

@TJ-91 PRs which fix CI are finally merged, so could you please rebase this on main to get it merged? Thanks!

[TJ-91]

@ni4 Passes all checks now, thanks!

[ni4]

Merging with two approvals. Thanks all!