Re: Comments on draft-ietf-secsh-filexfer-05.txt

[Erwin Bolwidt <erwin%klomp.org@localhost>]

Joseph Galbraith wrote:

> Erwin Bolwidt wrote:
>
>
> > 2. the SSH_FXP_EXTENDED_REPLY packet structure
> >
> > The structure of this packet is simple:
> >
> > byte   SSH_FXP_EXTENDED_REPLY
> > uint32 request-id
> > ... any request-specific data ...
> >
> > It differs from the other packets that use extension by the lack of an 
> > extension name.
> >
> > In the implementation of an SFTP server that I am creating, I have so 
> > far been able to parse packets free of any session information, and I 
> > think this is a nice and clean design: the packet layer deals with the 
> > wire protocol, and the session layer with packets parsed into memory 
> > structures.
> >
> > This is not possible in the case of the SSH_FXP_EXTENDED_REPLY packet. 
> > I assume that this problem has been observed by you, as the only 
> > extension for this packet that is defined in the specification, the 
> > "md5-hash" reply, includes an extension name as part of the 
> > request-specific data.
> >
> > My comment is, shouldn't the specification mandate the inclusion of an 
> > extension-name in the SSH_FXP_EXTENDED_REPLY packet? For compatibility 
> > with older protocol versions, this could be a requirement from a 
> > certain version of the protocol on.
>
>
> Actually, the fact that the EXTENDED_REPLY for the "md5-hash" extension
> includes the name is a typo-- I'll remove it in the next revision.
>
> The extended reply includes the request-id of the request, which
> should allow the client to match up request and response.
>
> I'm not sure why the server would need session information in order
> to send the reply w/o the name?  Perhaps you could be more specific
> about what you mean?  (Currently, I'm reading 'session information'
> as 'saved state'?)

I'm not only talking about the server; I wrote a generic library for 
both server and client, that translates the bytes of a packet into an 
object of an object-oriented language, in this case Java.
So far, for any packet except SSH_FXP_EXTENDED_REPLY, the library 
doesn't need any information to parse a packet, except for the packet 
itself. A method like this is sufficient:

public static SftpPacket parse(byte[] packetBytes);

Not in the case of SSH_FXP_EXTENDED_REPLY; to parse it, I would need to 
remember the SSH_FXP_EXTENDED packet to which it is a reply, in order to 
be able to parse the reply.

public static SftpPacket parse(SentPackets sent, byte[] packetBytes);

While parsing a packet was a straightforward, low-layer activity, it is 
now necessary to manage the information about sent packets. This makes 
the software design less transparent, and more prone to bugs.

If you were to write a protocol analyzer, this also creates problems: 
now the analyzer needs to remember sent packets to parse the response, 
and if the sent packet was missed, it cannot anymore.


I do realize that this argument can also be made against dynamically 
using packet types in the RESERVED_FOR_EXTENSIONS range; this is also 
something that I would rather avoid; SSH_FXP_EXTENDED and 
SSH_FXP_EXTENDED_REPLY should be sufficient for any purpose, in my opinion.


- Erwin Bolwidt