diff --git a/deltachat-rpc-client/src/deltachat_rpc_client/account.py b/deltachat-rpc-client/src/deltachat_rpc_client/account.py index 8d9106a9b8..9d0dd965a2 100644 --- a/deltachat-rpc-client/src/deltachat_rpc_client/account.py +++ b/deltachat-rpc-client/src/deltachat_rpc_client/account.py @@ -359,3 +359,7 @@ def import_self_keys(self, path) -> None: """Import keys.""" passphrase = "" # Importing passphrase-protected keys is currently not supported. self._rpc.import_self_keys(self.id, str(path), passphrase) + + def initiate_autocrypt_key_transfer(self) -> None: + """Send Autocrypt Setup Message.""" + return self._rpc.initiate_autocrypt_key_transfer(self.id) diff --git a/deltachat-rpc-client/src/deltachat_rpc_client/message.py b/deltachat-rpc-client/src/deltachat_rpc_client/message.py index 6d1d68ac4f..aff84cc7c6 100644 --- a/deltachat-rpc-client/src/deltachat_rpc_client/message.py +++ b/deltachat-rpc-client/src/deltachat_rpc_client/message.py @@ -52,6 +52,9 @@ def mark_seen(self) -> None: """Mark the message as seen.""" self._rpc.markseen_msgs(self.account.id, [self.id]) + def continue_autocrypt_key_transfer(self, setup_code: str) -> None: + self._rpc.continue_autocrypt_key_transfer(self.account.id, self.id, setup_code) + def send_webxdc_status_update(self, update: Union[dict, str], description: str) -> None: """Send a webxdc status update. This message must be a webxdc.""" if not isinstance(update, str): diff --git a/deltachat-rpc-client/tests/test_key_transfer.py b/deltachat-rpc-client/tests/test_key_transfer.py new file mode 100644 index 0000000000..27291b092e --- /dev/null +++ b/deltachat-rpc-client/tests/test_key_transfer.py @@ -0,0 +1,50 @@ +import pytest + +from deltachat_rpc_client import EventType +from deltachat_rpc_client.rpc import JsonRpcError + + +def wait_for_autocrypt_setup_message(account): + while True: + event = account.wait_for_event() + if event.kind == EventType.MSGS_CHANGED and event.msg_id != 0: + msg_id = event.msg_id + msg = account.get_message_by_id(msg_id) + if msg.get_snapshot().is_setupmessage: + return msg + + +def test_autocrypt_setup_message_key_transfer(acfactory): + alice1 = acfactory.get_online_account() + + alice2 = acfactory.get_unconfigured_account() + alice2.set_config("addr", alice1.get_config("addr")) + alice2.set_config("mail_pw", alice1.get_config("mail_pw")) + alice2.configure() + + setup_code = alice1.initiate_autocrypt_key_transfer() + msg = wait_for_autocrypt_setup_message(alice2) + + # Test that entering wrong code returns an error. + with pytest.raises(JsonRpcError): + msg.continue_autocrypt_key_transfer("7037-0673-6287-3013-4095-7956-5617-6806-6756") + + msg.continue_autocrypt_key_transfer(setup_code) + + +def test_ac_setup_message_twice(acfactory): + alice1 = acfactory.get_online_account() + alice2 = acfactory.get_unconfigured_account() + alice2.set_config("addr", alice1.get_config("addr")) + alice2.set_config("mail_pw", alice1.get_config("mail_pw")) + alice2.configure() + + # Send the first Autocrypt Setup Message and ignore it. + _setup_code = alice1.initiate_autocrypt_key_transfer() + wait_for_autocrypt_setup_message(alice2) + + # Send the second Autocrypt Setup Message and import it. + setup_code = alice1.initiate_autocrypt_key_transfer() + msg = wait_for_autocrypt_setup_message(alice2) + + msg.continue_autocrypt_key_transfer(setup_code) diff --git a/deltachat-rpc-client/tests/test_securejoin.py b/deltachat-rpc-client/tests/test_securejoin.py index d2275d2fd2..08cf51d055 100644 --- a/deltachat-rpc-client/tests/test_securejoin.py +++ b/deltachat-rpc-client/tests/test_securejoin.py @@ -4,6 +4,7 @@ import pytest from deltachat_rpc_client import Chat, EventType, SpecialContactId +from deltachat_rpc_client.rpc import JsonRpcError def test_qr_setup_contact(acfactory, tmp_path) -> None: @@ -26,17 +27,21 @@ def test_qr_setup_contact(acfactory, tmp_path) -> None: bob_contact_alice_snapshot = bob_contact_alice.get_snapshot() assert bob_contact_alice_snapshot.is_verified - # Test that if Bob changes the key, backwards verification is lost. + # Test that if Bob imports a key, + # backwards verification is not lost + # because default key is not changed. logging.info("Bob 2 is created") bob2 = acfactory.new_configured_account() bob2.export_self_keys(tmp_path) - logging.info("Bob imports a key") - bob.import_self_keys(tmp_path) + logging.info("Bob tries to import a key") + # Importing a second key is not allowed. + with pytest.raises(JsonRpcError): + bob.import_self_keys(tmp_path) - assert bob.get_config("key_id") == "2" + assert bob.get_config("key_id") == "1" bob_contact_alice_snapshot = bob_contact_alice.get_snapshot() - assert not bob_contact_alice_snapshot.is_verified + assert bob_contact_alice_snapshot.is_verified def test_qr_setup_contact_svg(acfactory) -> None: diff --git a/python/src/deltachat/message.py b/python/src/deltachat/message.py index 4d756ae4fe..d5e832be69 100644 --- a/python/src/deltachat/message.py +++ b/python/src/deltachat/message.py @@ -215,7 +215,7 @@ def continue_key_transfer(self, setup_code): """extract key and use it as primary key for this account.""" res = lib.dc_continue_key_transfer(self.account._dc_context, self.id, as_dc_charpointer(setup_code)) if res == 0: - raise ValueError("could not decrypt") + raise ValueError("Importing the key from Autocrypt Setup Message failed") @props.with_doc def time_sent(self): diff --git a/python/tests/test_0_complex_or_slow.py b/python/tests/test_0_complex_or_slow.py index 2366e90adb..d4b7974281 100644 --- a/python/tests/test_0_complex_or_slow.py +++ b/python/tests/test_0_complex_or_slow.py @@ -510,6 +510,7 @@ def test_see_new_verified_member_after_going_online(acfactory, tmp_path, lp): """ ac1, ac2 = acfactory.get_online_accounts(2) ac2_addr = ac2.get_config("addr") + acfactory.remove_preconfigured_keys() ac1_offl = acfactory.new_online_configuring_account(cloned_from=ac1) for ac in [ac1, ac1_offl]: ac.set_config("bcc_self", "1") @@ -560,6 +561,7 @@ def test_use_new_verified_group_after_going_online(acfactory, data, tmp_path, lp missing, cannot encrypt". """ ac1, ac2 = acfactory.get_online_accounts(2) + acfactory.remove_preconfigured_keys() ac2_offl = acfactory.new_online_configuring_account(cloned_from=ac2) for ac in [ac2, ac2_offl]: ac.set_config("bcc_self", "1") @@ -615,6 +617,7 @@ def test_verified_group_vs_delete_server_after(acfactory, tmp_path, lp): - Now the seconds device has all members verified. """ ac1, ac2 = acfactory.get_online_accounts(2) + acfactory.remove_preconfigured_keys() ac2_offl = acfactory.new_online_configuring_account(cloned_from=ac2) for ac in [ac2, ac2_offl]: ac.set_config("bcc_self", "1") diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index 53124c4d13..b5a9dd9e51 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -85,27 +85,6 @@ def test_configure_unref(tmp_path): lib.dc_context_unref(dc_context) -def test_export_import_self_keys(acfactory, tmp_path, lp): - ac1, ac2 = acfactory.get_online_accounts(2) - - dir = tmp_path / "exportdir" - dir.mkdir() - export_files = ac1.export_self_keys(str(dir)) - assert len(export_files) == 2 - for x in export_files: - assert x.startswith(str(dir)) - (key_id,) = ac1._evtracker.get_info_regex_groups(r".*xporting.*KeyId\((.*)\).*") - ac1._evtracker.consume_events() - - lp.sec("exported keys (private and public)") - for name in dir.iterdir(): - lp.indent(str(dir / name)) - lp.sec("importing into existing account") - ac2.import_self_keys(str(dir)) - (key_id2,) = ac2._evtracker.get_info_regex_groups(r".*stored.*KeyId\((.*)\).*") - assert key_id2 == key_id - - def test_one_account_send_bcc_setting(acfactory, lp): ac1 = acfactory.new_online_configuring_account() ac2 = acfactory.new_online_configuring_account() @@ -1597,53 +1576,6 @@ def assert_account_is_proper(ac): assert ac2.get_latest_backupfile(str(backupdir)) == path2 -def test_ac_setup_message(acfactory, lp): - # note that the receiving account needs to be configured and running - # before the setup message is send. DC does not read old messages - # as of Jul2019 - ac1 = acfactory.new_online_configuring_account() - ac2 = acfactory.new_online_configuring_account(cloned_from=ac1) - acfactory.bring_accounts_online() - - lp.sec("trigger ac setup message and return setupcode") - assert ac1.get_info()["fingerprint"] != ac2.get_info()["fingerprint"] - setup_code = ac1.initiate_key_transfer() - ev = ac2._evtracker.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED") - msg = ac2.get_message_by_id(ev.data2) - assert msg.is_setup_message() - assert msg.get_setupcodebegin() == setup_code[:2] - lp.sec("try a bad setup code") - with pytest.raises(ValueError): - msg.continue_key_transfer(str(reversed(setup_code))) - lp.sec("try a good setup code") - print("*************** Incoming ASM File at: ", msg.filename) - print("*************** Setup Code: ", setup_code) - msg.continue_key_transfer(setup_code) - assert ac1.get_info()["fingerprint"] == ac2.get_info()["fingerprint"] - - -def test_ac_setup_message_twice(acfactory, lp): - ac1 = acfactory.new_online_configuring_account() - ac2 = acfactory.new_online_configuring_account(cloned_from=ac1) - acfactory.bring_accounts_online() - - lp.sec("trigger ac setup message but ignore") - assert ac1.get_info()["fingerprint"] != ac2.get_info()["fingerprint"] - ac1.initiate_key_transfer() - ac2._evtracker.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED") - - lp.sec("trigger second ac setup message, wait for receive ") - setup_code2 = ac1.initiate_key_transfer() - ev = ac2._evtracker.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED") - msg = ac2.get_message_by_id(ev.data2) - assert msg.is_setup_message() - assert msg.get_setupcodebegin() == setup_code2[:2] - - lp.sec("process second setup message") - msg.continue_key_transfer(setup_code2) - assert ac1.get_info()["fingerprint"] == ac2.get_info()["fingerprint"] - - def test_qr_email_capitalization(acfactory, lp): """Regression test for a bug that resulted in failure to propagate verification via gossip in a verified group diff --git a/src/configure.rs b/src/configure.rs index 1fd561fbc0..854ac9b7f1 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -38,7 +38,7 @@ use crate::provider::{Protocol, Socket, UsernamePattern}; use crate::smtp::Smtp; use crate::sync::Sync::*; use crate::tools::time; -use crate::{chat, e2ee, provider}; +use crate::{chat, provider}; use crate::{stock_str, EventType}; use deltachat_contact_tools::addr_cmp; @@ -473,9 +473,6 @@ async fn configure(ctx: &Context, param: &EnteredLoginParam) -> Result Result { } } -async fn set_self_key(context: &Context, armored: &str, set_default: bool) -> Result<()> { +async fn set_self_key(context: &Context, armored: &str) -> Result<()> { // try hard to only modify key-state let (private_key, header) = SignedSecretKey::from_asc(armored)?; let public_key = private_key.split_public_key()?; @@ -170,16 +170,7 @@ async fn set_self_key(context: &Context, armored: &str, set_default: bool) -> Re public: public_key, secret: private_key, }; - key::store_self_keypair( - context, - &keypair, - if set_default { - key::KeyPairUse::Default - } else { - key::KeyPairUse::ReadOnly - }, - ) - .await?; + key::store_self_keypair(context, &keypair).await?; info!(context, "stored self key: {:?}", keypair.secret.key_id()); Ok(()) @@ -599,10 +590,10 @@ where } /// Imports secret key from a file. -async fn import_secret_key(context: &Context, path: &Path, set_default: bool) -> Result<()> { +async fn import_secret_key(context: &Context, path: &Path) -> Result<()> { let buf = read_file(context, path).await?; let armored = std::string::String::from_utf8_lossy(&buf); - set_self_key(context, &armored, set_default).await?; + set_self_key(context, &armored).await?; Ok(()) } @@ -624,8 +615,7 @@ async fn import_self_keys(context: &Context, path: &Path) -> Result<()> { "Importing secret key from {} as the default key.", path.display() ); - let set_default = true; - import_secret_key(context, path, set_default).await?; + import_secret_key(context, path).await?; return Ok(()); } @@ -643,14 +633,13 @@ async fn import_self_keys(context: &Context, path: &Path) -> Result<()> { } else { continue; }; - let set_default = !name_f.contains("legacy"); info!( context, "Considering key file: {}.", path_plus_name.display() ); - if let Err(err) = import_secret_key(context, &path_plus_name, set_default).await { + if let Err(err) = import_secret_key(context, &path_plus_name).await { warn!( context, "Failed to import secret key from {}: {:#}.", @@ -871,7 +860,7 @@ mod tests { assert_eq!(bytes, key.to_asc(None).into_bytes()); - let alice = &TestContext::new_alice().await; + let alice = &TestContext::new().await; if let Err(err) = imex(alice, ImexMode::ImportSelfKeys, Path::new(&filename), None).await { panic!("got error on import: {err:#}"); } @@ -893,7 +882,7 @@ mod tests { panic!("got error on export: {err:#}"); } - let context2 = TestContext::new_alice().await; + let context2 = TestContext::new().await; if let Err(err) = imex( &context2.ctx, ImexMode::ImportSelfKeys, @@ -920,15 +909,18 @@ mod tests { let alice = &TestContext::new_alice().await; let old_key = key::load_self_secret_key(alice).await?; - imex(alice, ImexMode::ImportSelfKeys, export_dir.path(), None).await?; - - let new_key = key::load_self_secret_key(alice).await?; - assert_ne!(new_key, old_key); - assert_eq!( - key::load_self_secret_keyring(alice).await?, - vec![new_key, old_key] + assert!( + imex(alice, ImexMode::ImportSelfKeys, export_dir.path(), None) + .await + .is_err() ); + // Importing a second key is not allowed anymore, + // even as a non-default key. + assert_eq!(key::load_self_secret_key(alice).await?, old_key); + + assert_eq!(key::load_self_secret_keyring(alice).await?, vec![old_key]); + let msg = alice.recv_msg(&sent).await; assert!(msg.get_showpadlock()); assert_eq!(msg.chat_id, alice.get_self_chat().await.id); diff --git a/src/imex/key_transfer.rs b/src/imex/key_transfer.rs index ce5d4da4e8..933abd3fc5 100644 --- a/src/imex/key_transfer.rs +++ b/src/imex/key_transfer.rs @@ -75,7 +75,7 @@ pub async fn continue_key_transfer( let file = open_file_std(context, filename)?; let sc = normalize_setup_code(setup_code); let armored_key = decrypt_setup_file(&sc, file).await?; - set_self_key(context, &armored_key, true).await?; + set_self_key(context, &armored_key).await?; context.set_config_bool(Config::BccSelf, true).await?; Ok(()) @@ -315,18 +315,19 @@ mod tests { alice2.recv_msg(&sent).await; let msg = alice2.get_last_msg().await; assert!(msg.is_setupmessage()); - - // Send a message that cannot be decrypted because the keys are - // not synchronized yet. - let sent = alice2.send_text(msg.chat_id, "Test").await; - let trashed_message = alice.recv_msg_opt(&sent).await; - assert!(trashed_message.is_none()); - assert_ne!(alice.get_last_msg().await.get_text(), "Test"); + assert_eq!( + crate::key::load_self_secret_keyring(&alice2).await?.len(), + 0 + ); // Transfer the key. alice2.set_config(Config::BccSelf, Some("0")).await?; continue_key_transfer(&alice2, msg.id, &setup_code).await?; assert_eq!(alice2.get_config_bool(Config::BccSelf).await?, true); + assert_eq!( + crate::key::load_self_secret_keyring(&alice2).await?.len(), + 1 + ); // Alice sends a message to self from the new device. let sent = alice2.send_text(msg.chat_id, "Test").await; diff --git a/src/key.rs b/src/key.rs index dc3aca7995..0582f5d28c 100644 --- a/src/key.rs +++ b/src/key.rs @@ -289,7 +289,7 @@ async fn generate_keypair(context: &Context) -> Result { .spawn_blocking(move || crate::pgp::create_keypair(addr, keytype)) .await??; - store_self_keypair(context, &keypair, KeyPairUse::Default).await?; + store_self_keypair(context, &keypair).await?; info!( context, "Keypair generated in {:.3}s.", @@ -326,18 +326,6 @@ pub(crate) async fn load_keypair(context: &Context) -> Result> { }) } -/// Use of a key pair for encryption or decryption. -/// -/// This is used by `store_self_keypair` to know what kind of key is -/// being saved. -#[derive(Debug, Clone, Eq, PartialEq)] -pub enum KeyPairUse { - /// The default key used to encrypt new messages. - Default, - /// Only used to decrypt existing message. - ReadOnly, -} - /// Store the keypair as an owned keypair for addr in the database. /// /// This will save the keypair as keys for the given address. The @@ -350,11 +338,7 @@ pub enum KeyPairUse { /// same key again overwrites it. /// /// [Config::ConfiguredAddr]: crate::config::Config::ConfiguredAddr -pub(crate) async fn store_self_keypair( - context: &Context, - keypair: &KeyPair, - default: KeyPairUse, -) -> Result<()> { +pub(crate) async fn store_self_keypair(context: &Context, keypair: &KeyPair) -> Result<()> { let mut config_cache_lock = context.sql.config_cache.write().await; let new_key_id = context .sql @@ -362,29 +346,28 @@ pub(crate) async fn store_self_keypair( let public_key = DcKey::to_bytes(&keypair.public); let secret_key = DcKey::to_bytes(&keypair.secret); - let is_default = match default { - KeyPairUse::Default => true, - KeyPairUse::ReadOnly => false, - }; - + // private_key and public_key columns + // are UNIQUE since migration 107, + // so this fails if we already have this key. transaction .execute( - "INSERT OR REPLACE INTO keypairs (public_key, private_key) + "INSERT INTO keypairs (public_key, private_key) VALUES (?,?)", (&public_key, &secret_key), ) .context("Failed to insert keypair")?; - if is_default { - let new_key_id = transaction.last_insert_rowid(); - transaction.execute( - "INSERT OR REPLACE INTO config (keyname, value) VALUES ('key_id', ?)", - (new_key_id,), - )?; - Ok(Some(new_key_id)) - } else { - Ok(None) - } + let new_key_id = transaction.last_insert_rowid(); + + // This will fail if we already have `key_id`. + // + // Setting default key is only possible if we don't + // have a key already. + transaction.execute( + "INSERT INTO config (keyname, value) VALUES ('key_id', ?)", + (new_key_id,), + )?; + Ok(Some(new_key_id)) }) .await?; @@ -405,7 +388,7 @@ pub async fn preconfigure_keypair(context: &Context, secret_data: &str) -> Resul let secret = SignedSecretKey::from_asc(secret_data)?.0; let public = secret.split_public_key()?; let keypair = KeyPair { public, secret }; - store_self_keypair(context, &keypair, KeyPairUse::Default).await?; + store_self_keypair(context, &keypair).await?; Ok(()) } @@ -700,6 +683,7 @@ i8pcjGO+IZffvyZJVRWfVooBJmWWbPB1pueo3tx8w3+fcuzpxz+RLFKaPyqXO+dD assert_eq!(pubkey.primary_key, KEYPAIR.public.primary_key); } + /// Tests that setting a default key second time is not allowed. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_save_self_key_twice() { // Saving the same key twice should result in only one row in @@ -714,13 +698,13 @@ i8pcjGO+IZffvyZJVRWfVooBJmWWbPB1pueo3tx8w3+fcuzpxz+RLFKaPyqXO+dD .unwrap() }; assert_eq!(nrows().await, 0); - store_self_keypair(&ctx, &KEYPAIR, KeyPairUse::Default) - .await - .unwrap(); + store_self_keypair(&ctx, &KEYPAIR).await.unwrap(); assert_eq!(nrows().await, 1); - store_self_keypair(&ctx, &KEYPAIR, KeyPairUse::Default) - .await - .unwrap(); + + // Saving a second key fails. + let res = store_self_keypair(&ctx, &KEYPAIR).await; + assert!(res.is_err()); + assert_eq!(nrows().await, 1); } diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index d48c189325..a5e8d36085 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -4705,8 +4705,10 @@ async fn test_outgoing_msg_forgery() -> Result<()> { // We need Bob only to encrypt the forged message to Alice's key, actually Bob doesn't // participate in the scenario. let bob = &TestContext::new().await; + assert_eq!(crate::key::load_self_secret_keyring(bob).await?.len(), 0); bob.configure_addr("bob@example.net").await; imex(bob, ImexMode::ImportSelfKeys, export_dir.path(), None).await?; + assert_eq!(crate::key::load_self_secret_keyring(bob).await?.len(), 1); let malice = &TestContext::new().await; malice.configure_addr(alice_addr).await; @@ -4714,6 +4716,7 @@ async fn test_outgoing_msg_forgery() -> Result<()> { .send_recv_accept(bob, malice, "hi from bob") .await .chat_id; + assert_eq!(crate::key::load_self_secret_keyring(bob).await?.len(), 1); let sent_msg = malice.send_text(malice_chat_id, "hi from malice").await; let msg = alice.recv_msg(&sent_msg).await; diff --git a/src/test_utils.rs b/src/test_utils.rs index 6bf1107e50..e645af4922 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -33,7 +33,7 @@ use crate::contact::{Contact, ContactId, Modifier, Origin}; use crate::context::Context; use crate::e2ee::EncryptHelper; use crate::events::{Event, EventEmitter, EventType, Events}; -use crate::key::{self, DcKey, KeyPairUse}; +use crate::key::{self, DcKey}; use crate::message::{update_msg_state, Message, MessageState, MsgId, Viewtype}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::peerstate::Peerstate; @@ -271,7 +271,7 @@ impl TestContextBuilder { let test_context = TestContext::new_internal(Some(name), self.log_sink).await; test_context.configure_addr(&addr).await; - key::store_self_keypair(&test_context, &key_pair, KeyPairUse::Default) + key::store_self_keypair(&test_context, &key_pair) .await .expect("Failed to save key"); test_context