Re-align the stubbed SSH agent socket with "real world" behavior
Marco Ricci

Marco Ricci commited on 2026-06-18 06:15:10
Zeige 3 geänderte Dateien mit 255 Einfügungen und 87 Löschungen.


Let the stubbed SSH agent tolerate incomplete request messages, or
multiple request messages at once, within the same `sendall` call.
Furthermore, change the error type to OSError (EBADF) for I/O on closed
socket connections (mirroring the behavior of UNIX domain sockets) and
when closing the connection, assert that no incomplete requests have
been issued to the agent.

With this change, the stubbed SSH agent socket now usefully models the
communication channel between agent and client, and can be sensibly used
to simulate communication and protocol errors (or their absence).  This
differentiates it—in terms of scope and use cases—from the agent
protocol response queue, which monkeypatches the client instead of
faking a server, and which operates on full request/response messages.
Add some commentary to the stubbed SSH agent socket docstring to this
effect.
... ...
@@ -687,7 +687,7 @@ class SSH_AGENTC(int, enum.Enum):  # noqa: N801
687 687
     EXTENSION = 27
688 688
     """"""
689 689
 
690
-    def __bytes__(self) -> bytes:
690
+    def __bytes__(self) -> bytes:  # pragma: no cover [unused]
691 691
         """Return the corresponding request code."""
692 692
         return int.to_bytes(self, 1, "big", signed=False)
693 693
 
... ...
@@ -724,7 +724,7 @@ class SSH_AGENT(int, enum.Enum):  # noqa: N801
724 724
     EXTENSION_RESPONSE = 29
725 725
     """"""
726 726
 
727
-    def __bytes__(self) -> bytes:
727
+    def __bytes__(self) -> bytes:  # pragma: no cover [unused]
728 728
         """Return the corresponding response code."""
729 729
         return int.to_bytes(self, 1, "big", signed=False)
730 730
 
... ...
@@ -308,12 +308,15 @@ class AgentProtocolResponseQueue:
308 308
     ([`AgentProtocolRequest`][data.AgentProtocolRequest]) to multiple
309 309
     response messages
310 310
     ([`AgentProtocolResponse`][data.AgentProtocolResponse]).  Whenever
311
-    [`_request_message`][] is called with a payload that matches
311
+    [`_send_request_message`][] is called with a payload that matches
312 312
     a [`AgentProtocolRequest`][data.AgentProtocolRequest] from the map,
313 313
     the corresponding
314 314
     [`AgentProtocolResponse`][data.AgentProtocolResponse] objects are
315
-    added to the queue, to be read by subsequent [`_response_pair`][]
316
-    calls.
315
+    added to the queue, to be read by subsequent
316
+    [`_get_response_pair`][] calls.  The [`_send_request_message`][] and
317
+    [`_get_response_pair`][] methods are function- and
318
+    interface-compatible with the internal methods of the same name in
319
+    [`ssh_agent.SSHAgentClient`][].
317 320
 
318 321
     The queue is a context manager.  When entered or exited, it
319 322
     verifies that the queue is empty.
... ...
@@ -390,13 +393,37 @@ class AgentProtocolResponseQueue:
390 393
     _types.BuiltinSSHAgentSocketProvider.STUB_AGENT
391 394
 )
392 395
 class StubbedSSHAgentSocket:
393
-    """A stubbed SSH agent presenting an [`_types.SSHAgentSocket`][]."""
396
+    """A stubbed SSH agent presenting an [`_types.SSHAgentSocket`][].
397
+
398
+    On the network protocol side, the agent implements the full
399
+    [`_types.SSHAgentSocket`][] interface, including pipelined and
400
+    unaligned agent requests.  However, on the application side, the
401
+    agent is intrinsically tied to [the set of SSH test
402
+    keys][data.ALL_KEYS], and only gives meaningful answers for
403
+    operations on the test keys and for agent operations in use by
404
+    [`ssh_agent.SSHAgentClient`][].  The agent does not actually
405
+    implement any cryptography; all cryptography-related answers are
406
+    derived from the recorded test key data.
407
+
408
+    It is not safe to further monkeypatch the agent's [`recv`][] or
409
+    [`sendall`][] methods on their own: either monkeypatch both of them,
410
+    or manipulate the [`send_to_client`][] bytes queue directly instead.
411
+    Given an [`ssh_agent.SSHAgentClient`][] connected to
412
+    a [`StubbedSSHAgentSocket`][], if the test ensures proper message
413
+    serialization and protocol framing and if the monkeypatching can be
414
+    expressed in terms of full request messages and full response
415
+    messages, prefer using a [`AgentProtocolResponseQueue`][] to
416
+    monkeypatch the client's high-level request/response-loop instead of
417
+    monkeypatching the low-level socket communication in this agent
418
+    socket.
419
+
420
+    """
394 421
 
395
-    _SOCKET_IS_CLOSED = "Socket is closed."
396 422
     _NO_FLAG_SUPPORT = "This stubbed SSH agent socket does not support flags."
397 423
     _PROTOCOL_VIOLATION = "SSH agent protocol violation."
398 424
     _INVALID_REQUEST = "Invalid request."
399 425
     _UNSUPPORTED_REQUEST = "Unsupported request."
426
+    _INCOMPLETE_REQUEST = "The last request was incomplete."
400 427
 
401 428
     HEADER_SIZE = 4
402 429
     CODE_SIZE = 1
... ...
@@ -435,8 +462,15 @@ class StubbedSSHAgentSocket:
435 462
         return self
436 463
 
437 464
     def __exit__(self, *args: object) -> None:
438
-        """Mark the agent's socket as closed."""
465
+        """Mark the agent's socket as closed.
466
+
467
+        Raises:
468
+            AssertionError:
469
+                The last request to the agent was incomplete.
470
+
471
+        """
439 472
         self.closed = True
473
+        assert not self.receive_from_client, self._INCOMPLETE_REQUEST
440 474
 
441 475
     def sendall(self, data: Buffer, flags: int = 0, /) -> None:
442 476
         """Send data to the SSH agent.
... ...
@@ -458,34 +492,66 @@ class StubbedSSHAgentSocket:
458 492
         Raises:
459 493
             AssertionError:
460 494
                 The flags argument, if specified, must be 0.
461
-            ValueError:
462
-                The agent's socket is already closed.  No further
463
-                requests can be sent.
495
+            OSError:
496
+                The socket connection is already closed.
464 497
 
465 498
         """
466 499
         assert not flags, self._NO_FLAG_SUPPORT
467
-        if self.closed:
468
-            raise ValueError(self._SOCKET_IS_CLOSED)
500
+        self._check_for_io_on_closed_connection()
469 501
         self.receive_from_client.extend(memoryview(data))
470
-        try:
471
-            self.parse_client_request_and_dispatch()
472
-        except ValueError:
473
-            payload = int.to_bytes(_types.SSH_AGENT.FAILURE, 1, "big")
474
-            self.send_to_client.extend(int.to_bytes(len(payload), 4, "big"))
475
-            self.send_to_client.extend(payload)
476
-        finally:
477
-            self.receive_from_client.clear()
502
+        while self.receive_from_client:
503
+            result: Buffer | Iterator[int]
504
+            if len(self.receive_from_client) < self.HEADER_SIZE:
505
+                break
506
+            count = int.from_bytes(
507
+                self.receive_from_client[: self.HEADER_SIZE],
508
+                "big",
509
+                signed=False,
510
+            )
511
+            if count:
512
+                code = int.from_bytes(
513
+                    self.receive_from_client[
514
+                        self.HEADER_SIZE : self.HEADER_SIZE + self.CODE_SIZE
515
+                    ],
516
+                    "big",
517
+                    signed=False,
518
+                )
519
+                request = bytes(
520
+                    self.receive_from_client[: self.HEADER_SIZE + count]
521
+                )
522
+                if len(request) < self.HEADER_SIZE + count:
523
+                    break
524
+                request_payload = request[self.HEADER_SIZE + self.CODE_SIZE :]
525
+
526
+                if code == _types.SSH_AGENTC.REQUEST_IDENTITIES:
527
+                    result = self.request_identities(list_extended=False)
528
+                elif code == _types.SSH_AGENTC.SIGN_REQUEST:
529
+                    result = self.sign(request_payload)
530
+                elif self._check_for_extension(code, "query"):
531
+                    result = self.query_extensions()
532
+                elif self._check_for_extension(
533
+                    code, "list-extended@putty.projects.tartarus.org"
534
+                ):
535
+                    result = self.request_identities(list_extended=True)
536
+                else:
537
+                    result = self._failure()
538
+            else:
539
+                request = bytes(self.receive_from_client[: self.HEADER_SIZE])
540
+                result = self._failure()
541
+            self.send_to_client.extend(
542
+                ssh_agent.SSHAgentClient.string(bytes(result))
543
+            )
544
+            self.receive_from_client[: len(request)] = b""
478 545
 
479 546
     def recv(self, count: int, flags: int = 0, /) -> bytes:
480 547
         """Read data from the SSH agent.
481 548
 
482 549
         As per the SSH agent protocol, data is only available to be read
483
-        immediately after a request via [`sendall`][].  Calls to
484
-        [`recv`][] at other points in time that attempt to read data
485
-        violate the protocol, and will fail.  Notwithstanding the last
486
-        sentence, at any point in time, though pointless, it is
487
-        additionally permissible to read 0 bytes from the agent, or any
488
-        number of bytes from a closed socket.
550
+        immediately after a request via [`sendall`][] and if the socket
551
+        connection is still open.  Calls to [`recv`][] at other points
552
+        in time that attempt to read data violate the protocol, and will
553
+        fail.  (A [`recv`][] of zero bytes does not read data.)  Calls
554
+        to [`recv`][] when the socket connection is closed always fail.
489 555
 
490 556
         Args:
491 557
             count:
... ...
@@ -495,8 +561,8 @@ class StubbedSSHAgentSocket:
495 561
 
496 562
         Returns:
497 563
             (A chunk of) the SSH agent's response to the most recent
498
-            request.  If reading 0 bytes, or if reading from a closed
499
-            socket, the returned chunk is always an empty byte string.
564
+            request.  If reading 0 bytes, the returned chunk is always
565
+            an empty byte string.
500 566
 
501 567
         Raises:
502 568
             AssertionError:
... ...
@@ -505,44 +571,25 @@ class StubbedSSHAgentSocket:
505 571
                 Alternatively, `recv` was called when there was no
506 572
                 response to be obtained, in violation of the SSH agent
507 573
                 protocol.
574
+            OSError:
575
+                The socket connection is already closed.
508 576
 
509 577
         """
510 578
         assert not flags, self._NO_FLAG_SUPPORT
511
-        assert not count or self.closed or self.send_to_client, (
512
-            self._PROTOCOL_VIOLATION
513
-        )
579
+        self._check_for_io_on_closed_connection()
580
+        assert not count or self.send_to_client, self._PROTOCOL_VIOLATION
514 581
         ret = bytes(self.send_to_client[:count])
515 582
         del self.send_to_client[:count]
516 583
         return ret
517 584
 
518
-    def parse_client_request_and_dispatch(self) -> None:
519
-        """Parse the client request and call the matching handler.
585
+    def _failure(self) -> bytes:
586
+        return bytes(_types.SSH_AGENT.FAILURE)
520 587
 
521
-        This agent supports the
522
-        [`SSH_AGENTC_REQUEST_IDENTITIES`][_types.SSH_AGENTC.REQUEST_IDENTITIES],
523
-        [`SSH_AGENTC_SIGN_REQUEST`][_types.SSH_AGENTC.SIGN_REQUEST] and
524
-        the [`SSH_AGENTC_EXTENSION`][_types.SSH_AGENTC.EXTENSION]
525
-        request types.
526
-
527
-        """
528
-
529
-        if len(self.receive_from_client) < self.HEADER_SIZE + self.CODE_SIZE:
530
-            raise ValueError(self._INVALID_REQUEST)
531
-        target_header = ssh_agent.SSHAgentClient.uint32(
532
-            len(self.receive_from_client) - self.HEADER_SIZE
533
-        )
534
-        if target_header != self.receive_from_client[: self.HEADER_SIZE]:
535
-            raise ValueError(self._INVALID_REQUEST)
536
-        code = _types.SSH_AGENTC(
537
-            int.from_bytes(
538
-                self.receive_from_client[
539
-                    self.HEADER_SIZE : self.HEADER_SIZE + self.CODE_SIZE
540
-                ],
541
-                "big",
542
-            )
543
-        )
588
+    def _check_for_io_on_closed_connection(self) -> None:
589
+        if self.closed:
590
+            raise OSError(errno.EBADF, os.strerror(errno.EBADF))
544 591
 
545
-        def is_enabled_extension(extension: str) -> bool:
592
+    def _check_for_extension(self, code: int, extension: str) -> bool:
546 593
         if (
547 594
             extension not in self.enabled_extensions
548 595
             or code != _types.SSH_AGENTC.EXTENSION
... ...
@@ -552,21 +599,6 @@ class StubbedSSHAgentSocket:
552 599
         extension_marker = b"\x1b" + string(extension.encode("ascii"))
553 600
         return self.receive_from_client.startswith(extension_marker, 4)
554 601
 
555
-        result: Buffer | Iterator[int]
556
-        if code == _types.SSH_AGENTC.REQUEST_IDENTITIES:
557
-            result = self.request_identities(list_extended=False)
558
-        elif code == _types.SSH_AGENTC.SIGN_REQUEST:
559
-            result = self.sign()
560
-        elif is_enabled_extension("query"):
561
-            result = self.query_extensions()
562
-        elif is_enabled_extension("list-extended@putty.projects.tartarus.org"):
563
-            result = self.request_identities(list_extended=True)
564
-        else:
565
-            raise ValueError(self._UNSUPPORTED_REQUEST)
566
-        self.send_to_client.extend(
567
-            ssh_agent.SSHAgentClient.string(bytes(result))
568
-        )
569
-
570 602
     def query_extensions(self) -> Iterator[int]:
571 603
         """Answer an `SSH_AGENTC_EXTENSION` request.
572 604
 
... ...
@@ -633,9 +665,14 @@ class StubbedSSHAgentSocket:
633 665
                     ssh_agent.SSHAgentClient.uint32(0)
634 666
                 )
635 667
 
636
-    def sign(self) -> bytes:
668
+    def sign(self, request_payload: bytes, /) -> bytes:
637 669
         """Answer an `SSH_AGENTC_SIGN_REQUEST` request.
638 670
 
671
+        Args:
672
+            request_payload:
673
+                The data of the sign request, without the protocol
674
+                framing or the request code.
675
+
639 676
         Returns:
640 677
             The bytes payload of the response, without the protocol
641 678
             framing.
... ...
@@ -647,17 +684,20 @@ class StubbedSSHAgentSocket:
647 684
         )
648 685
         spec = data.SSHTestKeyDeterministicSignatureClass.SPEC
649 686
         rfc6979 = data.SSHTestKeyDeterministicSignatureClass.RFC_6979
687
+        try:
650 688
             key_blob, rest = ssh_agent.SSHAgentClient.unstring_prefix(
651
-            self.receive_from_client[self.HEADER_SIZE + self.CODE_SIZE :]
689
+                request_payload
652 690
             )
653 691
             sign_data, rest = ssh_agent.SSHAgentClient.unstring_prefix(rest)
692
+        except ValueError:
693
+            return self._failure()
654 694
         if len(rest) != 4:
655
-            raise ValueError(self._INVALID_REQUEST)
695
+            return self._failure()
656 696
         flags = int.from_bytes(rest, "big")
657 697
         if flags:
658
-            raise ValueError(self._UNSUPPORTED_REQUEST)
698
+            return self._failure()
659 699
         if sign_data != vault.Vault.UUID:
660
-            raise ValueError(self._UNSUPPORTED_REQUEST)
700
+            return self._failure()
661 701
         for key in data.ALL_KEYS.values():
662 702
             if key.public_key_data == key_blob:
663 703
                 if spec in key.expected_signatures:
... ...
@@ -672,8 +712,8 @@ class StubbedSSHAgentSocket:
672 712
                     ) + ssh_agent.SSHAgentClient.string(
673 713
                         key.expected_signatures[rfc6979].signature
674 714
                     )
675
-                raise ValueError(self._UNSUPPORTED_REQUEST)
676
-        raise ValueError(self._UNSUPPORTED_REQUEST)
715
+                return self._failure()
716
+        return self._failure()
677 717
 
678 718
 
679 719
 # Standard variant
... ...
@@ -15,12 +15,15 @@ from __future__ import annotations
15 15
 import base64
16 16
 import contextlib
17 17
 import errno
18
+import math
18 19
 import os
19 20
 import pathlib
20 21
 import re
21 22
 from typing import TYPE_CHECKING
22 23
 
24
+import hypothesis
23 25
 import pytest
26
+from hypothesis import strategies
24 27
 
25 28
 from derivepassphrase import _types, ssh_agent, vault
26 29
 from tests import data, machinery
... ...
@@ -191,7 +194,6 @@ class Parametrize:
191 194
         "message",
192 195
         [
193 196
             pytest.param(b"\x00\x00\x00\x00", id="empty-message"),
194
-            pytest.param(b"\x00\x00\x00\x0f\x0d", id="truncated-message"),
195 197
             pytest.param(
196 198
                 b"\x00\x00\x00\x06\x1b\x00\x00\x00\x01\xff",
197 199
                 id="invalid-extension-name",
... ...
@@ -200,6 +202,10 @@ class Parametrize:
200 202
                 b"\x00\x00\x00\x11\x0d\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
201 203
                 id="sign-with-trailing-data",
202 204
             ),
205
+            pytest.param(
206
+                b"\x00\x00\x00\x05\x0d\x00\x00\x00\x00",
207
+                id="sign-without-payload",
208
+            ),
203 209
         ],
204 210
     )
205 211
     UNSUPPORTED_SSH_AGENT_MESSAGES = pytest.mark.parametrize(
... ...
@@ -243,6 +249,8 @@ class Parametrize:
243 249
                     ])
244 250
                 ),
245 251
                 id="sign-key-no-expected-signature",
252
+                # NOTE: only unsupported when stubbed agent has RFC 6979
253
+                # support disabled
246 254
             ),
247 255
             pytest.param(
248 256
                 ssh_agent.SSHAgentClient.string(
... ...
@@ -264,6 +272,79 @@ class Parametrize:
264 272
     )
265 273
 
266 274
 
275
+class Strategies:
276
+    """Common hypothesis data generation strategies."""
277
+
278
+    @strategies.composite
279
+    @staticmethod
280
+    def proper_bytestring_partition(
281
+        draw: strategies.DrawFn, bs: bytes | bytearray, /
282
+    ) -> list[bytes]:
283
+        """Partition a non-empty string into non-empty parts.
284
+
285
+        A string of length `n` is partitioned [`math.isqrt(n
286
+        - 1)`][math.isqrt] many times, such that each substring is
287
+        non-empty.
288
+
289
+        """
290
+        n = len(bs)
291
+        hypothesis.assume(n > 0)
292
+        num_divisions = math.isqrt(n - 1)
293
+        divisions = draw(
294
+            strategies.lists(
295
+                strategies.integers(1, n - 1),
296
+                min_size=num_divisions,
297
+                max_size=num_divisions,
298
+                unique=True,
299
+            ).map(sorted),
300
+            label="divisions",
301
+        )
302
+        result: list[bytes] = []
303
+        i = 0
304
+        for div in divisions:
305
+            result.append(bytes(bs[i:div]))
306
+            i = div
307
+        result.append(bytes(bs[i:n]))
308
+        return result
309
+
310
+    @strategies.composite
311
+    @staticmethod
312
+    def truncated_agent_request(
313
+        draw: strategies.DrawFn, /, min_size: int = 0, max_size: int = 100
314
+    ) -> bytes:
315
+        """Generate a private-use SSH agent request with truncated payload.
316
+
317
+        The message will adhere to specified size bounds before
318
+        truncation.  The request message code will be from the private
319
+        use area.
320
+
321
+        Args:
322
+            min_size:
323
+                The minimum size of the request payload, after truncation.
324
+            max_size:
325
+                The maximum size of the request payload, before truncation.
326
+
327
+        """
328
+        request_code = draw(
329
+            strategies.integers(240, 255), label="request_code"
330
+        )
331
+        full_message_size = draw(
332
+            strategies.integers(min_size + 1, max_size),
333
+            label="full_message_size",
334
+        )
335
+        truncated_message = draw(
336
+            strategies.binary(
337
+                min_size=min_size, max_size=full_message_size - 1
338
+            )
339
+        )
340
+        payload = bytearray(
341
+            int.to_bytes(1 + full_message_size, 4, "big", signed=False)
342
+        )
343
+        payload.extend(int.to_bytes(request_code, 1, "big", signed=False))
344
+        payload.extend(truncated_message)
345
+        return bytes(payload)
346
+
347
+
267 348
 class TestTestKeys:
268 349
     """Tests testing the test keys."""
269 350
 
... ...
@@ -543,13 +624,10 @@ class TestStubbedSSHAgentSocketProperOperations:
543 624
             # payload: SSH string "query"
544 625
             b"\x00\x00\x00\x05query"
545 626
         )
546
-        query_response = b""
547
-        with pytest.raises(
548
-            ValueError,
549
-            match=re.escape(machinery.StubbedSSHAgentSocket._SOCKET_IS_CLOSED),
550
-        ):
627
+        with pytest.raises(OSError, match=re.escape(os.strerror(errno.EBADF))):
551 628
             agent.sendall(query_request)
552
-        assert agent.recv(100) == query_response
629
+        with pytest.raises(OSError, match=re.escape(os.strerror(errno.EBADF))):
630
+            agent.recv(100)
553 631
 
554 632
     def test_no_recv_without_sendall(self) -> None:
555 633
         """The agent requires a message before sending a response."""
... ...
@@ -562,6 +640,43 @@ class TestStubbedSSHAgentSocketProperOperations:
562 640
             ):
563 641
                 agent.recv(100)
564 642
 
643
+    @hypothesis.given(
644
+        query_request_parts=Strategies.proper_bytestring_partition(
645
+            b"\x00\x00\x00\x0a\x1b\x00\x00\x00\x05query"
646
+        )
647
+    )
648
+    @hypothesis.example(
649
+        query_request_parts=[
650
+            b"",
651
+            b"\x00\x00\x00",
652
+            b"\x0a",
653
+            b"\x1b",
654
+            b"\x00\x00\x00\x05",
655
+            b"query",
656
+        ]
657
+    )
658
+    def test_piecemeal_sendall(self, query_request_parts: list[bytes]) -> None:
659
+        """The agent supports receiving messages incrementally."""
660
+        with machinery.StubbedSSHAgentSocket() as agent:
661
+            agent.enabled_extensions = frozenset({"query"})
662
+            for part in query_request_parts:
663
+                agent.sendall(part)
664
+            header = agent.recv(agent.HEADER_SIZE)
665
+            count = int.from_bytes(header, "big", signed=False)
666
+            payload = agent.recv(count)
667
+            assert len(payload) == count
668
+            assert bytes.startswith(
669
+                payload,
670
+                (
671
+                    bytes(_types.SSH_AGENT.SUCCESS),
672
+                    bytes(_types.SSH_AGENT.EXTENSION_RESPONSE),
673
+                ),
674
+            )
675
+            code = payload[: agent.CODE_SIZE]
676
+            assert code == bytes(_types.SSH_AGENT.SUCCESS) or code == bytes(
677
+                _types.SSH_AGENT.EXTENSION_RESPONSE
678
+            )
679
+
565 680
     @Parametrize.INVALID_SSH_AGENT_MESSAGES
566 681
     def test_invalid_ssh_agent_messages(
567 682
         self,
... ...
@@ -578,6 +693,19 @@ class TestStubbedSSHAgentSocketProperOperations:
578 693
             agent.sendall(message)
579 694
             assert agent.recv(100) == query_response
580 695
 
696
+    @hypothesis.given(message=Strategies.truncated_agent_request())
697
+    @hypothesis.example(message=b"\x00\x00\x00\x0f\xff")
698
+    def test_truncated_ssh_agent_message(self, message: bytes) -> None:
699
+        """The agent diagnoses a final truncated request message."""
700
+        with pytest.raises(  # noqa: SIM117
701
+            AssertionError,
702
+            match=re.escape(
703
+                machinery.StubbedSSHAgentSocket._INCOMPLETE_REQUEST
704
+            ),
705
+        ):
706
+            with machinery.StubbedSSHAgentSocket() as agent:
707
+                agent.sendall(message)
708
+
581 709
 
582 710
 class TestStubbedSSHAgentSocketSupportedAndUnsupportedFeatures:
583 711
     """Test the stubbed SSH agent socket: supported/unsupported features."""
584 712