Skip to content

Commit

Permalink
Fix image-create with ACLs. Fixes #1394.
Browse files Browse the repository at this point in the history
Signed-off-by: David Baird <dhbaird@gmail.com>
  • Loading branch information
dhbaird authored and imeoer committed Aug 17, 2023
1 parent 3faf95a commit f9ab2be
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 24 deletions.
2 changes: 1 addition & 1 deletion rafs/src/metadata/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,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() {
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 @@ -1854,7 +1854,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 @@ -2177,10 +2177,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 @@ -2191,35 +2210,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);
}
}
}
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

0 comments on commit f9ab2be

Please sign in to comment.