Bugzilla – Full Text Bug Listing
| Summary: | [terminal] [ssh] cannot programatically pass password to SshConnector | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] Target Management | Reporter: | Bryan Hunt <bhunt> | ||||||||||
| Component: | Terminal | Assignee: | Michael Scharf <eclipse> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Martin Oberhuber <mober.at+eclipse> | ||||||||||
| Severity: | major | ||||||||||||
| Priority: | P3 | CC: | eclipse | ||||||||||
| Version: | 3.2 | Flags: | mober.at+eclipse:
pmc_approved+
|
||||||||||
| Target Milestone: | 3.2 RC3 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
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.
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. 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.
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. 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
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.
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.
Released >= I20100527. 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 (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. |
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.