Skip to content

Commit 99499e0

Browse files
ndubchakCopilot
andauthored
engineering: Include PCR 7 into pcrlock policy for non-containerized UKI target OS (#221)
* Try including PCR 7 into pcrlock policy * Removed old check * Exclude PCR 7 on BM tests * Remove default PCR * Exclude PCR 7 for container tests * Update docs/Reference/Host-Configuration/API-Reference/Encryption.md Co-authored-by: Copilot <[email protected]> * Update crates/trident_api/src/config/host/storage/encryption.rs Co-authored-by: Copilot <[email protected]> * Corrected formatting * Updated PCR validation * Updated comment * Tried fixing * Tried fixing again * Do not overwrite Trident config for grub test * Updated task link * Corrected again --------- Co-authored-by: Copilot <[email protected]>
1 parent 5c4dacb commit 99499e0

File tree

24 files changed

+256
-208
lines changed

24 files changed

+256
-208
lines changed

.pipelines/templates/stages/testing_baremetal/update_host_config.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
def update_trident_host_config(
1212
*,
1313
host_configuration: dict,
14+
test_selection: dict,
1415
interface_name: str,
1516
interface_ip: str,
1617
interface_mac: Optional[str] = None,
@@ -80,6 +81,20 @@ def update_trident_host_config(
8081
"writableEtcOverlayHooks"
8182
] = True
8283

84+
# TODO: If this is a BM test with grub MOS -> UKI target OS flow, then only
85+
# request PCRs 4 and 11 in the PCRs section b/c we cannot currently include
86+
# PCR 7 into pcrlock policy as SecureBoot is disabled on BM machines.
87+
# Related ADO task:
88+
# https://dev.azure.com/mariner-org/polar/_workitems/edit/15566.
89+
storage = host_configuration.get("storage")
90+
if storage and "uki" in test_selection.get("compatible", []):
91+
encryption = storage.get("encryption")
92+
if encryption and "pcrs" in encryption:
93+
logging.info(
94+
"Detected UKI image, overwriting PCRs section to only include PCRs 4 and 11 and exclude PCR 7."
95+
)
96+
encryption["pcrs"] = ["boot-loader-code", "kernel-boot"]
97+
8398
logging.info(
8499
"Final trident_yaml content post all the updates: %s", host_configuration
85100
)
@@ -164,8 +179,12 @@ def main():
164179
with open(args.trident_yaml) as f:
165180
trident_yaml_content = yaml.safe_load(f)
166181

182+
with open(args.test_selection) as f:
183+
test_selection_content = yaml.safe_load(f)
184+
167185
update_trident_host_config(
168186
host_configuration=trident_yaml_content,
187+
test_selection=test_selection_content,
169188
interface_name=args.interface_name,
170189
interface_ip=args.oam_ip,
171190
interface_mac=args.oam_mac,

.pipelines/templates/stages/testing_vm/netlaunch-testing.yml

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,9 @@ stages:
172172
# Enable SecureBoot for all E2E tests.
173173
SECURE_BOOT="--secure-boot"
174174
175-
# If this is a usr-verity image; a signing certificate must be
176-
# set via --signing-cert; otherwise, set it to an empty string.
177-
# This can also be used for sysext signing.
175+
# If this is a usr-verity UKI image, a signing certificate must
176+
# be set via --signing-cert; otherwise, set it to an empty
177+
# string. This can also be used for sysext signing.
178178
TRIDENT_CONFIG="$(tridentConfigPath)/trident-config.yaml"
179179
IMAGE_URL=$(sudo yq '.image.url' "$TRIDENT_CONFIG")
180180
CERT_SEARCH_PATH="$(System.ArtifactsDirectory)/usrverity-testimage"
@@ -185,10 +185,24 @@ stages:
185185
SIGNING_CERT=""
186186
fi
187187
188+
# If this is a usr-verity UKI image and the test will run in a
189+
# container, then overwrite the PCRs to exclude PCR 7.
190+
ENCRYPTION_VALUE=$(sudo yq '.storage.encryption' "$TRIDENT_CONFIG")
191+
if [[ "$IMAGE_URL" == *usrverity.cosi && \
192+
"${{ parameters.runtimeEnv }}" == 'container' && \
193+
"$ENCRYPTION_VALUE" != "null" ]]; then
194+
sudo yq -i '
195+
.storage.encryption.pcrs = ["boot-loader-code", "kernel-boot"]
196+
' "$TRIDENT_CONFIG"
197+
198+
echo "Updated Trident config:"
199+
sudo yq '.' "$TRIDENT_CONFIG"
200+
fi
201+
188202
./bin/netlaunch \
189203
--iso ./artifacts/iso/$(installerISOName).iso \
190204
--config $(tridentSourceDirectory)/tools/vm-netlaunch.yaml \
191-
--trident $(tridentConfigPath)/trident-config.yaml \
205+
--trident "$TRIDENT_CONFIG" \
192206
--servefolder ./artifacts/test-image \
193207
--logstream \
194208
$MAX_FAILURES_FLAG \

crates/osutils/src/encryption.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ pub const KEY_SIZE: &str = "512";
2525
/// Size of the temporary recovery key file in bytes.
2626
const TMP_RECOVERY_KEY_SIZE: usize = 64;
2727

28-
/// Default PCR to seal encrypted volumes to. PCR 7 represents the `SecureBoot` state.
29-
pub const DEFAULT_PCR: Pcr = Pcr::Pcr7;
30-
3128
/// Runs `systemd-cryptenroll` to enroll a TPM 2.0 device for the given device of a LUKS2 encrypted
3229
/// volume.
3330
///
@@ -259,7 +256,7 @@ mod tests {
259256
let pcrs = BitFlags::from(Pcr::Pcr1) | BitFlags::from(Pcr::Pcr4);
260257
assert_eq!(to_tpm2_pcrs_arg(pcrs), "--tpm2-pcrs=1+4".to_string());
261258

262-
let single_pcr = BitFlags::from(DEFAULT_PCR);
259+
let single_pcr = BitFlags::from(Pcr::Pcr7);
263260
assert_eq!(to_tpm2_pcrs_arg(single_pcr), "--tpm2-pcrs=7".to_string());
264261

265262
let all_pcrs = BitFlags::<Pcr>::all();

crates/osutils/src/pcrlock.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ fn generate_tpm2_access_policy(pcrs: BitFlags<Pcr>) -> Result<(), Error> {
9595
pcrs.iter().map(|pcr| pcr.to_num()).collect::<Vec<_>>()
9696
);
9797

98-
// If running inside of a container AND pcrlock.json already exists in the ROS on the host,
99-
// copy it from the host and into the container
98+
// If running inside of a container AND pcrlock.json already exists in the target OS on the
99+
// host, copy it from the host into the container
100100
if container::is_running_in_container()
101101
.unstructured("Failed to determine if running in container")?
102102
{

crates/trident/src/engine/rollback.rs

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use enumflags2::BitFlags;
55
use log::{debug, info, trace, warn};
66

77
use osutils::{block_devices, efivar, lsblk, pcrlock, veritysetup, virt};
8-
use sysdefs::tpm2::Pcr;
98
use trident_api::{
109
constants::internal_params::VIRTDEPLOY_BOOT_ORDER_WORKAROUND,
1110
error::{InternalError, ReportError, ServicingError, TridentError, TridentResultExt},
@@ -87,23 +86,11 @@ pub fn validate_boot(datastore: &mut DataStore) -> Result<(), TridentError> {
8786
if ctx.is_uki()? {
8887
debug!("Regenerating pcrlock policy for current boot");
8988

90-
let pcrs = if !encryption.pcrs.is_empty() {
91-
encryption
92-
.pcrs
93-
.iter()
94-
.fold(BitFlags::empty(), |acc, &pcr| acc | BitFlags::from(pcr))
95-
} else {
96-
// TODO: Currently, we cannot seal to PCR 7 b/c not all measurements are
97-
// recognized by the .pcrlock file generation logic. Once that is resolved,
98-
// we want to have PCR 7 as the default. For now, we use PCRs 4 and 11. Related
99-
// ADO tasks:
100-
// https://dev.azure.com/mariner-org/polar/_workitems/edit/14523/ and
101-
// https://dev.azure.com/mariner-org/polar/_workitems/edit/14455/.
102-
//
103-
// Use default PCR 7 if none specified.
104-
//BitFlags::from(DEFAULT_PCR)
105-
BitFlags::from(Pcr::Pcr4) | BitFlags::from(Pcr::Pcr11)
106-
};
89+
// Get the PCRs from Host Configuration
90+
let pcrs = encryption
91+
.pcrs
92+
.iter()
93+
.fold(BitFlags::empty(), |acc, &pcr| acc | BitFlags::from(pcr));
10794

10895
// Get UKI and bootloader binaries for .pcrlock file generation
10996
let (uki_binaries, bootloader_binaries) =

crates/trident/src/engine/storage/encryption.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use osutils::{
1616
container,
1717
dependencies::{Dependency, DependencyResultExt},
1818
efivar,
19-
encryption::{self, KeySlotType, DEFAULT_PCR},
19+
encryption::{self, KeySlotType},
2020
lsblk::{self, BlockDeviceType},
2121
path::join_relative,
2222
pcrlock,
@@ -134,8 +134,13 @@ pub(super) fn create_encrypted_devices(
134134
pcrlock::generate_pcrlock_policy(BitFlags::from(Pcr::Pcr0), vec![], vec![])?;
135135
None
136136
} else {
137-
debug!("Target OS image is a grub image, so sealing against the default PCR 7");
138-
Some(BitFlags::from(DEFAULT_PCR))
137+
debug!("Target OS image is a grub image, so sealing against PCR 7");
138+
Some(
139+
encryption
140+
.pcrs
141+
.iter()
142+
.fold(BitFlags::empty(), |acc, &pcr| acc | BitFlags::from(pcr)),
143+
)
139144
};
140145

141146
// Check if `REENCRYPT_ON_CLEAN_INSTALL` internal param is set to true; if so, re-encrypt
@@ -226,8 +231,8 @@ pub(super) fn create_encrypted_devices(
226231
/// - `encryption_type`: The type of encryption to be used. Determines whether the device should be
227232
/// re-encrypted in-place, or whether a new LUKS2 volume should be initialized.
228233
/// - `pcr`: The PCR to seal the key against. This is an optional PCR for scenarios where encrypted
229-
/// volumes are sealed against the value of PCR 7 instead of a pcrlock policy, mainly for the
230-
/// grub MOS + grub ROS flow.
234+
/// volumes are sealed against the value of PCR 7 instead of a pcrlock policy, for the grub MOS +
235+
/// grub target OS flow.
231236
fn encrypt_and_open_device(
232237
device_path: &Path,
233238
device_name: &String,
@@ -302,7 +307,7 @@ struct LuksDumpSegment {
302307

303308
/// Returns paths of UKI and bootloader binaries that `systemd-pcrlock` tool should seal to. During
304309
/// encryption provisioning, returns binaries used for the current boot, as well as binaries that
305-
/// will be used in the future boot, i.e. in the ROS update image. During rollback validation,
310+
/// will be used in the future boot, i.e. in the target OS image. During rollback validation,
306311
/// returns binaries used for the current boot only.
307312
///
308313
/// Returns a tuple containing two vectors:

crates/trident/src/subsystems/storage/encryption.rs

Lines changed: 43 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use enumflags2::BitFlags;
88
use log::{debug, info, trace};
99

1010
use osutils::{
11-
encryption as osutils_encryption, files, path,
11+
container, efivar, encryption as osutils_encryption, files, path,
1212
pcrlock::{self, PCRLOCK_POLICY_JSON_PATH},
1313
};
1414
use sysdefs::tpm2::Pcr;
@@ -89,53 +89,43 @@ pub(super) fn validate_host_config(ctx: &EngineContext) -> Result<(), TridentErr
8989
}
9090

9191
// We've already validated that only supported PCRs, i.e. 4, 7, and/or 11, are specified;
92-
// but we also need to ensure that only valid PCRs are specified for each image type.
93-
if !encryption.pcrs.is_empty() {
94-
if ctx.is_uki()? {
95-
// TODO: Currently, we cannot seal to PCR 7 for grub MOS -> UKI ROS scenario b/c
96-
// not all measurements are recognized by the .pcrlock file generation logic. Once
97-
// that is resolved, we include PCR 7 into the valid list. For now, only PCRs 4 and
98-
// 11 are valid. Related ADO tasks:
99-
// https://dev.azure.com/mariner-org/polar/_workitems/edit/14523/ and
100-
// https://dev.azure.com/mariner-org/polar/_workitems/edit/14455/.
101-
let invalid_pcrs: Vec<_> = encryption
102-
.pcrs
92+
// but we also need to ensure that only PCR 7 is specified for grub images.
93+
if !ctx.is_uki()? {
94+
let invalid_pcrs: Vec<_> = encryption
95+
.pcrs
96+
.iter()
97+
.filter(|&&pcr| pcr != Pcr::Pcr7)
98+
.cloned()
99+
.collect();
100+
101+
if !invalid_pcrs.is_empty() {
102+
let pcrs_string = invalid_pcrs
103103
.iter()
104-
.filter(|&&pcr| pcr != Pcr::Pcr4 && pcr != Pcr::Pcr11)
105-
.cloned()
106-
.collect();
107-
108-
if !invalid_pcrs.is_empty() {
109-
let pcrs_string = invalid_pcrs
110-
.iter()
111-
.map(|pcr| pcr.to_num().to_string())
112-
.collect::<Vec<_>>()
113-
.join(", ");
104+
.map(|pcr| pcr.to_num().to_string())
105+
.collect::<Vec<_>>()
106+
.join(", ");
107+
return Err(TridentError::new(InvalidInputError::from(
108+
HostConfigurationDynamicValidationError::InvalidEncryptionPcrsForGrubImage {
109+
pcrs: pcrs_string,
110+
},
111+
)));
112+
}
113+
} else {
114+
// For UKI images, if PCR 7 is requested, we need to ensure that:
115+
// 1. Secure Boot is enabled,
116+
// 2. Trident is NOT running inside a container,
117+
// due to the limitations of `systemd-pcrlock`.
118+
if encryption.pcrs.contains(&Pcr::Pcr7) {
119+
if !efivar::secure_boot_is_enabled() {
114120
return Err(TridentError::new(InvalidInputError::from(
115-
HostConfigurationDynamicValidationError::InvalidEncryptionPcrsForUkiImage {
116-
pcrs: pcrs_string,
117-
},
121+
HostConfigurationDynamicValidationError::Pcr7EncryptionForUkiWhenSecureBootDisabled,
118122
)));
119123
}
120-
} else {
121-
let invalid_pcrs: Vec<_> = encryption
122-
.pcrs
123-
.iter()
124-
.filter(|&&pcr| pcr != Pcr::Pcr7)
125-
.cloned()
126-
.collect();
127124

128-
if !invalid_pcrs.is_empty() {
129-
let pcrs_string = invalid_pcrs
130-
.iter()
131-
.map(|pcr| pcr.to_num().to_string())
132-
.collect::<Vec<_>>()
133-
.join(", ");
125+
if container::is_running_in_container()? {
134126
return Err(TridentError::new(InvalidInputError::from(
135-
HostConfigurationDynamicValidationError::InvalidEncryptionPcrsForGrubImage {
136-
pcrs: pcrs_string,
137-
},
138-
)));
127+
HostConfigurationDynamicValidationError::Pcr7EncryptionForUkiWhenRunningInContainer,
128+
)));
139129
}
140130
}
141131
}
@@ -148,15 +138,15 @@ pub(super) fn validate_host_config(ctx: &EngineContext) -> Result<(), TridentErr
148138
/// persists the pcrlock policy to the update volume.
149139
///
150140
/// On clean install:
151-
/// 1. TODO: UKI MOS + UKI ROS -> re-generate pcrlock policy to include PCRs 4,7,11 as selected by
152-
/// the user; not implemented for now. Related ADO task:
141+
/// 1. TODO: UKI MOS + UKI target OS -> re-generate pcrlock policy to include PCRs 4,7,11 as
142+
/// selected by the user; not implemented for now. Related ADO task:
153143
/// https://dev.azure.com/mariner-org/polar/_workitems/edit/13059/.
154-
/// 2. Grub MOS + UKI ROS -> N/A, i.e. keep previous pcrlock policy that includes PCR 0 only,
155-
/// 3. Grub MOS + grub ROS -> N/A, i.e. still sealed to the value of PCR 7.
144+
/// 2. Grub MOS + UKI target OS -> N/A, i.e. keep previous pcrlock policy that includes PCR 0 only,
145+
/// 3. Grub MOS + grub target OS -> N/A, i.e. still sealed to the value of PCR 7.
156146
///
157147
/// On A/B update:
158-
/// 1. UKI ROS -> re-generate pcrlock policy to include PCRs 4,7,11 as selected by the user,
159-
/// 2. Grub ROS -> N/A, i.e. keep previous pcrlock policy that includes PCR 7 only.
148+
/// 1. UKI target OS -> re-generate pcrlock policy to include PCRs 4,7,11 as selected by the user,
149+
/// 2. Grub target OS -> N/A, i.e. keep previous pcrlock policy that includes PCR 7 only.
160150
#[tracing::instrument(name = "encryption_provision", skip_all)]
161151
pub fn provision(ctx: &EngineContext, mount_path: &Path) -> Result<(), TridentError> {
162152
if let Some(encryption) = &ctx.spec.storage.encryption {
@@ -168,23 +158,10 @@ pub fn provision(ctx: &EngineContext, mount_path: &Path) -> Result<(), TridentEr
168158
// On A/B update, use PCRs selected by the user through the API
169159
ServicingType::AbUpdate => {
170160
if ctx.is_uki()? {
171-
let bitflags = if !encryption.pcrs.is_empty() {
172-
encryption
173-
.pcrs
174-
.iter()
175-
.fold(BitFlags::empty(), |acc, &pcr| acc | BitFlags::from(pcr))
176-
} else {
177-
// TODO: Currently, we cannot seal to PCR 7 b/c not all measurements are
178-
// recognized by the .pcrlock file generation logic. Once that is resolved,
179-
// we want to have PCR 7 as the default. For now, we use PCRs 4 and 11. Related
180-
// ADO tasks:
181-
// https://dev.azure.com/mariner-org/polar/_workitems/edit/14523/ and
182-
// https://dev.azure.com/mariner-org/polar/_workitems/edit/14455/.
183-
//
184-
// Use default PCR if none specified.
185-
//BitFlags::from(osutils_encryption::DEFAULT_PCR)
186-
BitFlags::from(Pcr::Pcr4) | BitFlags::from(Pcr::Pcr11)
187-
};
161+
let bitflags = encryption
162+
.pcrs
163+
.iter()
164+
.fold(BitFlags::empty(), |acc, &pcr| acc | BitFlags::from(pcr));
188165
Some(bitflags)
189166
} else {
190167
debug!(
@@ -552,27 +529,8 @@ mod tests {
552529
}
553530
validate_host_config(&ctx).unwrap();
554531

555-
// Test case #2: If OS image is a UKI image, then only PCRs 4 and 11 are allowed.
532+
// Test case #2: If OS image is a UKI image AND PCRs only include 4 and 11, then pass.
556533
ctx.is_uki = Some(true);
557-
{
558-
let encryption = ctx.spec.storage.encryption.as_mut().unwrap();
559-
encryption.pcrs = vec![Pcr::Pcr4, Pcr::Pcr7, Pcr::Pcr11];
560-
}
561-
let pcrs_str = [Pcr::Pcr7]
562-
.iter()
563-
.map(|pcr| pcr.to_num().to_string())
564-
.collect::<Vec<_>>()
565-
.join(", ");
566-
assert_eq!(
567-
validate_host_config(&ctx).unwrap_err().kind(),
568-
&ErrorKind::InvalidInput(InvalidInputError::InvalidHostConfigurationDynamic {
569-
inner: HostConfigurationDynamicValidationError::InvalidEncryptionPcrsForUkiImage {
570-
pcrs: pcrs_str,
571-
}
572-
})
573-
);
574-
575-
// Test case #3: If OS image is a UKI image AND PCRs only include 4 and 11, then pass.
576534
{
577535
let encryption = ctx.spec.storage.encryption.as_mut().unwrap();
578536
encryption.pcrs = vec![Pcr::Pcr4, Pcr::Pcr11];

crates/trident/src/subsystems/storage/mod.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ mod tests {
222222
use tempfile::NamedTempFile;
223223
use url::Url;
224224

225-
use osutils::encryption::{self, DEFAULT_PCR};
225+
use osutils::encryption;
226226
use trident_api::{
227227
config::{
228228
AbUpdate, Disk as DiskConfig, Encryption, FileSystem, HostConfiguration, MountPoint,
@@ -232,6 +232,8 @@ mod tests {
232232
error::ErrorKind,
233233
};
234234

235+
use sysdefs::tpm2::Pcr;
236+
235237
fn get_ctx() -> EngineContext {
236238
EngineContext {
237239
servicing_type: ServicingType::CleanInstall,
@@ -331,7 +333,7 @@ mod tests {
331333
device_name: "luks-enc".to_owned(),
332334
device_id: "part5".to_owned(),
333335
}],
334-
pcrs: vec![DEFAULT_PCR],
336+
pcrs: vec![Pcr::Pcr7],
335337
..Default::default()
336338
}),
337339
..Default::default()

0 commit comments

Comments
 (0)