From c0448c11abbf1deb2bd85afe4ab43e25a39055ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Wed, 10 Jan 2024 18:22:08 -0500 Subject: [PATCH 1/3] shared/idmap: Don't change the json format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed to avoid a breaking change to the volatile keys in Incus and potentially in other tools using the serialized JSON format. Signed-off-by: Stéphane Graber --- shared/idmap/entry.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/shared/idmap/entry.go b/shared/idmap/entry.go index 03d34ad6ce1..cd6e2c1a49c 100644 --- a/shared/idmap/entry.go +++ b/shared/idmap/entry.go @@ -8,11 +8,11 @@ import ( // Entry is a single idmap entry (line). type Entry struct { - IsUID bool - IsGID bool - HostID int64 // id as seen on the host - i.e. 100000 - NSID int64 // id as seen in the ns - i.e. 0 - MapRange int64 + IsUID bool `json:"Isuid"` + IsGID bool `json:"Isgid"` + HostID int64 `json:"Hostid"` // id as seen on the host - i.e. 100000 + NSID int64 `json:"Nsid"` // id as seen in the ns - i.e. 0 + MapRange int64 `json:"Maprange"` } // ToLXCString converts an Entry into its LXC representation. From 64afc16c2ca74f8713246e34cbf5891f88f779d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Wed, 10 Jan 2024 18:46:22 -0500 Subject: [PATCH 2/3] shared/idmap: Document AddSafe and fix double records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This properly documents the working of AddSafe as well as fixes an issue where a new entry that's of both UID and GID types could intersect with more than one entry (if mixed with entries that aren't both UID and GID). Simply keep track of whether the entry was already added and if it did, process the intersection but don't add the entry again. Signed-off-by: Stéphane Graber --- shared/idmap/set.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/shared/idmap/set.go b/shared/idmap/set.go index a1b1fa82a01..30fbb89e12a 100644 --- a/shared/idmap/set.go +++ b/shared/idmap/set.go @@ -173,18 +173,20 @@ func (m *Set) ValidRanges() ([]*Range, error) { func (m *Set) AddSafe(i Entry) error { result := []Entry{} added := false + for _, e := range m.Entries { + // Check if the existing entry intersects with the new one. if !e.Intersects(i) { result = append(result, e) continue } + // Fail when the same hostid(s) are used in multiple entries. if e.HostIDsIntersect(i) { return ErrHostIDIsSubID } - added = true - + // Split the lower part of the entry (ids from begining of existing entry to start of new entry). lower := Entry{ IsUID: e.IsUID, IsGID: e.IsGID, @@ -193,6 +195,7 @@ func (m *Set) AddSafe(i Entry) error { MapRange: i.NSID - e.NSID, } + // Split the upper part of the entry (ids from new entry to end of existing entry). upper := Entry{ IsUID: e.IsUID, IsGID: e.IsGID, @@ -201,16 +204,28 @@ func (m *Set) AddSafe(i Entry) error { MapRange: e.MapRange - i.MapRange - lower.MapRange, } + // If the new entry doesn't completely cover the lower part of + // the existing entry, then add that to the set. if lower.MapRange > 0 { result = append(result, lower) } - result = append(result, i) + // Add the new entry in the middle. + if !added { + // With an entry matching both uid and gid, more than one + // intersection is possible, keep track of it to only put it in the set once. + added = true + result = append(result, i) + } + + // If the new entry doesn't completely cover the upper part of + // the existing entry, then add that to the set. if upper.MapRange > 0 { result = append(result, upper) } } + // If no intersection was found, just add the new entry to the set. if !added { result = append(result, i) } From fe01631b29094a6690a5656d1b8895467ca433b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Wed, 10 Jan 2024 20:28:49 -0500 Subject: [PATCH 3/3] incusd: Update instance_test for shared/idmap fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- cmd/incusd/instance_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cmd/incusd/instance_test.go b/cmd/incusd/instance_test.go index a481562a912..24fec5e544b 100644 --- a/cmd/incusd/instance_test.go +++ b/cmd/incusd/instance_test.go @@ -442,13 +442,11 @@ func (suite *containerTestSuite) TestContainer_findIdmap_raw() { suite.Req.Equal(int64(1000), map1.Entries[i].MapRange, "incorrect maprange") } - for _, i := range []int{1, 4} { - suite.Req.Equal(int64(1000), map1.Entries[i].HostID, "hostids don't match") - suite.Req.Equal(int64(1000), map1.Entries[i].NSID, "invalid nsid") - suite.Req.Equal(int64(1), map1.Entries[i].MapRange, "incorrect maprange") - } + suite.Req.Equal(int64(1000), map1.Entries[1].HostID, "hostids don't match") + suite.Req.Equal(int64(1000), map1.Entries[1].NSID, "invalid nsid") + suite.Req.Equal(int64(1), map1.Entries[1].MapRange, "incorrect maprange") - for _, i := range []int{2, 5} { + for _, i := range []int{2, 4} { suite.Req.Equal(host.HostID+1001, map1.Entries[i].HostID, "hostids don't match") suite.Req.Equal(int64(1001), map1.Entries[i].NSID, "invalid nsid") suite.Req.Equal(host.MapRange-1000-1, map1.Entries[i].MapRange, "incorrect maprange")