Skip to content
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

Jump to bottom

image-rs: add image block device dm-verity and mount #270

Merged
merged 1 commit into from
Jul 26, 2023
Merged

image-rs: add image block device dm-verity and mount #270

merged 1 commit into from
Jul 26, 2023

Conversation

ChengyuZhu6
Copy link
Member

@ChengyuZhu6 ChengyuZhu6 commented Jul 10, 2023 •
edited
Loading

Implement image block device integrity check using dm-verity and mount the verity device in image-rs.
Notes: the kernel should be upgrade to >=6.4 for mounting the verity device with erofs tarfs.

@ChengyuZhu6 ChengyuZhu6 force-pushed the image_block_verity_with_mount branch 3 times, most recently from 4662773 to f777e8e Compare July 10, 2023 01:33
@hsiangkao
Copy link

hsiangkao commented Jul 10, 2023 •
edited
Loading

the kernel should be upgrade to >=6.4 for mounting the verity device with erofs.
This is not accurate, only erofs tarfs support was landed in Linux 6.4.
Otherwise, if you just mount exist erofs images much like ext4 with dm-verity, there is no such requirement (5.4, 5.10, 5.15, 6.1 LTS are all supported. )

@ChengyuZhu6
Copy link
Member Author

the kernel should be upgrade to >=6.4 for mounting the verity device with erofs. This is not accurate, only erofs tarfs support was landed in Linux 6.4. Otherwise, if you just mount exist erofs images much like ext4 with dm-verity, there is no such requirement (5.4, 5.10, 5.15, 6.1 LTS are all supported. )

Thanks, I have updated.

@ChengyuZhu6 ChengyuZhu6 force-pushed the image_block_verity_with_mount branch from f777e8e to fca8f07 Compare July 11, 2023 01:37
@ChengyuZhu6 ChengyuZhu6 mentioned this pull request Jul 11, 2023
Merged
@ChengyuZhu6 ChengyuZhu6 force-pushed the image_block_verity_with_mount branch from 7e4d7f3 to fca8f07 Compare July 11, 2023 11:46
jiangliu
jiangliu reviewed Jul 11, 2023
View reviewed changes
image-rs/src/image.rs Outdated
@@ -39,7 +41,6 @@ use crate::nydus::{service, utils};
/// The reason for using the `/run` directory here is that in general HW-TEE,
/// the `/run` directory is mounted in `tmpfs`, which is located in the encrypted memory protected by HW-TEE.
pub const IMAGE_SECURITY_CONFIG_DIR: &str = "/run/image-security";

Copy link
Member

Choose a reason for hiding this comment

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

Unrelated change?

jiangliu
jiangliu reviewed Jul 11, 2023
View reviewed changes
image-rs/src/image.rs Outdated
@@ -421,6 +422,33 @@ impl ImageClient {

Ok(image_id)
}
/// set_image creates a mapping backed by image block device <source_device_path> and
Copy link
Member

Choose a reason for hiding this comment

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

Add a blank line above?

@ChengyuZhu6 ChengyuZhu6 force-pushed the image_block_verity_with_mount branch 5 times, most recently from 7c0e680 to 63e3078 Compare July 14, 2023 07:53
@ChengyuZhu6 ChengyuZhu6 requested a review from jiangliu July 14, 2023 07:53
@ChengyuZhu6 ChengyuZhu6 force-pushed the image_block_verity_with_mount branch from 63e3078 to 5ad0b28 Compare July 18, 2023 07:28
jiangliu
jiangliu reviewed Jul 18, 2023
View reviewed changes
image-rs/src/verity.rs Outdated
data_device_path: &str,
) -> Result<String> {
let veritysetup_path: &str = veritysetup_exists()?;
let output = Command::new(veritysetup_path)
Copy link
Member

Choose a reason for hiding this comment

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

How about using https://docs.rs/devicemapper/latest/devicemapper/struct.DM.html to get rid of the dependency on veritysetup binary?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't used devicemapper, but @jiangliu's comment seems right to me. Related to this: it looks like verity_option doesn't get sanity-checked or sanitized anywhere, so its values get plugged directly into the args to versitysetup when issuing the Command. Using a library call (@jiangliu's suggestion) and/or sanity-checking those values might make error handling/checking easier.

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't used devicemapper, but @jiangliu's comment seems right to me. Related to this: it looks like verity_option doesn't get sanity-checked or sanitized anywhere, so its values get plugged directly into the args to versitysetup when issuing the Command. Using a library call (@jiangliu's suggestion) and/or sanity-checking those values might make error handling/checking easier.

Yes, I agree with @jiangliu's comment and I am working on using the devicemapper library to handle/check the options.

@ChengyuZhu6 ChengyuZhu6 force-pushed the image_block_verity_with_mount branch 6 times, most recently from f9785a4 to 257362c Compare July 24, 2023 11:27
@ChengyuZhu6 ChengyuZhu6 force-pushed the image_block_verity_with_mount branch 15 times, most recently from 100ed06 to a4f6c55 Compare July 26, 2023 02:50
image-rs: add image block device dm-verity and mount
f5c3825 
Implement image block device integrity check using dm-verity and mount/umount the verity device in image-rs.

Signed-off-by: ChengyuZhu6 <[email protected]>
@ChengyuZhu6 ChengyuZhu6 force-pushed the image_block_verity_with_mount branch from a4f6c55 to f5c3825 Compare July 26, 2023 02:51
@jiangliu jiangliu merged commit af99f9a into confidential-containers:main Jul 26, 2023
@mythi mythi mentioned this pull request Jul 26, 2023
Merged
@ChengyuZhu6 ChengyuZhu6 mentioned this pull request Aug 2, 2023
Open
5 tasks
@ChengyuZhu6 ChengyuZhu6 deleted the image_block_verity_with_mount branch August 10, 2023 06:04
surajssd added a commit to surajssd/kata-containers that referenced this pull request Aug 11, 2023
@surajssd
static-checks: Install devmapper libraries
e4b13be 
After image-rs added the image-block-device integrity check using
dm-verity a new dependency is now needed, so install that.

Refer the following PR for more information:
confidential-containers/guest-components#270

Fixes: kata-containers#7580

Signed-off-by: Suraj Deshmukh <[email protected]>
surajssd added a commit to surajssd/kata-containers that referenced this pull request Aug 14, 2023
@surajssd
static-checks: Install devmapper libraries
8a4d09e 
After image-rs added the image-block-device integrity check using
dm-verity a new dependency is now needed, so install that.

Refer the following PR for more information:
confidential-containers/guest-components#270

Fixes: kata-containers#7580

Signed-off-by: Suraj Deshmukh <[email protected]>
surajssd added a commit to surajssd/kata-containers that referenced this pull request Aug 15, 2023
@surajssd
static-checks: Install devmapper libraries
52bca42 
After image-rs added the image-block-device integrity check using
dm-verity a new dependency is now needed, so install that.

Refer the following PR for more information:
confidential-containers/guest-components#270

Fixes: kata-containers#7580

Signed-off-by: Suraj Deshmukh <[email protected]>
@surajssd surajssd mentioned this pull request Aug 15, 2023
Closed
surajssd added a commit to surajssd/tests that referenced this pull request Aug 15, 2023
@surajssd
ci-setup-env-ubuntu: Install devmapper & clang
27baff9 
After image-rs added the image-block-device integrity check using
dm-verity a new dependency is now needed, so install that.

Refer the following PR for more information:
confidential-containers/guest-components#270

Signed-off-by: Suraj Deshmukh <[email protected]>
surajssd added a commit to surajssd/kata-containers that referenced this pull request Aug 15, 2023
@surajssd
static-checks: Install devmapper libraries
555ae95 
After image-rs added the image-block-device integrity check using
dm-verity a new dependency is now needed, so install that.

Refer the following PR for more information:
confidential-containers/guest-components#270

Fixes: kata-containers#7580

Signed-off-by: Suraj Deshmukh <[email protected]>
surajssd added a commit to surajssd/kata-containers that referenced this pull request Aug 16, 2023
@surajssd
static-checks: Install devmapper libraries
1ec85d7 
After image-rs added the image-block-device integrity check using
dm-verity a new dependency is now needed, so install that.

Refer the following PR for more information:
confidential-containers/guest-components#270

Fixes: kata-containers#7580

Signed-off-by: Suraj Deshmukh <[email protected]>
ChengyuZhu6 pushed a commit to ChengyuZhu6/kata-containers that referenced this pull request Aug 24, 2023
@surajssd
static-checks: Install devmapper libraries
fcc3bd4 
After image-rs added the image-block-device integrity check using
dm-verity a new dependency is now needed, so install that.

Refer the following PR for more information:
confidential-containers/guest-components#270

Fixes: kata-containers#7580

Signed-off-by: Suraj Deshmukh <[email protected]>
ChengyuZhu6 pushed a commit to ChengyuZhu6/kata-containers that referenced this pull request Aug 24, 2023
@surajssd
static-checks: Install devmapper libraries
89be034 
After image-rs added the image-block-device integrity check using
dm-verity a new dependency is now needed, so install that.

Refer the following PR for more information:
confidential-containers/guest-components#270

Fixes: kata-containers#7580

Signed-off-by: Suraj Deshmukh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiangliu jiangliu jiangliu approved these changes

@sameo sameo Awaiting requested review from sameo

@arronwy arronwy Awaiting requested review from arronwy

@lumjjb lumjjb Awaiting requested review from lumjjb

@jialez0 jialez0 Awaiting requested review from jialez0

@Xynnn007 Xynnn007 Awaiting requested review from Xynnn007

@portersrc portersrc Awaiting requested review from portersrc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ChengyuZhu6 @hsiangkao @portersrc @jiangliu