Re: filexfer draft ready for last call?

[der Mouse <mouse%Rodents.Montreal.QC.CA@localhost>]

> 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	       mouse%rodents.montreal.qc.ca@localhost
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B