Marco Ricci commited on 2026-06-13 18:52:55
Zeige 1 geänderte Dateien mit 89 Einfügungen und 1 Löschungen.
Detect and work around `ssh-agent` (OpenSSH 10.3 only) responding twice when answering a "query" extension request: once with the expected "extension response" or "success" message, and once with an unexpected/additional empty "success" message. We detect the extra empty "success" message in a generic and protocol compliant way, and re-align request-response-pair boundaries for any future requests. From the agent's perspective, the work-around does a read-only operation and should not change any permanent agent state, whether there are extra empty "success" messages or not. As written, the work-around tests specifically for output from OpenSSH 10.3, but is otherwise agnostic to the "extension response" message contents and could be easily adapted to trigger for other content patterns.
| ... | ... |
@@ -32,6 +32,79 @@ __all__ = ("SSHAgentClient",)
|
| 32 | 32 |
# a 4-byte/32-bit unsigned integer at the beginning. |
| 33 | 33 |
HEAD_LEN = 4 |
| 34 | 34 |
|
| 35 |
+# Needed for handling OpenSSH bug 3967. See |
|
| 36 |
+# _check_for_extra_success_response_after_query_extension_request. |
|
| 37 |
+_OPENSSH_AGENT_10_3_QUERY_EXTENSION_RESPONSE_PAYLOAD = bytes.fromhex("""
|
|
| 38 |
+00 00 00 05 71 75 65 72 79 |
|
| 39 |
+00 00 00 18 73 65 73 73 69 6f 6e 2d |
|
| 40 |
+62 69 6e 64 40 6f 70 65 6e 73 73 68 2e 63 6f 6d |
|
| 41 |
+""") |
|
| 42 |
+ |
|
| 43 |
+ |
|
| 44 |
+def _check_for_extra_empty_success_response_after_query_extension_request( |
|
| 45 |
+ client: SSHAgentClient, response_data: bytes | bytearray |
|
| 46 |
+) -> None: |
|
| 47 |
+ """Check for extra `SUCCESS` responses after a "query" extension request. |
|
| 48 |
+ |
|
| 49 |
+ OpenSSH's `ssh-agent` version 10.3 erroneously answers a "query" |
|
| 50 |
+ extension request with two response messages: the expected |
|
| 51 |
+ [`EXTENSION_RESPONSE`][_types.SSH_AGENT.EXTENSION_RESPONSE] or |
|
| 52 |
+ [`SUCCESS`][_types.SSH_AGENT.SUCCESS] message, and the unexpected |
|
| 53 |
+ extra `SUCCESS` message. This causes a misalignment of the request |
|
| 54 |
+ and response messages if further requests are later issued to the |
|
| 55 |
+ agent. |
|
| 56 |
+ |
|
| 57 |
+ ([OpenSSH acknowledges this as Bug 3967][OPENSSH_BZ3967], and |
|
| 58 |
+ intends to fix this in version 10.4.) |
|
| 59 |
+ |
|
| 60 |
+ This callback is intended to run immediately after issuing a "query" |
|
| 61 |
+ extension request to the agent, and after reading its response. If |
|
| 62 |
+ we detect the aforementioned OpenSSH behavior, then we re-align the |
|
| 63 |
+ request and response messages queue. Otherwise, we do nothing. In |
|
| 64 |
+ particular, it is safe to call us (at the aforementioned time) even |
|
| 65 |
+ on different SSH agents, or on versions of OpenSSH's agent that do |
|
| 66 |
+ not exhibit this bug. |
|
| 67 |
+ |
|
| 68 |
+ [OPENSSH_BZ3967]: https://bugzilla.mindrot.org/show_bug.cgi?id=3967 |
|
| 69 |
+ |
|
| 70 |
+ Args: |
|
| 71 |
+ client: |
|
| 72 |
+ The client connected to the SSH agent that may or may not |
|
| 73 |
+ exhibit this OpenSSH behavior. We assume that the client |
|
| 74 |
+ has just issued a "query" extension request, that the client |
|
| 75 |
+ has read and parsed the (first) response of the agent, that |
|
| 76 |
+ the response message was an `EXTENSION_RESPONSE` or |
|
| 77 |
+ a `SUCCESS` message, and that the response message (except |
|
| 78 |
+ for the response code) is exactly the contents of the |
|
| 79 |
+ `response_data` argument. |
|
| 80 |
+ response_data: |
|
| 81 |
+ The payload of the response message to a "query" extension |
|
| 82 |
+ request issued to the SSH agent immediately prior. |
|
| 83 |
+ |
|
| 84 |
+ """ |
|
| 85 |
+ if response_data == _OPENSSH_AGENT_10_3_QUERY_EXTENSION_RESPONSE_PAYLOAD: |
|
| 86 |
+ # We cannot blindly read from the agent socket to check |
|
| 87 |
+ # for a possible extra SUCCESS response: that would |
|
| 88 |
+ # deadlock if the extra response is missing. Instead, |
|
| 89 |
+ # we issue a second "query" request, and depending on |
|
| 90 |
+ # whether we next see the extra empty SUCCESS response or not, |
|
| 91 |
+ # we know whether to discard an additional two response |
|
| 92 |
+ # messages or not. |
|
| 93 |
+ # |
|
| 94 |
+ # (Principally any request that does *not* ellicit |
|
| 95 |
+ # a SUCCESS response and which does not modify the |
|
| 96 |
+ # agent's state would do for this.) |
|
| 97 |
+ response_code, response_data2 = client.request( |
|
| 98 |
+ _types.SSH_AGENTC.REQUEST_IDENTITIES, b"", response_code=None |
|
| 99 |
+ ) |
|
| 100 |
+ if response_code == _types.SSH_AGENT.SUCCESS and not response_data2: |
|
| 101 |
+ response_code, response_data2 = client._get_response_pair() # noqa: SLF001 |
|
| 102 |
+ assert response_code == _types.SSH_AGENT.IDENTITIES_ANSWER, ( |
|
| 103 |
+ "failed to re-synchronize with agent via REQUEST_IDENTITIES " |
|
| 104 |
+ 'after "query" EXTENSION_REQUEST: want IDENTITIES_ANSWER, ' |
|
| 105 |
+ f"got {response_code!r}, {response_data2!r}"
|
|
| 106 |
+ ) |
|
| 107 |
+ |
|
| 35 | 108 |
|
| 36 | 109 |
class TrailingDataError(RuntimeError): |
| 37 | 110 |
"""The result contained trailing data.""" |
| ... | ... |
@@ -736,6 +809,17 @@ class SSHAgentClient: |
| 736 | 809 |
|
| 737 | 810 |
""" |
| 738 | 811 |
try: |
| 812 |
+ # The draft-miller-ssh-agent-14 (2024-04-15) version of the |
|
| 813 |
+ # SSH agent protocol specification introduced the |
|
| 814 |
+ # SSH_AGENT_EXTENSION_RESPONSE protocol message, and |
|
| 815 |
+ # suggested the use of this message in the response for |
|
| 816 |
+ # SSH_AGENTC_EXTENSION requests. Older versions of the spec |
|
| 817 |
+ # used the SSH_AGENT_SUCCESS response, and even the newer |
|
| 818 |
+ # spec versions contain wording that allows such usage |
|
| 819 |
+ # across all SSH_AGENTC_EXTENSION requests. Both response |
|
| 820 |
+ # codes are used in the wild (`ssh-agent`/OpenSSH 10.3, |
|
| 821 |
+ # `pageant`/PuTTY 0.83), so both of them need to be |
|
| 822 |
+ # supported for compatibility reasons. |
|
| 739 | 823 |
response_data = self.request( |
| 740 | 824 |
_types.SSH_AGENTC.EXTENSION, |
| 741 | 825 |
self.string(b"query"), |
| ... | ... |
@@ -744,9 +828,13 @@ class SSHAgentClient: |
| 744 | 828 |
_types.SSH_AGENT.SUCCESS, |
| 745 | 829 |
}, |
| 746 | 830 |
) |
| 831 |
+ _check_for_extra_empty_success_response_after_query_extension_request( |
|
| 832 |
+ self, response_data |
|
| 833 |
+ ) |
|
| 747 | 834 |
except SSHAgentFailedError: |
| 748 | 835 |
# Cannot query extension support. Assume no extensions. |
| 749 |
- # This isn't necessarily true, e.g. for OpenSSH's ssh-agent. |
|
| 836 |
+ # This isn't necessarily true, e.g. for OpenSSH's ssh-agent |
|
| 837 |
+ # 10.2 and older. |
|
| 750 | 838 |
return frozenset() |
| 751 | 839 |
extensions: set[bytes] = set() |
| 752 | 840 |
msg = "Malformed response from SSH agent" |
| 753 | 841 |