Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support user password login ssh #129

Closed
wants to merge 6 commits into from
Closed

support user password login ssh #129

wants to merge 6 commits into from

Conversation

baoyachi
Copy link
Contributor

@baoyachi baoyachi commented Aug 3, 2023

fixed: #119

@jonhoo
Copy link
Collaborator

jonhoo commented Aug 3, 2023

This change is Reviewable

@codecov-commenter
Copy link

Codecov Report

Merging #129 (385f70d) into master (c7bb427) will decrease coverage by 2.44%.
The diff coverage is 38.66%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
Files Changed Coverage Δ
src/session.rs 60.39% <0.00%> (-14.92%) ⬇️
src/builder.rs 70.93% <52.72%> (-5.64%) ⬇️

if let Some(pass) = &self.password {
let mut init = Command::new("sshpass");

init.stdin(Stdio::null())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we deduplicate the code here?

Most of the args provided are the same

/// If connecting requires interactive authentication based on `STDIN` (such as reading a
/// password), the connection will fail. Consider setting up keypair-based authentication
/// instead.
/// password), consider use [`Session::connect_with_pass`] function instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// password), consider use [`Session::connect_with_pass`] function instead.
/// password), consider use [`Session::connect_with_pass`] function instead, though do note
/// that it passes the password via cmdline currently,
/// so it might leak your password to others
/// using the same machine and have privileges
/// to check /proc/$ssh_pid/cmdline

@@ -60,6 +60,7 @@ pub struct SessionBuilder {
user: Option<String>,
port: Option<String>,
keyfile: Option<PathBuf>,
password: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO we can use zeroize here to avoid leaking the password.

/// optimized later. If using this feature, pay attention to security risks.
///
/// For more options, see [`SessionBuilder`].
#[cfg(feature = "process-mux")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to put it under another feature "ssh-pass-login" since in the future we will implement this ourselves using pty, which will pull in additional features most don't use.

@NobodyXu
Copy link
Member

@baoyachi didn't you accidentally delete the repository?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

how to use Username and password authetication
4 participants