Skip to content

Commit

Permalink
fix: Add timeout to requests and skip dbus NSS lookup on dbus mock (#253
Browse files Browse the repository at this point in the history
)

dbus can do some NSS calls and, if this happens during authd startup,
the daemon will hang (this mainly affected cases where we wanted to run
authd with the examplebroker built-in as, in order to do that, we were
running a separate system bus of our own).

To prevent this from happening again and to add layers of protection,
our NSS requests will have a timeout and we are also skipping the dbus
calls to NSS in our dbus mock.

UDENG-2511
UDENG-2510
  • Loading branch information
denisonbarbosa authored Mar 18, 2024
2 parents 6555f2e + 9656454 commit e29f63e
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 14 deletions.
11 changes: 11 additions & 0 deletions internal/testutils/dbus.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,17 @@ func StartSystemBusMock() (func(), error) {
busCtx, busCancel := context.WithCancel(context.Background())
//#nosec:G204 // This is a test helper and we are in control of the arguments.
cmd := exec.CommandContext(busCtx, "dbus-daemon", "--config-file="+cfgPath, "--print-address=1")

// dbus-daemon can perform some NSS lookups when a client establishes a new connection, usually to determine the
// group membership of the caller -- if examplebroker is running via systemd activation and our NSS module is
// enabled the operation would time out, as the NSS module will send a request to authd that will never be answered,
// due to the fact that the socket is present but our gRPC server is not yet listening on it.
//
// Setting this environment variable causes the authd NSS module to return early and skip the lookup.
// For brevity, we reuse the same variable systemd uses when running the dbus service, which is already configured
// in our module.
cmd.Env = []string{"SYSTEMD_NSS_DYNAMIC_BYPASS=1"}

dbusStdout, err := cmd.StdoutPipe()
if err != nil {
busCancel()
Expand Down
3 changes: 2 additions & 1 deletion nss/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use tokio::net::UnixStream;
use tonic::transport::{Channel, Endpoint, Uri};
use tower::service_fn;

use crate::debug;
use crate::{debug, REQUEST_TIMEOUT};

pub mod authd {
tonic::include_proto!("authd");
Expand All @@ -26,6 +26,7 @@ pub async fn new_client() -> Result<NssClient<Channel>, Box<dyn Error>> {

// The URL must have a valid format, even though we don't use it.
let ch = Endpoint::try_from("https://not-used:404")?
.connect_timeout(REQUEST_TIMEOUT)
.connect_with_connector(service_fn(|_: Uri| {
UnixStream::connect(super::socket_path())
}))
Expand Down
13 changes: 8 additions & 5 deletions nss/src/group/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::error;
use crate::{error, REQUEST_TIMEOUT};
use libc::gid_t;
use libnss::group::{Group, GroupHooks};
use libnss::interop::Response;
Expand Down Expand Up @@ -45,8 +45,9 @@ fn get_all_entries() -> Response<Vec<Group>> {
}
};

let request = Request::new(authd::Empty {});
match client.get_group_entries(request).await {
let mut req = Request::new(authd::Empty {});
req.set_timeout(REQUEST_TIMEOUT);
match client.get_group_entries(req).await {
Ok(r) => Response::Success(group_entries_to_groups(r.into_inner().entries)),
Err(e) => {
error!("error when listing groups: {}", e.message());
Expand Down Expand Up @@ -75,7 +76,8 @@ fn get_entry_by_gid(gid: gid_t) -> Response<Group> {
}
};

let req = Request::new(authd::GetByIdRequest { id: gid });
let mut req = Request::new(authd::GetByIdRequest { id: gid });
req.set_timeout(REQUEST_TIMEOUT);
match client.get_group_by_gid(req).await {
Ok(r) => Response::Success(group_entry_to_group(r.into_inner())),
Err(e) => {
Expand Down Expand Up @@ -105,7 +107,8 @@ fn get_entry_by_name(name: String) -> Response<Group> {
}
};

let req = Request::new(authd::GetGroupByNameRequest { name });
let mut req = Request::new(authd::GetGroupByNameRequest { name });
req.set_timeout(REQUEST_TIMEOUT);
match client.get_group_by_name(req).await {
Ok(r) => Response::Success(group_entry_to_group(r.into_inner())),
Err(e) => {
Expand Down
7 changes: 6 additions & 1 deletion nss/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#[macro_use]
extern crate lazy_static; // used by libnss_*_hooks macros
extern crate lazy_static;
use std::time::Duration;

// used by libnss_*_hooks macros
use libnss::{interop::Response, libnss_group_hooks, libnss_passwd_hooks, libnss_shadow_hooks};

mod passwd;
Expand All @@ -19,6 +22,8 @@ mod logs;

mod client;

const REQUEST_TIMEOUT: Duration = Duration::from_secs(10);

/// socket_path returns the socket path to connect to the gRPC server.
///
/// It uses the AUTHD_NSS_SOCKET env value if set and the custom_socket feature is enabled,
Expand Down
11 changes: 7 additions & 4 deletions nss/src/passwd/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::error;
use crate::{error, REQUEST_TIMEOUT};
use libc::uid_t;
use libnss::interop::Response;
use libnss::passwd::{Passwd, PasswdHooks};
Expand Down Expand Up @@ -45,7 +45,8 @@ fn get_all_entries() -> Response<Vec<Passwd>> {
}
};

let req = Request::new(authd::Empty {});
let mut req = Request::new(authd::Empty {});
req.set_timeout(REQUEST_TIMEOUT);
match client.get_passwd_entries(req).await {
Ok(r) => Response::Success(passwd_entries_to_passwds(r.into_inner().entries)),
Err(e) => {
Expand Down Expand Up @@ -75,7 +76,8 @@ fn get_entry_by_uid(uid: uid_t) -> Response<Passwd> {
}
};

let req = Request::new(authd::GetByIdRequest { id: uid });
let mut req = Request::new(authd::GetByIdRequest { id: uid });
req.set_timeout(REQUEST_TIMEOUT);
match client.get_passwd_by_uid(req).await {
Ok(r) => Response::Success(passwd_entry_to_passwd(r.into_inner())),
Err(e) => {
Expand Down Expand Up @@ -105,10 +107,11 @@ fn get_entry_by_name(name: String) -> Response<Passwd> {
}
};

let req = Request::new(authd::GetPasswdByNameRequest {
let mut req = Request::new(authd::GetPasswdByNameRequest {
name,
should_pre_check: should_pre_check(),
});
req.set_timeout(REQUEST_TIMEOUT);
match client.get_passwd_by_name(req).await {
Ok(r) => Response::Success(passwd_entry_to_passwd(r.into_inner())),
Err(e) => {
Expand Down
8 changes: 5 additions & 3 deletions nss/src/shadow/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::error;
use crate::{error, REQUEST_TIMEOUT};
use libnss::interop::Response;
use libnss::shadow::{Shadow, ShadowHooks};
use tokio::runtime::Builder;
Expand Down Expand Up @@ -40,7 +40,8 @@ fn get_all_entries() -> Response<Vec<Shadow>> {
}
};

let req = Request::new(authd::Empty {});
let mut req = Request::new(authd::Empty {});
req.set_timeout(REQUEST_TIMEOUT);
match client.get_shadow_entries(req).await {
Ok(r) => Response::Success(shadow_entries_to_shadows(r.into_inner().entries)),
Err(e) => {
Expand Down Expand Up @@ -70,7 +71,8 @@ fn get_entry_by_name(name: String) -> Response<Shadow> {
}
};

let req = Request::new(authd::GetShadowByNameRequest { name });
let mut req = Request::new(authd::GetShadowByNameRequest { name });
req.set_timeout(REQUEST_TIMEOUT);
match client.get_shadow_by_name(req).await {
Ok(r) => Response::Success(shadow_entry_to_shadow(r.into_inner())),
Err(e) => {
Expand Down

0 comments on commit e29f63e

Please sign in to comment.