FIx some PQC issues.

[ni4]

Among other minor issues this PR adds RNP_EXPERIMENTAL_* defines to the rnp_export.h header, allowing to cut-off by default yet experimental features. Probably better solution would be to remove those function calls from the header at all, however don't see good solution for that at the moment.

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (05f59e9) 76.88% compared to head (977cf19) 76.89%.
Report is 1 commits behind head on main.

❗ Current head 977cf19 differs from pull request most recent head 3824009. Consider uploading reports for the commit 3824009 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2131      +/-   ##
==========================================
+ Coverage   76.88%   76.89%   +0.01%     
==========================================
  Files         193      193              
  Lines       36982    36976       -6     
==========================================
  Hits        28432    28432              
+ Misses       8550     8544       -6     

Files	Coverage Δ
src/lib/rnp.cpp	80.20% <ø> (+0.10%)	⬆️
src/librepgp/stream-parse.cpp	77.60% <0.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ni4]

@falko-strenzke what do you think about this solution on hiding PQC/v6 calls from the FFI API?

[falko-strenzke]

First of allo sorry for the delayed relpy.
As I understand it, this should force the application that uses RNP to also set the define RNP_EXPERIMENTAL_CRYPTO_REFRESH (or RNP_EXPERIMENTAL_PQC) in its own code. From our side this is fine.

[ni4]

@falko-strenzke Thanks for the reply. It's actually simpler, CMake will set those variables automatically and include into the rnp_export.h header, via this patch: https://github.com/rnpgp/rnp/pull/2131/files#diff-7567fde99fcba090ecb1958dd4ab9ba4367c10ba5d444e580cb61c3b404ced3a

[ni4]

@maxirmx @ribose-jeffreylau Could you please review this? Thanks!

[maxirmx]

(This is a suggestion to consider)

IMHO it will be much easier to support a single flags
Something like this:

#include <iostream>

//   Defintion of CRYPTO_REFRESH options
#define ENABLE_CRYPTO_REFRESH 0
#define ENABLE_EXPERIMENTAL_CRYPTO_REFRESH 1
#define DISABLE_CRYPTO_REFRESH  2

//   Selection of CRYPTO_REFRESH option
#define RNP_CRYPTO_REFRESH RNP_CRYPTO_REFRESH


int main(int, char**)
{
#if RNP_CRYPTO_REFRESH == ENABLE_CRYPTO_REFRESH
    std::cout << "ENABLE_CRYPTO_REFRESH" << std::endl;
#elif RNP_CRYPTO_REFRESH == ENABLE_EXPERIMENTAL_CRYPTO_REFRESH
    std::cout << "ENABLE_EXPERIMENTAL_CRYPTO_REFRESH" << std::endl;
#else
    std::cout << "DISABLE_CRYPTO_REFRESH (" << RNP_CRYPTO_REFRESH << ")" << std::endl;
#endif
return 0;
}

[ni4]

@maxirmx Thanks for noticing this, but it's suggested to work in a bit different way: both defines are for the same purpose, and this check is to make sure that they are in sync. The problem here is that one goes to the config.h.in, which is not installed and used only during compilation, and second is needed to be installed and used by another installed header rnp.h. This is needed to hide some function declarations from the rnp.h. So it's not the best solution, but I didn't find any better idea. Maybe you have some suggestion?

[maxirmx]

If it is a check that both defines are in sync I guess it shall be

#if defined(RNP_EXPERIMENTAL_CRYPTO_REFRESH) != defined(ENABLE_CRYPTO_REFRESH)

[ni4]

@maxirmx Indeed, thanks! Re-pushed.

[maxirmx]

(This is a suggestion to consider)
Pls refer to the note about CRYPTO_REFRESH flags

[ribose-jeffreylau]

Not a C expert so nothing to add...

[ni4]

Merging as it has 2 approvals and green CI.