Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

joining verified group via QR not working properly if contact is not already verified #4600

Closed
adbenitez opened this issue Aug 3, 2023 · 2 comments · Fixed by #4622
Closed
Assignees
Labels
bug Something is not working

Comments

@adbenitez
Copy link
Member

testing around I discovered this bug, info message shown as normal message (even if I got added correctly)
steps to reproduce:

  1. create new account B
  2. create new verified group in account B
  3. scan the QR with your other accout A, that should have not B verified already, that is: the group-join-QR will be the first time you also get verified with that contact

image

@adbenitez adbenitez added the bug Something is not working label Aug 3, 2023
@adbenitez adbenitez changed the title group join via QR not working properly if contact is not already verified joining verified group via QR not working properly if contact is not already verified Aug 3, 2023
@iequidoo
Copy link
Collaborator

Can be reproduced by the Python tests with this patch:

--- a/python/tests/test_0_complex_or_slow.py                                                                                                                                                                         
+++ b/python/tests/test_0_complex_or_slow.py                                                                                                                                                                         
@@ -136,6 +136,7 @@ def test_qr_verified_group_and_chatting(acfactory, lp):                                                                                                                                          
     lp.sec("ac2: read member added message")                                                                                                                                                                        
     msg = ac2._evtracker.wait_next_incoming_message()                                                                                                                                                               
     assert msg.is_encrypted()                                                                                                                                                                                       
+    assert msg.is_system_message()                                                                                                                                                                                  
     assert "added" in msg.text.lower()                                                                                                                                                                              
                                                                                                                                                                                                                     
     lp.sec("ac1: send message")

@iequidoo iequidoo self-assigned this Aug 15, 2023
@iequidoo
Copy link
Collaborator

iequidoo commented Aug 15, 2023

50.90 [events-ac2] INFO src/mimeparser.rs:300: decrypted message mime-body:
Content-Type: text/plain; charset=utf-8; format=flowed; delsp=no;                                                                                                                                                    
        protected-headers="v1";                                                                                                                                                                                      
Subject: hello
Chat-Verified: 1
Chat-Group-ID: eIIBkMWNYqg
Chat-Group-Name: =?utf-8?q?hello?=
Chat-Group-Member-Added: 2cyz8@ci.testrun.org
Secure-Join: vg-member-added
Secure-Join-Fingerprint: CCCB5AA9F6E1141C943165F1DB18B18CBCF70487
From: <ecj2q@ci.testrun.org>

I added member 2cyz8@ci.testrun.org.

50.90 [events-ac2] WARNING src/mimeparser.rs:813: Ignoring nested protected headers
50.90 [events-ac2] INFO src/receive_imf.rs:142: Received message has Message-Id: Gr.eIIBkMWNYqg.GmQhYmknOMi@ci.testrun.org
50.90 [events-ac2] INFO src/securejoin.rs:290: >>>>>>>>>>>>>>>>>>>>>>>>> secure-join message 'vg-member-added' received
50.90 [events-ac2] INFO src/securejoin/bobstate.rs:334: Bob Step 7 - handling vc-contact-confirm/vg-member-added message
50.90 [events-ac2] DC_EVENT_CONTACTS_CHANGED data1=0 data2=0
50.90 [events-ac2] INFO src/ephemeral.rs:567: Ephemeral loop waiting for deletion in 24h 0m 0s or interrupt
50.90 [events-ac2] INFO src/mimefactory.rs:1049: sending secure-join message 'vg-member-added-received' >>>>>>>>>>>>>>>>>>>>>>>>>
50.90 [events-ac2] INFO src/e2ee.rs:65: peerstate for "ecj2q@ci.testrun.org" is mutual
50.90 [events-ac2] DC_EVENT_MSGS_CHANGED data1=12 data2=17
50.91 [events-ac2] INFO src/scheduler.rs:714: smtp fake idle - interrupted
50.91 [events-ac2] INFO src/smtp.rs:656: Selected rows from SMTP queue: [3].
50.91 [events-ac2] INFO src/smtp.rs:551: Try number 1 to send message Msg#17 (entry 3) over SMTP
50.91 [events-ac2] DC_EVENT_CONNECTIVITY_CHANGED data1=0 data2=0
50.91 [events-ac2] DC_EVENT_MSGS_CHANGED data1=13 data2=18
50.91 [events-ac2] DC_EVENT_CHAT_MODIFIED data1=13 data2=0
50.91 [events-ac2] INFO src/chat.rs:1078: Set gossiped_timestamp for chat Chat#13 to 0.
50.91 [events-ac2] DC_EVENT_MSGS_CHANGED data1=13 data2=19
50.91 [events-ac2] DC_EVENT_CHAT_MODIFIED data1=13 data2=0
50.91 [events-ac2] WARNING src/receive_imf.rs:1571: Verification problem: The message was sent with non-verified encryption.
50.91 [events-ac2] INFO src/receive_imf.rs:1848: Recreating chat Chat#13 with members {ContactId(10), ContactId(1)}.
50.91 [events-ac2] DC_EVENT_CHAT_MODIFIED data1=13 data2=0
50.91 [events-ac2] WARNING src/receive_imf.rs:1041: Verification problem: The message was sent with non-verified encryption.
50.91 [events-ac2] INFO src/receive_imf.rs:1284: Message has 1 parts and is assigned to chat #Chat#13.
50.91 [events-ac2] INFO src/contact.rs:1632: Recently seen loop waiting for 0h 9m 51s or interrupt
50.91 [events-ac2] DC_EVENT_INCOMING_MSG data1=13 data2=20
50.91 [events-ac2] INFO src/imap.rs:1565: Successfully received 1 UIDs.
50.91 [events-ac2] INFO src/imap.rs:886: 1 mails read from "INBOX".is_info: cmd=Unknown, text='Member Me (2cyz8@ci.testrun.org) added by ecj2q@ci.testrun.org.'

50.91 [events-ac2] DC_EVENT_INCOMING_MSG_BUNCH data1=0 data2=0
-------------------------------------------------------------------------------------------- Captured stdout teardown --------------------------------------------------------------------------------------------

So, the problem is related to these messages (there are two of them):

Verification problem: The message was sent with non-verified encryption.

mime_parser.repl_msg_by_error() is called then which resets self.is_system_message from MemberAddedToGroup to Unknown. Then it is saved to the db as Param::Cmd. And then Message::is_info() called on such a message returns false:

    pub fn is_info(&self) -> bool {
        let cmd = self.param.get_cmd();
        self.from_id == ContactId::INFO                                                                                                                                                                              
            || self.to_id == ContactId::INFO
            || cmd != SystemMessage::Unknown && cmd != SystemMessage::AutocryptSetupMessage
    }

As i can see, the bug is introduced by 9cd000c "feat: Verified 1:1 chats (#4315)". Right now i have no idea how to rewrite the code properly, i'm not very familiar with it. @Hocuri

EDIT: Found a simple way to fix this. The reason is that mime_parser.decryption_info.peerstate is not updated when handling a Securejoin handshake, it's updated only in the db.

@iequidoo iequidoo assigned iequidoo and unassigned iequidoo Aug 15, 2023
iequidoo added a commit that referenced this issue Aug 16, 2023
…handshake (#4600)

Otherwise has_verified_encryption() would check a stale Peerstate object.
link2xt pushed a commit that referenced this issue Aug 24, 2023
…handshake (#4600)

Otherwise has_verified_encryption() would check a stale Peerstate object.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working
Projects
None yet
2 participants