Subject: filexfer draft ready for last call?

From: Joseph Galbraith <[email-masked]>
Date: Fri, 07 Jul 2006 17:18:05 -0600

I haven't really been doing much with the filexfer draft
of late.

I'm not sure how far we'll get with a last call, but I'd
kind of like to start heading in that direction.  So, if
anyone has anything that needs to be done, let's hear about
it...

Thanks,

Joseph

---

From: der Mouse <[email-masked]>
Date: Sun, 9 Jul 2006 01:28:00 -0400 (EDT)

> I haven't really been doing much with the filexfer draft of late.

> I'm not sure how far we'll get with a last call, but I'd kind of like
> to start heading in that direction.  So, if anyone has anything that
> needs to be done, let's hear about it...

What's the most recent?  filexfer-12?  That's the most recent I see on
ftp.ietf.org.

First, of course, it still has most of the same character-set issues it
always did.  If you don't want to even read about this again, skip the
next two paragraphs.

Section 6 tries to provide a nod in the direction of dealing with
systems that don't do UTF-8, but it still demands that filenames be
character sequences, which makes it unimplementable on systems (like
traditional Unices) where filenames are octet sequences, with
conversion to and from character sequences performed elsewhere if at
all (usually by the display device and input device, and therefore (a)
in a highly user- and often context-specific manner and (b) well beyond
the power of the ssh/sftp implementation to detect).  The draft
suggests using LC_CTYPE, but gives no hints on how the implementation
is supposed to detect what it is set to for the user in question - is
it expected to include a parser for the user's shell startup files?
(If indeed the user always uses the same LC_CTYPE, which may not be so.)

The simplest fix I see is to add a special charset-name for the
filename-charset extension which means, basically, "raw octet
sequences".  (For concreteness, I suggest "RAW-8BIT".)

There is also a possible problem in that it appears to demand that
files contain sequences of octets.  This could be a problem for, for
example, files (on, eg, a 36-bit machine) containing 9-bit bytes.  I'm
not sure this is worth fixing, given today's net.

Quite aside from that, there are some other issues, mostly but not
entirely minor editorial fixes.  Here are the ones I notice.

> Abstract

>    The SSH File Transfer Protocol provides secure file transfer
>    functionality over any reliable data stream.

s/data/octet/ here.  (I'm assuming "flow-controlled" is subsumed into
"reliable".)  You might also add "bidirectional" for completeness.

>    This protocol provides secure file transfer (and more generally file
>    system access.)

I would prefer to see the period outside the parens.  I don't know
whether this is just a mistake or whether you disagree philosophically
on this point, though other sentences ending with parentheses imply the
former.

>                     It is designed so that it could be used to implement
>    a secure remote file system service, as well as a secure file
>    transfer service.

s/,//

>    This protocol assumes that it runs over a secure channel, such as a
>    channel in [RFC4251], and that the server has already authenticated
>    the client, and that the identity of the client user is available to
>    the protocol.

Strike the first "and".

>    This document owes it's initial creation and protocol design to Tatu
>    Ylonen and Sami Lehtinen of SSH Communications Security Corp.

s/it's/its/

>       There are two packets, INIT and VERSION, which do not use the
>       request-id.
>       Packet descriptions in this document will contain the 'request-id'
>       field, but will not redefine it.

I'd prefer to see a blank line in the middle here, or else these two
sentences run together into a single paragraph.

>                             In other words, only fields after the last
>    field the implementation wishes to send are actually options.

s/options/optional/

> 5.1.  Client Initialization

>                                                    If the client wishes
>    to interoperate with servers that support discontinuous version

I'd prefer s/discontinuous/discontiguous/ (or perhaps noncontiguous).

> 5.4.  Supported Features

>                                                         All servers MUST
>    include as part of their version packet.

s/as/it as/

>    supported-attribute-mask
>       This mask MAY by applied to the 'File Attributes' valid-attribute-
>       flags field (Section 7.1) to ensure that no unsupported attributes
>       are present during a operation which writes attributes.

s/by/be/

Also, it's not clear whether this means "the client may want do this to
make things easier on the server" or "the server is allowed to silently
apply this mask".

>    supported-attribute-bits
>       This mask MAY by applied to the 'File Attributes' attrib-bits

s/by/be/

>       case, the server is responsible for translating the clients

s/clients/client's/

>       SSH_FXF_BLOCK_ADVISORY bits from Section 7.1.1.3, with a set bit

There is no section 7.1.1.3.  s/7/8/?

> 7.1.  valid-attribute-flags

>    New fields can only be added by incrementing the protocol version

I'd prefer s/only be added/be added only/.

> 7.3.  Size

>    number of bytes the client intends to transfer, but SHOULD NOT effect

s/effect/affect/ (the whole point of a file create request is to effect
file creation).

>    transfered in it's entirity.

s/it's/its/

>    If this field is present during a setstat operation, the file MUST be
>    extended or truncated to the specified size.

When a file is extended, is there any constraint on what appears in the
file between the old EOF point and the new?  I'd prefer to see this
spelled out explicitly - it is for write requests.

> 7.4.  allocation-size

>    If this field is present during a setstat operation, the file SHOULD
>    be extended or truncated to the specified size.  The 'size' of the
>    file may be affected by this operation.  If the operation succeeds,
>    the 'size' should be the minimum of the 'size' before the operation
>    and the new 'allocation-size'.

(1) Again, what content appears when a file is extended?

(2) The last-quoted sentence above appears to forbid truncating a file,
in contrast to the first-quoted sentence; what am I missing?

> 7.6.  Permissions

>    This protocol uses the following values for the symbols declared in
>    the POSIX standard.

> [...]

>    Implementations MUST NOT send bits that are not defined.

This makes it impossible to send, for example, VMS "delete permission"
bits.  Is this really desired, or is this being left up to
implementation experimentation before standardization?

> 7.9.  attrib-bits and attrib-bits-valid

>                  This allows both the server and the client to
>    communicate only the bits it knows about without inadvertently
>    twiddling bits they don't understand.

s/bits it knows/bits they know/

> 8.1.  Opening and Closing Files and Directories

>    an opaque, variable-length string) which may be used to access the

I'd prefer s/,// here.

> 8.1.1.3.  flags

>    SSH_FXF_APPEND_DATA
>       Data is always written at the end of the file.  The offset field
>       of the SSH_FXP_WRITE requests are ignored.

s/field/&s/ (or else s/are/is/).  Also, maybe, s/the// on the second
line.

>    SSH_FXF_BACKUP_STREAM

>                                      However, if the server has a well
>       defined backup stream format, their may be other uses for this
>       data outside the scope of this protocol.

s/their/there/

>    The following table is provided to assist in mapping POSIX semantics
>    to equivalent SFTP file open parameters:
>    O_RDONLY

I'd prefer to see a blank line after the colon, especially in view of
the blank lines between entries in the list.

> 8.2.1.  Reading Files

>    length

>       If the server specified a non-zero 'max-read-size' in its
>       'supported2' (Section 5.4) extension, then failure to return
>       'length' bytes indicates that EOF or an error occurred.

Doesn't this apply only if length <= max-read-size?

> 8.3.  Removing and Renaming Files

>    If flags does not include SSH_FXP_RENAME_OVERWRITE, and there already
>    exists a file with the name specified by newpath, the server MUST
>    respond with SSH_FX_FILE_ALREADY_EXISTS.

What is correct behaviour when the server cannot avoid a race?  For
example, I'm thinking of a Unix variant which renames directories only
with rename(), and there is no way to make rename() fail if the target
directory exists, is empty, and is writable by the calling user.  Under
these conditions, there is no way for the server to correctly rename a
directory without RENAME_OVERWRITE, since there will always be a window
of time during which the target directory could have been created and
then overwritten in violation of the (lack of) the flag.  Must servers
always fail renames they cannot implement correctly, or what?  I notice
the lack of a paragraph indicating what the server is to do if it
cannot implement the rename as specified by this flag.

> 8.7.  Dealing with Links

>    The SSH_FXP_LINK request creates a link (either hard or symbolic) on
>    the server.

>    existing-path
>       Specifies the path of an existing file system object to which the
>       new-link-path will refer.

This makes it impossible to create a symlink pointing to something that
does not currently exist.  This strikes me as an unreasonable
restriction.

> 8.8.  Byte-range locks

>                           or advisory (meaning that no other process can
>    obtain a conflicting lock, but the server does not enforce that no
>    operation violates the lock.

Missing ) at end-of-sentence.

I see no mention of whether this same process can lock overlapping
ranges, and if so, how this interacts with unlocking (see below).

I see no mention of whether locks to end-of-file always extend to
end-of-file or whether they always extend to where end-of-file was when
the lockw as established, or whether either behaviour is permissible.

I see no mention of whether it is permitted to lock ranges that are not
currently part of the file, and whether such a lock means anything.

>       Note that some server MAY return SSH_FX_OP_UNSUPPORTED if the
>       handle is a directory handle.

s/server/&s/

>    SSH_FXP_UNBLOCK removes a previously acquired byte-range lock on the
>    specified handle.

There is no explicit mention of whether it is possible to unlock only
part of a previously-taken lock, or whether locks must be unlocked in
toto or not at all.  There also is no indication of whether ranges
subject to multiple overlapping locks (if possible) must be unlocked
once per lock or whether the first unlock unlocks the range.  Nor do I
see any indication of what happens in response to an attempt to unlock
a range not currently locked, or partially not currently locked.

> 9.1.  Status Response

>                            Future protocol revisions will add additional
>       error codes without changing the version number.

s/will/may/, unless I suppose you already have revisions in the
pipeline which will do that.

>    SSH_FX_WRITE_PROTECT
>       The file is on read-only media, or the media is write protected.

>    SSH_FX_NO_MEDIA
>       The requested operation cannot be completed because there is no
>       media available in the drive.

"media" is plural (though the language seems to be unsure whether it is
a count noun or a mass noun).  Thus, s/is/are/ where the referent is
"media" (the last "is" for each case), or s/media/medium/ for the
corresponding referent.

>    SSH_FX_NO_SPACE_ON_FILESYSTEM
>       The requested operation cannot be completed because there is no
>       free space on the filesystem.

s/no/insufficient/, I would say.

>    SSH_FX_LOCK_CONFLICT
>       The file could not be opened because it is locked by another
>       process.

This description makes this status inappropriate for SSH_FXP_BLOCK
requests that fail due to a lock conflict, which makes little sense to
me.  s/file cound not be opened/request could not be performed/?  Or
perhaps SSH_FX_BYTE_RANGE_LOCK_REFUSED should be returned for that?
The description of SSH_FXP_BLOCK doesn't say.

>    SSH_FX_LINK_LOOP
>       Too many symbolic links encountered.

While this wording is technically correct, I'd be inclined to add "or,
an SSH_FXF_NOFOLLOW open encountered a symbolic link as the final
component".

>    SSH_FX_INVALID_PARAMETER
>       On of the parameters was out of range, or the parameters specified

s/On/&e/

> 9.3.  Data Response

>    end-of-file
>       This field is optional.  If it is present in the packet, it MUST
>       be true, and it indicates that EOF was reached during this read.
>       This can help the client avoid a round trip to determine whether a
>       short read was normal (due to EOF) or some other problem (limited
>       server buffer for example.)

Is there any requirement that this be set when a read less than
max-read-size returns less than the requested length?

> 9.4.  Name Response

>    end-of-list
>       If this field is present and true, there are no more entries to be
>       read.

The semantics of this field are unclear for NAME packets resulting from
things other than READDIR requests (READLINK and REALPATH, at present).

In passing, why is a false end-of-list permitted for NAME but not for
DATA?

> 12.  IANA Considerations

>    An IANA registry needs to be created containing:
>    o  The packet types define defined in Section 4.3
>    o  The extension specified in this draft, which are: [...]

Isn't that two registries?

> 13.  Security Considerations

>    It is assumed that both ends of the connection have been
>    authenticated and that the connection has privacy and integrity
>    features.  Such security issues are left to the underlying transport
>    protocol, except to note that if this is not the case, an attacker
>    could manipulate files on the server at will and thus wholly
>    compromise the server.

This was doing fine until the last six words.  They do not necessarily
follow from the rest; it's not at all unreasonable to imagine some kind
of "anonymous sftp", akin to anonymous FTP, allowing unauthenticated
access to certain restricted areas, presumably rather restricted access
(eg, no writes).  Such a thing need not allow "wholly compromis[ing]"
the server.

>    particular user (the user being authenticated externally to this
>    protocol, typically using [RFC4252].

Missing ) at end-of-sentence.

/~\ 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: Mon, 10 Jul 2006 15:40:31 -0600

> (If indeed the user always uses the same LC_CTYPE, which may not be so.)

Our server reads a per-user option file (because we don't execute
shell scripts.)

I believe many unix servers _do_ execute the shell environment
files because they run the sftp subsystem in an separate process
under the shell.

> sequences".  (For concreteness, I suggest "RAW-8BIT".)

So you proposal is that we continue to require the server do
best effort translation to UTF-8 unless the client turns
this off; however, we allow the server to advertise that
what you are likely to get if you turn off is RAW-8BIT?

The only change being that we allow the server to
specify something that basically says I have no
clue about the char-set of the filenames.

I can live with that-- but it still seems more useful
to have the server give some guess as to the content.

> not sure this is worth fixing, given today's net.

I agree, I don't think this is worth addressing.

> "reliable".)  You might also add "bidirectional" for completeness.

Done and done.

> on this point, though other sentences ending with parentheses imply the

Done.

>>    a secure remote file system service, as well as a secure file

Done.

>>    the client, and that the identity of the client user is available to

Done.

>>    Ylonen and Sami Lehtinen of SSH Communications Security Corp.

Done.

> sentences run together into a single paragraph.

Done.

>>    field the implementation wishes to send are actually options.

Done.

> I'd prefer s/discontinuous/discontiguous/ (or perhaps noncontiguous).

Used noncontiguous.

>>       are present during a operation which writes attributes.

Done.

> make things easier on the server" or "the server is allowed to silently

I've changed it to be 'applied by the client.'  Also, does the 
discussion after the list make this any clearer?

>>       This mask MAY by applied to the 'File Attributes' attrib-bits

Done.

>>       case, the server is responsible for translating the clients

Done.

> There is no section 7.1.1.3.  s/7/8/?

Done.

> I'd prefer s/only be added/be added only/.

Done.

> s/effect/affect/ (the whole point of a file create request is to effect

Fixed.

>>    transfered in it's entirity.

Done.

> spelled out explicitly - it is for write requests.

Done... identically to write requests.

>>    and the new 'allocation-size'.

I've reworked this section to avoid using 'extend' and 'truncate'
as this operation doesn't actually change the 'size' of the file
(unless the new 'allocation-size' is smaller.)

> (1) Again, what content appears when a file is extended?

In this case, the 'size' doesn't change; additional space
is reserved for the file, but the end-of-file isn't changed.

> in contrast to the first-quoted sentence; what am I missing?

Ugh... fixed now.

> implementation experimentation before standardization?

But no one would know how to interpret those bits-- a client
including those bits to a server that expected posix modes
could / would cause problems.  (And in fact we had a case
like this as AIX uses addition bits which we were sending
on the wire which caused problems when the server on a
non-aix system tried to set them.)

I'd be happy to add the VMS bits.  I'd probably need at least
some rough description of what needs to be added (if I remember
correctly, doesn't VMS also have a SYSTEM field -- four fields
of four bits each?)

> s/bits it knows/bits they know/

Fixed.

>>    an opaque, variable-length string) which may be used to access the

Done.

> s/field/&s/ (or else s/are/is/).  Also, maybe, s/the// on the second

Done.

>>       data outside the scope of this protocol.

Done.

> the blank lines between entries in the list.

Done.

> Doesn't this apply only if length <= max-read-size?

Fixed.

> cannot implement the rename as specified by this flag.

Clarified.  'SSH_FXP_RENAME_ATOMIC' specifies whether the client
needs the operation to be atomic, or whether a racey emulation is
acceptable.

> does not currently exist.  This strikes me as an unreasonable

Fixed.

> Missing ) at end-of-sentence.

Fixed.

> ranges, and if so, how this interacts with unlocking (see below).

Hmmm... well, my prefered answer is that
overlapping ranges are not allowed.

> the lockw as established, or whether either behaviour is permissible.

I've clarified this as being 'current eof' -- the equivalent of
doing a SSH_FXP_STAT to retrieve the size and then issuing the
request based on that.

> currently part of the file, and whether such a lock means anything.

Clarified -- it is permissible to lock outside the file,
the same rules apply (i.e., if you block write access
to a range outside the file and someone else tries to
extend the file into that range, it should fail.)

>>       handle is a directory handle.

Fixed.

> toto or not at all.

Clarified.

   There also is no indication of whether ranges
> once per lock or whether the first unlock unlocks the range.

Not an issue due to no overlapping ranges (an excellent reason
to not do overlapping ranges :-)

   Nor do I
> a range not currently locked, or partially not currently locked.

Fixed.

> pipeline which will do that.

Done.

> "media" (the last "is" for each case), or s/media/medium/ for the

Uggh... there is no medium in the drive or are no media both
sound strange to me.  Are you sure that 'media' isn't really
pretty much used as a singular noun in spite of what the
dictionary says?

> s/no/insufficient/, I would say.

Done.

> The description of SSH_FXP_BLOCK doesn't say.

Indeed, SSH_FXP_BLOCK does return SSH_FX_BYTE_RANGE_LOCK_REFUSED.
SSH_FX_LOCK_CONFLICT is returned from SSH_FXP_OPEN.

> an SSH_FXF_NOFOLLOW open encountered a symbolic link as the final

Done.

>>       On of the parameters was out of range, or the parameters specified

Fixed.

> max-read-size returns less than the requested length?

No... it is still possible that the short read is not due to
EOF, but rather some other error (for example a bad media
sector.)

> things other than READDIR requests (READLINK and REALPATH, at present).

Specified that the field should either be omitted or true for 
requests other than SSH_FXP_READDIR.

> In passing, why is a false end-of-list permitted for NAME but not for

Someone pointed out that forcing it to have a certain value kind
of locked us in for future expansion of the packet in the NAME
case, but I didn't pick up on the same thing for data.

false is now permitted in both cases.

> Isn't that two registries?

Yes, I think you are correct.

> (eg, no writes).  Such a thing need not allow "wholly compromis[ing]"

I've changed it to read 'an attacker may be able to manipulate
files on the server and thus wholly compromise the server.'

Wholly compromising the server does pretty much follow
from manipulating files at will... in your example,
manipulating files at will isn't possible.

> Missing ) at end-of-sentence.

Fixed.

Thanks for you feedback.

Joseph

---

From: der Mouse <[email-masked]>
Date: Tue, 11 Jul 2006 00:48:01 -0400 (EDT)

>> First, of course, it still has most of the same character-set issues
>> it always did.  [...]

> So you proposal is that we continue to require the server do best
> effort translation to UTF-8 unless the client turns this off;
> however, we allow the server to advertise that what you are likely to
> get if you turn off is RAW-8BIT?

Yes.  Basically, a way for the server to say "my filenames are
charset-blind octet sequences; if you insist on UTF-8 you're likely to
get unpleasant surprises".

> The only change being that we allow the server to specify something
> that basically says I have no clue about the char-set of the
> filenames.

Right.

> I can live with that-- but it still seems more useful to have the
> server give some guess as to the content.

Possibly; it depends on whether you think a guess made with no
information behind it is more or less valuable than the information
that there is no information on which to base a guess.  I can see it
going either way, depending on the context.

>>>    supported-attribute-mask
>> Also, it's not clear whether this means "the client may want do this
>> to make things easier on the server" or "the server is allowed to
>> silently apply this mask".

Somewhat.  I'm glad to see it clarified at the point of definition,
though.

>>>    This protocol uses the following values for the symbols declared
>>>    in the POSIX standard.  [...]
>> This makes it impossible to send, for example, VMS "delete
>> permission" bits.  Is this really desired, or is this being left up
>> to implementation experimentation before standardization?
> But no one would know how to interpret those bits--

That depends.  If the sftp spec says that these bits indicate delete
permission, then everyone knows what they mean, even if they might not
have any good local analog available.

In other respects, sftp seems to try to be a union of all the common
possibilities (eg, the way it handles locking); it seems odd for it to
take the opposite tack when it comes to permission bits.

> I'd be happy to add the VMS bits.  I'd probably need at least some
> rough description of what needs to be added (if I remember correctly,
> doesn't VMS also have a SYSTEM field -- four fields of four bits
> each?)

That sounds right; it's been far too long since I used VMS, though, so
I'd have to go ask someone who still uses it, or dig up some docs.

>>>       [...] because there is no media available in the drive.
>> "media" is plural [...].
> Uggh... there is no medium in the drive or are no media both sound
> strange to me.  Are you sure that 'media' isn't really pretty much
> used as a singular noun in spite of what the dictionary says?

Hmm.  Actually, I think a good case could be made that it is becoming
(or perhaps even has become) a mass noun (at least when applied to
computer storage).  And mass nouns usually act grammatically singular,
so you are probably right.

I usually try to reword the sentence to avoid depending on the number
of "media".  In these cases, I'd perhaps try

	The file is on read-only (or write-protected0 media.
	...because the drive is empty.

On reflection, I now think that rewording is best, with leaving it the
way it was is second best.  Switching to plural at least has historical
pedantry on its side, though it feels sufficiently odd to my modern eye
to relegate it to third place for me.

>> Or perhaps SSH_FX_BYTE_RANGE_LOCK_REFUSED should be returned for
>> [SSH_FXP_BLOCK conflicts]?  The description of SSH_FXP_BLOCK doesn't
>> say.

Just in case: I trust you made SSH_FXP_BLOCK call out which it uses?

> Wholly compromising the server does pretty much follow from
> manipulating files at will... in your example, manipulating files at
> will isn't possible.

Well, I suppose it's a matter of interpretation of what "at will"
means.  I'm not about to lose any sleep over it, especially since to my
eye the rewording you gave fixes it.

/~\ The ASCII				der Mouse
\ / Ribbon Campaign