Skip to content

Commit 17ca87b

Browse files
feat: extract admin rights check and do it also for windows (#1861)
1 parent efbdd25 commit 17ca87b

File tree

11 files changed

+126
-19
lines changed

11 files changed

+126
-19
lines changed

.github/workflows/push_pr_checks_tests.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,16 +455,21 @@ jobs:
455455
if: '!cancelled()'
456456
run: cargo test --workspace --exclude 'newrelic_agent_control' --all-targets
457457

458-
- name: Run tests agent control lib excluding root-required tests (on-host)
458+
- name: Run tests agent control lib
459459
if: '!cancelled()'
460460
shell: bash
461461
run: make -C agent-control test/onhost
462462

463-
- name: Run integration tests
463+
- name: Run integration tests excluding non root-required tests only available for unix (on-host)
464464
if: '!cancelled()'
465465
shell: bash
466466
run: make -C agent-control test/onhost/integration
467467

468+
- name: Run onHost root-required integration-tests only (windows runner has elevated permissions)
469+
if: '!cancelled()'
470+
shell: bash
471+
run: make -C agent-control test/onhost/root/integration
472+
468473
- name: Run documentation tests
469474
if: '!cancelled()'
470475
run: cargo test --locked --doc

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ reqwest = { version = "0.12.24", default-features = false, features = [
4444
] }
4545
rstest = "0.26.1"
4646
windows = "0.62.2"
47+
windows-sys = "0.61.2"
4748

4849
[profile.release]
4950
# remove debug symbols from the release binary

agent-control/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ windows = { workspace = true, features = [
8181
"Win32_System_Threading",
8282
] }
8383

84+
[target.'cfg(target_os = "windows")'.dependencies]
85+
windows-sys = { workspace = true }
86+
8487
[dev-dependencies]
8588
assert_cmd = { workspace = true }
8689
predicates = { workspace = true }

agent-control/src/bin/main_onhost.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use newrelic_agent_control::event::ApplicationEvent;
1212
use newrelic_agent_control::event::channel::{EventPublisher, pub_sub};
1313
use newrelic_agent_control::http::tls::install_rustls_default_crypto_provider;
1414
use newrelic_agent_control::instrumentation::tracing::TracingGuardBox;
15+
use newrelic_agent_control::utils::is_elevated::is_elevated;
1516
use std::error::Error;
1617
use std::process::ExitCode;
1718
use tracing::{error, info, trace};
@@ -36,9 +37,9 @@ fn _main(
3637
agent_control_run_config: AgentControlRunConfig,
3738
_tracer: Vec<TracingGuardBox>, // Needs to take ownership of the tracer as it can be shutdown on drop
3839
) -> Result<(), Box<dyn Error>> {
39-
#[cfg(all(target_family = "unix", not(feature = "disable-asroot")))]
40-
if !nix::unistd::Uid::effective().is_root() {
41-
return Err("Program must run as root".into());
40+
#[cfg(not(feature = "disable-asroot"))]
41+
if !is_elevated()? {
42+
return Err("Program must run with elevated permissions".into());
4243
}
4344

4445
#[cfg(not(feature = "multiple-instances"))]

agent-control/src/utils.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
pub mod binary_metadata;
2+
pub mod is_elevated;
23
pub mod retry;
34
pub mod thread_context;
45
pub mod threads;
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#[cfg(target_family = "windows")]
2+
use windows_sys::Win32::Foundation::{CloseHandle, HANDLE};
3+
#[cfg(target_family = "windows")]
4+
use windows_sys::Win32::Security::{
5+
GetTokenInformation, TOKEN_ELEVATION, TOKEN_QUERY, TokenElevation,
6+
};
7+
#[cfg(target_family = "windows")]
8+
use windows_sys::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken};
9+
10+
#[derive(Debug, thiserror::Error)]
11+
#[error("{0}")]
12+
pub struct IsElevatedError(String);
13+
14+
pub fn is_elevated() -> Result<bool, IsElevatedError> {
15+
#[cfg(target_family = "unix")]
16+
return Ok(nix::unistd::Uid::effective().is_root());
17+
18+
#[cfg(target_family = "windows")]
19+
is_elevated_windows()
20+
}
21+
22+
#[cfg(target_family = "windows")]
23+
fn is_elevated_windows() -> Result<bool, IsElevatedError> {
24+
unsafe {
25+
let mut token_handle: HANDLE = std::ptr::null_mut();
26+
let process = GetCurrentProcess();
27+
28+
// https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-openprocesstoken
29+
// An access token contains the security information for a logon session and every process
30+
// executed on behalf of the user has a copy of the token. Here we get that token.
31+
if OpenProcessToken(process, TOKEN_QUERY, &mut token_handle) == 0 {
32+
return Err(IsElevatedError("Failed to open process token.".to_string()));
33+
}
34+
35+
let mut elevation = TOKEN_ELEVATION { TokenIsElevated: 0 };
36+
let mut return_length = 0;
37+
38+
// https://learn.microsoft.com/en-us/windows/win32/api/securitybaseapi/nf-securitybaseapi-gettokeninformation
39+
// GetTokenInformation requires a pointer to a buffer (elevation) the function fills with the requested information.
40+
// The second parameter is the class (TokenElevation) we want to get information from the Token.
41+
let result = GetTokenInformation(
42+
token_handle,
43+
TokenElevation,
44+
&mut elevation as *mut _ as *mut _,
45+
std::mem::size_of::<TOKEN_ELEVATION>() as u32,
46+
&mut return_length,
47+
);
48+
49+
CloseHandle(token_handle);
50+
51+
if result == 0 {
52+
return Err(IsElevatedError(
53+
"Failed to get token information to check user rights.".to_string(),
54+
));
55+
}
56+
57+
Ok(elevation.TokenIsElevated != 0)
58+
}
59+
}

agent-control/tests/on_host/cli.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ fn does_not_run_if_no_root() -> Result<(), Box<dyn std::error::Error>> {
6767
)?;
6868
let mut cmd = cargo_bin_cmd!("newrelic-agent-control");
6969
cmd.arg("--local-dir").arg(dir.path());
70-
cmd.assert()
71-
.failure()
72-
.stdout(predicate::str::contains("Program must run as root"));
70+
cmd.assert().failure().stdout(predicate::str::contains(
71+
"Program must run with elevated permissions",
72+
));
7373
Ok(())
7474
}
7575

agent-control/tests/on_host/logging/file_logging.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,10 @@ fn default_log_level_no_root() {
4848
// Expecting to fail as non_root
4949
// Asserting content is logged to stdout as well
5050
cmd.assert().failure().stdout(
51-
predicate::str::is_match(TIME_FORMAT.to_owned() + "ERROR.*Program must run as root")
52-
.unwrap(),
51+
predicate::str::is_match(
52+
TIME_FORMAT.to_owned() + "ERROR.*Program must run with elevated permissions",
53+
)
54+
.unwrap(),
5355
);
5456

5557
// The behavior of the appender functionality is already unit tested as part of the sub-agent

agent-control/tests/on_host/logging/level.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ fn default_log_level_no_root() {
4646
.unwrap(),
4747
)
4848
.stdout(
49-
predicate::str::is_match(TIME_FORMAT.to_owned() + "ERROR.*Program must run as root")
50-
.unwrap(),
49+
predicate::str::is_match(
50+
TIME_FORMAT.to_owned() + "ERROR.*Program must run with elevated permissions",
51+
)
52+
.unwrap(),
5153
);
5254
}
5355

@@ -122,8 +124,10 @@ fn debug_log_level_no_root() {
122124
.unwrap(),
123125
)
124126
.stdout(
125-
predicate::str::is_match(TIME_FORMAT.to_owned() + "ERROR.*Program must run as root")
126-
.unwrap(),
127+
predicate::str::is_match(
128+
TIME_FORMAT.to_owned() + "ERROR.*Program must run with elevated permissions",
129+
)
130+
.unwrap(),
127131
);
128132
}
129133

0 commit comments

Comments
 (0)