Tim, All,
I've started a new thread to specifically look at the problem with sftp_kio not obtaining the port number to open the connection for sftp without the user manually providing it in the url. (eg: sftp://myhost:nonStdPort). This is part of bug http://bugs.pearsoncomputing.net/show_bug.cgi?id=897
Tim's commit of GIT hash e72f492 fixed the ability of sftp to connect to remote hosts, but if the remote host ssh runs on a port other than 22, the connection fails due to sftp no longer getting the correct host/port pair from either the system-wide config /etc/ssh/ssh_config or the user's ~/.ssh/config.
This worked correctly before the sftp bug was introduced and it continues to work for fish://, but I cannot find where the config files are read. It appears to be something read during ssh startup from the system-wide or user .ssh/config file. That is hinted to in the fish/README file:
NOTE: From version 1.1.3 on, compression is no longer turned on auto- matically. You have to specify it via ~/.ssh/config or wherever your local ssh client reads its settings. The same goes for all other connection parameters. OpenSSH for example has a powerful configuration file syntax which lets you configure access differently for each host, something I do not intend to duplicate. Read the ssh_config(5) man page for details.
The variable in sftp associated with the port is 'mPort', but I cannot tell where this could be set (yes, I know, just grep it, but somehow I'm getting lost in whether this is piped or cached and read somewhere rather than just declared and assigned) It appears to begin in ksshprocess.cpp with:
bool KSshProcess::setOptions(const SshOptList& opts) { kdDebug(KSSHPROC) << "KSshProcess::setOptions()" << endl; mArgs.clear(); SshOptListConstIterator it; TQString cmd, subsystem; mPassword = mUsername = mHost = TQString::null; TQCString tmp; for(it = opts.begin(); it != opts.end(); ++it) { switch( (*it).opt ) { <snip> case SSH_PORT: mArgs.append("-p"); tmp.setNum((*it).num); mArgs.append(tmp); mPort = (*it).num; break; <snip>
Also, both sftp:// and fish:// make use of an AuthInfo struct that references a port, but I cannot figure out how to tell what this contains:
kio_sftp.cpp: // Setup AuthInfo for use with password caching and the kio_sftp.cpp: AuthInfo info; kio_sftp.cpp: info.url.setPort(mPort);
fish/fish.h: /** AuthInfo object used for logging in */ fish/fish.h: KIO::AuthInfo connectionAuth;
It is frustrating because it still works fine in fish, so we should just be able to look there and find it, but it is apparently not that easy either...
Now regardless of how you look at it, the port information must be read from either /etc/ssh/ssh_config or ~/.ssh/config before the ssh connection is opened because that is the only place this information exists and it is imperative that you have that connection before attempting to open a connection -- or the connection will simply be refused and fail. So this is taking place in fish somewhere and it was taking place in sftp somewhere as well -- the questions is where??
What say the experts?? I've grepped and looked until I'm blue in the face and I've exhausted what I can do.
Tim, All,
I've started a new thread to specifically look at the problem with sftp_kio not obtaining the port number to open the connection for sftp without the user manually providing it in the url. (eg: sftp://myhost:nonStdPort). This is part of bug http://bugs.pearsoncomputing.net/show_bug.cgi?id=897
Tim's commit of GIT hash e72f492 fixed the ability of sftp to connect to remote hosts, but if the remote host ssh runs on a port other than 22, the connection fails due to sftp no longer getting the correct host/port pair from either the system-wide config /etc/ssh/ssh_config or the user's ~/.ssh/config.
This worked correctly before the sftp bug was introduced and it continues to work for fish://, but I cannot find where the config files are read. It appears to be something read during ssh startup from the system-wide or user .ssh/config file. That is hinted to in the fish/README file:
NOTE: From version 1.1.3 on, compression is no longer turned on auto- matically. You have to specify it via ~/.ssh/config or wherever your local ssh client reads its settings. The same goes for all other connection parameters. OpenSSH for example has a powerful configuration file syntax which lets you configure access differently for each host, something I do not intend to duplicate. Read the ssh_config(5) man page for details.
The variable in sftp associated with the port is 'mPort', but I cannot tell where this could be set (yes, I know, just grep it, but somehow I'm getting lost in whether this is piped or cached and read somewhere rather than just declared and assigned) It appears to begin in ksshprocess.cpp with:
bool KSshProcess::setOptions(const SshOptList& opts) { kdDebug(KSSHPROC) << "KSshProcess::setOptions()" << endl; mArgs.clear(); SshOptListConstIterator it; TQString cmd, subsystem; mPassword = mUsername = mHost = TQString::null; TQCString tmp; for(it = opts.begin(); it != opts.end(); ++it) { switch( (*it).opt ) {
<snip> case SSH_PORT: mArgs.append("-p"); tmp.setNum((*it).num); mArgs.append(tmp); mPort = (*it).num; break; <snip>
Also, both sftp:// and fish:// make use of an AuthInfo struct that references a port, but I cannot figure out how to tell what this contains:
kio_sftp.cpp: // Setup AuthInfo for use with password caching and the kio_sftp.cpp: AuthInfo info; kio_sftp.cpp: info.url.setPort(mPort);
fish/fish.h: /** AuthInfo object used for logging in */ fish/fish.h: KIO::AuthInfo connectionAuth;
It is frustrating because it still works fine in fish, so we should just be able to look there and find it, but it is apparently not that easy either...
Now regardless of how you look at it, the port information must be read from either /etc/ssh/ssh_config or ~/.ssh/config before the ssh connection is opened because that is the only place this information exists and it is imperative that you have that connection before attempting to open a connection -- or the connection will simply be refused and fail. So this is taking place in fish somewhere and it was taking place in sftp somewhere as well -- the questions is where??
What say the experts?? I've grepped and looked until I'm blue in the face and I've exhausted what I can do.
-- David C. Rankin, J.D.,P.E.
I am not convinced that kio_sftp (or kio_fish) reads ANYTHING from ~/.ssh/config, rather I suspect it relies on implicit ssh behavior when ssh is invoked in the background. Something probably changed there; I will look into the problem shortly.
Tim
On 04/24/2012 12:51 PM, Timothy Pearson wrote:
I am not convinced that kio_sftp (or kio_fish) reads ANYTHING from ~/.ssh/config, rather I suspect it relies on implicit ssh behavior when ssh is invoked in the background. Something probably changed there; I will look into the problem shortly.
Tim
I'm looking too. I'm reading the ssh man pages. I'm guessing that there is a separate call to ssh somewhere that causes ssh to return the port number for the host or that there is a way that this information is read 'on-the-fly' when 'ssh hostname' is given.
When I'm in a directory in the code line tdebase/kioslave/sftp -- is there some c++ 'tool' I can run that will scan the code and give me a representation of how the files and functions are called? Kind of like a process diagram? Something that says 'this file is executed first, second, etc..' That would really help me find my way around.
Especially with things like bool KSshProcess::setOptions(const SshOptList& opts) -- what the heck is SshOptList and where is it filled? That's the type question that really hangs me up. I can grep it, but that doesn't help 90% of the time.
ksshprocess seems to talk the most about setting up the connection. eg:
ksshprocess.h
* To setup a SSH connection first create a list of options to use and tell * KSshProcess about your options. Then start the ssh connection. Once the * connection is setup use the stdin, stdout, stderr, and pty file descriptors * to communicate with ssh. For a detailed example of how to use, see * ksshprocesstest.cpp.
ksshprocess.cpp
* To start ssh we take the arguments the user gave us * in the SshOptList and build the ssh command arguments based on the version * of ssh we are using. This command and its arguments are passed to * PTYProcess for execution. Once ssh is started we scan each line of input * from stdin, stderr, and the pty for recognizable strings. The recognizable * strings are taken from several string tables. Each table contains a string * for each specific version of ssh we support and a string for a generic * version of OpenSSH and commercial SSH incase we don't recognized the * specific ssh version strings (as when a new SSH version is released after * a release of KSshProcess). There are tables for ssh version strings, * password prompts, new host key errors, different host key errors, * messages than indicate a successful connect, authentication errors, etc. * If we find user interaction is necessary, for instance to provide a * password or passphrase, we return a err code to the user who can send * a message to KSshProcess, using one of several methods, to correct * the error.
I have no doubt you will find the key to unlock the chest far before I do...
On 04/24/2012 01:38 PM, David C. Rankin wrote:
I'm looking too. I'm reading the ssh man pages. I'm guessing that there is a separate call to ssh somewhere that causes ssh to return the port number for the host or that there is a way that this information is read 'on-the-fly' when 'ssh hostname' is given.
AARGH!!!
We have been thinking about this thing 'bass-ackwards'. All sftp has to do is read what 'ssh' spits out upon connection. You don't care about reading any config files prior to calling 'ssh', 'ssh' takes care of that for you automagically!
Think about it:
13:43 providence:~/tde/tmp/patch/tdebase/kioslave/sftp> ssh -v phoenix OpenSSH_5.9p1, OpenSSL 1.0.1a 19 Apr 2012 debug1: Reading configuration data /home/david/.ssh/config debug1: /home/david/.ssh/config line 36: Applying options for phoenix debug1: Reading configuration data /etc/ssh/ssh_config debug1: /etc/ssh/ssh_config line 20: Applying options for * debug1: Connecting to phoenix [192.168.7.15] port 6630. debug1: Connection established.
When ssh is called, it reads BOTH /etc/ssh/ssh_config and ~/.ssh/config and acts accordingly. This means the only reason sftp://hostname fails when ssh is on a non-standard port is that the sftp:// code is NOT interpreting the ssh response correctly and is erroneously closing a perfectly good connection that ssh has established. Basically -- the sftp code just gets stuck waiting on some response that has changed in the new openssh -- doesn't know what to do with the response it gets -- and just sits there until it times out with an error.
You will have to be the wizard that figures out where this happens, but is suspect it is in ksshprocess with the version tables that are set up at the top of the file :)
On 04/24/2012 01:48 PM, David C. Rankin wrote:
When ssh is called, it reads BOTH /etc/ssh/ssh_config and ~/.ssh/config and acts accordingly. This means the only reason sftp://hostname fails when ssh is on a non-standard port is that the sftp:// code is NOT interpreting the ssh response correctly and is erroneously closing a perfectly good connection that ssh has established. Basically -- the sftp code just gets stuck waiting on some response that has changed in the new openssh -- doesn't know what to do with the response it gets -- and just sits there until it times out with an error.
Tim,
I've added a couple of more 'guesses' at where the code can be failing to bug 897. At least to just provide a possible starting point of things to eliminate.
On 04/24/2012 01:48 PM, David C. Rankin wrote:
When ssh is called, it reads BOTH /etc/ssh/ssh_config and ~/.ssh/config and acts accordingly. This means the only reason sftp://hostname fails when ssh is on a non-standard port is that the sftp:// code is NOT interpreting the ssh response correctly and is erroneously closing a perfectly good connection that ssh has established. Basically -- the sftp code just gets stuck waiting on some response that has changed in the new openssh -- doesn't know what to do with the response it gets -- and just sits there until it times out with an error.
Tim,
I've added a couple of more 'guesses' at where the code can be failing to bug 897. At least to just provide a possible starting point of things to eliminate.
-- David C. Rankin, J.D.,P.E.
This should be fixed in GIT hash 073dc86, let me know if the problem persists.
Tim
On 04/24/2012 06:46 PM, Timothy Pearson wrote:
This should be fixed in GIT hash 073dc86, let me know if the problem persists.
Tim
Do I need to rebuild both tdelibs and tdebase this time or just tdebase ?
Can't wait to see what it was! Hell yah!
On 04/24/2012 06:46 PM, Timothy Pearson wrote:
This should be fixed in GIT hash 073dc86, let me know if the problem persists.
Tim
Do I need to rebuild both tdelibs and tdebase this time or just tdebase ?
Just tdebase.
Tim
On 04/25/2012 12:14 AM, Timothy Pearson wrote:
Just tdebase.
Tim
looks like it is finally fixed. Only error I saw was when "Adding" the sftp://server/ directory to the 'places' part of the file-save dialog. When I rt-clicked and chose 'Add Entry' -> the url was blank, even though I had already navigated to sftp://server/. It has always provided a directory name and the kio (eg: fish://, sftp:// and the current remote directory as the default directory to add. This is probably not sftp related, although the one I have with fish did what it was supposed to when it was added a while back??
Regardless, them are small potatoes compared to getting sftp:// back up and running! 2 years it was down. The white wizard pulled it off. Good job Tim.
Now even though I have the patch and I've looked at it, I still don't follow what changes you made - so could you help a brother out -- What was it??
On 04/25/2012 12:14 AM, Timothy Pearson wrote:
Just tdebase.
Tim
looks like it is finally fixed. Only error I saw was when "Adding" the sftp://server/ directory to the 'places' part of the file-save dialog. When I rt-clicked and chose 'Add Entry' -> the url was blank, even though I had already navigated to sftp://server/. It has always provided a directory name and the kio (eg: fish://, sftp:// and the current remote directory as the default directory to add. This is probably not sftp related, although the one I have with fish did what it was supposed to when it was added a while back??
Regardless, them are small potatoes compared to getting sftp:// back up and running! 2 years it was down. The white wizard pulled it off. Good job Tim.
Glad to be of assistance! :-)
Now even though I have the patch and I've looked at it, I still don't follow what changes you made - so could you help a brother out -- What was it??
You would need to know some of how kio_sftp operates to know what I did, so I will try to sum it up below:
kio_sftp can be invoked in two ways, as you are aware: one with a port number, and one without. Internally, kio_sftp runs an ssh process in verbose sftp mode in order to view/modify files on the remote system. To to this, kio_sftp passes required parameters and optional parameters, which kio_sftp generates internally based on information given, to the ssh process during process invocation.
Optional parameters include the port number--if the port number is not manually specified when kio_sftp is invoked, the port number parameter should not be passed to the ssh process, allowing ssh to connect on whatever default or preconfigured port it desires.
The recent problem arose due to the fact that the port number was ALWAYS being specified via the removed code. The new code sets mPort to -1 if a port number was not manually passed to kio_sftp, thus indicating that the optional port parameter should NOT be passed to the ssh process on invocation.
Does this make sense?
Tim
On 04/25/2012 02:05 AM, Timothy Pearson wrote:
The recent problem arose due to the fact that the port number was ALWAYS being specified via the removed code. The new code sets mPort to -1 if a port number was not manually passed to kio_sftp, thus indicating that the optional port parameter should NOT be passed to the ssh process on invocation.
Does this make sense?
Tim
Yep,
Makes perfect sense. I looked over that part of the code 15 times, looking at it with a strange look on my face (wondering - "Is that right?"), but not being able to see through the process to what happens either way and what difference it made. That is the "You would need to know some of how kio_sftp operates to know what I did..." that I simply didn't have.
I'm glad it is fixed!
Uh-oh! Just saw the update 'openssh-6.0p1-1' ready to install. We'll go give it a test and report back. I'm sure it will be no issue :)
On 04/25/2012 09:28 AM, David C. Rankin wrote:
Uh-oh! Just saw the update 'openssh-6.0p1-1' ready to install. We'll go give it a test and report back. I'm sure it will be no issue :)
sftp continues to work just fine with openssh-6.0p1-1. Bug 897 appears to be done :)
On 04/25/2012 02:05 AM, Timothy Pearson wrote:
Does this make sense?
Tim
Yep,
I also see you reverted changes to ksshprocess.cpp as well. It is amazing that the total fix only required removing 5 lines of code and adding:
mPort = -1;
How did you debug this? Did you just add kdDebug messages? Use some tool that allows you to set some sort of breakpoint in the code and then watch mPort? If you can pass those nuggets along, that will help Darrell and I come up to speed to better help with this type debugging.
On 04/25/2012 02:05 AM, Timothy Pearson wrote:
Does this make sense?
Tim
Yep,
I also see you reverted changes to ksshprocess.cpp as well.
??? Nothing should have been reverted. :-)
It is amazing that the total fix only required removing 5 lines of code and adding:
mPort = -1;
How did you debug this? Did you just add kdDebug messages? Use some tool that allows you to set some sort of breakpoint in the code and then watch mPort? If you can pass those nuggets along, that will help Darrell and I come up to speed to better help with this type debugging.
Generally I read the code and insert debug printf() statements in the most likely execution paths, near any chunks of code that I think are directly handling the feature that has the bug in it. These printf() statements spew the status of any local variables that I think might be relevant to the bug report; i.e. if there is a problem with port numbers, I look for the sections of code that deal with port numbers, then print the port numbers (at a minimum) at the beginning and end of those sections.
Tim
On 04/25/2012 12:16 PM, Timothy Pearson wrote:
Generally I read the code and insert debug printf() statements in the most likely execution paths, near any chunks of code that I think are directly handling the feature that has the bug in it. These printf() statements spew the status of any local variables that I think might be relevant to the bug report; i.e. if there is a problem with port numbers, I look for the sections of code that deal with port numbers, then print the port numbers (at a minimum) at the beginning and end of those sections.
Tim
OK, that I can do -- I just need a faster box to build on :)
The reversion I was talking about was this stuff. They are not part of the final fix, but I saw them in the diffs I did between old/new and now they are gone again:
diff -uNrb tdebase.orig/kioslave/sftp/ksshprocess.cpp tdebase/kioslave/sftp/ksshprocess.cpp --- tdebase.orig/kioslave/sftp/ksshprocess.cpp 2012-02-08 20:13:49.000000000 -0600 +++ tdebase/kioslave/sftp/ksshprocess.cpp 2012-04-23 15:39:54.000000000 -0500 @@ -569,7 +569,9 @@ // If we still don't have anything in our buffer so there must // not be anything on the pty or stderr. Setup a select() // to wait for some data from SSH. - if( buffer.empty() ) { + // Hack around select() failure on newer systems + unsigned long milliseconds = 0; + while ((buffer.size() == 0) && (milliseconds < (60*1000))) { //kdDebug(KSSHPROC) << "KSshProcess::getLine(): " << // "Line buffer empty, calling select() to wait for data." << endl; int errfd = ssh.stderrFd(); @@ -616,14 +618,18 @@ // had data on it first. if( FD_ISSET(ptyfd, &rfds) ) { ptyLine = ssh.readLineFromPty(false); + if (ptyLine.size() > 0) { buffer.prepend(TQString(ptyLine)); + } //kdDebug(KSSHPROC) << "KSshProcess::getLine(): " // "line from pty -" << ptyLine << endl; }
if( FD_ISSET(errfd, &rfds) ) { errLine = ssh.readLineFromStderr(false); + if (errLine.size() > 0) { buffer.prepend(TQString(errLine)); + } //kdDebug(KSSHPROC) << "KSshProcess::getLine(): " // "line from err -" << errLine << endl; } @@ -638,6 +644,10 @@ "Exception on std err file descriptor." << endl; }
+ if (buffer.size() == 0) { + milliseconds++; + usleep(1000); + } } }
-
David C. Rankin -
Timothy Pearson