Skip to content

Commit

Permalink
fix(shadowsocks): run_as_user abort if error occurs (#1328)
Browse files Browse the repository at this point in the history
  • Loading branch information
zonyitoo committed Oct 18, 2023
1 parent 64d9cf3 commit ce344c6
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 25 deletions.
15 changes: 6 additions & 9 deletions src/service/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@ use shadowsocks_service::shadowsocks::relay::socks5::Address;
use shadowsocks_service::{
acl::AccessControl,
config::{
read_variable_field_value,
Config,
ConfigType,
LocalConfig,
LocalInstanceConfig,
ProtocolType,
read_variable_field_value, Config, ConfigType, LocalConfig, LocalInstanceConfig, ProtocolType,
ServerInstanceConfig,
},
local::{loadbalancing::PingBalancer, Server},
Expand All @@ -37,8 +32,7 @@ use shadowsocks_service::{
use crate::logging;
use crate::{
config::{Config as ServiceConfig, RuntimeMode},
monitor,
vparser,
monitor, vparser,
};

#[cfg(feature = "local-dns")]
Expand Down Expand Up @@ -850,7 +844,10 @@ pub fn create(matches: &ArgMatches) -> Result<(Runtime, impl Future<Output = Exi

#[cfg(unix)]
if let Some(uname) = matches.get_one::<String>("USER") {
crate::sys::run_as_user(uname);
if let Err(err) = crate::sys::run_as_user(uname) {
eprintln!("failed to change as user, error: {err}");
return Err(crate::EXIT_CODE_INSUFFICIENT_PARAMS.into());
}
}

info!("shadowsocks local {} build {}", crate::VERSION, crate::BUILD_TIME);
Expand Down
8 changes: 5 additions & 3 deletions src/service/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ use shadowsocks_service::{
use crate::logging;
use crate::{
config::{Config as ServiceConfig, RuntimeMode},
monitor,
vparser,
monitor, vparser,
};

/// Defines command line options
Expand Down Expand Up @@ -478,7 +477,10 @@ pub fn create(matches: &ArgMatches) -> Result<(Runtime, impl Future<Output = Exi

#[cfg(unix)]
if let Some(uname) = matches.get_one::<String>("USER") {
crate::sys::run_as_user(uname);
if let Err(err) = crate::sys::run_as_user(uname) {
eprintln!("failed to change as user, error: {err}");
return Err(crate::EXIT_CODE_INSUFFICIENT_PARAMS.into());
}
}

info!("shadowsocks manager {} build {}", crate::VERSION, crate::BUILD_TIME);
Expand Down
8 changes: 5 additions & 3 deletions src/service/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ use shadowsocks_service::{
use crate::logging;
use crate::{
config::{Config as ServiceConfig, RuntimeMode},
monitor,
vparser,
monitor, vparser,
};

/// Defines command line options
Expand Down Expand Up @@ -498,7 +497,10 @@ pub fn create(matches: &ArgMatches) -> Result<(Runtime, impl Future<Output = Exi

#[cfg(unix)]
if let Some(uname) = matches.get_one::<String>("USER") {
crate::sys::run_as_user(uname);
if let Err(err) = crate::sys::run_as_user(uname) {
eprintln!("failed to change as user, error: {err}");
return Err(crate::EXIT_CODE_INSUFFICIENT_PARAMS.into());
}
}

info!("shadowsocks server {} build {}", crate::VERSION, crate::BUILD_TIME);
Expand Down
22 changes: 12 additions & 10 deletions src/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ pub fn adjust_nofile() {

/// setuid(), setgid() for a specific user or uid
#[cfg(unix)]
pub fn run_as_user(uname: &str) {
use log::warn;
pub fn run_as_user(uname: &str) -> std::io::Result<()> {
use log::error;
use std::{
ffi::{CStr, CString},
io::Error,
io::{Error, ErrorKind},
};

unsafe {
Expand All @@ -101,8 +101,7 @@ pub fn run_as_user(uname: &str) {
};

if pwd.is_null() {
warn!("user {} not found", uname);
return;
return Err(Error::new(ErrorKind::InvalidInput, format!("user {} not found", uname)));
}

let pwd = &*pwd;
Expand All @@ -111,37 +110,40 @@ pub fn run_as_user(uname: &str) {
if libc::setgid(pwd.pw_gid as libc::gid_t) != 0 {
let err = Error::last_os_error();

warn!(
error!(
"could not change group id to user {:?}'s gid: {}, uid: {}, error: {}",
CStr::from_ptr(pwd.pw_name),
pwd.pw_gid,
pwd.pw_uid,
err
);
return;
return Err(err);
}

if libc::initgroups(pwd.pw_name, pwd.pw_gid.try_into().unwrap()) != 0 {

Check warning on line 123 in src/sys.rs

View workflow job for this annotation

GitHub Actions / clippy-check (ubuntu-latest)

useless conversion to the same type: `u32`

warning: useless conversion to the same type: `u32` --> src/sys.rs:123:42 | 123 | if libc::initgroups(pwd.pw_name, pwd.pw_gid.try_into().unwrap()) != 0 { | ^^^^^^^^^^^^^^^^^^^^^ | = help: consider removing `.try_into()` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion = note: `#[warn(clippy::useless_conversion)]` on by default
let err = Error::last_os_error();
warn!(
error!(
"could not change supplementary groups to user {:?}'s gid: {}, uid: {}, error: {}",
CStr::from_ptr(pwd.pw_name),
pwd.pw_gid,
pwd.pw_uid,
err
);
return;
return Err(err);
}

if libc::setuid(pwd.pw_uid) != 0 {
let err = Error::last_os_error();
warn!(
error!(
"could not change user id to user {:?}'s gid: {}, uid: {}, error: {}",
CStr::from_ptr(pwd.pw_name),
pwd.pw_gid,
pwd.pw_uid,
err
);
return Err(err);
}
}

Ok(())
}

0 comments on commit ce344c6

Please sign in to comment.