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

From: "Richard Whalen" <[email-masked]>
Date: Thu, 21 Apr 2005 10:58:37 -0400

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

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)

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.)

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.

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.

---

From: Joseph Galbraith <[email-masked]>
Date: Thu, 21 Apr 2005 10:54:19 -0600

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

Congratulations!

> "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
 > 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

---

From: der Mouse <[email-masked]>
Date: Thu, 21 Apr 2005 13:09:33 -0400 (EDT)

>> 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 [...]
> Hmmm... I didn't specify this because I don't think we know enough
> about how somone might possible use this.

I think this is the right tack to take.  Until we have some experience
with such extensions, I think it's premature to standardize how to do
them.

>> 9.1.2 [...]
> Done.  Do people think this is superior to the MD5 stuff?

I do, because it subsumes the md5-hash stuff: specify an algorithm-list
containing only md5 and it amounts to much the same thing.  The only
difference I see is the quick-check stuff, and that can be done too at
the cost of another round trip.

However, there is one unclear spot.  It's not clear whether there is
any spec for which algorithms the server supports - the text says that
"[c]urrently supported algorithms are [..list..]", but it's not clear
whether that means all servers MUST support all of them, or those are
the algorithms supported by all known implementations, or those are the
algorithms supported by any known implementations, or those are the
algorithms that may be used without DNS extension syntax but there is
no intent to imply that anyone does/should/must support any of them, or
perhaps even something else.

I can't recall whether anyone caught this before, but in case not: in
9, describing "extended-request", I see "..extensions have use the..",
which needs to have "have" struck.

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML	       [email-masked]
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

---

From: Joseph Galbraith <[email-masked]>
Date: Thu, 21 Apr 2005 12:42:57 -0600

> "[c]urrently supported algorithms are [..list..]", but it's not clear

Does it clear it up if I change it to ...currently defined
algorithsm...?

> which needs to have "have" struck.

Thanks, I reworded the section to use the boiler-plate text for
this with the xref to the architecture draft.

I'll probably publish in the next couple of days, so keep the
feedback coming.

Thanks,

Joseph

---

From: der Mouse <[email-masked]>
Date: Thu, 21 Apr 2005 16:31:16 -0400 (EDT)

>> I do, [...].  However, there is one unclear spot.  It's not clear
>> whether there is any spec for which algorithms the server supports -
>> the text says that "[c]urrently supported algorithms are
>> [..list..]", but it's not clear [...]

Yes.  If any algorithms are MUSTs, they should be listed; if not, I'd
prefer to see that explicitly stated, since (almost?) everywhere else,
there's at least one MUST-implement algorithm.  (In case it's not
clear, I have no problem with having no MUST-implement choices here; I
just would like to see it stated unambiguously.)

/~\ The ASCII				der Mouse
\ / Ribbon Campaign