Skip to content

Commit daea817

Browse files
kvinwangkvinwang
authored
Merge pull request #784 from Dstack-TEE/fix/absolute-env-path-overrides
fix(attest): require absolute env path overrides
2 parents 499a704 + 3842bd9 commit daea817

2 files changed

Lines changed: 146 additions & 18 deletions

File tree

‎dstack/cc-eventlog/src/tcg.rs‎

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// SPDX-License-Identifier: Apache-2.0
66

77
use crate::{codecs::VecOf, tdx::TdxEvent};
8-
use anyhow::{Context, Result};
8+
use anyhow::{bail, Context, Result};
99
use scale::Decode;
1010
use std::path::PathBuf;
1111

@@ -302,9 +302,7 @@ impl TcgEventLog {
302302
}
303303

304304
pub fn decode_from_ccel_file() -> Result<Self> {
305-
let path = std::env::var_os(CCEL_FILE_ENV)
306-
.map(PathBuf::from)
307-
.unwrap_or_else(|| PathBuf::from(CCEL_FILE));
305+
let path = ccel_file_path()?;
308306
let data = fs_err::read(&path)
309307
.with_context(|| format!("failed to read CCEL from {}", path.display()))?;
310308
Self::decode(&mut data.as_slice()).context("failed to decode CCEL")
@@ -320,6 +318,27 @@ impl TcgEventLog {
320318
}
321319
}
322320

321+
/// Resolve the CCEL path, honoring `DSTACK_CCEL_FILE` when set.
322+
///
323+
/// Overrides must be non-empty absolute paths so relative values cannot silently
324+
/// resolve against the process working directory.
325+
fn ccel_file_path() -> Result<PathBuf> {
326+
let Some(value) = std::env::var_os(CCEL_FILE_ENV) else {
327+
return Ok(PathBuf::from(CCEL_FILE));
328+
};
329+
if value.is_empty() {
330+
bail!("empty path override from {CCEL_FILE_ENV}");
331+
}
332+
let path = PathBuf::from(value);
333+
if !path.is_absolute() {
334+
bail!(
335+
"path override from {CCEL_FILE_ENV} must be absolute: {}",
336+
path.display()
337+
);
338+
}
339+
Ok(path)
340+
}
341+
323342
fn parse_spec_id_event_log<I: scale::Input>(
324343
input: &mut I,
325344
) -> Result<(TcgEvent, TcgEfiSpecIdEvent)> {
@@ -379,3 +398,48 @@ impl TryFrom<TcgEvent> for TdxEvent {
379398
})
380399
}
381400
}
401+
402+
#[cfg(test)]
403+
mod tests {
404+
use super::*;
405+
406+
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
407+
408+
#[test]
409+
fn ccel_override_rejects_empty_and_relative() {
410+
let _env_guard = ENV_LOCK.lock().unwrap();
411+
let previous = std::env::var_os(CCEL_FILE_ENV);
412+
413+
std::env::set_var(CCEL_FILE_ENV, "");
414+
assert!(ccel_file_path()
415+
.unwrap_err()
416+
.to_string()
417+
.contains("empty path override"));
418+
419+
std::env::set_var(CCEL_FILE_ENV, "relative/ccel.bin");
420+
assert!(ccel_file_path()
421+
.unwrap_err()
422+
.to_string()
423+
.contains("must be absolute"));
424+
425+
std::env::set_var(CCEL_FILE_ENV, "/abs/ccel.bin");
426+
assert_eq!(ccel_file_path().unwrap(), PathBuf::from("/abs/ccel.bin"));
427+
428+
match previous {
429+
Some(value) => std::env::set_var(CCEL_FILE_ENV, value),
430+
None => std::env::remove_var(CCEL_FILE_ENV),
431+
}
432+
}
433+
434+
#[test]
435+
fn ccel_defaults_without_override() {
436+
let _env_guard = ENV_LOCK.lock().unwrap();
437+
let previous = std::env::var_os(CCEL_FILE_ENV);
438+
std::env::remove_var(CCEL_FILE_ENV);
439+
assert_eq!(ccel_file_path().unwrap(), PathBuf::from(CCEL_FILE));
440+
match previous {
441+
Some(value) => std::env::set_var(CCEL_FILE_ENV, value),
442+
None => std::env::remove_var(CCEL_FILE_ENV),
443+
}
444+
}
445+
}

‎dstack/tdx-attest/src/linux.rs‎

Lines changed: 78 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@
1212
//! 2. VSock - via QGS (Quote Generation Service) over vsock
1313
//! 3. TDVMCALL - via `/dev/tdx_guest` ioctl (legacy)
1414
15+
use std::ffi::OsStr;
1516
use std::fs::{self, File, OpenOptions};
1617
use std::io::{Read, Write};
1718
use std::os::unix::io::AsRawFd;
18-
use std::path::Path;
19+
use std::path::{Path, PathBuf};
1920
use std::sync::Mutex;
2021
use std::thread;
2122
use std::time::{Duration, Instant};
@@ -188,15 +189,39 @@ fn should_try_configfs() -> bool {
188189
|| Path::new(CONFIGFS_BASE).is_dir()
189190
}
190191

192+
/// Parse an environment path override.
193+
///
194+
/// Overrides must be non-empty absolute paths so a mis-set relative value like
195+
/// `"."` cannot silently operate on the process working directory.
196+
fn absolute_env_path(var: &str, value: impl AsRef<OsStr>) -> Result<PathBuf> {
197+
let value = value.as_ref();
198+
if value.is_empty() {
199+
return Err(TdxAttestError::NotSupported(format!(
200+
"empty path override from {var}"
201+
)));
202+
}
203+
let path = PathBuf::from(value);
204+
if !path.is_absolute() {
205+
return Err(TdxAttestError::NotSupported(format!(
206+
"path override from {var} must be absolute: {}",
207+
path.display()
208+
)));
209+
}
210+
Ok(path)
211+
}
212+
191213
/// Return whether a TDX guest provider is available through either the legacy
192214
/// misc device or the standard TSM report interface.
193215
pub fn is_tdx_available() -> bool {
194216
if Path::new(TDX_GUEST_DEVICE).exists() {
195217
return true;
196218
}
197219

198-
if let Some(path) = std::env::var_os(CONFIGFS_PATH_ENV) {
199-
return verify_configfs_provider(Path::new(&path)).unwrap_or(false);
220+
if let Some(value) = std::env::var_os(CONFIGFS_PATH_ENV) {
221+
let Ok(path) = absolute_env_path(CONFIGFS_PATH_ENV, &value) else {
222+
return false;
223+
};
224+
return verify_configfs_provider(&path).unwrap_or(false);
200225
}
201226

202227
if verify_configfs_provider(Path::new(CONFIGFS_DEFAULT)).unwrap_or(false) {
@@ -264,9 +289,9 @@ pub fn extend_rtmr(index: u32, _event_type: u32, digest: [u8; 48]) -> Result<()>
264289
))
265290
}
266291

267-
fn rtmr_sysfs_base() -> Result<Option<std::path::PathBuf>> {
268-
if let Some(path) = std::env::var_os(RTMR_SYSFS_PATH_ENV) {
269-
let path = std::path::PathBuf::from(path);
292+
fn rtmr_sysfs_base() -> Result<Option<PathBuf>> {
293+
if let Some(value) = std::env::var_os(RTMR_SYSFS_PATH_ENV) {
294+
let path = absolute_env_path(RTMR_SYSFS_PATH_ENV, &value)?;
270295
if path.as_os_str().len() < 240 && path.is_dir() {
271296
return Ok(Some(path));
272297
}
@@ -278,7 +303,7 @@ fn rtmr_sysfs_base() -> Result<Option<std::path::PathBuf>> {
278303

279304
Ok(Path::new(RTMR_SYSFS_BASE)
280305
.is_dir()
281-
.then(|| std::path::PathBuf::from(RTMR_SYSFS_BASE)))
306+
.then(|| PathBuf::from(RTMR_SYSFS_BASE)))
282307
}
283308

284309
fn extend_rtmr_via_sysfs(base: &Path, index: u32, digest: &[u8; 48]) -> Result<()> {
@@ -374,15 +399,14 @@ fn get_quote_via_configfs(report_data: &TdxReportData) -> Result<Vec<u8>> {
374399
}
375400

376401
fn prepare_configfs() -> Result<String> {
377-
if let Ok(path) = std::env::var(CONFIGFS_PATH_ENV) {
378-
if path.len() < 240
379-
&& Path::new(&path).is_dir()
380-
&& verify_configfs_provider(Path::new(&path))?
381-
{
382-
return Ok(path);
402+
if let Ok(value) = std::env::var(CONFIGFS_PATH_ENV) {
403+
let path = absolute_env_path(CONFIGFS_PATH_ENV, &value)?;
404+
if path.as_os_str().len() < 240 && path.is_dir() && verify_configfs_provider(&path)? {
405+
return Ok(path.display().to_string());
383406
}
384407
return Err(TdxAttestError::NotSupported(format!(
385-
"invalid configfs path from env: {path}"
408+
"invalid configfs path from env: {}",
409+
path.display()
386410
)));
387411
}
388412

@@ -727,4 +751,44 @@ mod tests {
727751
if message.contains("invalid configfs path from env")
728752
));
729753
}
754+
755+
#[test]
756+
fn absolute_env_path_rejects_empty_and_relative() {
757+
assert!(absolute_env_path("TEST_PATH", "").is_err());
758+
assert!(absolute_env_path("TEST_PATH", ".").is_err());
759+
assert!(absolute_env_path("TEST_PATH", "relative/path").is_err());
760+
assert_eq!(
761+
absolute_env_path("TEST_PATH", "/abs/path").unwrap(),
762+
PathBuf::from("/abs/path")
763+
);
764+
}
765+
766+
#[test]
767+
fn relative_configfs_override_is_not_available() {
768+
let previous = std::env::var_os(CONFIGFS_PATH_ENV);
769+
std::env::set_var(CONFIGFS_PATH_ENV, ".");
770+
let available = is_tdx_available();
771+
match previous {
772+
Some(value) => std::env::set_var(CONFIGFS_PATH_ENV, value),
773+
None => std::env::remove_var(CONFIGFS_PATH_ENV),
774+
}
775+
// relative overrides are ignored; only a real guest device can still pass
776+
assert_eq!(available, Path::new(TDX_GUEST_DEVICE).exists());
777+
}
778+
779+
#[test]
780+
fn relative_rtmr_override_is_rejected() {
781+
let previous = std::env::var_os(RTMR_SYSFS_PATH_ENV);
782+
std::env::set_var(RTMR_SYSFS_PATH_ENV, ".");
783+
let result = rtmr_sysfs_base();
784+
match previous {
785+
Some(value) => std::env::set_var(RTMR_SYSFS_PATH_ENV, value),
786+
None => std::env::remove_var(RTMR_SYSFS_PATH_ENV),
787+
}
788+
assert!(matches!(
789+
result,
790+
Err(TdxAttestError::NotSupported(message))
791+
if message.contains("must be absolute")
792+
));
793+
}
730794
}

0 commit comments

Comments
 (0)