- Hans-Jurgen May pointed out that trying SCP or SFTP over a SOCKS proxy

  crashed libcurl. This is now addressed by making sure we use "plain send"
  internally when doing the socks handshake instead of the Curl_write()
  function which is designed to use the "target" protocol. That's then SCP or
  SFTP in this case. I also took the opportunity and cleaned up some ssh-
  related #ifdefs in the code for readability.

6 files changed, 65 insertions, 20 deletions

diff --git a/CHANGES b/CHANGES
index ae1cdf6b3..edec49675 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,14 @@
 
                                   Changelog
 
+Daniel Stenberg (20 Jun 2008)
+- Hans-Jurgen May pointed out that trying SCP or SFTP over a SOCKS proxy
+  crashed libcurl. This is now addressed by making sure we use "plain send"
+  internally when doing the socks handshake instead of the Curl_write()
+  function which is designed to use the "target" protocol. That's then SCP or
+  SFTP in this case. I also took the opportunity and cleaned up some ssh-
+  related #ifdefs in the code for readability.
+
 Daniel Stenberg (19 Jun 2008)
 - Christopher Palow fixed a curl_multi_socket() issue which previously caused
   libcurl to not tell the app properly when a socket was closed (when the name
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index cf04e5dc8..e48fdb53e 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -22,6 +22,7 @@ This release includes the following bugfixes:
  o Fixed the multi interface connection re-use with NSS-built libcurl
  o connection re-use when using the multi interface with pipelining enabled
  o curl_multi_socket() socket callback fix for close/re-create sockets case
+ o SCP or SFTP over socks proxy crashed
 
 This release includes the following known bugs:
 
@@ -39,6 +40,6 @@ This release would not have looked like this without help, code, reports and
 advice from friends like these:
 
  Lenny Rachitsky, Axel Tillequin, Arnaud Ebalard, Yang Tse, Dan Fandrich,
- Rob Crittenden, Dengminwen, Christopher Palow
+ Rob Crittenden, Dengminwen, Christopher Palow, Hans-Jürgen May
 
         Thanks! (and sorry if I forgot to mention someone)
diff --git a/lib/sendf.c b/lib/sendf.c
index 3869a4d4f..52edbc4ce 100644
--- a/lib/sendf.c
+++ b/lib/sendf.c
@@ -356,16 +356,12 @@ CURLcode Curl_write(struct connectdata *conn,
   int num = (sockfd == conn->sock[SECONDARYSOCKET]);
 
   if(conn->ssl[num].state == ssl_connection_complete)
-    /* only TRUE if SSL enabled */
     bytes_written = Curl_ssl_send(conn, num, mem, len);
-#ifdef USE_LIBSSH2
-  else if(conn->protocol & PROT_SCP)
+  else if(Curl_ssh_enabled(conn, PROT_SCP))
     bytes_written = Curl_scp_send(conn, num, mem, len);
-  else if(conn->protocol & PROT_SFTP)
+  else if(Curl_ssh_enabled(conn, PROT_SFTP))
     bytes_written = Curl_sftp_send(conn, num, mem, len);
-#endif /* !USE_LIBSSH2 */
   else if(conn->sec_complete)
-    /* only TRUE if krb enabled */
     bytes_written = Curl_sec_send(conn, num, mem, len);
   else
     bytes_written = send_plain(conn, num, mem, len);
@@ -376,6 +372,29 @@ CURLcode Curl_write(struct connectdata *conn,
   return retcode;
 }
 
+/*
+ * Curl_write_plain() is an internal write function that sends data to the
+ * server using plain sockets only. Otherwise meant to have the exact same
+ * proto as Curl_write()
+ */
+CURLcode Curl_write_plain(struct connectdata *conn,
+                          curl_socket_t sockfd,
+                          const void *mem,
+                          size_t len,
+                          ssize_t *written)
+{
+  ssize_t bytes_written;
+  CURLcode retcode;
+  int num = (sockfd == conn->sock[SECONDARYSOCKET]);
+
+  bytes_written = send_plain(conn, num, mem, len);
+
+  *written = bytes_written;
+  retcode = (-1 != bytes_written)?CURLE_OK:CURLE_SEND_ERROR;
+
+  return retcode;
+}
+
 static CURLcode pausewrite(struct SessionHandle *data,
                            int type, /* what type of data */
                            char *ptr,
@@ -574,8 +593,7 @@ int Curl_read(struct connectdata *conn, /* connection data */
       return -1; /* -1 from Curl_ssl_recv() means EWOULDBLOCK */
     }
   }
-#ifdef USE_LIBSSH2
-  else if(conn->protocol & (PROT_SCP|PROT_SFTP)) {
+  else if(Curl_ssh_enabled(conn, (PROT_SCP|PROT_SFTP))) {
     if(conn->protocol & PROT_SCP)
       nread = Curl_scp_recv(conn, num, buffertofill, bytesfromsocket);
     else if(conn->protocol & PROT_SFTP)
@@ -589,7 +607,6 @@ int Curl_read(struct connectdata *conn, /* connection data */
       /* since it is negative and not EGAIN, it was a protocol-layer error */
       return CURLE_RECV_ERROR;
   }
-#endif /* !USE_LIBSSH2 */
   else {
     if(conn->sec_complete)
       nread = Curl_sec_read(conn, sockfd, buffertofill,
diff --git a/lib/sendf.h b/lib/sendf.h
index 7ffa4768a..2d507ee23 100644
--- a/lib/sendf.h
+++ b/lib/sendf.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2007, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2008, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -62,12 +62,18 @@ void Curl_read_rewind(struct connectdata *conn,
 int Curl_read(struct connectdata *conn, curl_socket_t sockfd,
               char *buf, size_t buffersize,
               ssize_t *n);
-/* internal write-function, does plain socket, SSL and krb4 */
+/* internal write-function, does plain socket, SSL, SCP, SFTP and krb4 */
 CURLcode Curl_write(struct connectdata *conn,
                     curl_socket_t sockfd,
                     const void *mem, size_t len,
                     ssize_t *written);
 
+/* internal write-function, does plain sockets ONLY */
+CURLcode Curl_write_plain(struct connectdata *conn,
+                          curl_socket_t sockfd,
+                          const void *mem, size_t len,
+                          ssize_t *written);
+
 /* the function used to output verbose information */
 int Curl_debug(struct SessionHandle *handle, curl_infotype type,
                char *data, size_t size,
diff --git a/lib/socks.c b/lib/socks.c
index 06a513e80..d2cb65522 100644
--- a/lib/socks.c
+++ b/lib/socks.c
@@ -242,8 +242,9 @@ CURLcode Curl_SOCKS4(const char *proxy_name,
     }
 
     /* Send request */
-    code = Curl_write(conn, sock, (char *)socksreq, packetsize + hostnamelen,
-                      &written);
+    code = Curl_write_plain(conn, sock, (char *)socksreq,
+                            packetsize + hostnamelen,
+                            &written);
     if((code != CURLE_OK) || (written != packetsize + hostnamelen)) {
       failf(data, "Failed to send SOCKS4 connect request.");
       return CURLE_COULDNT_CONNECT;
@@ -251,7 +252,8 @@ CURLcode Curl_SOCKS4(const char *proxy_name,
     if (protocol4a && hostnamelen == 0) {
       /* SOCKS4a with very long hostname - send that name separately */
       hostnamelen = (ssize_t)strlen(hostname) + 1;
-      code = Curl_write(conn, sock, (char *)hostname, hostnamelen, &written);
+      code = Curl_write_plain(conn, sock, (char *)hostname, hostnamelen,
+                              &written);
       if((code != CURLE_OK) || (written != hostnamelen)) {
         failf(data, "Failed to send SOCKS4 connect request.");
         return CURLE_COULDNT_CONNECT;
@@ -432,8 +434,8 @@ CURLcode Curl_SOCKS5(const char *proxy_name,
 
   Curl_nonblock(sock, FALSE);
 
-  code = Curl_write(conn, sock, (char *)socksreq, (2 + (int)socksreq[1]),
-                      &written);
+  code = Curl_write_plain(conn, sock, (char *)socksreq, (2 + (int)socksreq[1]),
+                          &written);
   if((code != CURLE_OK) || (written != (2 + (int)socksreq[1]))) {
     failf(data, "Unable to send initial SOCKS5 request.");
     return CURLE_COULDNT_CONNECT;
@@ -502,7 +504,7 @@ CURLcode Curl_SOCKS5(const char *proxy_name,
     memcpy(socksreq + len, proxy_password, (int) pwlen);
     len += pwlen;
 
-    code = Curl_write(conn, sock, (char *)socksreq, len, &written);
+    code = Curl_write_plain(conn, sock, (char *)socksreq, len, &written);
     if((code != CURLE_OK) || (len != written)) {
       failf(data, "Failed to send SOCKS5 sub-negotiation request.");
       return CURLE_COULDNT_CONNECT;
@@ -613,7 +615,7 @@ CURLcode Curl_SOCKS5(const char *proxy_name,
     *((unsigned short*)&socksreq[8]) = htons((unsigned short)remote_port);
   }
 
-  code = Curl_write(conn, sock, (char *)socksreq, packetsize, &written);
+  code = Curl_write_plain(conn, sock, (char *)socksreq, packetsize, &written);
   if((code != CURLE_OK) || (written != packetsize)) {
     failf(data, "Failed to send SOCKS5 connect request.");
     return CURLE_COULDNT_CONNECT;
diff --git a/lib/ssh.h b/lib/ssh.h
index e87b21ba3..9bbeaeda8 100644
--- a/lib/ssh.h
+++ b/lib/ssh.h
@@ -8,7 +8,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2007, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2008, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -37,6 +37,17 @@ ssize_t Curl_sftp_send(struct connectdata *conn, int sockindex,
                        const void *mem, size_t len);
 ssize_t Curl_sftp_recv(struct connectdata *conn, int sockindex,
                        char *mem, size_t len);
+bool Curl_ssh_enabled(struct connectdata *conn,
+                      int prot);
+
+#define Curl_ssh_enabled(conn,prot) (conn->protocol & prot)
+
+#else /* USE_LIBSSH2 */
+#define Curl_ssh_enabled(x,y) 0
+#define Curl_scp_send(a,b,c,d) 0
+#define Curl_sftp_send(a,b,c,d) 0
+#define Curl_scp_recv(a,b,c,d) 0
+#define Curl_sftp_recv(a,b,c,d) 0
 
 #endif /* USE_LIBSSH2 */