IETF-SSH archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: filexfer draft ready for last call?
der Mouse wrote:
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.)
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.
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".)
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.
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.
I agree, I don't think this is worth addressing.
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.
Done and done.
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.
Done.
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/,//
Done.
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".
Done.
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/
Done.
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.
Done.
In other words, only fields after the last
field the implementation wishes to send are actually options.
s/options/optional/
Done.
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).
Used 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/
Done.
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".
I've changed it to be 'applied by the client.' Also, does the
discussion after the list make this any clearer?
supported-attribute-bits
This mask MAY by applied to the 'File Attributes' attrib-bits
s/by/be/
Done.
case, the server is responsible for translating the clients
s/clients/client's/
Done.
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/?
Done.
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/.
Done.
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).
Fixed.
transfered in it's entirity.
s/it's/its/
Done.
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.
Done... identically to 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'.
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.
(2) The last-quoted sentence above appears to forbid truncating a file,
in contrast to the first-quoted sentence; what am I missing?
Ugh... fixed now.
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?
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?)
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/
Fixed.
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.
Done.
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.
Done.
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/
Done.
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.
Done.
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?
Fixed.
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.
Clarified. 'SSH_FXP_RENAME_ATOMIC' specifies whether the client
needs the operation to be atomic, or whether a racey emulation is
acceptable.
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.
Fixed.
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.
Fixed.
I see no mention of whether this same process can lock overlapping
ranges, and if so, how this interacts with unlocking (see below).
Hmmm... well, my prefered answer is that
overlapping ranges are not allowed.
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'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.
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.
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.)
Note that some server MAY return SSH_FX_OP_UNSUPPORTED if the
handle is a directory handle.
s/server/&s/
Fixed.
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.
Clarified.
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.
Not an issue due to no overlapping ranges (an excellent reason
to not do overlapping ranges :-)
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.
Fixed.
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.
Done.
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.
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?
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.
Done.
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.
Indeed, SSH_FXP_BLOCK does return SSH_FX_BYTE_RANGE_LOCK_REFUSED.
SSH_FX_LOCK_CONFLICT is returned from SSH_FXP_OPEN.
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".
Done.
SSH_FX_INVALID_PARAMETER
On of the parameters was out of range, or the parameters specified
s/On/&e/
Fixed.
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?
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.)
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).
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
DATA?
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.
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?
Yes, I think you are correct.
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.
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.
particular user (the user being authenticated externally to this
protocol, typically using [RFC4252].
Missing ) at end-of-sentence.
Fixed.
Thanks for you feedback.
Joseph
Home |
Main Index |
Thread Index |
Old Index