Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 313991 - [terminal] [ssh] cannot programatically pass password to SshConnector
Summary: [terminal] [ssh] cannot programatically pass password to SshConnector
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: Terminal (show other bugs)
Version: 3.2   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 3.2 RC3   Edit
Assignee: Michael Scharf CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-21 18:25 EDT by Bryan Hunt CLA
Modified: 2010-05-28 15:16 EDT (History)
1 user (show)

See Also:
mober.at+eclipse: pmc_approved+


Attachments
proposed patch (917 bytes, patch)
2010-05-21 18:25 EDT, Bryan Hunt CLA
mober.at+eclipse: iplog+
Details | Diff
example application (7.19 KB, application/zip)
2010-05-22 10:02 EDT, Bryan Hunt CLA
no flags Details
patch to allow providing a password in the settings (1.92 KB, patch)
2010-05-24 09:16 EDT, Michael Scharf CLA
no flags Details | Diff
final released patch (5.26 KB, patch)
2010-05-27 06:30 EDT, Martin Oberhuber CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bryan Hunt CLA 2010-05-21 18:25:54 EDT
Created attachment 169563 [details]
proposed patch

I'm using the TerminalViewControlFactory to create a  terminal in my own view using the SshConnector.  It's not working correctly because the SshSettings does not load the password from the settings store.  Here is a patch to fix the problem.
Comment 1 Martin Oberhuber CLA 2010-05-21 18:47:37 EDT
Comment on attachment 169563 [details]
proposed patch

The patch might seem like a no-brainer but I am a little concerned about security here. We should avoid keeping passwords in plaintext wherever possible - be it in memory or in files.

Do you have a real use-case for this? In general, I would always prefer using private/public key authentication with SSH, perhaps even using a passphrase on the keyring (I don't think we support storing the passphrase at the moment in the terminal SSH connector, it will always be asked interactively).

Also, can you post a short snippet that shows how exactly you are using the TerminalViewFactory? This may help better understanding your case.
Comment 2 Michael Scharf CLA 2010-05-21 19:49:48 EDT
I would agree with Martin. I did exclude the password because it will be saved in plain text somewhere in the .metadata directory.

I would consider this patch as *very* harmful. If you can provide a patch can guarantee that the "Password" property is never saved then I would consider accepting it.
Comment 3 Bryan Hunt CLA 2010-05-22 10:02:15 EDT
Created attachment 169586 [details]
example application

Here is how I'm using your API.  If there's a better way to do this, a pointer to the documentation would be helpful.
Comment 4 Michael Scharf CLA 2010-05-24 09:06:46 EDT
I see... Well, if we only read the Password form the settings (and do not store it) then it is in the responsibility of the settings provider to deal with the security problem....

Martin. I think we can apply the patch. Because it will allow clients to have their own way of getting the password.
Comment 5 Michael Scharf CLA 2010-05-24 09:16:02 EDT
Created attachment 169674 [details]
patch to allow providing a password in the settings

I think this is a non critical fix that can go into the next RC
Comment 6 Martin Oberhuber CLA 2010-05-26 14:35:22 EDT
I'm going to consider this for RC3. Arguably, as long as we don't ISetingsStore.set("Password") to something there shouldn't be any risk.
Comment 7 Martin Oberhuber CLA 2010-05-27 06:30:52 EDT
Created attachment 170162 [details]
final released patch

I checked again, and I'm going to release the change.

A bit more work was needed since this was the first SSH Terminal change in the TM 3.2 release, so the plugin micro version had to be updated and a few Copyright Years had to be updated.
Comment 8 Martin Oberhuber CLA 2010-05-27 06:32:38 EDT
Released >= I20100527.
Comment 9 Martin Oberhuber CLA 2010-05-28 14:55:46 EDT
Hi Bryan, 

are you actually self-employed (making this contribution during your spare time), or working for a company? I would like to know this for our contributor's list
www.eclipse.org/dsdp/tm/development/contributors.php

Thanks,
Martin
Comment 10 Bryan Hunt CLA 2010-05-28 15:16:56 EDT
(In reply to comment #9)
> Hi Bryan, 
> 
> are you actually self-employed (making this contribution during your spare
> time), or working for a company? I would like to know this for our
> contributor's list
> www.eclipse.org/dsdp/tm/development/contributors.php

I'm employed by an Eclipse member company; however, I did this on my own time so my employer should not be named unless necessary.