Telethon icon indicating copy to clipboard operation
Telethon copied to clipboard

Security: Implement perfect forward secrecy for MTProto

Open habcawa opened this issue 7 months ago • 2 comments

Hi!

Thanks for the great work with Telethon.

Quite a while ago I have implemented support for perfect forward secrecy in MTProto (cf. https://core.telegram.org/api/pfs) and would love to share this work with the community.

I understand that v1 is feature frozen, however, I think that this provides an advantage in security for all users and is not a new feature per se but rather a security fix. This PR also fixes two bugs (type confusion, error handling on seq_no too low/too high) that can cause issues.

I have tested this with both a personal account and a bot token and did not encouter any problems. I have also tested the behavior on the expiration of a temporary auth key.

habcawa avatar May 20 '25 14:05 habcawa

Looks good. It's much appreciated. [...]

I am happy to hear! Thanks, also for your comments. I'll provide a patch shortly.

  • If this breaks, I don't know if I'll be able to provide a timely fix myself.
  • There'll likely be an expectation that v2 (if I ever finish that) will support PFS too.

At least for the latter issue, I can offer that I'll give implementing this in v2 a try in the next weeks. I would presume that most of the heavy lifting is already done, but I have never worked with v2 and would need to get familiar with the codebase.

I'm assuming the performance hit is not something we should worry about, since by default they expire after a day (although the first connection might be noticeably slower, specially with pure-Python encryption on mobile devices).

I would think so, too. After the temporary key has been generated and bound, performance should be back to normal until the key needs to be changed.

habcawa avatar May 21 '25 17:05 habcawa

Thanks for your review @andrew-ld.

the AUTH_KEY_PERM_EMPTY error should also be handled, sometimes telegram may forget which persistent key the temporary key is bound to, again the key must be regenerated.

I fixed this and will provide a patch shortly. I am not sure if this is the correct place, so please let me know about any further changes you would like to see. Accessing a private method is also not really a nice thing here.

first it does not handle the -404 error code at the transport level, telegram clearly says that temporary keys could be deleted from the server before they expire, you have to regenerate the key in this case [...] in the case of -404 if the key was generated within 60 seconds of the error you simply have to reconnect instead of regenerating it because it happens that immediately after generating it the server still does not accept the key.

Yes, I have also read that part of the docs. Therefore, the current implementation should always attempt to regenerate the key if I am not missing something. I have changed the following to test this:

diff --git a/telethon/network/authenticator.py b/telethon/network/authenticator.py
index e32df6f9..9507112b 100644
--- a/telethon/network/authenticator.py
+++ b/telethon/network/authenticator.py
@@ -44,7 +44,7 @@ async def do_authentication(sender, tmp_auth_key=False, tmp_auth_key_expires_s =
     new_nonce = int.from_bytes(os.urandom(32), 'little', signed=True)
 
     if tmp_auth_key:
-        expires_at = int(time.time()) + tmp_auth_key_expires_s
+        expires_at = int(time.time()) + 10 # tmp_auth_key_expires_s
         pq_inner_data = bytes(PQInnerDataTemp(
             pq=rsa.get_byte_array(pq), p=p, q=q,
             nonce=res_pq.nonce,

If I run the following program, the expired key is handled, as expected, by reconnecting (which triggers the generation of a new key).

client = TelegramClient('a', api_id, api_hash)
async for dialog in client.iter_dialogs():
        await client.send_message(dialog.id, f'Hello, {dialog.name}! {time.time()}')
        print('Send 1 done')
        await asyncio.sleep(10)
        
        await client.send_message(dialog.id, f'Hello, {dialog.name} 2! {time.time()}')
        print('Send 2 done')
        await asyncio.sleep(2)
        break

As before, please let me know if I am missing something here.

habcawa avatar May 21 '25 18:05 habcawa

i dont think that always re-generate the temp auth key is a good idea, the client must keep the generated temp key in memory and re-generate only if needed

Sorry for the late reply - could you please elaborate what changes you'd like to see in this regard?

habcawa avatar Oct 10 '25 14:10 habcawa

(rebased to the current tip of the v1 branch)

habcawa avatar Nov 07 '25 12:11 habcawa

I can offer that I'll give implementing this in v2 a try […]

I've changed my mind on v2 as pure Python. If I'm to maintain grammers and Telethon, the only thing that makes sense is to reuse grammers' core for Telethon. This way most of the important logic can be pure Rust, in one place, and Telethon v2 can simply offer a nice interface on top.

So, if you're still up for the challenge, if grammers-mtsender supported temporary auth keys, and Telethon v2 is ever a thing, Telethon v2 will probably make use of grammers-mtsender (and all crates below it) and just abstract it.

I recently completed a significant refactor of all grammers crates (except grammers-client, which Telethon v2 wouldn't use), and I'm fairly happy with the state they're in now. So in my mind they'd be stable enough to absorb this sort of change too.

Lonami avatar Nov 07 '25 22:11 Lonami

OK note to self, don't merge large changes when I'm tired.

https://github.com/LonamiWebs/Telethon/pull/4618#discussion_r2505758053 still needs to be addressed (should likely be on __call__ and not here)

Plus @andrew-ld left some comments that would still be worth addressing in a follow-up.

Lonami avatar Nov 07 '25 23:11 Lonami