On Thu, Nov 27, 2008 at 7:41 AM, Daniel Stenberg <daniel_at_haxx.se> wrote:
> On Wed, 26 Nov 2008, Ken Hirsch wrote:
>
>> In 2007 there was some talk on this list of always sending a PROT command
>> after a PBSZ, even in implicit SSL mode. I think this would be a good idea.
>> We communicate with 4 other customers that use FTPS and our communication
>> with their servers continued to work after I added a 'PROT P' command to
>> them. I'm not sure what software they use, but it's not FileZillas Server
>> and I know that they are not all the same software.
>
> Right, I think for implicit FTPS that can be a sensible approach.

[...]

> Doesn't setting CURLOPT_USE_SSL option to CURLUSESSL_ALL also "fix" the
> problem?

No, it doesn't.

The relevant code (in lib/ftp.c) is this function:
static CURLcode ftps_setup_connection(struct connectdata * conn)
{
  struct SessionHandle *data = conn->data;

  conn->ssl[SECONDARYSOCKET].use = data->set.ftp_ssl != CURLUSESSL_CONTROL;
  return ftp_setup_connection(conn);
}
and this part of ftp_statemach_act:
    case FTP_PBSZ:
      /* FIX: check response code */

      /* For TLS, the data connection can have one of two security levels.

      1) Clear (requested by 'PROT C')

      2)Private (requested by 'PROT P')
      */
      if(!conn->ssl[SECONDARYSOCKET].use) {
        NBFTPSENDF(conn, "PROT %c",
                   data->set.ftp_ssl == CURLUSESSL_CONTROL ? 'C' : 'P');
        state(conn, FTP_PROT);
      }
      else {
        result = ftp_state_pwd(conn);
        if(result)
          return result;
      }

      break;

I propose doing away with ftps_setup_connection and just calling
ftp_setup_connection instead, then simplifying the code in
ftp_statemach_act to:
    case FTP_PBSZ:
      NBFTPSENDF(conn, "PROT %c",
                 data->set.ftp_ssl == CURLUSESSL_CONTROL ? 'C' : 'P');
      state(conn, FTP_PROT);

      break;

I have tried this code with ftps: and ftp: combined with --ftp-ssl,
-ftp-ssl-reqd, --ftp-ssl-control and it seems to do the right thing.

Should I send a patch with this change?

Ken Hirsch
Received on 2008-11-28