Re: I-D ACTION:draft-ietf-secsh-filexfer-08.txt

[Joseph Galbraith <galb-list%vandyke.com@localhost>]

Richard Whalen wrote:
> Sorry for the delay in getting comments in, but I've been out due to my wife having a baby.

Congratulations!

> section 4.6 version re-negotiation
> "If the client and server have negotiated any version higher than version '3'..."
> should be:
> "If the client and server have negotiated any version greater than or equal to version
> '3'..." (or something with similar meaning)

Got this one.

> section 3.3 mentions the possibility of using SSH_FXP_EXTENDED
> to negotiate the uses for packet types 210-255, and refers to
> the section on the extensions.  But the section on extensions (9.)
> does not mention a formal way of negotiating usage.  How about
> something like:
>
> byte SSH_FXP_EXTENDED
> uint32 request-id
> string "negotiate-extension"
> string extension-name
>
> the returned packet would be something of:
> byte SSH_FXP_EXTENDED_REPLY
> uint32 request-id
> uint32 status (SSH_FX_OK, SSH_FX_OP_UNSUPPORTED, SSH_FX_FAILURE)
> uint32 value to use if SSH_FX_OK, optional secondary status if failure
> A status value of SSH_FX_OP_UNSUPPORTED would indicate that the
> "negotiate-extension" extended command is not supported. No
> secondary status is present in this case.
>
> A status value of SSH_FX_FAILURE would indicate that
> "negotiate-extension" is supported, but that a opcode
> number could not be assigned.  A secondary status of
> SSH_FX_OP_UNSUPPORTED would indicate that the requested
> extension is not supported, or negotiation to assign it
> a number is not supported.  (Note that support of the
> extension can be determined by the "supported-features"
> extension in the SSH_FXP_VERSION packet.)

Hmmm... I didn't specify this because I don't think we know
enough about how somone might possible use this.

I'm imagining that the client would send an extension to
the server to request that it start using a feature that
requires addition packet type(s) (and perhaps to
communicate additional information to the server.)

The response from the server would either be SSH_FXP_STATUS
indicating an error, or a SSH_FXP_EXTENDED_REPLY in the
success case.  One of the things the EXTENDED_REPLY packet
would include is the packet type values the server is going
to use for the feature (it might contain other things.)

I like this better because it avoids the needed for the
nested error conditions, allows additional flexibility
(what if additional information needs to be sent to the
server or return by the server?  What if the feature needs
more than one packet type?)

I guess I should probably include this in the draft in some
form.

> 9.1.2 Could "check-file" be modified to be "check-file"
> or "check-file-handle" (The first accepting a filename,
> the second a handle), this would allow the implementation
> to avoid having to do an FXP_OPEN first.

Done.  Do people think this is superior to the MD5
stuff?  This was kind of a off-the-cuff proposal
for a way to match more rsync features w/ sftp.

If people prefer this one to MD5, I'll pull the
md5 one.

> Whether or not the suggested change is made, the
> description for the handle has some awkwardness to it:
> "If ACE4_READ_DATA MUST was not included when the file
> was opened, the server MUST return STATUS_PERMISSION_DENIED."
> The first MUST looks like it is extraneous.

Fixed.

Thanks,

Joseph