Repository navigation
fix: fix the order of arguments to merge_openpgp_certificates() call - #8847
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
15f5e1f to
7af89b8
Compare
7af89b8 to
aaa7e3c
Compare
| /// Tests that Direct Key Signature from the new certificate is preferred | ||
| /// when the timestamp is the same. | ||
| #[test] | ||
| fn test_merge_openpgp_certificates_tiebreak() -> Result<()> { |
There was a problem hiding this comment.
this test also passes on main, so it's not about what this PR changes which is what i expected it would do. Wouldn't it be a more interesting test to import two same-second signed certs via import_public_key and see that the second one wins?
There was a problem hiding this comment.
Added similar test_import_public_key_tiebreak, it is failing without the fix.
| }); | ||
| let users: Vec<SignedUser> = best_user.into_iter().collect(); | ||
|
|
||
| let public_subkeys = old_public_subkeys; |
There was a problem hiding this comment.
this unconditionally takes the keys form the stored cert instead of the received one, without any same-second timestamp comparison. This probably only matters with Autocrypt2 because our ac1 keys never change.
There was a problem hiding this comment.
Merging of subkeys is implemented #8344, but it is currently a draft and has no tests.
hpk42
left a comment
hpk42
left a comment
There was a problem hiding this comment.
i think it's ok to merge. ac1 sub keys don't change but for ac2 we will need to revisit this merging, and probably have more refined logic for merging incoming/stored subkeys.
It is not obvious that relay list is comma-separated.
…ts() pk_encrypt() already uses current timestamp to select the key for encryption, no need to use a potentially different timestamp to sign the message.
This tests that the order of arguments to merge_openpgp_certificates() is important if signature timestamp is the same.
This fixes flaky tests/test_securejoin.py::test_qr_scan_updates_new_relay_address which hanged during the second securejoin on fast network (e.g. when testing against local container) as Bob received securejoin pubkey message from Alice but did not merge the new relay list.
aaa7e3c to
d755d69
Compare
This fixes flaky tests/test_securejoin.py::test_qr_scan_updates_new_relay_address which hanged during the second securejoin on fast network (e.g. when testing against local container) as Bob received securejoin pubkey message from Alice but did not merge the new relay list.