Skip to content

Commit

Permalink
Guard against configuration mistakes leading to security issues
Browse files Browse the repository at this point in the history
We should protect a user against config mistakes, where they forget to
set auth-type appropriately, while providing authenticator-specific
parameters.
  • Loading branch information
robklg committed Jun 20, 2024
1 parent 011fa18 commit 8923db5
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 12 deletions.
24 changes: 24 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ unftp-auth-jsonfile = { version = "0.3.4", optional = true }
unftp-sbe-rooter = "0.2.1"
unftp-sbe-restrict = "0.1.2"
url = "2.5.0"
strum = { version = "0.26.2", features = ["derive"] }
strum_macros = "0.26.2"

[target.'cfg(unix)'.dependencies]
unftp-auth-pam = { version = "0.2.5", optional = true }
Expand Down
13 changes: 7 additions & 6 deletions src/args.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::app;
use clap::{Arg, ArgEnum, Command};
use std::str::FromStr;
use strum_macros::{Display, EnumString};

pub const AUTH_JSON_PATH: &str = "auth-json-path";
pub const AUTH_PAM_SERVICE: &str = "auth-pam-service";
Expand Down Expand Up @@ -45,13 +46,13 @@ pub const USR_JSON_PATH: &str = "usr-json-path";
pub const USR_HTTP_URL: &str = "usr-http-url";
pub const VERBOSITY: &str = "verbosity";

#[derive(ArgEnum, Clone, Debug)]
#[allow(non_camel_case_types)]
#[derive(Debug, EnumString, Display, PartialEq)]
#[strum(serialize_all = "lowercase")]
pub enum AuthType {
anonymous,
pam,
rest,
json,
Anonymous,
Pam,
Rest,
Json,
}

#[derive(ArgEnum, Clone, Debug)]
Expand Down
49 changes: 43 additions & 6 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::infra::userdetail_http::HTTPUserDetailProvider;
use crate::{
app::libunftp_version, args::FtpsClientAuthType, auth::DefaultUserProvider, notify::FTPListener,
};
use args::AuthType;
use auth::LookupAuthenticator;
use clap::ArgMatches;
use domain::events::{EventDispatcher, FTPEvent, FTPEventPayload};
Expand Down Expand Up @@ -93,13 +94,49 @@ fn load_user_file(
fn make_auth(
m: &clap::ArgMatches,
) -> Result<Arc<dyn auth_spi::Authenticator<user::User> + Send + Sync + 'static>, String> {
let mut auth: LookupAuthenticator = match m.value_of(args::AUTH_TYPE) {
None | Some("anonymous") => make_anon_auth(),
Some("pam") => make_pam_auth(m),
Some("rest") => make_rest_auth(m),
Some("json") => make_json_auth(m),
unknown_type => Err(format!("unknown auth type: {}", unknown_type.unwrap())),
let default_auth_type = AuthType::Anonymous.to_string();
let input_auth_type = m.value_of(args::AUTH_TYPE).unwrap_or(&default_auth_type);
let auth_type_variant = match input_auth_type.parse::<AuthType>() {
Ok(auth_type_variant) => auth_type_variant,
Err(strum::ParseError::VariantNotFound) => {
return Err(format!("unknown auth type: {}", input_auth_type))
}
};

let mut auth: LookupAuthenticator = match auth_type_variant {
AuthType::Anonymous => make_anon_auth(),
AuthType::Pam => make_pam_auth(m),
AuthType::Rest => make_rest_auth(m),
AuthType::Json => make_json_auth(m),
}?;

if auth_type_variant != AuthType::Pam && m.is_present(args::AUTH_PAM_SERVICE) {
return Err(format!(
"parameter {} set while auth_type is set to {}",
args::AUTH_PAM_SERVICE,
auth_type_variant
));
} else if auth_type_variant != AuthType::Json && m.is_present(args::AUTH_JSON_PATH) {
return Err(format!(
"parameter {} set while auth_type is set to {}",
args::AUTH_JSON_PATH,
auth_type_variant
));
} else if auth_type_variant != AuthType::Rest
&& [
args::AUTH_REST_URL,
args::AUTH_REST_REGEX,
args::AUTH_REST_SELECTOR,
]
.iter()
.any(|&arg| m.is_present(arg))
{
return Err(format!(
"REST auth parameter(s) set while auth_type is set to {}",
auth_type_variant
));
}

auth.set_usr_detail(
match (
m.value_of(args::USR_JSON_PATH),
Expand Down

0 comments on commit 8923db5

Please sign in to comment.