Skip to content

Commit ad23a6d

Browse files
Fix QueryResponse wire format to match IETF spec and OpenSSH
Vec<String>::encode from ssh-encoding adds an outer u32 byte-count prefix that the SSH agent protocol does not use for extension query responses. Replace with flat per-string encoding/decoding to produce wire bytes identical to OpenSSH's process_ext_query. Add compliance test asserting byte-identical output to OpenSSH for the "session-bind@openssh.com" query response. Fixes: #1 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1a88d58 commit ad23a6d

1 file changed

Lines changed: 63 additions & 4 deletions

File tree

src/proto/extension/message.rs

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,28 @@ pub struct QueryResponse {
2525

2626
impl Encode for QueryResponse {
2727
fn encoded_len(&self) -> Result<usize, EncodingError> {
28-
self.extensions.encoded_len()
28+
self.extensions.iter().try_fold(0usize, |acc, s| {
29+
acc.checked_add(s.encoded_len()?)
30+
.ok_or(EncodingError::Length)
31+
})
2932
}
3033

3134
fn encode(&self, writer: &mut impl Writer) -> Result<(), EncodingError> {
32-
self.extensions.encode(writer)
35+
for ext in &self.extensions {
36+
ext.encode(writer)?;
37+
}
38+
Ok(())
3339
}
3440
}
3541

3642
impl Decode for QueryResponse {
3743
type Error = ProtoError;
3844

3945
fn decode(reader: &mut impl Reader) -> Result<Self, Self::Error> {
40-
let extensions = Vec::<String>::decode(reader)?;
41-
46+
let mut extensions = Vec::new();
47+
while !reader.is_finished() {
48+
extensions.push(String::decode(reader)?);
49+
}
4250
Ok(Self { extensions })
4351
}
4452
}
@@ -134,6 +142,7 @@ mod tests {
134142
use testresult::TestResult;
135143

136144
use super::*;
145+
use crate::proto::{Extension, Response};
137146

138147
fn round_trip<T>(msg: T) -> TestResult
139148
where
@@ -149,6 +158,56 @@ mod tests {
149158
Ok(())
150159
}
151160

161+
#[test]
162+
fn query_response_roundtrip() -> TestResult {
163+
round_trip(QueryResponse {
164+
extensions: vec!["session-bind@openssh.com".into(), "foo@example.com".into()],
165+
})
166+
}
167+
168+
#[test]
169+
fn query_response_empty_roundtrip() -> TestResult {
170+
round_trip(QueryResponse { extensions: vec![] })
171+
}
172+
173+
/// Verify that encoding a QueryResponse through Response::ExtensionResponse
174+
/// produces wire bytes identical to OpenSSH's process_ext_query.
175+
///
176+
/// Reference: openssh-portable/ssh-agent.c process_ext_query()
177+
/// Wire format (after outer message frame):
178+
/// byte SSH_AGENT_EXTENSION_RESPONSE (29)
179+
/// string "query"
180+
/// string "session-bind@openssh.com"
181+
/// [end of message]
182+
#[test]
183+
fn query_response_matches_openssh_wire_format() -> TestResult {
184+
let query = QueryResponse {
185+
extensions: vec!["session-bind@openssh.com".into()],
186+
};
187+
let response = Response::ExtensionResponse(Extension::new_message(query)?);
188+
189+
let mut buf: Vec<u8> = vec![];
190+
response.encode(&mut buf)?;
191+
192+
#[rustfmt::skip]
193+
let expected: &[u8] = &[
194+
// byte: SSH_AGENT_EXTENSION_RESPONSE
195+
0x1d,
196+
// string: "query"
197+
0x00, 0x00, 0x00, 0x05,
198+
0x71, 0x75, 0x65, 0x72, 0x79,
199+
// string: "session-bind@openssh.com"
200+
0x00, 0x00, 0x00, 0x18,
201+
0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x2d,
202+
0x62, 0x69, 0x6e, 0x64, 0x40, 0x6f, 0x70, 0x65,
203+
0x6e, 0x73, 0x73, 0x68, 0x2e, 0x63, 0x6f, 0x6d,
204+
];
205+
206+
assert_eq!(buf, expected);
207+
208+
Ok(())
209+
}
210+
152211
#[test]
153212
fn parse_bind() -> TestResult {
154213
let mut buffer: &[u8] = &[

0 commit comments

Comments
 (0)