On 10/19/2016 08:20 PM, Zhao, Joe wrote:
> There is no scp.c in curl package 7.36.0

I have scp and sftp being used with libssh2 and libcurl very neatly for
years also. Very stable.

However I am generally doing updates of libcurl often and simply doing
a recompile of libcurl as well as openssl and libssh2 isn't that big a
deal. The code written years ago ( mostly in C ) works and keeps working
and after some testing I promote the code up to my testing tier for a
whole slew of users to test. After another week I promote upwards to the
production layer and have been doing this, for years. LibCurl is without
a doubt one of them most stable slick bits of code out there and yes I
have the tee-shirt too. :-)

So ... I shall glance at your code but if it works then it will keep
working.

Also, for the love of all that is holy on the internet and elsewhere
stop top posting.

> I haven't got the chance to review the source code of 7.51.0 version.

I'm using 7.50.3 at the moment. That is up to date :

     https://curl.haxx.se/changes.html

> Do we have to rewrite our application code for using the new curl
> library?

Not very likely but I have not seen ALL of it. I sure don't need to
update mine. I sure don't go digging into several hundred thousand
lines of code ( about 220,000+ at last count ) in my work world every
time libCurl or openssl or libssh2 updates. That is for sure.

Are there changes happening and coders at work ? Yes. However code
that worked five years ago .. keeps on working. Pretty well tested.

> Here is our application code for getting a file using scp via curl library.
>

> char remotePath[4096]; /* Have big enough buffer to construct remote
> server name, userid, passwd, filename & path */

You may be better off to calloc/malloc what you need to be super safe
and also ( at least in my stuff ) do a character test on the strings to
be sure they are utf8 valid with no special chars to make life hell.

> char err[4096];
> int attempt = 1;
> int retries;
> int retryInterval;

cool .. a recyle on attempts may be needed with a delay stairstep
function that drives the wait time upwards to some cutoff.

> XftpCurlFile xftpLocalfile = { pXftpParams->localFileName, NULL};

yeah .. about this, do you know the file exists and you can get to it ?

do you stat and check this file ?

     struct stat fid_check_exists;
     stat_foo = stat ( pXftpParams->localFileName, &fid_check_exists );

Just an idea.

     if ( stat_foo != 0 ) {
         /* deal with it */
         exit ( EXIT_FAILURE ); /* or whatever works */
     }

> retries = bUseRetries ? 1 : 0;
> retryInterval = 15;

sec ? millisec ? just curious.

>
> curl_global_init(CURL_GLOBAL_DEFAULT);
> do
> {
> if (attempt > 1)
> delay(retryInterval * 1000);

oh .. okay . .so secs I guess.

> curl = curl_easy_init();
> if (curl)
> {

If you want ssh then I am suggesting you use key files too .. unless
you don't need them. Have this somewhere :

   char* ssh_pub_key_file = "/somepath/keys/foo/bar_rsa.id.pub";
   char* ssh_priv_key_file = "/somepath/keys/foo/bar_rsa.id";
   char* ssh_pass = "";

then in this big loopy "if" somewhere :

         curl_easy_setopt ( curl, CURLOPT_SSH_PUBLIC_KEYFILE,
                                  ssh_pub_key_file );

         curl_easy_setopt ( curl, CURLOPT_SSH_PRIVATE_KEYFILE,
                                  ssh_priv_key_file );

         curl_easy_setopt ( curl, CURLOPT_KEYPASSWD,
                                  ssh_pass );

The password here is the passphrase on the key. You may not need this.

         curl_easy_setopt ( curl, CURLOPT_VERBOSE, 1L );
            /* you have this somewhere I see */

         curl_easy_setopt ( curl, CURLOPT_DEBUGFUNCTION, audit_log );

Not sure if you want that either but it is nice to track events and log
them.

> // Build TFTP/FTP/SFTP string required for CURL
> if (xftpURLGet(curl, remotePath, sizeof(remotePath),
> pXftpParams->hostname, pXftpParams->username,
> pXftpParams->password,
> pXftpParams->remoteFileName,
> pXftpParams->hostmode) == False)
> {
> curl_easy_cleanup(curl);
> curl_global_cleanup();
> return XftpStatus_CurlInitFailed;
> }
>
> if (curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT,
> pXftpParams->hostmode == XftpMode_Sftp ?
> XFTP_DEFAULT_SFTP_TIMEOUT :
> XFTP_DEFAULT_FTP_TIMEOUT) != CURLE_OK)
> {
> curl_easy_cleanup(curl);
> curl_global_cleanup();
> return XftpStatus_CurlInitFailed;
> }
>
> if (curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, XFTP_DEFAULT_LOW_SPEED_LIMIT) != CURLE_OK)
> {
> curl_easy_cleanup(curl);
> curl_global_cleanup();
> return XftpStatus_CurlInitFailed;
> }

this looks duplicate.

>
> if (curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME , XFTP_DEFAULT_LOW_SPEED_TIME) != CURLE_OK)
> {
> curl_easy_cleanup(curl);
> curl_global_cleanup();
> return XftpStatus_CurlInitFailed;
> }

>
> if (curl_easy_setopt(curl, CURLOPT_URL, remotePath) != CURLE_OK)
> {
> curl_easy_cleanup(curl);
> curl_global_cleanup();
> return XftpStatus_CurlInitFailed;
> }
>
> /* Define our callback to get called when there's data to be written */
> if (curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, my_fwrite) != CURLE_OK)
> {
> curl_easy_cleanup(curl);
> curl_global_cleanup();
> return XftpStatus_CurlInitFailed;
> }
>
> /* Set a pointer to our struct to pass to the callback */
> if (curl_easy_setopt(curl, CURLOPT_WRITEDATA, &xftpLocalfile) != CURLE_OK)
> {
> curl_easy_cleanup(curl);
> curl_global_cleanup();
> return XftpStatus_CurlInitFailed;
> }
>
> /* Set transfer mode to Binary (0 == BIN) */
> if (curl_easy_setopt(curl, CURLOPT_TRANSFERTEXT, 0) != CURLE_OK)
> {
> curl_easy_cleanup(curl);
> curl_global_cleanup();
> return XftpStatus_CurlInitFailed;
> }

> /* Switch on full protocol/debug output */

cool.

> curl_easy_setopt(curl, CURLOPT_VERBOSE, XFTP_VERBOSE_LEVEL);
> /* Switch on full protocol/debug output */
> curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, err);
> res = curl_easy_perform(curl);
> /* always cleanup */
> curl_easy_cleanup(curl);
>
> if (xftpLocalfile.stream)
> {
> fclose(xftpLocalfile.stream); /* close the local file */
> }
> }
> } while ((res != CURLE_OK) && (attempt++ <= retries));

hrmmm .. okay ..

> curl_global_cleanup();
> return xftpCurlResultToGsStatus(res);
> } /* xftp_get_file_e */

Looks pretty much good stuff. I don't see any stringent error checking
or some other enterprise features for hey the connection failed and
lets back out and pause for 500ms and then try again and then try again
after 1000ms etc etc with some audit logging and syslog messages.

Also ... you are fetching a file from a Windows server of some sort? So
you are not using LDAP or any sort of active directory authentication
foo here I guess.

I am using, at the moment :

$ ls -loptr /usr/local/lib | grep -i "curl"
-rwxr-xr-x 1 root 843840 Sep 24 00:38 libcurl.so.4.4.0
lrwxrwxrwx 1 root 16 Sep 24 00:38 libcurl.so.4 ->
libcurl.so.4.4.0*
lrwxrwxrwx 1 root 16 Sep 24 00:38 libcurl.so -> libcurl.so.4.4.0*
-rwxr-xr-x 1 root 1161 Sep 24 00:38 libcurl.la
-rw-r--r-- 1 root 1719032 Sep 24 00:38 libcurl.a

$ /usr/local/bin/curl --version
curl 7.50.3 (sparc64-sun-solaris2.10) libcurl/7.50.3 OpenSSL/1.0.2j
zlib/1.2.8 libidn/1.33 libssh2/1.7.0
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps
pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: IDN IPv6 Largefile NTLM NTLM_WB SSL libz TLS-SRP UnixSockets

Works like a charm.

The application codebase that processes the secure sftp of files to
destination URL's as well as fetching files hasn't changed in years.

Not sure what else to say.

Dennis

-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2016-10-20