IETF-SSH archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



I've cc'd this discussion to the ietf mailing list
(and removed most of the other cc's)

Erwin Bolwidt wrote:

Dear editors of the IETF sftp specification,

I would like to make two comments on the draft-ietf-secsh-filexfer-05.txt specification, and one question:

- on the max-read-size field in the "supported" extension.
- on the SSH_FXP_EXTENDED_REPLY packet structure
- should the name of an extension be encoded as US-ASCII or UTF-8?

1. The max-read-size field in the "supported" extension

I believe that the specification is too strict where it defined the max-read-size field. I quote:


[...]


But how meaningful is this? For a Unix implemention, one can guarantee a certain max-read-size for regular files, but for character devices or named pipes, such a guarantee cannot be made, unless reads are peformed in a blockin fashion.

The specification of the SSH_FXP_DATA packet mentions this: " The data string may be at most the number of bytes requested in a SSH_FXP_READ request, but may also be shorter if end of file is reached or if the read is from something other than a regular file."

Wouldn't it be more meaningful to relax the defintion of max-read-size to only include files of type SSH_FILEXFER_TYPE_REGULAR ?

I intend to remove this field for the next version (which I hope to
ship soon.)

The motivation behind this is that when using multiple overlapped
reads to read file data, getting a short read is difficult to handle.

However, you raise several good points (and I think someone else
wasn't very fond of the field either.)  I myself was not terribly fond
of the field, but was trying to solve a problem.

I also now suspect that a client that correctly handles out-of-order
request completion correctly, will have less trouble handling short
reads.

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'?)

Of course, as an extension author, you can include the name if you
wish for extensions you are designing...

3. should the name of an extension be encoded as US-ASCII or UTF-8

The specification doesn't mention an encoding for extension names in the places where they are used. I assume that they should be US-ASCII then, is this true?

It is indeed US-ASCII. I'll update the draft to clarify this.

Thanks,

Joseph



Home | Main Index | Thread Index | Old Index