pqc-thunderbird / rnp

Manual clone of the repository https://github.com/rnpgp/rnp
Other
0 stars 0 forks source link

PKESK and SEIPD logic: parameter object's type should become ternary #7

Closed falko-strenzke closed 1 year ago

falko-strenzke commented 1 year ago

Currently, various booleans are used in the code to indictate which packet type is processed.

for instance in stream-write.cpp: pgp_dest_encrypted_param_t

bool                       has_mdc; /* encrypted with mdc, i.e. tag 18 */
bool                       aead;    /* we use AEAD encryption */
bool    is_v2_seipd = false;

These should be united in single enum: mdc, aead and v2seipd

This needs to be addressed analoguously for encryption.

falko-strenzke commented 1 year ago

stream-write.cpp:

in the code pgp_source_encrypted_param_t->aead is used to check for whether to use AEAD packets.

pgp_source_encrypted_param_t should receive the functions:

Once the whole code has been refactored independently by the RNP maintainers the boolean parameters can be replaced by a ternary enum. For the PQC integration we would for now only use the functions in our new code.

stream-read-cpp:

Here we will take the same approach as for stream-parse.cpp outlined above, as the use of the fields is identical.

ni4 commented 1 year ago

Sure, this should be refactored to some expandable enum (which could be expandable even further, having two variants of AEAD encrypted packet). I'll do a refactoring PR soon.

ni4 commented 1 year ago

PR with proposed changes: https://github.com/rnpgp/rnp/pull/2014 Does it look good to you? To adopt seipd_v2 it should be enough to add new value to rnp::AuthType, updating places where it's value is checked.

falko-strenzke commented 1 year ago

This PR looks good to me. Will you adapt it into the main branch? Then we can merge it into our code.

ni4 commented 1 year ago

Thanks. Yeah, it will be merged to the main branch once PR got approved by other team members. I could recommend to do periodical rebases on the main to have less possible merge conflicts afterwards.

ni4 commented 1 year ago

JFYI: PR is already merged.

falko-strenzke commented 1 year ago

:+1: great, we're integrating it