Bugs item #1759542, was opened at 2007-07-24 18:23
Message generated for change (Comment added) made by jayesh_a_shah
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=1759542&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: bad behaviour
Status: Open
Resolution: None
Priority: 7
Private: No
Submitted By: Jayesh A Shah (jayesh_a_shah)
Assigned to: Daniel Stenberg (bagder)
Summary: curl writing to a wrong file descriptor causing corruption

Initial Comment:
Hi,

We are facing a issue with curl library in secure mode.

I have written down a simple program to demonstrate the issue with the curl /openssl library. This issue is seen even with latest curl library (7.16.4) and ssl (0.9.8e) libraries.

In this sample program, I do the following:

1. Initialize curl and ssl parameters
2. create a get thread
3. create a apply thread
4. wait for threads

get thread:
 
1. In the get thread, I get one file from a remote server and then notify the apply thread using global variable. To be specific, I am downloading file abc.dat to local directory as xyz.dat

In our real application, we make use of ace instead of pthreads and Ace_Message_Queue for notifying apply thread.

Apply thread:

 Wait for get thread to finish getting the data.
  open the file /root/test.dat.
  sleep for 10 secs
  close the fd for /root/test.dat

To compile this program:
  g++ -g -I<curl include path> main.cpp -lpthread -L<curl lib dir> -lcurl -L<ssl lib dir> -lssl -lcrypto -ldl -lz -o my_curl

To run this program in non secure mode:

./my_curl

To run this program in secuee mode:

./my_curl secure

When I run the program in non secure mode, there are no issues. When I run it in non secure mode, the file /root/test.dat gets corrupted.

-------------------------------------------------------Non Secure Mode:

[root_at_IMITS027 host]# ll /root/test.dat
-rw-r--r-- 1 root root 0 Jul 24 15:15 /root/test.dat
[root_at_IMITS027 host]# rm xyz.dat
rm: remove regular file `xyz.dat'? y
[root_at_IMITS027 host]# ./my_curl
Assuming non secure mode
 [root_at_IMITS027 host]# ll /root/test.dat
-rw-r--r-- 1 root root 0 Jul 24 15:15 /root/test.dat
[root_at_IMITS027 host]# ll xyz.dat
-rw-r--r-- 1 root root 595661 Jul 24 15:15 xyz.dat

Secure Mode:

[root_at_IMITS027 host]# rm xyz.dat
rm: remove regular file `xyz.dat'? y
[root_at_IMITS027 host]# rm /root/test.dat
[root_at_IMITS027 host]# touch /root/test.dat
[root_at_IMITS027 host]# ll /root/test.dat
-rw-r--r-- 1 root root 0 Jul 24 15:15 /root/test.dat
[root_at_IMITS027 host]# ./my_curl secure
running in secure mode
[root_at_IMITS027 host]# ll /root/test.dat
-rw-r--r-- 1 root root 37 Jul 24 15:16 /root/test.dat

Program code:
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
// File : main.cpp
//
// Description:
//

#include <pthread.h>
#include <openssl/crypto.h>
#include <curl/curl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>

struct FtpFile {
  char *filename;
  FILE *stream;
};

int my_fwrite(void *buffer, size_t size, size_t nmemb, void *stream)
{
  struct FtpFile *out=(struct FtpFile *)stream;
  if(out && !out->stream) {
    /* open file for writing */
    out->stream=fopen(out->filename, "wb");
    if(!out->stream)
      return -1; /* failure, can't open file to write */
  }
  return fwrite(buffer, size, nmemb, out->stream);
}

bool secure = false;
volatile bool getdone = false;

#define MUTEX_TYPE pthread_mutex_t
#define MUTEX_SETUP(x) pthread_mutex_init(&(x), NULL)
#define MUTEX_CLEANUP(x) pthread_mutex_destroy(&(x))
#define MUTEX_LOCK(x) pthread_mutex_lock(&(x))
#define MUTEX_UNLOCK(x) pthread_mutex_unlock(&(x))
#define THREAD_ID pthread_self( )

void handle_error(const char *file, int lineno, const char *msg){
     fprintf(stderr, "** %s:%i %s\n", file, lineno, msg);
     /* exit(-1); */
 }

/* This array will store all of the mutexes available to OpenSSL. */
static MUTEX_TYPE *mutex_buf= NULL;

static void locking_function(int mode, int n, const char * file, int line)
{
  if (mode & CRYPTO_LOCK)
    MUTEX_LOCK(mutex_buf[n]);
  else
    MUTEX_UNLOCK(mutex_buf[n]);
}

static unsigned long id_function(void)
{
  return ((unsigned long)THREAD_ID);
}

int thread_setup(void)
{
  int i;

  curl_global_init(CURL_GLOBAL_ALL);
  mutex_buf = (MUTEX_TYPE *)malloc(CRYPTO_num_locks( ) * sizeof(MUTEX_TYPE));
  if (!mutex_buf)
    return 0;
  for (i = 0; i < CRYPTO_num_locks( ); i++)
    MUTEX_SETUP(mutex_buf[i]);
  CRYPTO_set_id_callback(id_function);
  CRYPTO_set_locking_callback(locking_function);
  return 1;
}

int thread_cleanup(void)
{
  int i;

  curl_global_cleanup();
  if (!mutex_buf)
    return 0;
  CRYPTO_set_id_callback(NULL);
  CRYPTO_set_locking_callback(NULL);
  for (i = 0; i < CRYPTO_num_locks( ); i++)
    MUTEX_CLEANUP(mutex_buf[i]);
  free(mutex_buf);
  mutex_buf = NULL;
  return 1;
}

void * applydata(void * in)
{
        while(!getdone);
        int fd = open("/root/test.dat", O_RDWR);
        sleep(10);
        close(fd);
        return 0;
}

void * getdata(void * in)
{

    CURL *curl;
    CURLcode res;
    struct FtpFile ftpfile={
        "xyz.dat", /* name to store the file as if succesful */
        NULL
   };

   curl = curl_easy_init();
   curl_easy_setopt(curl, CURLOPT_USERPWD, "jayesh:passwd") ;
   curl_easy_setopt(curl, CURLOPT_PORT, 0) ;
   curl_easy_setopt(curl,CURLOPT_URL, "ftp://10.0.1.27//home/jayesh/abc.dat");
 
   if(secure)
   {
    // Setting SSL for both the data and control channels
    curl_easy_setopt(curl, CURLOPT_FTP_SSL, CURLFTPSSL_ALL);

    curl_easy_setopt(curl, CURLOPT_FTPSSLAUTH, CURLFTPAUTH_DEFAULT);

    //set the ssl version
    curl_easy_setopt(curl,CURLOPT_SSLVERSION,CURL_SSLVERSION_SSLv3);

    // Set up using the default engine
    curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1);

    // Don't perform peer or host verification
    curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0);
    curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0);

    curl_easy_setopt(curl, CURLOPT_SSLKEY, "passwd2");
    }

    /* Define our callback to get called when there's data to be written */
    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, my_fwrite);
    /* Set a pointer to our struct to pass to the callback */
    curl_easy_setopt(curl, CURLOPT_WRITEDATA, &ftpfile);

    /* Switch on full protocol/debug output ?*/
    curl_easy_setopt(curl, CURLOPT_VERBOSE, true);

    res = curl_easy_perform(curl);
    if(CURLE_OK != res)
    {
      /* we failed */
      fprintf(stderr, "curl told us %d\n", res);
    }
        
    // this is only for test, we would send postmsg
    // only on success in out app
    getdone = true;

    sleep(1);
    /* always cleanup */
    curl_easy_cleanup(curl);

   if(ftpfile.stream)
    fclose(ftpfile.stream); /* close the local file */

   return 0;
}

int main(int argc, char ** argv)
{

   if(argc != 2 )
   {
        printf("Assuming non secure mode\n");
        secure = false;
   } else
   {
     if(!strcmp(argv[1], "secure"))
     {
        printf("running in secure mode\n");
        secure = true;
     } else
     {
        printf("running in non secure mode\n");
        secure = false;
     }
   }

    thread_setup();
    /* create two threads with message queue */

    pthread_t tid[2];
    pthread_create(&tid[0], NULL, getdata, NULL);
    pthread_create(&tid[1], NULL, applydata, NULL);
   

  /* now wait for all threads to terminate */
  for(int i=0; i< 2; i++) {
    pthread_join(tid[i], NULL);
    fprintf(stderr, "Thread %d terminated\n", i);
  }

  thread_cleanup();

  return 0;
}

---------------------------------------------------

----------------------------------------------------------------------

>Comment By: Jayesh A Shah (jayesh_a_shah)
Date: 2007-07-27 08:32

Message:
Logged In: YES
user_id=1852319
Originator: YES

I'm not sure if there is an SSL mismatch or not. That was just guess
because it does not hang in non secure mode (i.e. when SSL is not being
used) and since I could eliminate the hang by not setting connssl->use =
FALSE. I was thinking maybe it had something to do with the fact that the
control is still setup as SSL but now the data is no longer going to have
SSL flag set. The hang could be something completely different though.

Below I have included out put from both a non hang and a hang run. I also
have included a very simple program that does not use threads and calls
curl_easy_perform multiple times before calling curl_easy_cleanup that
reproduces the hang. I compile with g++ but it should compile with gcc as
there is no c++ code in it. You will need to edit the code and provide the
correct values for CURLOPT_USERPWD, CURLOPT_URL, and CURLOPT_SSLKEY. There
are no command line options. It will just use secure mode as that is the
one with the issue. I also have a call to open ./test.dat after the
curl_easy_perform and before calling curl_easy_cleanup as that will
reproduce the fd corruption from the original code before the patch. After
a successful run, ./test.dat should have size 0, if not, then the fd
corruption has occured. You should delete ./test.dat before each run.

Here is a detailed description of what I did to reproduce the hang and how
I came to the conclusion that is was the line connssl->use = FALSE; that
seemed to cause the issue.

* applied the patch, ran with no loops. It worked and no fd corruption

* ran with patched code and a loop around curl_easy_perform. it hung
somewhere in the second call to curl_easy_perform

* cloned the Curl_ossl_close function. As in my temp fix, called it
Curl_ossl_close_handle. In ftp.c called this new function instead of
Curl_ssl_close (yes I now know that is not correct and that it should
really go through sslgen.c, but since I only have open ssl it was simplier
to just do that for now). In the new function I started with only

void Curl_ossl_close_handle(struct connectdata *conn, int sockindex)
{
  struct ssl_connect_data *connssl = &conn->ssl[sockindex];

  if(connssl->handle) {
    (void)SSL_shutdown(connssl->handle);
    SSL_set_connect_state(connssl->handle);
  }
}

ran again with the loop around curl_easy_perform and it worked
successfully. No hang.

* modfied the new function as follows

void Curl_ossl_close_handle(struct connectdata *conn, int sockindex)
{
  struct ssl_connect_data *connssl = &conn->ssl[sockindex];

  if(connssl->handle) {
    (void)SSL_shutdown(connssl->handle);
    SSL_set_connect_state(connssl->handle);

    SSL_free (connssl->handle);
    connssl->handle = NULL;
  }
}

ran again with the loop around curl_easy_perform and it worked. No hang
 

* modfied the new function as follows

void Curl_ossl_close_handle(struct connectdata *conn, int sockindex)
{
  struct ssl_connect_data *connssl = &conn->ssl[sockindex];

  if(connssl->handle) {
    (void)SSL_shutdown(connssl->handle);
    SSL_set_connect_state(connssl->handle);

    SSL_free (connssl->handle);
    connssl->handle = NULL;
  }
  if(connssl->ctx) {
    SSL_CTX_free (connssl->ctx);
    connssl->ctx = NULL;
  }
}

ran again with the loop around curl_easy_perform and it worked. No hang

* modified the new funtion as follows

void Curl_ossl_close_handle(struct connectdata *conn, int sockindex)
{
  struct ssl_connect_data *connssl = &conn->ssl[sockindex];

  if(connssl->handle) {
    (void)SSL_shutdown(connssl->handle);
    SSL_set_connect_state(connssl->handle);

    SSL_free (connssl->handle);
    connssl->handle = NULL;
  }
  if(connssl->ctx) {
    SSL_CTX_free (connssl->ctx);
    connssl->ctx = NULL;
  }
  connssl->use = FALSE; /* get back to ordinary socket usage */
}

ran again with the loop around curl_easy_perform and this time it hung
just as with the patch. Stuck some where in the second call to
curl_easy_perform

* commented out

  // connssl->use = FALSE;

ran again with loop around curl_easy_perform (just to make sure) and it
worked. No hang

* I went back to the original patched code. ran again with loop around
curl_easy_perform. Once again it hung as it did originally.

* finally I tried commenting out
  
   // connssl->use = FALSE;

in Curl_ossl_close from the original patched code. ran again with loop
around curl_easy_perform and that too hung. I didn't expected hang (as it
was very similar with my new fucntion), but thought it might not work
correctly as there are other places that also call Curl_ssl_close and by
not setting connssl->use = FALSE it might cause other issues.

So I don't know why it hangs. Just that it does with a pretty simple
program.

Here is the out put for both a non hang and a hang. the second run is the
hang. In all hang cases the last output line is

* DO-MORE phase ends with 0

------------------------- B E G I N R U N -------------------------

============== LOOP COUNT: 0 =============

* About to connect() to 192.168.80.175 port 21 (#0)
* Trying 192.168.80.175... * connected
* Connected to 192.168.80.175 (192.168.80.175) port 21 (#0)
* FTP 0x8145bd0 state change from STOP to WAIT220
< 220 ProFTPD 1.2.10 Server (ProFTPD Default Installation)
[192.168.80.175]
> AUTH SSL
* FTP 0x8145bd0 state change from WAIT220 to AUTH
< 234 AUTH SSL successful
* successfully set certificate verify locations:
* CAfile: /usr/local/share/curl/curl-ca-bundle.crt
  CApath: none
* SSL connection using DHE-RSA-AES256-SHA
* Server certificate:
* subject: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* start date: 2005-07-01 20:19:53 GMT
* expire date: 2006-07-01 20:19:53 GMT
* issuer: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* SSL certificate verify result: unable to get local issuer certificate
(20), continuing anyway.
> USER svsystems
* FTP 0x8145bd0 state change from AUTH to USER
< 331 Password required for svsystems.
> PASS svsHillview
* FTP 0x8145bd0 state change from USER to PASS
< 230 User svsystems logged in.
> PBSZ 0
* FTP 0x8145bd0 state change from PASS to PBSZ
< 200 PBSZ 0 successful
> PROT P
* FTP 0x8145bd0 state change from PBSZ to PROT
< 200 Protection set to Private
> PWD
* FTP 0x8145bd0 state change from PROT to PWD
< 257 "/home/svsystems" is current directory.
* Entry path is '/home/svsystems'
* FTP 0x8145bd0 state change from PWD to STOP
* protocol connect phase DONE
* DO phase starts
> CWD /home
* FTP 0x8145bd0 state change from STOP to CWD
< 250 CWD command successful
> CWD gregz
< 250 CWD command successful
> CWD src
< 250 CWD command successful
> CWD fd-test
< 250 CWD command successful
> EPSV
* FTP 0x8145bd0 state change from CWD to PASV
* Connect data stream passively
< 229 Entering Extended Passive Mode (|||54611|)
* Trying 192.168.80.175... * connected
* Connecting to 192.168.80.175 (192.168.80.175) port 54611
* FTP 0x8145bd0 state change from PASV to STOP
* DO phase is complete
* DO-MORE phase starts
> TYPE I
* FTP 0x8145bd0 state change from STOP to RETR_TYPE
< 200 Type set to I
> SIZE get.dat
* FTP 0x8145bd0 state change from RETR_TYPE to RETR_SIZE
< 213 540
> RETR get.dat
* FTP 0x8145bd0 state change from RETR_SIZE to RETR
< 150 Opening BINARY mode data connection for get.dat (540 bytes)
* Doing the SSL/TLS handshake on the data stream
* successfully set certificate verify locations:
* CAfile: /usr/local/share/curl/curl-ca-bundle.crt
  CApath: none
* SSL connection using DHE-RSA-AES256-SHA
* Server certificate:
* subject: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* start date: 2005-07-01 20:19:53 GMT
* expire date: 2006-07-01 20:19:53 GMT
* issuer: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* SSL certificate verify result: unable to get local issuer certificate
(20), continuing anyway.
* Maxdownload = -1
* Getting file with size: 540
* FTP 0x8145bd0 state change from RETR to STOP
* DO-MORE phase ends with 0
* nread <= 0, server closed connection, bailing
* Remembering we are in dir /home/gregz/src/fd-test/
< 226 Transfer complete.
* Connection #0 to host 192.168.80.175 left intact

============== LOOP COUNT: 1 =============

* Re-using existing connection! (#0) with host 192.168.80.175
* Connected to 192.168.80.175 (192.168.80.175) port 21 (#0)
* Request has same path as previous transfer
* DO phase starts
> EPSV
* FTP 0x8145bd0 state change from STOP to PASV
* Connect data stream passively
< 229 Entering Extended Passive Mode (|||54613|)
* Trying 192.168.80.175... * connected
* Connecting to 192.168.80.175 (192.168.80.175) port 54613
* FTP 0x8145bd0 state change from PASV to STOP
* DO phase is complete
* DO-MORE phase starts
* FTP 0x8145bd0 state change from STOP to RETR_TYPE
> SIZE get.dat
* FTP 0x8145bd0 state change from RETR_TYPE to RETR_SIZE
< 213 540
> RETR get.dat
* FTP 0x8145bd0 state change from RETR_SIZE to RETR
< 150 Opening BINARY mode data connection for get.dat (540 bytes)
* Doing the SSL/TLS handshake on the data stream
* successfully set certificate verify locations:
* CAfile: /usr/local/share/curl/curl-ca-bundle.crt
  CApath: none
* SSL connection using DHE-RSA-AES256-SHA
* Server certificate:
* subject: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* start date: 2005-07-01 20:19:53 GMT
* expire date: 2006-07-01 20:19:53 GMT
* issuer: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* SSL certificate verify result: unable to get local issuer certificate
(20), continuing anyway.
* Maxdownload = -1
* Getting file with size: 540
* FTP 0x8145bd0 state change from RETR to STOP
* DO-MORE phase ends with 0
* nread <= 0, server closed connection, bailing
* Remembering we are in dir /home/gregz/src/fd-test/
< 226 Transfer complete.
* Connection #0 to host 192.168.80.175 left intact
> QUIT
* FTP 0x8145bd0 state change from STOP to QUIT
< 221 Goodbye.
* FTP 0x8145bd0 state change from QUIT to STOP
* Closing connection #0
------------------------- E N D R U N -------------------------

------------------------- B E G I N R U N -------------------------

============== LOOP COUNT: 0 =============

* About to connect() to 192.168.80.175 port 21 (#0)
* Trying 192.168.80.175... * connected
* Connected to 192.168.80.175 (192.168.80.175) port 21 (#0)
* FTP 0x8145bd0 state change from STOP to WAIT220
< 220 ProFTPD 1.2.10 Server (ProFTPD Default Installation)
[192.168.80.175]
> AUTH SSL
* FTP 0x8145bd0 state change from WAIT220 to AUTH
< 234 AUTH SSL successful
* successfully set certificate verify locations:
* CAfile: /usr/local/share/curl/curl-ca-bundle.crt
  CApath: none
* SSL connection using DHE-RSA-AES256-SHA
* Server certificate:
* subject: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* start date: 2005-07-01 20:19:53 GMT
* expire date: 2006-07-01 20:19:53 GMT
* issuer: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* SSL certificate verify result: unable to get local issuer certificate
(20), continuing anyway.
> USER svsystems
* FTP 0x8145bd0 state change from AUTH to USER
< 331 Password required for svsystems.
> PASS svsHillview
* FTP 0x8145bd0 state change from USER to PASS
< 230 User svsystems logged in.
> PBSZ 0
* FTP 0x8145bd0 state change from PASS to PBSZ
< 200 PBSZ 0 successful
> PROT P
* FTP 0x8145bd0 state change from PBSZ to PROT
< 200 Protection set to Private
> PWD
* FTP 0x8145bd0 state change from PROT to PWD
< 257 "/home/svsystems" is current directory.
* Entry path is '/home/svsystems'
* FTP 0x8145bd0 state change from PWD to STOP
* protocol connect phase DONE
* DO phase starts
> CWD /home
* FTP 0x8145bd0 state change from STOP to CWD
< 250 CWD command successful
> CWD gregz
< 250 CWD command successful
> CWD src
< 250 CWD command successful
> CWD fd-test
< 250 CWD command successful
> EPSV
* FTP 0x8145bd0 state change from CWD to PASV
* Connect data stream passively
< 229 Entering Extended Passive Mode (|||54566|)
* Trying 192.168.80.175... * connected
* Connecting to 192.168.80.175 (192.168.80.175) port 54566
* FTP 0x8145bd0 state change from PASV to STOP
* DO phase is complete
* DO-MORE phase starts
> TYPE I
* FTP 0x8145bd0 state change from STOP to RETR_TYPE
< 200 Type set to I
> SIZE get.dat
* FTP 0x8145bd0 state change from RETR_TYPE to RETR_SIZE
< 213 540
> RETR get.dat
* FTP 0x8145bd0 state change from RETR_SIZE to RETR
< 150 Opening BINARY mode data connection for get.dat (540 bytes)
* Doing the SSL/TLS handshake on the data stream
* successfully set certificate verify locations:
* CAfile: /usr/local/share/curl/curl-ca-bundle.crt
  CApath: none
* SSL connection using DHE-RSA-AES256-SHA
* Server certificate:
* subject: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* start date: 2005-07-01 20:19:53 GMT
* expire date: 2006-07-01 20:19:53 GMT
* issuer: /C=US/ST=CA/L=Santa
Clara/O=Inmage/OU=Inmage/CN=Inmage/emailAddress=Inmage_at_inmage.com
* SSL certificate verify result: unable to get local issuer certificate
(20), continuing anyway.
* Maxdownload = -1
* Getting file with size: 540
* FTP 0x8145bd0 state change from RETR to STOP
* DO-MORE phase ends with 0
* nread <= 0, server closed connection, bailing
* Remembering we are in dir /home/gregz/src/fd-test/
< 226 Transfer complete.
* Connection #0 to host 192.168.80.175 left intact

============== LOOP COUNT: 1 =============

* Re-using existing connection! (#0) with host 192.168.80.175
* Connected to 192.168.80.175 (192.168.80.175) port 21 (#0)
* Request has same path as previous transfer
* DO phase starts
> EPSV
* FTP 0x8145bd0 state change from STOP to PASV
* Connect data stream passively
< 229 Entering Extended Passive Mode (|||54569|)
* Trying 192.168.80.175... * connected
* Connecting to 192.168.80.175 (192.168.80.175) port 54569
* FTP 0x8145bd0 state change from PASV to STOP
* DO phase is complete
* DO-MORE phase starts
* FTP 0x8145bd0 state change from STOP to RETR_TYPE
> SIZE get.dat
* FTP 0x8145bd0 state change from RETR_TYPE to RETR_SIZE
< 213 540
> RETR get.dat
* FTP 0x8145bd0 state change from RETR_SIZE to RETR
< 150 Opening BINARY mode data connection for get.dat (540 bytes)
* Maxdownload = -1
* Getting file with size: 540
* FTP 0x8145bd0 state change from RETR to STOP
* DO-MORE phase ends with 0

Here is the simple program that should reproduce the hang with the patch
applied.

#include <openssl/crypto.h>
#include <curl/curl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>

#define REPEAT_COUNT 2

struct FtpFile {
    char *filename;
    FILE *stream;
};
 
int my_fwrite(void *buffer, size_t size, size_t nmemb, void *stream)
{
    struct FtpFile *out=(struct FtpFile *)stream;
    if (out && !out->stream) {
        out->stream=fopen(out->filename, "wb");
        if(!out->stream) {
            return -1;
        }
    }
    return fwrite(buffer, size, nmemb, out->stream);
}
 
void * getdata()
{
    int fd;

    CURL *curl;
    CURLcode res;
    struct FtpFile ftpfile = { "xyz.dat", NULL };

    curl = curl_easy_init();
    curl_easy_setopt(curl, CURLOPT_USERPWD, "user:passwd") ;
 /* set to your user:passwd */
    curl_easy_setopt(curl, CURLOPT_PORT, 0) ;
    curl_easy_setopt(curl, CURLOPT_URL,
"ftp://0.0.0.0//some/dir/get.dat"); /* set to your URL get file */
    curl_easy_setopt(curl, CURLOPT_FTP_SSL, CURLFTPSSL_ALL);
    curl_easy_setopt(curl, CURLOPT_FTPSSLAUTH, CURLFTPAUTH_DEFAULT);
    curl_easy_setopt(curl, CURLOPT_SSLVERSION,CURL_SSLVERSION_SSLv3);
    curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1);
    curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0);
    curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0);
    curl_easy_setopt(curl, CURLOPT_SSLKEY, "passwd2");
/* set to your sslkey passwd */
    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, my_fwrite);
    curl_easy_setopt(curl, CURLOPT_WRITEDATA, &ftpfile);
    curl_easy_setopt(curl, CURLOPT_VERBOSE, true);

    for (int i = 0; i < REPEAT_COUNT; ++i) {
        printf("\n============== LOOP COUNT: %d =============\n\n", i);
        res = curl_easy_perform(curl);
        if (CURLE_OK != res) {
            fprintf(stderr, "curl told us %d\n", res);
            break;
        }
    }

    fd = open("./test.dat", O_CREAT | O_RDWR);

    curl_easy_cleanup(curl);

    if (-1 == fd) {
        fprintf(stderr, "open ./test.dat failed %s\n", strerror(errno));
    } else {
        close(fd);
    }
    
    if (ftpfile.stream) {
        fclose(ftpfile.stream);
    }
 
    return 0;
}
 
int main(int , char **)
{
    printf("------------------------- B E G I N R U N
-------------------------\n");
    getdata();
    printf("------------------------- E N D R U N
-------------------------\n");
    return 0;

}

I hope this helps.

~greg

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2007-07-27 03:09

Message:
Logged In: YES
user_id=1110
Originator: NO

Hi Jayesh and Greg!

1) I don't see how multiple calls to curl_easy_perform() before
curl_easy_cleanup() should matter since this patch affects Curl_ftp_done()
which is called after each FTP transfer and closes the SSL layer for the
data connection (which is setup new and fresh for each new FTP transfer).

2) the "connssl->use = FALSE" like is what sets that the particular
socket/connection is no longer uses SSL, so it would be very interesting
to get to know where or how it "hangs" after that has been set since
libcurl should from then on not attempt to use anything SSL-like on that
connection.

3) You said "there is a mismatch between the 2 sides". Exactly how is that
mismatch happening with my suggested patch applied? I don't see it...

----------------------------------------------------------------------

Comment By: Jayesh A Shah (jayesh_a_shah)
Date: 2007-07-27 02:20

Message:
Logged In: YES
user_id=1852319
Originator: YES

I've tested the patch. While it does solve the fd corruption issue, it
introduces a hang if multiple calls are made to curl_easy_perform prior to
calling curl_easy_cleanup.

I see that while I mentioned a hang issue when trying to use
Curl_ossl_shutdown. I forgot to mention that I had a hang issue when I
initially had done what you did with respect to having the function
Curl_ossl_close only close one of the handles at a time. That is the reason
I had created the separate function Curl_ossl_close_handle and only had it
do the shutdown and set connection state. Not knowing the curl code I
thought that freeing the handle and certificates (knowing that they were
going to be reused with out calling cleanup) was going to cause issues.

I investigated a little more and the hang seems to be do to the following
line

  connssl->use = FALSE; /* get back to ordinary socket usage */

I.e. if everything in Curl_ossl_close is done except that line, then there
is no hang and it seems to work OK. Although I'm not sure it makes sense to
free the handle and certificates until the cleanup. Again I don't know the
curl code well enough to know it that is OK or not.

I think (note I did not confirm this) what happens is that after that
line, the SECONDARYSOCKET is no longer going to use SSL. The other side is
going to use SSL. So there is a mismatch between the 2 sides. Not sure why
that causes a hang.

I also noticed that while I did mention I was a colleague of Jayesh, I
didn't mention my name, Greg. Sorry about that.

~greg

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2007-07-26 14:59

Message:
Logged In: YES
user_id=1110
Originator: NO

I submit a patch for the fix, as I suggested in my previous comment. Does
this fix the problem for you?
File Added: ftp-close-ssl.patch

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2007-07-26 03:20

Message:
Logged In: YES
user_id=1110
Originator: NO

Thanks for the description and yes, I can see the problem now.

Wouldn't the correct fix be to simply shutdown the SSL on the secondary
connection (only) before the close is done in Curl_ftp_done() ? (Of course
given that the second connection uses SSL)

----------------------------------------------------------------------

Comment By: Jayesh A Shah (jayesh_a_shah)
Date: 2007-07-26 02:54

Message:
Logged In: YES
user_id=1852319
Originator: YES

I should have mentioned that Curl_ossl_shutdown was initial tried instead
of creating the Curl_ossl_close_handle. However it caused hangs if multiple
curl_easy_performs were down with out calling curl_easy_cleanup.

Also another note on threads. This issue is reproducible with out the use
of threads. All you need to do is in getdata after the call to
curl_easy_perform and before the call to curl_easy_cleanup do an open of
test.dat (just like the applydata function) but don't close the fd until
after the call to curl_easy_cleanup. You could then in main just call
getdata directly with no threads and you will see that test.dat gets the
socket writes from the curl_easy_cleanup written to it.

----------------------------------------------------------------------

Comment By: Jayesh A Shah (jayesh_a_shah)
Date: 2007-07-26 02:18

Message:
Logged In: YES
user_id=1852319
Originator: YES

I'm a colleague of Jayesh's and he asked me to look into this for him.

To answer you questions this sample code fails on various Linux distros
for RedHat and Suse. As for threads, the issue is only related to threads
in the sense that to reproduce this issue requires the curl_easy_perform to
happen in a separate thread from the one where the test.dat is being open.
And the test.dat needs to be opened after the curl_easy_perform completes
but before the curl_easy_cleanup is run. So thread safe C libraries should
not come into play for this specific issue.

To be clear, this is only an issue when using secure mode (i.e. SSL). I
looked at version 7.16.4 and the issue (at least for FTP) is that in ftp.c
in the function Curl_ftp_done a call is made to sclose on the data socket
(at or near line 3118). The problem is that in secure mode, this closes the
socket but not the SSL part. So later when curl_easy_cleanup is called a
call is made to Curl_ossl_close (in ssluse.c). In there a call is made to
SSL_shutdown which eventually tries to write some data to the socket. In
this case the fd for that socket is no longer valid as it was closed in
FTP_done. In particular the fd has been reissued by the os and is now for
the file test.dat. That is where the data for the SSL write ends up. I
believe in general SSL needs to do some stuff before calling sclose on the
socket (it has been a long time since I have worked with SSL so I don't
remember exactly what specifically needs to happen with respect to SSL
prior to the sclose). Once the socket is closed then no more writes should
be attempted to that socket.

With that in mind, I created a temp fix. What was done was to break up the
Curl_ossl_close into 2 functions. Curl_ossl_close_handle and
Curl_oss_close. The code that does the SSL shutdown and set the connect
state from the original Curl_ossl_close was moved into
Curl_ossl_close_handle. Curl_ossl_close now calls Curl_ossl_close_handle
and continues to do what it was doing. In ftp.c before the sclose a call
was added to Curl_ossl_close_handle for the ssl handle that is associated
with the socket that is going to be closed. This is probably not complete
(as there maybe other places when SSL is being used where this needs to be
done before the sclose call) and perhaps not even the correct solution (I
am not at all familiar with the curl code, so the new
Curl_ossl_close_handle maybe doing more then it needs to or maybe not
enough), but it does seem eliminate the corruption (i.e. having SSL write
to the test.dat fd).

If I find anything else out I will let you know.

Anyway here are the diffs for you to see exactly what was done

diff ssluse.c new-ssluse.c
703a704,716
> * This function is called when an SSL handle is closed.
> */
> void Curl_ossl_close_handle( struct ssl_connect_data * connssl)
> {
> if(connssl->use) {
> if(connssl->handle) {
> (void)SSL_shutdown(connssl->handle);
> SSL_set_connect_state(connssl->handle);
> }
> }
> }
>
> /*
708,732c721,747
< int i;
< /*
< ERR_remove_state() frees the error queue associated with
< thread pid. If pid == 0, the current thread will have its
< error queue removed.
<
< Since error queue data structures are allocated
< automatically for new threads, they must be freed when
< threads are terminated in oder to avoid memory leaks.
< */
< ERR_remove_state(0);
<
< for(i=0; i<2; i++) {
< struct ssl_connect_data *connssl = &conn->ssl[i];
<
< if(connssl->handle) {
< (void)SSL_shutdown(connssl->handle);
< SSL_set_connect_state(connssl->handle);
<
< SSL_free (connssl->handle);
< connssl->handle = NULL;
< }
< if(connssl->ctx) {
< SSL_CTX_free (connssl->ctx);
< connssl->ctx = NULL;

---
>     struct ssl_connect_data * connssl;
>     if(conn->ssl[FIRSTSOCKET].use) {
>         int i;
>         /*
>           ERR_remove_state() frees the error queue associated with
>           thread pid.  If pid == 0, the current thread will have its
>           error queue removed.
> 
>           Since error queue data structures are allocated
>           automatically for new threads, they must be freed when
>           threads are terminated in oder to avoid memory leaks.
>         */
>         ERR_remove_state(0);
> 
>         for(i=0; i<2; i++) {
>             connssl = &conn->ssl[i];
>             if (connssl->handle) {          
>                 Curl_ossl_close_handle(connssl);
>                 SSL_free (connssl->handle);
>                 connssl->handle = NULL;
>             }
>         }
>         if(connssl->ctx) {
>             SSL_CTX_free (connssl->ctx);
>             connssl->ctx = NULL;
>         }
>         connssl->use = FALSE; /* get back to ordinary socket usage */
734,735d748
<     connssl->use = FALSE; /* get back to ordinary socket usage */
<   }
diff ssluse.h new-ssluse.h
34a35
> void Curl_ossl_close_handle( struct ssl_connect_data * connssl); /*
close a SSL handle */
diff ftp.c new-ftp.c
3117c3117,3118
< 
---
>   
>   Curl_ossl_close_handle(&conn->ssl[SECONDARYSOCKET]);
----------------------------------------------------------------------
Comment By: Dan Fandrich (dfandrich)
Date: 2007-07-24 21:59
Message:
Logged In: YES 
user_id=236775
Originator: NO
The test.dat file descriptor doesn't even exist until after the main curl
transfer runs; only curl_easy_cleanup() runs while the file is open, so if
it's a curl problem, that's where it would be. What are those 37 bytes that
test.dat contains at the end? Is the xyz.dat file transferred correctly?
Can you post the libcurl debug logs during this transfer (especially the
part during the curl_easy_cleanup)?
What platform is this, anyway? Are you using thread safe C libraries? 
----------------------------------------------------------------------
Comment By: Daniel Stenberg (bagder)
Date: 2007-07-24 19:53
Message:
Logged In: YES 
user_id=1110
Originator: NO
(please don't change the priority field)
I don't understand what the problem is. Writing to "wrong file
descriptor"? What does that mean exactly?
The writing to the file isn't even done by libcurl, it is done by your
provided callback!
----------------------------------------------------------------------
Comment By: Jayesh A Shah (jayesh_a_shah)
Date: 2007-07-24 19:03
Message:
Logged In: YES 
user_id=1852319
Originator: YES
Please note : There was a small typo in my submission ...
I had written:
---------------------------------------------------------------------------
When I run the program in non secure mode, there are no issues. When I
run
it in non secure mode, the file /root/test.dat gets corrupted.
----------------------------------------------------------------------------
Read it as :
When I run the program in non secure mode, there are no issues. When I
run
it in secure mode, the file /root/test.dat gets corrupted.
----------------------------------------------------------------------
Comment By: Jayesh A Shah (jayesh_a_shah)
Date: 2007-07-24 19:03
Message:
Logged In: YES 
user_id=1852319
Originator: YES
Please note : There was a small typo in my submission ...
I had written:
---------------------------------------------------------------------------
When I run the program in non secure mode, there are no issues. When I
run
it in non secure mode, the file /root/test.dat gets corrupted.
----------------------------------------------------------------------------
Read it as :
When I run the program in non secure mode, there are no issues. When I
run
it in secure mode, the file /root/test.dat gets corrupted.
----------------------------------------------------------------------
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=1759542&group_id=976