This change depends on async-imap update that resets the timeout
every time an `* OK Still here` is received.
Reducing timeout allows to detect lost connections
not later than 6 minutes
because Delta Chat will attempt to finish IDLE with DONE
after 5 minutes without keepalives
and will either get TCP RST directly
or, worst case, wait another minute for TCP socket read timeout.
Ad-hoc groups don't have grpid-s that can be used to identify them across devices and thus wasn't
synced until now.
The same problem already exists for assigning messages to ad-hoc groups and this assignment is done
by `get_parent_message()` and `lookup_chat_by_reply()`. Let's reuse this logic for the
synchronisation, it works well enough and this way we have less surprises than if we try to
implement grpids for ad-hoc groups. I.e. add an `Msgids` variant to `chat::SyncId` analogous to the
"References" header in messages and put two following Message-IDs to a sync message:
- The latest message A having `DownloadState::Done` and the state to be one of `InFresh, InNoticed,
InSeen, OutDelivered, OutMdnRcvd`.
- The message that A references in `In-Reply-To`.
This way the logic is almost the same to what we have in `Chat::prepare_msg_raw()` (the difference
is that we don't use the oldest Message-ID) and it's easier to reuse the existing code.
NOTE: If a chat has only an OutPending message f.e., the synchronisation wouldn't work, but trying
to work in such a corner case has no significant value and isn't worth complicating the code.
- Use TestContextManager
- Actually run receive_imf rather than only mimeparser on "received" messages
- Check that received message parts actually have a padlock
parse_mime_recursive() skips empty text parts,
so there may be no parts as the result of parsing.
In this case an empty part is added.
However, because it is added with parts.push()
rather than add_single_part(),
it is added without a padlock even if the message is encrypted.
`do_add_single_part()` adds padlock (GuaranteeE2EE param)
and should be used to add parts instead.
This test reproduces a bug preventing joining the group with a QR code
if the group already has a contact with inconsistent key state,
which means both Autocrypt and verified key exist,
but don't match.
This can happen when an invite QR code created by this contact
is scanned as scanning an invite code creates unprotected group
with the inviter for info messages.
If securejoin process never finishes because the inviter is offline,
group remains in this unprotected state with added inviter.
Normally the group becomes verified when a "Member added" (vg-member-added)
message is received in the chat.
However, current code checks that all members
of the chat are verified
(have a green checkmark, use verified key in 1:1 chat)
before marking the group as verified and fails otherwise.
- Remove "Detected Autocrypt-mime message" logs printed for every incoming Autocrypt message.
- Print only a single line at the beginning of receive_imf with both the Message-ID and seen flag.
- Print Securejoin step only once, inside handle_securejoin_handshake or observe_securejoin_on_other_device.
- Do not log "Not creating ad-hoc group" every time ad-hoc group is not created, log when it is created instead.
- Log ID of the chat where Autocrypt-Gossip for all members is received.
- Do not print "Secure-join requested." for {vg,vc}-request, we already log the step.
- Remove ">>>>>>>>>>>>>>>>>>>>>>>>>" noise from securejoin logs.
Otherwise it looks like the message creating a protected group is not verified. For this, use
`sent_timestamp` of the received message as an upper limit of the sort timestamp (`msgs.timestamp`)
of the protection message. As the protection message is added to the chat earlier, this way its
timestamp is always less or eq than the received message's timestamp.
Test that on the second device of a protected group creator the first message is
`SystemMessage::ChatProtectionEnabled` and the second one is the message populating the group.
Allowing outgoing unencrypted messages in groups with 2 members
breaks the test
`python/tests/test_0_complex_or_slow.py::test_verified_group_vs_delete_server_after`
This centralizes all Securejoin/verification checks and updates in one
place right before add_parts() even before we assign the message to
the chat, so we can decouple chat logic from verification logic.
It's not necessary and in other places like add_contact_to_chat_ex() sync messages are also sent
only if there are no system messages sent like MemberAddedToGroup.
We already synchronise status/footer when we see a self-sent message with a Chat-Version
header. Would be nice to do the same for display name.
But let's do it the same way as for `Config::{MdnsEnabled,ShowEmails}`. Otherwise, if we sync the
display name using the "From" header, smth like `Param::StatusTimestamp` is needed then to reject
outdated display names. Also this timestamp needs to be updated when `Config::Displayname` is set
locally. Also this wouldn't work if system time isn't synchronised on devices. Also using multiple
approaches to sync different config values would lead to more code and bugs while having almost no
value -- using "From" only saves some bytes and allows to sync some things w/o the synchronisation
itself to be enabled. But the latter also can be a downside -- if it's usual synchonisation, you can
(potentially) disable it and share the same email account across people in some organisation
allowing them to have different display names. With using "From" for synchronisation such a
capability definitely requires a new config option.
Motivation: Syncing these options will improve UX in very most cases and should be done. Other
candidates are less clear or are advanced options, we can reconsider that at some point later.
Approach:
- Sync options one-by-one when the corresponding option is set (even if to the same value).
- Don't sync when an option is reset to a default as defaults may differ across client versions.
- Check on both sides that the option should be synced so that if there are different client
versions, the synchronisation of the option is either done or not done in both directions.
Moreover, receivers of a config value need to check if a key can be synced because some settings
(e.g. Avatar path) could otherwise lead to exfiltration of files from a receiver's device if we
assume an attacker to have control of a device in a multi-device setting or if multiple users are
sharing an account.
- Don't sync `SyncMsgs` itself.
If a message is encrypted, but unsigned:
- Don't set `MimeMessage::from_is_signed`.
- Remove "secure-join-fingerprint" and "chat-verified" headers from `MimeMessage`.
- Minor: Preserve "Subject" from the unencrypted top level if there's no "Subject" in the encrypted
part, this message is displayed w/o a padlock anyway.
Apparently it didn't lead to any vulnerabilities because there are checks for
`MimeMessage::signatures.is_empty()` in all necessary places, but still the code looked dangerous,
especially because `from_is_singed` var name didn't correspond to its actual value (it was rather
`from_is_encrypted_maybe_signed`).
Currently this leads to
DEBUG root:rpc.py:136 account_id=1 got an event {'kind': 'Warning', 'msg': 'src/scheduler.rs:711: send_smtp_messages failed: failed to send message: failed to update retries count: database is locked: Error code 5: The database file is locked'}
and
FAILED tests/test_webxdc.py::test_webxdc_insert_lots_of_updates - deltachat_rpc_client.rpc.JsonRpcError: {'code': -1, 'message': 'database is locked\n\nCaused by:\n Error code 5: The database file is locked'}
Without this change
when the test returns a `Result`, `cargo test` does not show
the line number.
To make the tests as easy to debug as the panicking tests,
enable `backtrace` feature on `anyhow` and add debug information
to add source line numbers to backtraces.
Now running `RUST_BACKTRACE=1 cargo test` produces backtraces
with the line numbers. For example:
Error: near ",": syntax error in SELECT COUNT(,,*) FROM msgs_status_updates; at offset 13
Caused by:
Error code 1: SQL error or missing database
Stack backtrace:
0: <core::result::Result<T,F> as core::ops::try_trait::FromResidual<core::result::Result<core::convert::Infallible,E>>>::from_residual
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/result.rs:1962:27
1: deltachat::sql::Sql::query_row::{{closure}}::{{closure}}
at ./src/sql.rs:466:23
2: deltachat::sql::Sql::call::{{closure}}::{{closure}}
at ./src/sql.rs:379:55
3: tokio::runtime::context::runtime_mt::exit_runtime
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/runtime/context/runtime_mt.rs:35:5
4: tokio::runtime::scheduler::multi_thread::worker::block_in_place
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/runtime/scheduler/multi_thread/worker.rs:438:9
5: tokio::runtime::scheduler::block_in_place::block_in_place
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/runtime/scheduler/block_in_place.rs:20:5
6: tokio::task::blocking::block_in_place
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/task/blocking.rs:78:9
7: deltachat::sql::Sql::call::{{closure}}
at ./src/sql.rs:379:19
8: deltachat::sql::Sql::query_row::{{closure}}
at ./src/sql.rs:469:10
9: deltachat::sql::Sql::count::{{closure}}
at ./src/sql.rs:443:76
10: deltachat::webxdc::tests::change_logging_webxdc::{{closure}}
at ./src/webxdc.rs:2644:18
11: <core::pin::Pin<P> as core::future::future::Future>::poll
at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/future/future.rs:125:9
12: tokio::runtime::park::CachedParkThread::block_on::{{closure}}
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/runtime/park.rs:282:63
13: tokio::runtime::coop::with_budget
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/runtime/coop.rs:107:5
tokio::runtime::coop::budget
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/runtime/coop.rs:73:5
tokio::runtime::park::CachedParkThread::block_on
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/runtime/park.rs:282:31
14: tokio::runtime::context::blocking::BlockingRegionGuard::block_on
at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.34.0/src/runtime/context/blocking.rs:66:9
...
Line 10 of the backtrace contains the line number in the test (2644).
Limit the number of IMAP connections to 1 per minute regardless of the reason of reconnection, but
allow one immediate retry. This is more reliable than ratelimiting only in error conditions because
ratelimiting can't be skipped by mistake. Anyway connections shouldn't be frequent in normal
operation mode.
