Browse Source

ENGINE-828: removed username from matching heuristics in update_identity

IPS-2
Krista Bennett 11 months ago
parent
commit
efde49fb78
4 changed files with 21 additions and 82 deletions
  1. +10
    -21
      src/keymanagement.c
  2. +3
    -4
      test/src/DefaultFromEmailTest.cc
  3. +8
    -7
      test/src/UserIdCollisionTest.cc
  4. +0
    -50
      test/test_mails/CanonicalFrom2.2BobToAliceOpenPGP_NoKey.eml

+ 10
- 21
src/keymanagement.c View File

@ -783,8 +783,6 @@ DYNAMIC_API PEP_STATUS update_identity(
bool input_has_user_id = !EMPTYSTR(identity->user_id);
bool input_has_username = !EMPTYSTR(identity->username);
bool input_has_real_id = input_has_user_id ? (strstr(identity->user_id, "TOFU_") != identity->user_id) : false;
bool input_name_is_addr = input_has_username ? strcmp(identity->username, identity->address) == 0 : false;
bool weak_input_name = input_name_is_addr || !input_has_username;
char* default_own_id = NULL;
pEp_identity* stored_ident = NULL;
@ -871,11 +869,10 @@ DYNAMIC_API PEP_STATUS update_identity(
// following properties (not in order of priority, and not for every case - the logic here is a mess):
//
// 1. Did the input have a user_id?
// 2. Did the input hava a username?
// 2. Did the input have a username? N.B. This is, as of ENGINE-828, less important than it was.
// 3. Is the input user_id a real id?
// 4. Is the stored user_id a real id?
// 5. Does the stored user_id have a username?
// 6. Do the names match?
//
// Based on this, if we find an acceptable candidate, we do one:
//
@ -883,8 +880,7 @@ DYNAMIC_API PEP_STATUS update_identity(
// (this may be different than 1, though in practice it seems we always do both)
// 2. Patch the output identity's user_id from the stored identity
//
// If we find none, in the case if the has-username-but-no-user_id input case, we'll try a TOFU id
// fetch before giving up on stored identity candidates.
// If we find none, we'll try a TOFU id fetch before giving up on stored identity candidates.
//
// Acceptable candidates are then passed to prepare_update_identity which will patch usernames and
// find any applicable keys.
@ -916,13 +912,6 @@ DYNAMIC_API PEP_STATUS update_identity(
bool candidate_has_username = !EMPTYSTR(candidate->username);
bool candidate_name_is_addr = candidate_has_username ? strcmp(candidate->address, candidate->username) == 0 : false;
// No longer necessary, as we don't compare usernames anymore
// bool weak_candidate_name = !candidate_has_username || candidate_name_is_addr;
//
// bool names_match = (weak_candidate_name && weak_input_name) ||
// ((input_has_username && candidate_has_username) &&
// (strcmp(identity->username, candidate->username) == 0));
// This is where the optimisation gets a little weird:
//
// Decide whether to accept and patch the database and stored id from the input,
@ -983,14 +972,14 @@ DYNAMIC_API PEP_STATUS update_identity(
snprintf(identity->user_id, strlen(identity->address) + 6,
"TOFU_%s", identity->address);
// Try one last time to see if there is an ident for us with a TOFU id, if there was no ID but there
// was a usernames
if (input_has_username) {
status = get_identity(session,
identity->address,
identity->user_id,
&stored_ident);
}
// Try one last time to see if there is an ident for us with a TOFU id
//
// We no longer use the username as a qualifying condition.
//
status = get_identity(session,
identity->address,
identity->user_id,
&stored_ident);
}
}


+ 3
- 4
test/src/DefaultFromEmailTest.cc View File

@ -834,10 +834,9 @@ TEST_F(DefaultFromEmailTest, check_pEp_v2_2_import_two_alternate_available) {
///////////////////////////////////////////////////////////////
// Identity Key:
// Bob: known pEp partner
// Carol: known OpenPGP partner
// Sylvia: unknown pEp partner
// John: unknown OpenPGP partner
// Bob: known partner
// Sylvia: unknown partner
//
// Case 1: Partner didn't have our key


+ 8
- 7
test/src/UserIdCollisionTest.cc View File

@ -264,7 +264,11 @@ TEST_F(UserIdCollisionTest, simple_tofu_collision) {
// Create TOFU identity, set its FPR in the DB, test real id collision
// with different usernames. Real ID shouldn't pick up the TOFU information
// OR its key, since key election has been removed.
//
//
// This should no longer fail, because we don't use username matches to determine
// what happens in update_identity. So we SHOULD replace the user_id and consolidate
// identities.
//
TEST_F(UserIdCollisionTest, simple_tofu_collision_different_usernames) {
slurp_and_import_key(session,alex_keyfile);
tofu_alex->username = strdup("Alexander Hamilton");
@ -279,19 +283,16 @@ TEST_F(UserIdCollisionTest, simple_tofu_collision_different_usernames) {
ASSERT_OK;
ASSERT_STREQ(tofu_alex->fpr, alex_keyid);
// Ok, so we had a TOFU id with a real username. It's different from THIS
// username. As such, we shouldn't find a key here, because things won't
// be merged. // FIXME: This should change, probably. Discuss with Volker.
status = update_identity(session, real_alex);
ASSERT_OK;
ASSERT_NULL(real_alex->fpr);
ASSERT_NOTNULL(real_alex->fpr);
ASSERT_STREQ(real_alex->fpr, alex_keyid);
bool tofu_still_exists = false;
status = exists_person(session, tofu_alex, &tofu_still_exists);
ASSERT_OK;
// SHOULD still exist, because we don't replace when usernames differ
ASSERT_TRUE(tofu_still_exists);
ASSERT_FALSE(tofu_still_exists);
}
//


+ 0
- 50
test/test_mails/CanonicalFrom2.2BobToAliceOpenPGP_NoKey.eml View File

@ -1,50 +0,0 @@
From: Bob Dog <pep.test.bob@pep-project.org>
To: Alice Spivak Hyatt <pep.test.alice@pep-project.org>
Subject: =?utf-8?Q?p=E2=89=A1p?=
X-pEp-Version: 2.2
MIME-Version: 1.0
Content-Type: multipart/encrypted; boundary="1ab1a98f4371002878f6ebd414063add";
protocol="application/pgp-encrypted"
--1ab1a98f4371002878f6ebd414063add
Content-Type: application/pgp-encrypted
Version: 1
--1ab1a98f4371002878f6ebd414063add
Content-Type: application/octet-stream
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="msg.asc"
-----BEGIN PGP MESSAGE-----
wcBMA4rbkTfsG51OAQf/TKQhJ1ediKZeBfX7h6vkl1eyUdDIOWWfKojHStzzvIvp
EZ3AQ9PV5nO0nDtZZA2QwRBRk5Fuzc15A7+Fv31Q4iU5KWoh0u4jHwMb8Cp/UMAB
IEwBLxWK6vcs4DEynBAOkXOs38mwEYVkVy2zw3axvjpfBMqCukw+Q7s6ahca7VIU
dBvb/DbmDojOzJJ+lR9Z74zzMCDrjvu4iNIfA0wxMwHk7OG3A26g2Bub/l2VWB3o
9l5NeuynkrWcXq/G3YS/1nMiZxQmw7oolY9qItZ6IPvE/6qqHeydCdYvcam2ViSH
FBKlmDug4pJ43gF5EK/b8D13N2o1E7T/jD7e2w19FcHATANaAgXZcwg/TQEH/RGk
HXM8uoPQlJFtt1yA+snC3m7K+qyABjvzeT25IdpgyvPOHPPA9oVp1Hv2vtzE+JI5
qwo2U6WtyTGxysIbg33GDGCjph6Ix3YzMvxRjeF944YBt4GP0JtTbWVcOcJ6rS1/
4UHEwNdO69RElOVNdqM4s2oqqJv8yXdl9exR8tAaZIqVU3O9GWcXmqLtdlnA7Ody
5cQUI4+vtewlofsqWP2c7DolixLZgVl0eJLbkBHcG0rHqk7paZXietDETr9dC2Zn
0VSEwgaP5HC/M8S6/oEGz5erhHVblff1qxTj8DhAcHuYZyXX7SR4bddZj38ronjL
0zwS7zeceS3AAqYtcJzSwiEBd51eCRejJd7IkDzVstYDH4u4LwOKMJGPofV2WBCV
1xAJ3XogN2hu0HrkOLBtAaCzKo/9SPa30tkgFLNyLskoHu6Ryk6H1vw3fkbIjvgF
OG2bDMhvzoMg/Fe+Y09C3FvNQ6772gVSgLx3tF7Qx6CIyMFt6vkUjvoCMpyuFdq+
rbqB4qf+woI4tDPoXq5dCZncEQheX4KteTp77L+Dktf8cib7wRU7+NRlhYb6eeNe
AbdbX/FX9y3Sn0MN1E6yqpTrxkXMb22r+R//3vGVlLQCtYS28bzRh+s0tfbCXjzX
lLusXjGjaw6LIZcY7JMJ9vItSAoblYCj0YvJSbLZUVdDoqHmSJSXdlB9fEdSPS2a
KaHtSiOWxKit7wr6Uk6MMGujKKNkiPtEAuo+LhiUya/D/wC25ln13oBGMCPW7aEr
ywx+Hrk7RlpXBM2HLgoPgSKBDdyP4a3JUssGttErme3ro4f2Vofo9ZSJ6IuoVfVz
OUjcoAlDv0zzLycL1jMTY4DncVgFS1o2lVIRajK+XCcDw/fmgNgZ5bBuWMrylJGx
NChl6ilQFcdPTh38f2mCJAg600uxFC2psVPXLlYYN90q09OlfrIVY6R4WIyHePHY
EvKtRB4hYs2j+gGkgch/+oZ3nXG/NfrqmK8gELAsLX7/hY+lhJ3u21t//GLAHQyZ
+5A1guJI+zYj27+S2VyfDNA93Lwvax22KNvHaBaEjqgqGl3kMiy2OklHhl9TX+xR
MupGqBf/CQcRDpLZru40VbPYhQvGRcXvHMfZkrSkeLM4WJ8ko5KfTCuIQRjEZt/9
3QECCnewnbyK5OE82wlMr0M23J5BT8/M7ofs8f1sLP0W2VuHs6A8DZWKwjjA23C5
cOksznRgQ+gsylL6NeqajUZ4juzuMWz88uH55Kign7dEov0HaDXDFGOjgJOXV4oD
MJCB7hBAzJMeZTWdCgUquS5S283tir9hT3XjD6S5eN7p4w==
=mEEA
-----END PGP MESSAGE-----
--1ab1a98f4371002878f6ebd414063add--

Loading…
Cancel
Save