Skip to main content

Last Call Review of draft-ietf-behave-ftp64-
review-ietf-behave-ftp64-tsvdir-lc-gont-2011-06-07-00

Request Review of draft-ietf-behave-ftp64
Requested revision No specific revision (document currently at 12)
Type Last Call Review
Team Transport Area Directorate (tsvdir)
Deadline 2011-06-03
Requested 2011-05-27
Authors Iljitsch van Beijnum
I-D last updated 2011-06-07
Completed reviews Secdir Last Call review of -?? by Donald E. Eastlake 3rd
Tsvdir Last Call review of -?? by Fernando Gont
Assignment Reviewer Fernando Gont
State Completed
Request Last Call review on draft-ietf-behave-ftp64 by Transport Area Directorate Assigned
Completed 2011-06-07
review-ietf-behave-ftp64-tsvdir-lc-gont-2011-06-07-00
Hello,

I've reviewed this document as part of the transport area directorate's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the
document's authors for their information and to allow them to address
any issues raised. The authors should consider this review together with
any other last-call comments they receive. Please always CC
tsv-dir at ietf.org if you reply to or forward this review.


SUMMARY
-------

This draft is on the right track but has open issues, described in the
review.


TECHNICAL
---------

* Section 1, page 3:
   In 5 cases, issuing the EPSV
   command to the server led to a significant delay, in 3 cases followed
   by a control channel reset.

Could you please clarify what was the cause of the delay? And, btw, have
you guys put the details of your results publicly available somewhere?


* Section 1, page 3-4:
   Clients that want to engage in more complex behavior, such as server-
   to-server transfers, may make an FTP application layer gateway (ALG)
   go into transparent mode by issuing AUTH or ALGS commands as
   explained in Section 5.

I thought server-to-server transfer had been banned for many years now
(it was exploited for address scan purposes). Am I missing something?


* Section 5:
   In the second case, an implementation MUST have the ability to track
   and update TCP sequence numbers when translating packets as well as
   the ability to break up packets into smaller packets after
[....]

What about the TCP URG flag and the Urgent Pointer? FTP is one of the
few legacy applications that make use of TCP's urgent mode. And while
use in new applications is deprecated, and TCP urgent mode itself is
unreliable (see RFC 6093), you should make an explicit decision about
what to do with TCP urgent mode when used for the FTP control channel.

When packets are translated individually, the ALG should update the
Urgent Pointer if/where necessary. If the ALG terminates the IPv6 TCP
session, there's the question of whether urgent mode should be "copied"
from the IPv6 session to the IPv4 session.


* Section 5.1, page 7:
For the time being, ALG implementations may employ
   one of the following strategies regarding LANG negotiation

Should you s/may/MAY/?


* Section 5.1, page 8:
   3.  Block LANG negotiation by translating the LANG command to a NOOP
       command, and translating the resulting 200 response into a
       response appropriate for unsupported commands, such as 500.  Text
       is sent in the default language.

I think you should mandate which exact code the 200 should be translated
to, rather than just provide an example.


EDITORIAL
---------

* Abstract, page 1

   The File Transfer Protocol (FTP) has a very long history, and despite
   the fact that today, other options exist to perform file transfers,
   FTP is still in common use.

Remove the comma between "today" and "others"


* Abstract, page 1

   translators
   are used to bridge that gap, FTP is made to work through these
   translators as best it can.

s/as best as it can/to the best possible extent/?


* Section 5.1 (page 7) and elsewhere:
   So the situation where the client and server try to
   negotiate a language that the ALG doesn't support can't be avoided.

Expand doesn't to "does not", "can't" to "cannot", etc.


* Section 5.1, page 8:
   Note that [RFC2640] section 3.1 specifies new handling for spaces and
   the CR character in path names.

Rephrase to "Note that Section 3.1 of [RFC2640]..."


* Section 11, page 12:
   ALGs MUST support the new ALGS (ALG status) command that allows
   command MUST be passed on to the server without modification, and the
   clients to query and set the ALG's status.

I had trouble parsing this sentence.


* Section 11, page 12:
   A client can use the ALGS command to request the ALG's status and to
   enable and disable EPSV to PASV and, if implemented, EPRT to PORT
   translation.

Please rephrase to "...disable EPSV to PASV translation and, if
implemented, EPRT to PORT translation".


Thanks!

Best regards,
-- 
Fernando Gont
e-mail: fernando at gont.com.ar || fgont at acm.org
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1