Don't depend on these 3 cleartext headers for the question whether we
download a message.
This PR will waste a bit of bandwidth for people who use the legacy
show_emails option; apart from that, there is no user-visible change
yet. It's a preparation for being able to remove these headers, in order
to further reduce unencrypted metadata.
Removing In-Reply-To and References will be easy; removing Chat-Version
must happen at least one release after the PR here is released, so that
people don't miss messages. Also, maybe some nerds depend on the
Chat-Version header for server-side filtering of messages, but we shall
have this discussion at some other time.
For the question whether a message should be moved, we do still depend
on them; this will be fixed with
https://github.com/chatmail/core/pull/7780.
When both this PR and #7780 are merged, we can stop requesting
Chat-Version header during prefetch.
I assume that the problem was that sometimes, alice2 or fiona doesn't
accept alice's smeared timestamp, because `calc_sort_timestamp()`
doesn't allow the timestamp of a received message to be in the future. I
tried this patch:
```diff
diff --cc src/chat.rs
index 9565437cf,9565437cf..a2e4f97d0
--- a/src/chat.rs
+++ b/src/chat.rs
@@@ -46,6 -46,6 +46,7 @@@ use crate::receive_imf::ReceivedMsg
use crate::smtp::{self, send_msg_to_smtp};
use crate::stock_str;
use crate::sync::{self, Sync::*, SyncData};
++use crate::timesmearing::MAX_SECONDS_TO_LEND_FROM_FUTURE;
use crate::tools::{
IsNoneOrEmpty, SystemTime, buf_compress, create_broadcast_secret, create_id,
create_outgoing_rfc724_mid, create_smeared_timestamp, create_smeared_timestamps, get_abs_path,
@@@ -1212,7 -1212,7 +1213,11 @@@ SELECT id, rfc724_mid, pre_rfc724_mid,
received: bool,
incoming: bool,
) -> Result<i64> {
-- let mut sort_timestamp = cmp::min(message_timestamp, smeared_time(context));
++ let mut sort_timestamp = cmp::min(
++ message_timestamp,
++ // Add MAX_SECONDS_TO_LEND_FROM_FUTURE in order to allow other senders to do timesmearing, too:
++ smeared_time(context) + MAX_SECONDS_TO_LEND_FROM_FUTURE,
++ );
let last_msg_time: Option<i64> = if always_sort_to_bottom {
// get newest message for this chat
```
...maybe this patch makes sense anyways, but you still get the problem
that the message sent by alice2 (i.e. the add-fiona message) will have
an earlier timestamp than the message sent by alice, because alice
already sent more messages, and therefore has more timesmearing-seconds.
It's unsure it makes sense to modify calc_sort_timestamp() this way because if some chat member has the clock in the future (even unintentionally), their fresh messages will be sorted to the bottom relatively to others' fresh messages. Maybe it's even better to limit the message timestamp ("Date") by the current system time there.
To really fix the problem, we could send a serial number together with the timestamp, that distinguishes two messages sent in the same second. But since we haven't gotten complaints about message ordering since some time, let's just leave things as they are.
Since all this timesmearing is a bit best-effort right now, I decided to
instead just make the test more relaxed.
fix https://github.com/chatmail/core/issues/7880
depends on #7754 (merged)
With this change, a securejoin message is just ignored if the contact
was deleted in the meantime; apparently the user is not interested in
the securejoin process anymore if they deleted the contact.
But other, parallel securejoin processes must not be affected; the test
also tests this.
This was initially done in the IMAP loop
to set is_chatmail for existing users.
They should all have the setting configured
by now unless they install some very old backup.
Setting during the configuration is needed
for Delta Chat Desktop because it caches the value
internally:
<https://github.com/deltachat/deltachat-desktop/issues/6068>
Close https://github.com/chatmail/core/issues/7396. Before reviewing,
you should read the issue description of
https://github.com/chatmail/core/issues/7396.
I recommend to review with hidden whitespace changes.
TODO:
- [x] Implement the new protocol
- [x] Make Rust tests pass
- [x] Make Python tests pass
- [x] Test it manually on a phone
- [x] Print the sent messages, and check that they look how they should:
[test_secure_join_group_with_mime_printed.txt](https://github.com/user-attachments/files/24800556/test_secure_join_group.txt)
- [x] Fix bug: If Alice has a second device, then Bob's chat won't be
shown yet on that second device. Also, Bob's contact isn't shown in her
contact list. As soon as either party writes something into the chat,
the that shows up and everything is fine. All of this is still a way
better UX than in WhatsApp, where Bob always has to write first 😂
Still, I should fix that.
- This is actually caused by a larger bug: AUTH tokens aren't synced if
there is no corresponding INVITE token.
- Fixed by 6b658a0e0
- [x] Either make a new `auth_tokens` table with a proper UNIQUE bound,
or put a UNIQUE bound on the `tokens` table
- [x] Benchmarking
- [x] TODOs in the code, maybe change naming of the new functions
- [x] Write test for interop with older DC (esp. that the original
securejoin runs if you remove the &v=3 param)
- [x] From a cryptography perspective, is it fine that vc-request is
encrypted with AUTH, rather than a separate secret (like INVITE)?
- [x] Make sure that QR codes without INVITE work, so that we can remove
it eventually
- [x] Self-review, and comment on some of my code changes to explain
what they do
- [x] ~~Maybe use a new table rather than reusing AUTH token.~~ See
https://github.com/chatmail/core/pull/7754#discussion_r2728544725
- [ ] Update documentation; I'll do that in a separate PR. All necessary
information is in the https://github.com/chatmail/core/issues/7396 issue
description
- [ ] Update tests and other code to use the new names (e.g.
`request-pubkey` rather than `request` and `pubkey` rather than
`auth-required`); I'll do that in a follow-up PR
**Backwards compatibility:**
Everything works seamlessly in my tests. If both devices are updated,
then the new protocol is used; otherwise, the old protocol is used. If
there is a not-yet-updated second device, it will correctly observe the
protocol, and mark the chat partner as verified.
Note that I removed the `Auto-Submitted: auto-replied` header from
securejoin messages. We don't need it ourselves, it's a cleartext header
that leaks too much information, and I can't see any reason to have it.
---------
Co-authored-by: iequidoo <117991069+iequidoo@users.noreply.github.com>
There is no need to store copy of public key
next to the secret key because public key is a subset of the secret key
and can be obtained by using SignedSecretKey.public_key()
or SignedSecretKey.to_public_key().
Rpc.start() now calls get_system_info() after launching the server
to verify it started successfully. If the server exits early (e.g.
due to an invalid accounts directory), the core error message from
stderr is captured and included in the raised JsonRpcError.
The reader_loop now unblocks pending RPC requests when the server
closes stdout, so callers never hang on a dead server.
Export JsonRpcError from the deltachat_rpc_client package.
Add test_early_failure verifying that Rpc.start() raises with
the actual core error message for invalid accounts directories.
Timestamp renewal was introduced in 1dbf924c6a "feat:
chat::resend_msgs: Guarantee strictly increasing time in the Date header" so that re-sent messages
can be deduplicated on the reciver side, but the deduplication logic doesn't depend on "Date"
anymore.
fix https://github.com/chatmail/core/issues/7863
`test_import_encrypted_bak_into_encrypted_acct` CFFI test fails because
it tests that trying to import an encrypted account with a wrong
passphrase into an already-encrypted database will fail, but leave the
already-encrypted database (esp, leave it with the same password).
But in order to reset the database after a failed login attempt, I'm
using this code:
```rust
context.sql.close().await;
fs::remove_file(context.sql.dbfile.as_path())
.await
.log_err(context)
.ok();
context
.sql
.open(context, "".to_string()) // <-- NOTE THIS LINE
.await
.log_err(context)
.ok();
```
We're not remembering the password, so, we can't just pass the correct
password there.
Since password-protected databases are not really supported anyways, we
decided to just skip the test.
I also tried two tricks for deleting everything [found on
Stackoverflow](https://stackoverflow.com/questions/525512/drop-all-tables-command),
but neither of them managed to actually reset the database (i.e. they
led to a failed Rust test, because asserting
`!context2.is_configured().await?` failed):
```rust
context
.sql
.call_write(|conn| {
let mut stmt = conn.prepare(
"select 'drop table ' || name || ';' from sqlite_master where type = 'table';",
)?;
let mut iter = stmt.query(())?;
while iter.next()?.is_some() {}
Ok(())
})
.await
.log_err(context)
.ok();
context
.sql
.run_migrations(context)
.await
.log_err(context)
.ok();
```
```rust
context
.sql
.transaction(|t| {
t.execute_batch(
"
PRAGMA writable_schema = 1;
delete from sqlite_master where type in ('table', 'index', 'trigger');
PRAGMA writable_schema = 0",
)?;
Ok(())
})
.await
.log_err(context)
.ok();
context
.sql
.run_migrations(context)
.await
.log_err(context)
.ok();
```
---------
Co-authored-by: l <link2xt@testrun.org>
I sometimes find it hard to find the most recent migration.
This PR moves the migrations::run() function to the end of the file, so
that it's easy to find the most recent migration - it's at the end of
the file.
There are no changes except for switching the ordering of the functions.
The webxdc file name itself isn't informative for users. Still, send and display it if the webxdc
manifest can't be parsed, it's better than sending "Mini App" and this isn't a normal case anyway.
This code does not expect the variable to be unset,
so use indexing to fail with KeyError instead.
Otherwise getenv() returns None which is then converted to "none" string by formatting
and the test only fails because of connection attempts to "none" domain.
fix#7877
The bug was: If there is no chat description, and the chat description
is set to an empty string, the INSERT statement inserted a row with an
empty chat description, and therefore from the view of the INSERT
statement, something changed.
This PR fixes this by simply loading the chat description first, and
comparing it.
instead of Alice saying to Bob "You changed the chat description",
we now say "[Chat description changed, please update ...]
i was also considering to say "[Chat description changed to:\n\n...]"
but then there is no incentive for ppl to update, and chat descriptions
for chat creation would still be missing. and this is probably far more
often used.
successor of https://github.com/chatmail/core/pull/7829
