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.

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

Fix image-create with ACLs. Fixes #1394. #1395

Merged
merged 1 commit into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion rafs/src/metadata/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl RafsXAttrs {
return Err(einval!("xattr key/value is too big"));
}
for p in RAFS_XATTR_PREFIXES {
if buf.len() > p.as_bytes().len() && &buf[..p.as_bytes().len()] == p.as_bytes() {
if buf.len() >= p.as_bytes().len() && &buf[..p.as_bytes().len()] == p.as_bytes() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps so; I am not 100% sure. I apologize that I have yet produced a test in bats to correspond to testing this. Thank you for the time and consideration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

*That I have not yet produced, I mean!

Copy link
Collaborator

@imeoer imeoer Aug 8, 2023

Choose a reason for hiding this comment

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

Hi @dhbaird, you can add a related test case here, it will be used by the smoke test automatically.
https://github.com/dragonflyoss/image-service/blob/34ee8255dea73819f4d634f83260d1fa4fe68c97/smoke/tests/texture/layer.go#L89

self.pairs.insert(name, value);
return Ok(());
}
Expand Down
89 changes: 66 additions & 23 deletions rafs/src/metadata/layout/v6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,7 @@ impl RafsXAttrs {
for (key, value) in self.pairs.iter() {
let (index, prefix_len) = Self::match_prefix(key)
.map_err(|_| einval!(format!("invalid xattr key {:?}", key)))?;
if key.len() <= prefix_len {
if key.len() < prefix_len {
return Err(einval!(format!("invalid xattr key {:?}", key)));
}
if value.len() > u16::MAX as usize {
Expand Down Expand Up @@ -2408,10 +2408,29 @@ mod tests {
let mut reader: Box<dyn RafsIoRead> = Box::new(r);

let mut xattrs = RafsXAttrs::new();
xattrs.add(OsString::from("user.nydus"), vec![1u8]).unwrap();
// These xattrs are in "e_name_index" order for easier reading:
xattrs
.add(OsString::from("security.rafs"), vec![2u8, 3u8])
.unwrap();
xattrs
.add(
OsString::from("system.posix_acl_access"),
vec![4u8, 5u8, 6u8],
)
.unwrap();
xattrs
.add(
OsString::from("system.posix_acl_default"),
vec![7u8, 8u8, 9u8, 10u8],
)
.unwrap();
xattrs
.add(
OsString::from("trusted.abc"),
vec![11u8, 12u8, 13u8, 14u8, 15u8],
)
.unwrap();
xattrs.add(OsString::from("user.nydus"), vec![1u8]).unwrap();
xattrs.store_v6(&mut writer).unwrap();
writer.flush().unwrap();

Expand All @@ -2422,35 +2441,59 @@ mod tests {
assert_eq!(header.h_shared_count, 0u8);

let target1 = RafsV6XattrEntry {
e_name_len: 4u8,
e_name_index: 6u8,
e_value_size: u16::to_le(2u16),
e_name_len: 5u8, // "nydus"
e_name_index: 1u8, // EROFS_XATTR_INDEX_USER
e_value_size: u16::to_le(1u16),
};

let target2 = RafsV6XattrEntry {
e_name_len: 5u8,
e_name_index: 1u8,
e_value_size: u16::to_le(1u16),
e_name_len: 0u8, // ""
e_name_index: 2u8, // EROFS_XATTR_INDEX_POSIX_ACL_ACCESS
e_value_size: u16::to_le(3u16),
};

let mut entry1 = RafsV6XattrEntry::new();
reader.read_exact(entry1.as_mut()).unwrap();
assert!((entry1 == target1 || entry1 == target2));
let target3 = RafsV6XattrEntry {
e_name_len: 0u8, // ""
e_name_index: 3u8, // EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT
e_value_size: u16::to_le(4u16),
};

size += size_of::<RafsV6XattrEntry>()
+ entry1.name_len() as usize
+ entry1.value_size() as usize;
let target4 = RafsV6XattrEntry {
e_name_len: 3u8, // "abc"
e_name_index: 4u8, // EROFS_XATTR_INDEX_TRUSTED
e_value_size: u16::to_le(5u16),
};

reader
.seek_to_offset(round_up(size as u64, size_of::<RafsV6XattrEntry>() as u64))
.unwrap();
let target5 = RafsV6XattrEntry {
e_name_len: 4u8, // "rafs"
e_name_index: 6u8, // EROFS_XATTR_INDEX_SECURITY
e_value_size: u16::to_le(2u16),
};

let mut entry2 = RafsV6XattrEntry::new();
reader.read_exact(entry2.as_mut()).unwrap();
if entry1 == target1 {
assert!(entry2 == target2);
} else {
assert!(entry2 == target1);
let targets = vec![target1, target2, target3, target4, target5];

let mut entries: Vec<RafsV6XattrEntry> = Vec::new();
entries.reserve(targets.len());
for _i in 0..targets.len() {
let mut entry = RafsV6XattrEntry::new();
reader.read_exact(entry.as_mut()).unwrap();
size += round_up(
(size_of::<RafsV6XattrEntry>()
+ entry.e_name_len as usize
+ entry.e_value_size as usize) as u64,
size_of::<RafsV6XattrEntry>() as u64,
) as usize;
reader.seek_to_offset(size as u64).unwrap();
entries.push(entry);
}

for (i, target) in targets.iter().enumerate() {
let j = entries
.iter()
.position(|entry| entry == target)
.unwrap_or_else(|| panic!("Test failed for: target{}", i + 1));
// Note: swap_remove() is faster than remove() when order doesn't matter:
entries.swap_remove(j);
}
}

Expand Down
12 changes: 12 additions & 0 deletions smoke/tests/texture/layer.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ func MakeLowerLayer(t *testing.T, workDir string, makers ...LayerMaker) *tool.La
// Set file xattr (only `security.capability` xattr is supported in OCI layer)
tool.Run(t, fmt.Sprintf("setcap CAP_NET_RAW+ep %s", filepath.Join(workDir, "dir-1/file-2")))

// Note: The following test is omitted for now because containerd does not
// support created layers with any xattr except "security." xattrs described
// in this issue: https://github.com/containerd/containerd/issues/8947
// Create file with an ACL:
//layer.CreateFile(t, "acl-file.txt", []byte(""))
// The following xattr key and value are equivalent to running this ACL
// command: "setfacl -x user:root:rwx acl-file.txt"
//layer.SetXattr(t, "acl-file.txt", "system.posix_acl_access", []byte{
// 0x02, 0x00, 0x00, 0x00, 0x01, 0x00, 0x07, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0x02, 0x00, 0x07, 0x00,
// 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x05, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0x10, 0x00, 0x07, 0x00,
// 0xFF, 0xFF, 0xFF, 0xFF, 0x20, 0x00, 0x05, 0x00, 0xFF, 0xFF, 0xFF, 0xFF });

// Customized files
for _, maker := range makers {
maker(t, layer)
Expand Down
Loading