Skip to content

Commit

Permalink
Added password-factor=N option to determine which authentication fa…
Browse files Browse the repository at this point in the history
…ctor is the password. This is helpful when the password is read from the config file or from the keychain. Fixes #78
  • Loading branch information
ancwrd1 committed Feb 16, 2025
1 parent 206ca5d commit 31f016c
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 33 deletions.
9 changes: 9 additions & 0 deletions snx-rs-gui/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ struct MyWidgets {
tunnel_type: gtk::ComboBoxText,
user_name: gtk::Entry,
password: gtk::Entry,
password_factor: gtk::Entry,
no_dns: gtk::CheckButton,
search_domains: gtk::Entry,
ignored_domains: gtk::Entry,
Expand Down Expand Up @@ -116,6 +117,7 @@ impl MyWidgets {
self.ike_lifetime.text().parse::<u32>()?;
self.esp_lifetime.text().parse::<u32>()?;
self.ike_port.text().parse::<u16>()?;
self.password_factor.text().parse::<usize>()?;

let dns_servers = self.dns_servers.text();
if !dns_servers.is_empty() {
Expand Down Expand Up @@ -175,6 +177,7 @@ impl SettingsDialog {
let tunnel_type = gtk::ComboBoxText::builder().build();
let user_name = gtk::Entry::builder().text(&params.user_name).build();
let password = gtk::Entry::builder().text(&params.password).visibility(false).build();
let password_factor = gtk::Entry::builder().text(params.password_factor.to_string()).build();

let no_dns = gtk::CheckButton::builder().active(params.no_dns).build();

Expand Down Expand Up @@ -399,6 +402,7 @@ impl SettingsDialog {
tunnel_type,
user_name,
password,
password_factor,
no_dns,
search_domains,
ignored_domains,
Expand Down Expand Up @@ -473,6 +477,7 @@ impl SettingsDialog {
};
params.user_name = self.widgets.user_name.text().into();
params.password = self.widgets.password.text().into();
params.password_factor = self.widgets.password_factor.text().parse()?;
params.no_dns = self.widgets.no_dns.is_active();
params.search_domains = self
.widgets
Expand Down Expand Up @@ -746,6 +751,10 @@ impl SettingsDialog {
mfa_prompts.pack_start(&self.widgets.mfa_prompts, false, true, 0);
misc_box.pack_start(&mfa_prompts, false, true, 6);

let password_factor = self.form_box("Index of password factor, 1..N");
password_factor.pack_start(&self.widgets.password_factor, false, true, 0);
misc_box.pack_start(&password_factor, false, true, 6);

let no_keychain = self.form_box("Do not store passwords in the keychain");
no_keychain.pack_start(&self.widgets.no_keychain, false, true, 0);
misc_box.pack_start(&no_keychain, false, true, 6);
Expand Down
11 changes: 11 additions & 0 deletions snx-rs/src/cmdline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ pub struct CmdlineParams {
#[clap(long = "password", short = 'p', help = "Password in base64-encoded form")]
pub password: Option<String>,

#[clap(
long = "password-factor",
short = 'Y',
help = "Numerical index of the password factor, 1..N [default: 1]"
)]
pub password_factor: Option<usize>,

#[clap(long = "config-file", short = 'c', help = "Read parameters from config file")]
pub config_file: Option<PathBuf>,

Expand Down Expand Up @@ -225,6 +232,10 @@ impl CmdlineParams {
let _ = other.decode_password();
}

if let Some(password_factor) = self.password_factor {
other.password_factor = password_factor;
}

if let Some(log_level) = self.log_level {
other.log_level = log_level.to_string();
}
Expand Down
11 changes: 6 additions & 5 deletions snx-rs/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,17 +139,18 @@ async fn main_standalone(params: TunnelParams) -> anyhow::Result<()> {
connector.authenticate().await?
};

let mut first_mfa = true;
let mut mfa_index = 0;

while let SessionState::PendingChallenge(challenge) = session.state.clone() {
match challenge.mfa_type {
MfaType::PasswordInput => {
mfa_index += 1;

let prompt = mfa_prompts
.pop_front()
.unwrap_or_else(|| AuthPrompt::new_password(&challenge.prompt));
.unwrap_or_else(|| AuthPrompt::new("", &challenge.prompt));

let input = if !params.password.is_empty() && first_mfa && prompt.is_password() {
first_mfa = false;
let input = if !params.password.is_empty() && mfa_index == params.password_factor {
Ok(params.password.clone())
} else {
TtyPrompt.get_secure_input(&prompt)
Expand All @@ -172,7 +173,7 @@ async fn main_standalone(params: TunnelParams) -> anyhow::Result<()> {
session = connector.challenge_code(session, &otp).await?;
}
MfaType::UserNameInput => {
let prompt = AuthPrompt::new("", "username", &challenge.prompt);
let prompt = AuthPrompt::new("Username is required for authentication", &challenge.prompt);
let input = TtyPrompt.get_plain_input(&prompt)?;
session = connector.challenge_code(session, &input).await?;
}
Expand Down
23 changes: 11 additions & 12 deletions snxcore/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub struct ServiceController<B, P> {
mfa_prompts: Option<VecDeque<AuthPrompt>>,
password_from_keychain: String,
username: String,
first_mfa: bool,
mfa_index: usize,
browser_controller: B,
}

Expand All @@ -64,7 +64,7 @@ where
mfa_prompts: None,
password_from_keychain: String::new(),
username: String::new(),
first_mfa: true,
mfa_index: 0,
browser_controller,
}
}
Expand Down Expand Up @@ -105,14 +105,12 @@ where
}

async fn process_mfa_request(&mut self, mfa: &MfaChallenge) -> anyhow::Result<ConnectionStatus> {
let first_mfa = self.first_mfa;

match self.get_mfa_input(mfa).await {
Ok(input) => {
let result = self.do_challenge_code(input.clone()).await;
if result.is_ok()
&& mfa.mfa_type == MfaType::PasswordInput
&& first_mfa
&& self.mfa_index == self.params.password_factor
&& !self.params.no_keychain
&& !input.is_empty()
{
Expand All @@ -130,23 +128,23 @@ where
async fn get_mfa_input(&mut self, mfa: &MfaChallenge) -> anyhow::Result<String> {
match mfa.mfa_type {
MfaType::PasswordInput => {
self.mfa_index += 1;

let prompt = self
.mfa_prompts
.as_mut()
.and_then(|p| p.pop_front())
.unwrap_or_else(|| AuthPrompt::new_password(&mfa.prompt));
.unwrap_or_else(|| AuthPrompt::new("", &mfa.prompt));

if !self.params.password.is_empty() && self.first_mfa && prompt.is_password() {
self.first_mfa = false;
if !self.params.password.is_empty() && self.mfa_index == self.params.password_factor {
Ok(self.params.password.clone())
} else if !self.password_from_keychain.is_empty() && self.first_mfa && prompt.is_password() {
self.first_mfa = false;
} else if !self.password_from_keychain.is_empty() && self.mfa_index == self.params.password_factor {
Ok(self.password_from_keychain.clone())
} else {
let prompt = if self.params.server_prompt {
prompt
} else {
AuthPrompt::new_password(&mfa.prompt)
AuthPrompt::new("", &mfa.prompt)
};
let input = self.prompt.get_secure_input(&prompt)?;
Ok(input)
Expand All @@ -169,7 +167,7 @@ where
}
}
MfaType::UserNameInput => {
let prompt = AuthPrompt::new("", "username", &mfa.prompt);
let prompt = AuthPrompt::new("Username is required for authentication", &mfa.prompt);
let input = self.prompt.get_plain_input(&prompt)?;
self.username = input.clone();

Expand Down Expand Up @@ -257,6 +255,7 @@ where
}

async fn fill_mfa_prompts(&mut self) {
self.mfa_index = 0;
self.mfa_prompts
.replace(server_info::get_mfa_prompts(&self.params).await.unwrap_or_default());
}
Expand Down
16 changes: 1 addition & 15 deletions snxcore/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,32 +136,18 @@ pub enum TunnelServiceResponse {
#[derive(Debug, Clone, PartialEq)]
pub struct AuthPrompt {
pub header: String,
pub factor_type: String,
pub prompt: String,
}

impl AuthPrompt {
pub fn new<H, F, S>(header: H, factor_type: F, prompt: S) -> Self
pub fn new<H, S>(header: H, prompt: S) -> Self
where
H: AsRef<str>,
F: AsRef<str>,
S: AsRef<str>,
{
Self {
header: header.as_ref().to_owned(),
factor_type: factor_type.as_ref().to_owned(),
prompt: prompt.as_ref().to_owned(),
}
}

pub fn new_password<S: AsRef<str>>(prompt: S) -> Self {
Self {
header: String::new(),
factor_type: "password".to_owned(),
prompt: prompt.as_ref().to_owned(),
}
}
pub fn is_password(&self) -> bool {
self.factor_type == "password" || self.factor_type == "user_defined"
}
}
4 changes: 4 additions & 0 deletions snxcore/src/model/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ pub struct TunnelParams {
pub server_name: String,
pub user_name: String,
pub password: String,
pub password_factor: usize,
pub log_level: String,
pub search_domains: Vec<String>,
pub ignore_search_domains: Vec<String>,
Expand Down Expand Up @@ -299,6 +300,7 @@ impl Default for TunnelParams {
server_name: String::new(),
user_name: String::new(),
password: String::new(),
password_factor: 1,
log_level: "off".to_owned(),
search_domains: Vec::new(),
ignore_search_domains: Vec::new(),
Expand Down Expand Up @@ -351,6 +353,7 @@ impl TunnelParams {
"server-name" => params.server_name = v,
"user-name" => params.user_name = v,
"password" => params.password = v,
"password-factor" => params.password_factor = v.parse().unwrap_or(1),
"log-level" => params.log_level = v,
"search-domains" => params.search_domains = v.split(',').map(|s| s.trim().to_owned()).collect(),
"ignore-search-domains" => {
Expand Down Expand Up @@ -413,6 +416,7 @@ impl TunnelParams {
"password={}",
base64::engine::general_purpose::STANDARD.encode(&self.password)
)?;
writeln!(buf, "password-factor={}", self.password_factor)?;
writeln!(buf, "search-domains={}", self.search_domains.join(","))?;
writeln!(buf, "ignore-search-domains={}", self.ignore_search_domains.join(","))?;
writeln!(
Expand Down
1 change: 0 additions & 1 deletion snxcore/src/server_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ pub async fn get_mfa_prompts(params: &TunnelParams) -> anyhow::Result<VecDeque<A
LoginDisplayLabelSelect::LoginDisplayLabel(map) => map.get("password").map(|label| {
AuthPrompt::new(
map.get("header").map(ToOwned::to_owned).unwrap_or_default(),
factor.factor_type,
format!("{}: ", label),
)
}),
Expand Down

0 comments on commit 31f016c

Please sign in to comment.