Have the client send machine id to terminal rather than vice versa

The terminal is the trusted by user party here.

Author: kovidgoyal

docs/dnd-protocol.rst

@@ -48,16 +48,6 @@ The list of MIME types is optional, it is needed if the program wants to accept
 exotic or private use MIME types on platforms such as macOS, where the system
 does not deliver drop events unless the MIME type is registered.
 
-The terminal emulator may respond to this escape code with an escape code of
-the form::
-
-    OSC _dnd_code ; t=a ; machine id ST
-
-Here, the :ref:`machine id <machine_id>` is an id that identifies the machine
-the terminal is running on and can be used by the client to determine whether
-to request remote files from the terminal when a drop occurs.
-See :ref:`below <machine_id>` for the semantics of the machine id.
-
 When the client is done accepting drops, or at exit, it should send the escape
 code::
 
@@ -143,12 +133,17 @@ Dropping from remote machines
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 In order to support dropping of files from remote machines, the client
-can use the :ref:`machine id <machine_id>` previously sent by the terminal.
-If it is different from the id of the machine the client is running on, it
-can choose to request remote files, as follows.
+must inform the terminal of its :ref:`machine id <machine_id>` using the escape code::
+
+    OSC _dnd_code ; t=a:x=1 ; machine id ST
 
-Clients can first request the :rfc:`text/uri-list <2483>` MIME
-type to get a list of dropped URIs. For every URI in the list, they can
+Then, the client must first request the :rfc:`text/uri-list <2483>` MIME
+type to get a list of dropped URIs. When responding to this request,
+the terminal will send the usual ``t=r`` responses, but, in addition,
+if the client has sent its machine id and the terminal determines that
+the client is on a different machine based on the id, it will add the ``X=1``
+key to its response. The client should use this key to determine if it wants to
+request data for entries in the URI list. For every URI in the list, the client can
 send the terminal emulator a data request of the form::
 
     OSC _dnd_code ; t=r:x=idx:y=subidx ST
@@ -486,11 +481,22 @@ Key      Value                 Default    Description
 Machine id
 -----------------
 
-The machine id is used to detect when a drag is started on a remote machine. It
-is of the form: ``version:ASCII printable chars``. The leading ``version`` field
-allows for changing the format or semantics of this field in the future. The
-actual id is the machine id (the contents of :file:`/etc/machine-id` on
-Linux/BSD and :file:`HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Cryptography\\MachineGuid` on Windows and ``IOPlatformUUID`` on macOS). This machine id is then hashed using a :rfc:`HMAC <2104>`
-with :rfc:`SHA-256 <6234>` as the digest algorithm and the key being the ASCII bytes:
-``tty-dnd-protocol-machine-id``. The hashing is done so as to not easily leak the
-actual machine id and to ensure that the value is of fixed size.
+The machine id is used to detect when the source and destination machines for a
+drag and drop are different. It is of the form: ``version:ASCII printable
+chars``. The leading ``version`` field allows for changing the format or
+semantics of this field in the future. The actual id is the machine id (the
+contents of :file:`/etc/machine-id` on Linux/BSD and
+:file:`HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Cryptography\\MachineGuid` on
+Windows and ``IOPlatformUUID`` on macOS). This machine id is then hashed using
+a :rfc:`HMAC <2104>` with :rfc:`SHA-256 <6234>` as the digest algorithm and the
+key being the ASCII bytes: ``tty-dnd-protocol-machine-id``. The hashing is done
+so as to not easily leak the actual machine id and to ensure that the value is
+of fixed size. This gives a final value of::
+
+    1:hashed machine id hexadecimal encoded
+
+In the future, the ``version`` field may increase if the hashing algorithm is
+changed. If the terminal sees a version it does not understand, it must assume
+that the machine id does not match, aka the source and destination machines are
+different. This assumption means that remote drag and drop will still work, just with
+reduced performance in case of version mismatch.

kitty/dnd.c

@@ -136,11 +136,13 @@ drop_free_data(Window *w) {
 static void
 reset_drop(Window *w) {
     bool wanted = w->drop.wanted; uint32_t cid = w->drop.client_id;
+    bool is_remote_client = w->drop.is_remote_client;
     drop_free_data(w);
     zero_at_ptr(&w->drop);
     if (wanted) {
         w->drop.wanted = wanted;
         w->drop.client_id = cid;
+        w->drop.is_remote_client = is_remote_client;
     }
 }
 
@@ -255,10 +257,23 @@ queue_payload_to_child(id_type id, uint32_t client_id, PendingData *pending, con
     if (pending->count) check_for_pending_writes();
 }
 
+static bool
+is_same_machine(const char *client_machine_id, size_t sz) {
+    if (!sz || !client_machine_id) return true;
+    if (sz < 20) return false;
+    if (client_machine_id[0] != '1' || client_machine_id[1] != ':') return false;
+    client_machine_id = client_machine_id + 2; sz -= 2;
+    const char *host_machine_id = machine_id();
+    if (!host_machine_id) return true;
+    const size_t hsz = strlen(host_machine_id);
+    return sz == hsz && memcmp(client_machine_id, host_machine_id, sz) == 0;
+}
+
 void
 drop_register_window(Window *w, const uint8_t *payload, size_t payload_sz, bool on, uint32_t client_id, bool more) {
     w->drop.wanted = on;
     w->drop.client_id = client_id;
+    w->drop.is_remote_client = false;
     if (!on) { drop_free_data(w); zero_at_ptr(&w->drop); return; }
     if (!payload || !payload_sz) return;
     size_t sz = w->drop.registered_mimes ? strlen(w->drop.registered_mimes) : 0;
@@ -286,13 +301,11 @@ drop_register_window(Window *w, const uint8_t *payload, size_t payload_sz, bool
         }
     }
     free(w->drop.registered_mimes); w->drop.registered_mimes = NULL;
-    const char* host_machine_id = machine_id();
-    if (host_machine_id) {
-        char header[32] = {0};
-        int n = snprintf(header, sizeof(header), "\x1b]%d;t=a", DND_CODE);
-        queue_payload_to_child(
-            w->id, w->drop.client_id, &w->drop.pending, header, n, host_machine_id, strlen(host_machine_id), false);
-    }
+}
+
+void
+drop_register_machine_id(Window *w, const uint8_t *machine_id, size_t sz) {
+    w->drop.is_remote_client = !is_same_machine((const char*)machine_id, sz);
 }
 
 void
@@ -471,9 +484,12 @@ drop_dispatch_data(Window *w, const char *mime, const char *data, ssize_t sz) {
     } else {
         char buf[128];
         int header_size = snprintf(buf, sizeof(buf), "\x1b]%d;t=r", DND_CODE);
+        const bool is_uri_list = strcmp(mime, "text/uri-list") == 0;
+        if (is_uri_list) header_size += snprintf(
+            buf + header_size, sizeof(buf) - header_size, ":X=%d", w->drop.is_remote_client);
         header_size += drop_append_request_keys(w, buf + header_size, sizeof(buf) - header_size);
         queue_payload_to_child(w->id, w->drop.client_id, &w->drop.pending, buf, header_size, sz ? data : NULL, sz, true);
-        if (strcmp(mime, "text/uri-list") == 0) {
+        if (is_uri_list) {
             w->drop.uri_list_sz += sz;
             w->drop.uri_list = realloc(w->drop.uri_list, w->drop.uri_list_sz);
             if (w->drop.uri_list) memcpy(w->drop.uri_list + w->drop.uri_list_sz - sz, data, sz);
@@ -1102,14 +1118,8 @@ cancel_drag(Window *w, int error_code) {
 
 void
 drag_start_offerring(Window *w, const char *client_machine_id, size_t sz) {
-    ds.can_offer = true; ds.is_remote_client = false;
-    if (sz && client_machine_id) {
-        const char *host_machine_id = machine_id();
-        if (host_machine_id) {
-            size_t hsz = strlen(host_machine_id);
-            if (hsz != sz || memcmp(host_machine_id, client_machine_id, sz) != 0) ds.is_remote_client = true;
-        }
-    }
+    ds.can_offer = true;
+    ds.is_remote_client = !is_same_machine(client_machine_id, sz);
 }
 
 void

kitty/dnd.h

@@ -10,6 +10,7 @@
 
 
 void drop_register_window(Window *w, const uint8_t *payload, size_t payload_sz, bool on, uint32_t client_id, bool more);
+void drop_register_machine_id(Window *w, const uint8_t *machine_id, size_t sz);
 void drop_move_on_child(Window *w, const char **mimes, size_t num_mimes, bool is_drop);
 void drop_left_child(Window *w);
 void drop_free_data(Window *w);

kitty/fast_data_types.pyi

@@ -1832,7 +1832,7 @@ class StreamingBase64Decoder:
     def needs_more_data(self) -> bool: ...
 
 
-class StreamingBase64Encodeer:
+class StreamingBase64Encoder:
     def __init__(self, add_trailing_bytes: bool = True) -> None: ...
     # encode the specified data
     def encode(self, data: ReadableBuffer) -> bytes: ...

kitty/screen.c

@@ -1528,7 +1528,10 @@ screen_handle_dnd_command(Screen *self, const DnDCommand *cmd, const uint8_t *pa
     Window *w = window_for_window_id(self->window_id);
     if (!w) return;
     switch(cmd->type) {
-        case 'a': drop_register_window(w, payload, cmd->payload_sz, true, cmd->client_id, cmd->more); break;
+        case 'a':
+            if (cmd->cell_x == 1) drop_register_machine_id(w, payload, cmd->payload_sz);
+            else drop_register_window(w, payload, cmd->payload_sz, true, cmd->client_id, cmd->more);
+            break;
         case 'A': drop_register_window(w, NULL, 0, false, cmd->client_id, cmd->more); break;
         case 'm': drop_set_status(w, cmd->operation, (const char*)payload, cmd->payload_sz, cmd->more); break;
         case 'r': {

kitty/state.h

@@ -269,7 +269,7 @@ typedef struct Window {
         bool is_hovering;
     } scrollbar;
     struct {
-        bool wanted, hovered, dropped;
+        bool wanted, hovered, dropped, is_remote_client;
         uint32_t client_id;
         char *registered_mimes;
         char *uri_list; size_t uri_list_sz;

kitty_tests/dnd.py

@@ -3,13 +3,14 @@
 
 import errno
 import re
-from base64 import standard_b64decode, standard_b64encode
+from base64 import standard_b64encode
 from contextlib import contextmanager
 from functools import partial
 
 from kitty.fast_data_types import (
     DND_CODE,
     Screen,
+    StreamingBase64Decoder,
     dnd_set_test_write_func,
     dnd_test_cleanup_fake_window,
     dnd_test_create_fake_window,
@@ -30,10 +31,6 @@ def _osc(payload: str) -> bytes:
 
 def client_register(mimes: str = '', client_id: int = 0) -> bytes:
     """Escape code a client sends to start accepting drops (t=a)."""
-    meta = f'{DND_CODE};t=a'
-    if client_id:
-        meta += f':i={client_id}'
-    return _osc(f'{meta};{mimes}')
 
 
 def client_unregister(client_id: int = 0) -> bytes:
@@ -257,14 +254,17 @@ def parse_escape_codes_b64(data: bytes) -> list[dict]:
     """Like *parse_escape_codes* but base64-decodes each chunk's payload."""
     result = parse_escape_codes(data)
     for entry in result:
+        d = StreamingBase64Decoder()
+        decoded = b''
         decoded_chunks = []
-        full = b''
-        for chunk in entry['chunks']:
-            dec = standard_b64decode(chunk + b'==') if chunk else b''
+        for c in entry['chunks']:
+            dec = d.decode(c)
             decoded_chunks.append(dec)
-            full += dec
+            decoded += dec
+        # if d.needs_more_data():
+        #     raise AssertionError('Incomplete base64 data')
+        entry['payload'] = decoded
         entry['chunks'] = decoded_chunks
-        entry['payload'] = full
     return result
 
 
@@ -332,13 +332,19 @@ class TestDnDProtocol(BaseTest):
     def _assert_no_output(self, capture: _WriteCapture, window_id: int) -> None:
         self.ae(capture.peek(window_id), b'', 'unexpected output to child')
 
-    def _register_for_drops(self, screen, cap, wid, mimes='text/plain text/uri-list', client_id=0) -> None:
-        parse_bytes(screen, client_register(mimes, client_id=client_id))
-        events = self._get_events(cap, wid)
-        self.assertEqual(len(events), 1, events)
-        self.ae(events[0]['type'], 'a')
-        self.ae(events[0]['payload'].strip().decode(), machine_id())
-
+    def _register_for_drops(
+        self, screen, cap, wid, mimes='text/plain text/uri-list', client_id=0, register_machine_id=True
+    ) -> None:
+        meta = f'{DND_CODE};t=a'
+        if client_id:
+            meta += f':i={client_id}'
+        r = _osc(f'{meta};{mimes}')
+        parse_bytes(screen, r)
+        if register_machine_id:
+            if not isinstance(register_machine_id, str):
+                register_machine_id = machine_id()
+            parse_bytes(screen, _osc(f'{DND_CODE};t=a:x=1;1:{register_machine_id}'))
+        self._assert_no_output(cap, wid)
 
     def _get_events(self, capture: _WriteCapture, window_id: int) -> list[dict]:
         return parse_escape_codes(capture.consume(window_id))
@@ -642,15 +648,16 @@ def _setup_uri_drop(self, screen, wid, cap, uri_list_data: bytes, mimes=None):
         """Register, drop, deliver text/uri-list data, discard move/drop events."""
         if mimes is None:
             mimes = ['text/plain', 'text/uri-list']
-        self._register_for_drops(screen, cap, wid, 'text/plain text/uri-list')
+        self._register_for_drops(screen, cap, wid, 'text/plain text/uri-list', register_machine_id='remote')
         dnd_test_set_mouse_pos(wid, 0, 0, 0, 0)
         dnd_test_fake_drop_event(wid, True, mimes)
         cap.consume(wid)
         # Client requests and receives the URI list (idx=2 for text/uri-list in the default MIME list)
         uri_idx = mimes.index('text/uri-list') + 1  # 1-based
         parse_bytes(screen, client_request_data(uri_idx))
         dnd_test_fake_drop_data(wid, 'text/uri-list', uri_list_data)
-        cap.consume(wid)  # discard t=r data for text/uri-list
+        events = parse_escape_codes_b64(cap.consume(wid))
+        self.assertEqual(events[0]['meta']['X'], '1')
 
     def test_uri_file_transfer_basic(self) -> None:
         """URI file request sends the content of a regular file as t=r chunks."""