[PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Patrick Ohly]

The /etc passwd files in a rootfs consist of the default entries from
base-passwd plus anything that gets added by preinst scripts or
extrausers.bbclass.

The execution order of preinst scripts is not perfectly deterministic,
or at least unrelated changes caused it to change in a
non-deterministic way, resulting in irrelevant changes in the order of
passwd entries.

Such re-ordering is bad for reproducible builds and file-based update
mechanisms like swupd which work best if changes are as minimal as
possible.

To achieve that, the files get sorted in a post-processing command,
enabled by default. It would be slightly nicer to keep entries from
base-passwd at the beginning of the files, but that's harder to
implement.

The order of the entries should not matter, but in obscure cases where
it does (like having multiple entries for the same numeric ID) this
behavior can be disabled by setting SORT_PASSWD_POSTPROCESS_COMMAND to
an empty string.

Fixes: YOCTO #10520

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/rootfs-postcommands.bbclass | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/meta/classes/rootfs-postcommands.bbclass b/meta/classes/rootfs-postcommands.bbclass
index 8d48a2d..4ca031f 100644
--- a/meta/classes/rootfs-postcommands.bbclass
+++ b/meta/classes/rootfs-postcommands.bbclass
@@ -30,6 +30,13 @@ ROOTFS_POSTPROCESS_COMMAND += 'empty_var_volatile;'
 SSH_DISABLE_DNS_LOOKUP ?= " ssh_disable_dns_lookup ; "
 ROOTFS_POSTPROCESS_COMMAND_append_qemuall = "${SSH_DISABLE_DNS_LOOKUP}"
 
+# Sort the user and group entries in /etc in order to make the content
+# deterministic. Package installs are not deterministic, causing the ordering
+# of entries to change between builds. In case that this isn't desired,
+# the command can be overridden.
+SORT_PASSWD_POSTPROCESS_COMMAND ??= "sort_passwd; "
+ROOTFS_POSTPROCESS_COMMAND += "${SORT_PASSWD_POSTPROCESS_COMMAND}"
+
 systemd_create_users () {
 	for conffile in ${IMAGE_ROOTFS}/usr/lib/sysusers.d/systemd.conf ${IMAGE_ROOTFS}/usr/lib/sysusers.d/systemd-remote.conf; do
 		[ -e $conffile ] || continue
@@ -146,6 +153,19 @@ ssh_disable_dns_lookup () {
 	fi
 }
 
+sort_passwd () {
+	for i in passwd shadow group gshadow; do
+		for suffix in "" "-"; do
+			file="${IMAGE_ROOTFS}/${sysconfdir}/$i$suffix"
+			if [ -f $file ]; then
+				sort $file >$file.tmp
+				cat $file.tmp >$file
+				rm $file.tmp
+			fi
+		done
+	done
+}
+
 #
 # Enable postinst logging if debug-tweaks is enabled
 #
-- 
2.1.4

Re: [PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Phil Blundell]

On Thu, 2017-01-05 at 16:38 +0100, Patrick Ohly wrote:
> The /etc passwd files in a rootfs consist of the default entries from
> base-passwd plus anything that gets added by preinst scripts or
> extrausers.bbclass.
> 
> The execution order of preinst scripts is not perfectly
> deterministic,
> or at least unrelated changes caused it to change in a
> non-deterministic way, resulting in irrelevant changes in the order
> of
> passwd entries.
> 
> Such re-ordering is bad for reproducible builds and file-based update
> mechanisms like swupd which work best if changes are as minimal as
> possible.
> 
> To achieve that, the files get sorted in a post-processing command,
> enabled by default.

Won't the numeric UIDs still be non-deterministic, though?  If the goal
is reproducible builds then it doesn't sound as though this quite fixes
the problem.

p.

Re: [PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Patrick Ohly]

On Fri, 2017-01-06 at 21:17 +0000, Phil Blundell wrote:
> On Thu, 2017-01-05 at 16:38 +0100, Patrick Ohly wrote:
> > The /etc passwd files in a rootfs consist of the default entries from
> > base-passwd plus anything that gets added by preinst scripts or
> > extrausers.bbclass.
> > 
> > The execution order of preinst scripts is not perfectly
> > deterministic,
> > or at least unrelated changes caused it to change in a
> > non-deterministic way, resulting in irrelevant changes in the order
> > of
> > passwd entries.
> > 
> > Such re-ordering is bad for reproducible builds and file-based update
> > mechanisms like swupd which work best if changes are as minimal as
> > possible.
> > 
> > To achieve that, the files get sorted in a post-processing command,
> > enabled by default.
> 
> Won't the numeric UIDs still be non-deterministic, though?  If the goal
> is reproducible builds then it doesn't sound as though this quite fixes
> the problem.

Yes, but there's already a solution for that problem:
useradd-staticids.bbclass

I was assuming that someone who wants identical files is already using
that. Should it be mentioned in a comment next to the new feature?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Phil Blundell]

On Sat, 2017-01-07 at 09:06 +0100, Patrick Ohly wrote:
> 
Yes, but there's already a solution for that problem:
> useradd-staticids.bbclass
> 
> I was assuming that someone who wants identical files is already
> using
> that. Should it be mentioned in a comment next to the new feature?

That sounds like a good idea.  And in that case, maybe it would be
better to sort on uid rather than username?  That would preserve the
"traditional" ordering in the file, i.e. root first.

p.

Re: [PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Patrick Ohly]

On Sat, 2017-01-07 at 09:59 +0000, Phil Blundell wrote:
> On Sat, 2017-01-07 at 09:06 +0100, Patrick Ohly wrote:
> > 
> Yes, but there's already a solution for that problem:
> > useradd-staticids.bbclass
> > 
> > I was assuming that someone who wants identical files is already
> > using
> > that. Should it be mentioned in a comment next to the new feature?
> 
> That sounds like a good idea.  And in that case, maybe it would be
> better to sort on uid rather than username?  That would preserve the
> "traditional" ordering in the file, i.e. root first.

I had thought about that, but then did not pursue that further because
it would have made sorting quite a bit more complex (needs to know about
line content, id not present in each file).

I can give it a try, though, if that's considered worth some additional
complexity.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Richard Purdie]

On Sat, 2017-01-07 at 20:00 +0100, Patrick Ohly wrote:
> On Sat, 2017-01-07 at 09:59 +0000, Phil Blundell wrote:
> > 
> > On Sat, 2017-01-07 at 09:06 +0100, Patrick Ohly wrote:
> > > 
> > > 
> > Yes, but there's already a solution for that problem:
> > > 
> > > useradd-staticids.bbclass
> > > 
> > > I was assuming that someone who wants identical files is already
> > > using
> > > that. Should it be mentioned in a comment next to the new
> > > feature?
> > That sounds like a good idea.  And in that case, maybe it would be
> > better to sort on uid rather than username?  That would preserve
> > the
> > "traditional" ordering in the file, i.e. root first.
> I had thought about that, but then did not pursue that further
> because
> it would have made sorting quite a bit more complex (needs to know
> about
> line content, id not present in each file).
> 
> I can give it a try, though, if that's considered worth some
> additional
> complexity.

I think those functions can be python functions and this might not be
too bad to write in python....

Cheers,

Richard

Re: [PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Phil Blundell]

On Sat, 2017-01-07 at 22:52 +0000, Richard Purdie wrote:
> On Sat, 2017-01-07 at 20:00 +0100, Patrick Ohly wrote:
> > 
> > On Sat, 2017-01-07 at 09:59 +0000, Phil Blundell wrote:
> > > 
> > > That sounds like a good idea.  And in that case, maybe it would
> > > be
> > > better to sort on uid rather than username?  That would preserve
> > > the
> > > "traditional" ordering in the file, i.e. root first.
> > I had thought about that, but then did not pursue that further
> > because
> > it would have made sorting quite a bit more complex (needs to know
> > about
> > line content, id not present in each file).
> > 
> > I can give it a try, though, if that's considered worth some
> > additional
> > complexity.
> 
> I think those functions can be python functions and this might not be
> too bad to write in python....

I'm not sure it's that complicated.  Maybe I'm overlooking something
obvious, but something like "sort -t: -k3n" seems like it ought to
suffice for sorting /etc/passwd and /etc/group on numeric id.

I didn't understand Patrick's comment about "id not present in each
file" though, maybe that's the key to the extra complexity.

p.

Re: [PATCH 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Patrick Ohly]

On Sun, 2017-01-08 at 09:36 +0000, Phil Blundell wrote:
> I'm not sure it's that complicated.  Maybe I'm overlooking something
> obvious, but something like "sort -t: -k3n" seems like it ought to
> suffice for sorting /etc/passwd and /etc/group on numeric id.
> 
> I didn't understand Patrick's comment about "id not present in each
> file" though, maybe that's the key to the extra complexity.

/etc/gshadow doesn't contain numeric IDs, so sorting has to be done
based on information gathered from /etc/groups. Not terribly difficult,
just more code.

I also wonder how to handle comment lines - remove them (easy) or
associate with the following entry (nicer)?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

[PATCH V2 1/1] rootfs-postcommands.bbclass: sort passwd entries

[Patrick Ohly]

The /etc passwd files in a rootfs consist of the default entries from
base-passwd plus anything that gets added via package installation,
EXTRA_USERS_PARAMS and/or system sysusers.

The execution order of preinst scripts is not perfectly deterministic,
or at least unrelated changes caused it to change in a
non-deterministic way, resulting in irrelevant changes in the order of
passwd entries.

useradd-staticids.bbclass ensures that the numeric IDs don't change,
but re-ordering can still occur, which is bad for reproducible builds
and file-based update mechanisms like swupd which work best if changes
are as minimal as possible.

To achieve that, the files get sorted in a post-processing command,
enabled by default. Sorting is based primarily on the numeric IDs, so
for example, the "root" user continues to be listed first. "nobody"
now is at the end, which wasn't the case before.

The order of the entries should not matter, but in obscure cases where
it does (like having multiple entries for the same numeric ID) this
behavior can be disabled by setting SORT_PASSWD_POSTPROCESS_COMMAND to
an empty string.

Fixes: YOCTO #10520

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---

Notes:
    Changes since V1:
     * switched to sorting by numeric ID
     * try harder to run after other postprocess commands which add
       users and groups (content is deterministic but not fully sorted
       otherwise)

 meta/classes/rootfs-postcommands.bbclass | 22 ++++++++++++++++
 meta/lib/rootfspostcommands.py           | 44 ++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 meta/lib/rootfspostcommands.py

diff --git a/meta/classes/rootfs-postcommands.bbclass b/meta/classes/rootfs-postcommands.bbclass
index 8d48a2d..53a4fda 100644
--- a/meta/classes/rootfs-postcommands.bbclass
+++ b/meta/classes/rootfs-postcommands.bbclass
@@ -30,6 +30,23 @@ ROOTFS_POSTPROCESS_COMMAND += 'empty_var_volatile;'
 SSH_DISABLE_DNS_LOOKUP ?= " ssh_disable_dns_lookup ; "
 ROOTFS_POSTPROCESS_COMMAND_append_qemuall = "${SSH_DISABLE_DNS_LOOKUP}"
 
+# Sort the user and group entries in /etc by ID in order to make the content
+# deterministic. Package installs are not deterministic, causing the ordering
+# of entries to change between builds. In case that this isn't desired,
+# the command can be overridden.
+#
+# Note that useradd-staticids.bbclass has to be used to ensure that
+# the numeric IDs of dynamically created entries remain stable.
+#
+# We want this to run as late as possible, in particular after
+# systemd_sysusers_create and set_user_group. Using _append is not
+# enough for that, set_user_group is added that way and would end
+# up running after us.
+SORT_PASSWD_POSTPROCESS_COMMAND ??= " sort_passwd; "
+python () {
+    d.appendVar('ROOTFS_POSTPROCESS_COMMAND', '${SORT_PASSWD_POSTPROCESS_COMMAND}')
+}
+
 systemd_create_users () {
 	for conffile in ${IMAGE_ROOTFS}/usr/lib/sysusers.d/systemd.conf ${IMAGE_ROOTFS}/usr/lib/sysusers.d/systemd-remote.conf; do
 		[ -e $conffile ] || continue
@@ -146,6 +163,11 @@ ssh_disable_dns_lookup () {
 	fi
 }
 
+python sort_passwd () {
+    import rootfspostcommands
+    rootfspostcommands.sort_passwd(d.expand('${IMAGE_ROOTFS}${sysconfdir}'))
+}
+
 #
 # Enable postinst logging if debug-tweaks is enabled
 #
diff --git a/meta/lib/rootfspostcommands.py b/meta/lib/rootfspostcommands.py
new file mode 100644
index 0000000..6a9b8b4
--- /dev/null
+++ b/meta/lib/rootfspostcommands.py
@@ -0,0 +1,44 @@
+import os
+
+def sort_file(filename, mapping):
+    """
+    Sorts a passwd or group file based on the numeric ID in the third column.
+    If a mapping is given, the name from the first column is mapped via that
+    dictionary instead (necessary for /etc/shadow and /etc/gshadow). If not,
+    a new mapping is created on the fly and returned.
+    """
+    new_mapping = {}
+    with open(filename, 'rb+') as f:
+        lines = f.readlines()
+        # No explicit error checking for the sake of simplicity. /etc
+        # files are assumed to be well-formed, causing exceptions if
+        # not.
+        for line in lines:
+            entries = line.split(b':')
+            name = entries[0]
+            if mapping is None:
+                id = int(entries[2])
+            else:
+                id = mapping[name]
+            new_mapping[name] = id
+        # Sort by numeric id first, with entire line as secondary key
+        # (just in case that there is more than one entry for the same id).
+        lines.sort(key=lambda line: (new_mapping[line.split(b':')[0]], line))
+        # We overwrite the entire file, i.e. no truncate() necessary.
+        f.seek(0)
+        f.write(b''.join(lines))
+    return new_mapping
+
+def sort_passwd(sysconfdir):
+    """
+    Sorts passwd and group files in a rootfs /etc directory by ID.
+    """
+    for suffix in '', '-':
+        for main, shadow in (('passwd', 'shadow'),
+                             ('group', 'gshadow')):
+            filename = os.path.join(sysconfdir, main + suffix)
+            if os.path.exists(filename):
+                mapping = sort_file(filename, None)
+                filename = os.path.join(sysconfdir, shadow + suffix)
+                if os.path.exists(filename):
+                    sort_file(filename, mapping)
-- 
2.1.4