Skip to content

Commit 13cb153

Browse files
jleboncgwalters
authored andcommitted
ostree-ext/store: Label layers of plain OCI images using booted policy
When importing layers from a plain OCI image (i.e. without `/ostree`), right now we don't do any initial labeling. So all the real labeling happens during the merge commit. This causes a lot of file duplication. We'll fix that more categorically in a later patch, but as a first pass let's at least do the initial import with _an_ SELinux policy; a natural choice is to use the one from the booted deployment. In the common case where we're upgrading, the policies are likely similar enough and so this significantly reduces file duplication in the first place. (There's also the case at install time where we're not yet in a booted commit but may have an SELinux policy lying around; I didn't bother trying to support that because it seems fine to be a bit less efficient there for simpler code.) See also #1637. Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
1 parent 850ce55 commit 13cb153

File tree

4 files changed

+82
-15
lines changed

4 files changed

+82
-15
lines changed

crates/lib/src/cli.rs

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,7 +1100,8 @@ async fn upgrade(
11001100

11011101
if opts.check {
11021102
let imgref = imgref.clone().into();
1103-
let mut imp = crate::deploy::new_importer(repo, &imgref).await?;
1103+
let mut imp =
1104+
crate::deploy::new_importer(repo, &imgref, Some(&booted_ostree.deployment)).await?;
11041105
match imp.prepare().await? {
11051106
PrepareResult::AlreadyPresent(_) => {
11061107
println!("No changes in: {imgref:#}");
@@ -1125,10 +1126,26 @@ async fn upgrade(
11251126
let use_unified = crate::deploy::image_exists_in_unified_storage(storage, imgref).await?;
11261127

11271128
let fetched = if use_unified {
1128-
crate::deploy::pull_unified(repo, imgref, None, opts.quiet, prog.clone(), storage)
1129-
.await?
1129+
crate::deploy::pull_unified(
1130+
repo,
1131+
imgref,
1132+
None,
1133+
opts.quiet,
1134+
prog.clone(),
1135+
storage,
1136+
Some(&booted_ostree.deployment),
1137+
)
1138+
.await?
11301139
} else {
1131-
crate::deploy::pull(repo, imgref, None, opts.quiet, prog.clone()).await?
1140+
crate::deploy::pull(
1141+
repo,
1142+
imgref,
1143+
None,
1144+
opts.quiet,
1145+
prog.clone(),
1146+
Some(&booted_ostree.deployment),
1147+
)
1148+
.await?
11321149
};
11331150
let staged_digest = staged_image.map(|s| s.digest().expect("valid digest in status"));
11341151
let fetched_digest = &fetched.manifest_digest;
@@ -1289,9 +1306,26 @@ async fn switch_ostree(
12891306
};
12901307

12911308
let fetched = if use_unified {
1292-
crate::deploy::pull_unified(repo, &target, None, opts.quiet, prog.clone(), storage).await?
1309+
crate::deploy::pull_unified(
1310+
repo,
1311+
&target,
1312+
None,
1313+
opts.quiet,
1314+
prog.clone(),
1315+
storage,
1316+
Some(&booted_ostree.deployment),
1317+
)
1318+
.await?
12931319
} else {
1294-
crate::deploy::pull(repo, &target, None, opts.quiet, prog.clone()).await?
1320+
crate::deploy::pull(
1321+
repo,
1322+
&target,
1323+
None,
1324+
opts.quiet,
1325+
prog.clone(),
1326+
Some(&booted_ostree.deployment),
1327+
)
1328+
.await?
12951329
};
12961330

12971331
if !opts.retain {
@@ -1428,7 +1462,15 @@ async fn edit_ostree(
14281462
return crate::deploy::rollback(storage).await;
14291463
}
14301464

1431-
let fetched = crate::deploy::pull(repo, new_spec.image, None, opts.quiet, prog.clone()).await?;
1465+
let fetched = crate::deploy::pull(
1466+
repo,
1467+
new_spec.image,
1468+
None,
1469+
opts.quiet,
1470+
prog.clone(),
1471+
Some(&booted_ostree.deployment),
1472+
)
1473+
.await?;
14321474

14331475
// TODO gc old layers here
14341476

crates/lib/src/deploy.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,16 @@ impl ImageState {
9898
pub(crate) async fn new_importer(
9999
repo: &ostree::Repo,
100100
imgref: &ostree_container::OstreeImageReference,
101+
booted_deployment: Option<&ostree::Deployment>,
101102
) -> Result<ostree_container::store::ImageImporter> {
102103
let config = new_proxy_config();
103104
let mut imp = ostree_container::store::ImageImporter::new(repo, imgref, config).await?;
104105
imp.require_bootable();
105106
// We do our own GC/prune in deploy::prune(), so skip the importer's internal one.
106107
imp.disable_gc();
108+
if let Some(deployment) = booted_deployment {
109+
imp.set_sepolicy_commit(deployment.csum().to_string());
110+
}
107111
Ok(imp)
108112
}
109113

@@ -112,11 +116,15 @@ pub(crate) async fn new_importer_with_config(
112116
repo: &ostree::Repo,
113117
imgref: &ostree_container::OstreeImageReference,
114118
config: ostree_ext::containers_image_proxy::ImageProxyConfig,
119+
booted_deployment: Option<&ostree::Deployment>,
115120
) -> Result<ostree_container::store::ImageImporter> {
116121
let mut imp = ostree_container::store::ImageImporter::new(repo, imgref, config).await?;
117122
imp.require_bootable();
118123
// We do our own GC/prune in deploy::prune(), so skip the importer's internal one.
119124
imp.disable_gc();
125+
if let Some(deployment) = booted_deployment {
126+
imp.set_sepolicy_commit(deployment.csum().to_string());
127+
}
120128
Ok(imp)
121129
}
122130

@@ -459,11 +467,12 @@ pub(crate) async fn prepare_for_pull(
459467
repo: &ostree::Repo,
460468
imgref: &ImageReference,
461469
target_imgref: Option<&OstreeImageReference>,
470+
booted_deployment: Option<&ostree::Deployment>,
462471
) -> Result<PreparedPullResult> {
463472
let imgref_canonicalized = imgref.clone().canonicalize()?;
464473
tracing::debug!("Canonicalized image reference: {imgref_canonicalized:#}");
465474
let ostree_imgref = &OstreeImageReference::from(imgref_canonicalized);
466-
let mut imp = new_importer(repo, ostree_imgref).await?;
475+
let mut imp = new_importer(repo, ostree_imgref, booted_deployment).await?;
467476
if let Some(target) = target_imgref {
468477
imp.set_target(target);
469478
}
@@ -517,6 +526,7 @@ pub(crate) async fn prepare_for_pull_unified(
517526
imgref: &ImageReference,
518527
target_imgref: Option<&OstreeImageReference>,
519528
store: &Storage,
529+
booted_deployment: Option<&ostree::Deployment>,
520530
) -> Result<PreparedPullResult> {
521531
// Get or initialize the bootc container storage (same as used for LBIs)
522532
let imgstore = store.get_ensure_imgstore()?;
@@ -562,7 +572,7 @@ pub(crate) async fn prepare_for_pull_unified(
562572
config.skopeo_cmd = Some(cmd);
563573

564574
// Use the preparation flow with the custom config
565-
let mut imp = new_importer_with_config(repo, &ostree_imgref, config).await?;
575+
let mut imp = new_importer_with_config(repo, &ostree_imgref, config, booted_deployment).await?;
566576
if let Some(target) = target_imgref {
567577
imp.set_target(target);
568578
}
@@ -613,8 +623,9 @@ pub(crate) async fn pull_unified(
613623
quiet: bool,
614624
prog: ProgressWriter,
615625
store: &Storage,
626+
booted_deployment: Option<&ostree::Deployment>,
616627
) -> Result<Box<ImageState>> {
617-
match prepare_for_pull_unified(repo, imgref, target_imgref, store).await? {
628+
match prepare_for_pull_unified(repo, imgref, target_imgref, store, booted_deployment).await? {
618629
PreparedPullResult::AlreadyPresent(existing) => {
619630
// Log that the image was already present (Debug level since it's not actionable)
620631
const IMAGE_ALREADY_PRESENT_ID: &str = "5c4d3e2f1a0b9c8d7e6f5a4b3c2d1e0f9";
@@ -726,8 +737,9 @@ pub(crate) async fn pull(
726737
target_imgref: Option<&OstreeImageReference>,
727738
quiet: bool,
728739
prog: ProgressWriter,
740+
booted_deployment: Option<&ostree::Deployment>,
729741
) -> Result<Box<ImageState>> {
730-
match prepare_for_pull(repo, imgref, target_imgref).await? {
742+
match prepare_for_pull(repo, imgref, target_imgref, booted_deployment).await? {
731743
PreparedPullResult::AlreadyPresent(existing) => {
732744
// Log that the image was already present (Debug level since it's not actionable)
733745
const IMAGE_ALREADY_PRESENT_ID: &str = "5c4d3e2f1a0b9c8d7e6f5a4b3c2d1e0f9";

crates/lib/src/install.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,10 +1074,11 @@ async fn install_container(
10741074
&spec_imgref,
10751075
Some(&state.target_imgref),
10761076
storage,
1077+
None,
10771078
)
10781079
.await?
10791080
} else {
1080-
prepare_for_pull(repo, &spec_imgref, Some(&state.target_imgref)).await?
1081+
prepare_for_pull(repo, &spec_imgref, Some(&state.target_imgref), None).await?
10811082
};
10821083

10831084
let pulled_image = match prepared {
@@ -2764,6 +2765,7 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> {
27642765
None,
27652766
opts.quiet,
27662767
prog.clone(),
2768+
None,
27672769
)
27682770
.await?;
27692771
(fetched, new_spec)

crates/ostree-ext/src/container/store.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ pub struct ImageImporter {
301301
target_imgref: Option<OstreeImageReference>,
302302
no_imgref: bool, // If true, do not write final image ref
303303
disable_gc: bool, // If true, don't prune unused image layers
304+
/// Optional commit to use as SELinux policy source for non-ostree container layers.
305+
sepolicy_commit: Option<String>,
304306
/// If true, require the image has the bootable flag
305307
require_bootable: bool,
306308
/// Do not attempt to contact the network
@@ -653,6 +655,7 @@ impl ImageImporter {
653655
no_imgref: false,
654656
ostree_v2024_3: ostree::check_version(2024, 3),
655657
disable_gc: false,
658+
sepolicy_commit: None,
656659
require_bootable: false,
657660
offline: false,
658661
imgref: imgref.clone(),
@@ -694,6 +697,13 @@ impl ImageImporter {
694697
self.disable_gc = true;
695698
}
696699

700+
/// Set the commit to use as SELinux policy source when importing
701+
/// non-ostree container layers. Has no effect on ostree-native
702+
/// containers (which have their own base commit).
703+
pub fn set_sepolicy_commit(&mut self, commit: String) {
704+
self.sepolicy_commit = Some(commit);
705+
}
706+
697707
/// Determine if there is a new manifest, and if so return its digest.
698708
/// This will also serialize the new manifest and configuration into
699709
/// metadata associated with the image, so that invocations of `[query_cached]`
@@ -1373,10 +1383,11 @@ impl ImageImporter {
13731383
self.imgref.imgref.transport,
13741384
)
13751385
.await?;
1376-
// An important aspect of this is that we SELinux label the derived layers using
1377-
// the base policy.
1386+
// SELinux label derived layers using the base policy. For non-ostree
1387+
// containers (base_commit is None), fall back to the caller-provided
1388+
// sepolicy_commit (typically the booted deployment's commit).
13781389
let opts = crate::tar::WriteTarOptions {
1379-
base: base_commit.clone(),
1390+
base: base_commit.clone().or(self.sepolicy_commit.clone()),
13801391
selinux: true,
13811392
allow_nonusr: root_is_transient,
13821393
retain_var: self.ostree_v2024_3,

0 commit comments

Comments
 (0)