Skip to content

vmgstool: allow read/write to unencrypted file in encrypted vmgs #1384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+59 −55
Open

vmgstool: allow read/write to unencrypted file in encrypted vmgs #1384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8200916
allow writing unencrypted file to encrypted vmgs
tjones60 May 17, 2025
9b827a3
fix
tjones60 May 20, 2025
52f0e86
unknown gsp
tjones60 May 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions vm/vmgs/vmgs/src/vmgs_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ pub struct VmgsFileInfo {
pub allocated_bytes: u64,
/// Number of valid bytes in the file.
pub valid_bytes: u64,
/// Whether this file is encrypted.
pub encrypted: bool,
}

// Aggregates fully validated data from the FILE_TABLE and EXTENDED_FILE_TABLE
Expand Down Expand Up @@ -411,6 +413,7 @@ impl Vmgs {
Ok(VmgsFileInfo {
allocated_bytes: block_count_to_byte_count(fcb.allocated_blocks.get()),
valid_bytes: fcb.valid_bytes,
encrypted: fcb.attributes.encrypted() || fcb.attributes.authenticated(),
})
}

Expand Down
109 changes: 55 additions & 54 deletions vm/vmgs/vmgstool/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ enum Error {
EmptyFile,
#[error("Invalid VMGS file size: {0} {1}")]
InvalidVmgsFileSize(u64, String),
#[error("VMGS file is encrypted, but no encryption key was provided")]
AlreadyEncrypted,
#[error("Key file IO")]
KeyFile(#[source] std::io::Error),
#[error("Key must be {0} bytes long, is {1} bytes instead")]
Expand All @@ -85,6 +83,10 @@ enum Error {
FileIdExists(FileId),
#[error("VMGS file is encrypted using GspById")]
GspByIdEncryption,
#[error("VMGS file is encrypted using an unknown encryption scheme")]
GspUnknown,
#[error("VMGS file is using an unknown encryption algorithm")]
EncryptionUnknown,
}

/// Automation requires certain exit codes to be guaranteed
Expand All @@ -103,6 +105,7 @@ enum ExitCode {
ErrorNotFound = 4,
ErrorV1 = 5,
ErrorGspById = 6,
ErrorGspUnknown = 7,
}

#[derive(Debug, Clone, Copy)]
Expand Down Expand Up @@ -341,6 +344,7 @@ fn main() {
Error::Vmgs(VmgsError::FileInfoAllocated) => ExitCode::ErrorNotFound,
Error::V1Format => ExitCode::ErrorV1,
Error::GspByIdEncryption => ExitCode::ErrorGspById,
Error::GspUnknown => ExitCode::ErrorGspUnknown,
_ => ExitCode::Error,
};

Expand Down Expand Up @@ -454,7 +458,7 @@ async fn vmgs_file_update_key(
new_key_path: impl AsRef<Path>,
) -> Result<(), Error> {
let new_encryption_key = read_key_path(new_key_path)?;
let mut vmgs = vmgs_file_open(file_path, key_path, OpenMode::ReadWrite, false).await?;
let mut vmgs = vmgs_file_open(file_path, key_path, OpenMode::ReadWrite).await?;

vmgs_update_key(&mut vmgs, encryption_alg, new_encryption_key.as_ref()).await
}
Expand Down Expand Up @@ -603,7 +607,7 @@ async fn vmgs_file_write(
eprintln!("Read {} bytes", buf.len());

let encrypt = key_path.is_some();
let mut vmgs = vmgs_file_open(file_path, key_path, OpenMode::ReadWrite, false).await?;
let mut vmgs = vmgs_file_open(file_path, key_path, OpenMode::ReadWrite).await?;

vmgs_write(&mut vmgs, file_id, &buf, encrypt, allow_overwrite).await?;

Expand All @@ -619,11 +623,12 @@ async fn vmgs_write(
) -> Result<(), Error> {
eprintln!("Writing File ID {} ({:?})", file_id.0, file_id);

if !allow_overwrite {
if let Ok(info) = vmgs.get_file_info(file_id) {
if info.valid_bytes > 0 {
return Err(Error::FileIdExists(file_id));
}
if let Ok(info) = vmgs.get_file_info(file_id) {
if !allow_overwrite && info.valid_bytes > 0 {
return Err(Error::FileIdExists(file_id));
}
if !encrypt && info.encrypted {
eprintln!("Warning: overwriting encrypted file with plaintext data")
}
}

Expand All @@ -649,13 +654,17 @@ async fn vmgs_file_read(
raw_stdout: bool,
) -> Result<(), Error> {
let decrypt = key_path.is_some();
let mut vmgs = vmgs_file_open(file_path, key_path, OpenMode::ReadOnly, true).await?;
let mut vmgs = vmgs_file_open(file_path, key_path, OpenMode::ReadOnly).await?;

let file_info = vmgs.get_file_info(file_id)?;
if !decrypt && file_info.encrypted {
eprintln!("Warning: Reading encrypted file without decrypting");
}

let buf = vmgs_read(&mut vmgs, file_id, decrypt).await?;

eprintln!("Read {} bytes", buf.len());
let data_size = vmgs.get_file_info(file_id)?.valid_bytes as usize;
if buf.len() != data_size {
if buf.len() != file_info.valid_bytes as usize {
eprintln!("Warning: Bytes read from VMGS doesn't match file info");
}

Expand Down Expand Up @@ -893,7 +902,6 @@ async fn vmgs_file_open(
file_path: impl AsRef<Path>,
key_path: Option<impl AsRef<Path>>,
open_mode: OpenMode,
encrypted_no_key_ok: bool,
) -> Result<Vmgs, Error> {
eprintln!("Opening VMGS File: {}", file_path.as_ref().display());
let file = fs_err::OpenOptions::new()
Expand All @@ -910,7 +918,7 @@ async fn vmgs_file_open(
.map_err(Error::InvalidDisk)?;
let encryption_key = key_path.map(read_key_path).transpose()?;

let res = vmgs_open(disk, encryption_key.as_deref(), encrypted_no_key_ok).await;
let res = vmgs_open(disk, encryption_key.as_deref()).await;

if matches!(
res,
Expand All @@ -925,11 +933,7 @@ async fn vmgs_file_open(
}

#[cfg_attr(not(with_encryption), expect(unused_mut), expect(unused_variables))]
async fn vmgs_open(
disk: Disk,
encryption_key: Option<&[u8]>,
encrypted_no_key_ok: bool,
) -> Result<Vmgs, Error> {
async fn vmgs_open(disk: Disk, encryption_key: Option<&[u8]>) -> Result<Vmgs, Error> {
let mut vmgs: Vmgs = Vmgs::open(disk, None).await?;

if let Some(encryption_key) = encryption_key {
Expand All @@ -941,12 +945,6 @@ async fn vmgs_open(
}
#[cfg(not(with_encryption))]
unreachable!("Encryption requires the encryption feature");
} else if vmgs.is_encrypted() {
if encrypted_no_key_ok {
eprintln!("Warning: Opening encrypted file without decrypting");
} else {
return Err(Error::AlreadyEncrypted);
}
}

Ok(vmgs)
Expand Down Expand Up @@ -977,7 +975,7 @@ async fn vmgs_file_query_file_size(
file_path: impl AsRef<Path>,
file_id: FileId,
) -> Result<(), Error> {
let vmgs = vmgs_file_open(file_path, None as Option<PathBuf>, OpenMode::ReadOnly, true).await?;
let vmgs = vmgs_file_open(file_path, None as Option<PathBuf>, OpenMode::ReadOnly).await?;

let file_size = vmgs_query_file_size(&vmgs, file_id)?;

Expand All @@ -996,15 +994,14 @@ fn vmgs_query_file_size(vmgs: &Vmgs, file_id: FileId) -> Result<u64, Error> {
async fn vmgs_file_query_encryption(file_path: impl AsRef<Path>) -> Result<(), Error> {
print!("{} is ", file_path.as_ref().display());

let vmgs = vmgs_file_open(file_path, None as Option<PathBuf>, OpenMode::ReadOnly, true).await?;
let vmgs = vmgs_file_open(file_path, None as Option<PathBuf>, OpenMode::ReadOnly).await?;

match (
vmgs.get_encryption_algorithm(),
vmgs_get_encryption_scheme(&vmgs),
) {
(EncryptionAlgorithm::NONE, VmgsEncryptionScheme::None) => {
println!("not encrypted");
// Returning an error for HA to easily parse
(EncryptionAlgorithm::NONE, scheme) => {
println!("not encrypted (encryption scheme: {scheme:?})");
Err(Error::NotEncrypted)
}
(EncryptionAlgorithm::AES_GCM, VmgsEncryptionScheme::GspKey) => {
Expand All @@ -1015,8 +1012,17 @@ async fn vmgs_file_query_encryption(file_path: impl AsRef<Path>) -> Result<(), E
println!("encrypted with AES GCM encryption algorithm using GspById");
Err(Error::GspByIdEncryption)
}
(EncryptionAlgorithm::AES_GCM, VmgsEncryptionScheme::None) => {
println!(
"encrypted with AES GCM encryption algorithm using an unknown encryption scheme"
);
Err(Error::GspUnknown)
}
(alg, scheme) => {
unreachable!("Invalid encryption algorithm ({alg:?}) / scheme ({scheme:?})");
println!(
"using an unknown encryption algorithm: {alg:?} (encryption scheme: {scheme:?})"
);
Err(Error::EncryptionUnknown)
}
}
}
Expand Down Expand Up @@ -1135,7 +1141,6 @@ mod tests {
path: impl AsRef<Path>,
open_mode: OpenMode,
encryption_key: Option<&[u8]>,
encrypted_no_key_ok: bool,
) -> Result<Vmgs, Error> {
let file = fs_err::OpenOptions::new()
.read(true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a test covering this case?

Expand All @@ -1148,16 +1153,15 @@ mod tests {
.map_err(Error::Vhd1)?,
)
.unwrap();
let vmgs = vmgs_open(disk, encryption_key, encrypted_no_key_ok).await?;
let vmgs = vmgs_open(disk, encryption_key).await?;
Ok(vmgs)
}

async fn test_vmgs_query_file_size(
file_path: impl AsRef<Path>,
file_id: FileId,
) -> Result<u64, Error> {
let vmgs =
vmgs_file_open(file_path, None as Option<PathBuf>, OpenMode::ReadOnly, true).await?;
let vmgs = vmgs_file_open(file_path, None as Option<PathBuf>, OpenMode::ReadOnly).await?;

vmgs_query_file_size(&vmgs, file_id)
}
Expand All @@ -1166,8 +1170,7 @@ mod tests {
async fn test_vmgs_query_encryption(
file_path: impl AsRef<Path>,
) -> Result<EncryptionAlgorithm, Error> {
let vmgs =
vmgs_file_open(file_path, None as Option<PathBuf>, OpenMode::ReadOnly, true).await?;
let vmgs = vmgs_file_open(file_path, None as Option<PathBuf>, OpenMode::ReadOnly).await?;

Ok(vmgs.get_encryption_algorithm())
}
Expand All @@ -1179,8 +1182,7 @@ mod tests {
encryption_key: Option<&[u8]>,
new_encryption_key: &[u8],
) -> Result<(), Error> {
let mut vmgs =
test_vmgs_open(file_path, OpenMode::ReadWrite, encryption_key, false).await?;
let mut vmgs = test_vmgs_open(file_path, OpenMode::ReadWrite, encryption_key).await?;

vmgs_update_key(&mut vmgs, encryption_alg, new_encryption_key).await
}
Expand All @@ -1196,7 +1198,7 @@ mod tests {
async fn read_invalid_file() {
let (_dir, path) = new_path();

let result = test_vmgs_open(path, OpenMode::ReadOnly, None, false).await;
let result = test_vmgs_open(path, OpenMode::ReadOnly, None).await;

assert!(result.is_err());
}
Expand All @@ -1207,7 +1209,7 @@ mod tests {

test_vmgs_create(&path, None, false, None).await.unwrap();

let mut vmgs = test_vmgs_open(path, OpenMode::ReadOnly, None, false)
let mut vmgs = test_vmgs_open(path, OpenMode::ReadOnly, None)
.await
.unwrap();
let result = vmgs_read(&mut vmgs, FileId::FILE_TABLE, false).await;
Expand All @@ -1221,7 +1223,7 @@ mod tests {

test_vmgs_create(&path, None, false, None).await.unwrap();

let mut vmgs = test_vmgs_open(path, OpenMode::ReadWrite, None, false)
let mut vmgs = test_vmgs_open(path, OpenMode::ReadWrite, None)
.await
.unwrap();

Expand All @@ -1242,7 +1244,7 @@ mod tests {

test_vmgs_create(&path, None, false, None).await.unwrap();

let mut vmgs = test_vmgs_open(path, OpenMode::ReadWrite, None, false)
let mut vmgs = test_vmgs_open(path, OpenMode::ReadWrite, None)
.await
.unwrap();

Expand Down Expand Up @@ -1292,7 +1294,7 @@ mod tests {
.await
.unwrap();

let mut vmgs = test_vmgs_open(path, OpenMode::ReadWrite, Some(&encryption_key), false)
let mut vmgs = test_vmgs_open(path, OpenMode::ReadWrite, Some(&encryption_key))
.await
.unwrap();

Expand Down Expand Up @@ -1326,7 +1328,7 @@ mod tests {

test_vmgs_create(&path, None, false, None).await.unwrap();

let result = test_vmgs_open(path, OpenMode::ReadWrite, Some(&encryption_key), false).await;
let result = test_vmgs_open(path, OpenMode::ReadWrite, Some(&encryption_key)).await;

assert!(result.is_err());
}
Expand All @@ -1339,7 +1341,7 @@ mod tests {
test_vmgs_create(&path, None, false, None).await.unwrap();

{
let mut vmgs = test_vmgs_open(&path, OpenMode::ReadWrite, None, false)
let mut vmgs = test_vmgs_open(&path, OpenMode::ReadWrite, None)
.await
.unwrap();

Expand Down Expand Up @@ -1371,7 +1373,7 @@ mod tests {
.unwrap();

{
let mut vmgs = test_vmgs_open(&path, OpenMode::ReadWrite, Some(&encryption_key), false)
let mut vmgs = test_vmgs_open(&path, OpenMode::ReadWrite, Some(&encryption_key))
.await
.unwrap();

Expand Down Expand Up @@ -1448,7 +1450,7 @@ mod tests {
.unwrap();

{
let mut vmgs = test_vmgs_open(&path, OpenMode::ReadWrite, Some(&encryption_key), false)
let mut vmgs = test_vmgs_open(&path, OpenMode::ReadWrite, Some(&encryption_key))
.await
.unwrap();

Expand All @@ -1467,10 +1469,9 @@ mod tests {
.unwrap();

{
let mut vmgs =
test_vmgs_open(&path, OpenMode::ReadOnly, Some(&new_encryption_key), false)
.await
.unwrap();
let mut vmgs = test_vmgs_open(&path, OpenMode::ReadOnly, Some(&new_encryption_key))
.await
.unwrap();

let read_buf = vmgs_read(&mut vmgs, FileId::BIOS_NVRAM, true)
.await
Expand All @@ -1479,7 +1480,7 @@ mod tests {
}

// Old key should no longer work
let result = test_vmgs_open(&path, OpenMode::ReadOnly, Some(&encryption_key), false).await;
let result = test_vmgs_open(&path, OpenMode::ReadOnly, Some(&encryption_key)).await;
assert!(result.is_err());
}

Expand All @@ -1496,7 +1497,7 @@ mod tests {
.await
.unwrap();

let mut vmgs = test_vmgs_open(&path, OpenMode::ReadWrite, Some(&encryption_key), false)
let mut vmgs = test_vmgs_open(&path, OpenMode::ReadWrite, Some(&encryption_key))
.await
.unwrap();

Expand Down
2 changes: 1 addition & 1 deletion vm/vmgs/vmgstool/src/uefi_nvram.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ async fn vmgs_file_open_nvram(
key_path: Option<impl AsRef<Path>>,
open_mode: OpenMode,
) -> Result<HclCompatNvram<VmgsStorageBackend>, Error> {
let vmgs = vmgs_file_open(file_path, key_path, open_mode, false).await?;
let vmgs = vmgs_file_open(file_path, key_path, open_mode).await?;
let encrypted = vmgs.is_encrypted();

open_nvram(vmgs, encrypted)
Expand Down