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

consistent uid allocation #36

Closed
cgwalters opened this issue Oct 21, 2014 · 8 comments
Closed

consistent uid allocation #36

cgwalters opened this issue Oct 21, 2014 · 8 comments

Comments

@cgwalters
Copy link
Member

Because ostree ships numeric uid/gid, we need some mechanism to ensure that packages get assigned the same uids across tree composes.

See https://bugzilla.gnome.org/show_bug.cgi?id=729118

A possible fix would be to force the mapping to be listed in the .json file.

@edsantiago
Copy link
Contributor

I can envision several ways of solving this, all of them requiring careful thought and attention to detail. And time.

In the meantime we have a window during which an (unlikely) uid/gid change can go by unnoticed. I believe it makes sense to try to catch those before they go out the door. How about adding a step near the end of a compose, running diff of /etc/passwd and /etc/group against a known-good (stashed) copy, and aborting the compose if there's any difference?

@cgwalters
Copy link
Member Author

+1 to the idea of a quick sanity check. Perhaps this should live in rpm-ostree-toolbox?

Note we actually do have a stashed copy - the one in the previous tree! We can extract it like so:

ostree --repo=repo checkout -U --subpath=/usr/lib fedora-atomic/rawhide/x86_64/docker-host^ usrlib-previous
ostree --repo=repo checkout -U --subpath=/usr/lib fedora-atomic/rawhide/x86_64/docker-host usrlib-new
parse/diff usrlib-previous/passwd usrlib-new/passwd, and same for group

@cgwalters
Copy link
Member Author

(ostree currently coredumps if you try --subpath=/usr/lib/passwd, that should be fixed)

@a13m
Copy link

a13m commented Oct 22, 2014

( I am reminded of when people started getting into similar issues in the Conary world, and in the end the solution was a UID registry: https://opensource.sas.com/conarywiki/index.php/rPath_Linux:RPath_UID_Registry ... not my favorite solution.)

I guess maybe the sanity check is the best we can do right now. I would rather see something where userlist gets checked against a previously-allocated set of users during the compose instead of just checking after, but you would have to hijack "useradd" and "groupadd" commands in post scripts. (Just using the previous passwd and group files as a starting point for a compose doesn't work, of course, because then you won't properly deal with package removals)

@cgwalters
Copy link
Member Author

In Fedora there are many actually static uid/gids as well: https://fedoraproject.org/wiki/Packaging:UsersAndGroups

But there's a reason we have dynamic allocation: there are more daemons in the world than would fit in the < 1000 range. And in the general case, the package set used to compose a tree could include dynamic set that varies over time.

@edsantiago
Copy link
Contributor

Uh-oh on the sanity check: I have one in progress in rpm-ostree-toolbox, but I now believe that this is too late in the process: the check should happen before corrupt data gets checked into the repo. Possibly just before the post-yum break in rpmostree_compose_builtin_tree(), and almost certainly done by an external helper script. Any objection to that approach?

EDIT: and, duh, with this approach the helper can be a fixup tool, not just a check-and=warn.

@cgwalters
Copy link
Member Author

While a lot of the code in compose tree would be about 20% the length in a scripting language, I'd like to keep the direction of moving to C libraries (hawkey/libsolv + librepo) for the core code, because eventually to do package layering requires deep integration between the ostree side (C, though introspection bindings available) and RPM (C, though bindings available).

Secondly there are things that are inefficient to do in most scripting languages (e.g. avoiding lots of string allocations when processing package data and blowing up memory usage), or tend to be broken (e.g. xattrs).

As for fixup, yes I think so. It's going to be ugly though until we get to the point where we have control over what's happening in the rpm %posts. I think https://fedoraproject.org/wiki/Changes/SystemdSysusers would potentially help with that.

Short term though, something like listing the name -> uid mappings in the treefile json, then before commit, parse the generated /usr/lib/{passwd,group}, diff it versus what we expect. Error out if there are any unallocated users. Then do a recursive walk, look up the filesystem uid in the generated map, resolve back to name, then chown if it doesn't match the preallocated one?

edsantiago added a commit that referenced this issue Nov 3, 2014
This function looks like a possible place in which to
implement issue #36[1], consistent UID/GID allocation.
With that in mind, this commit implements a small set
of unit tests to act as a starting point for said work.

The work won't be easy: the calling interface needs to
be changed, either by adding an ostree (against which
to compare) or a JSON pointer to config settings with
an enumeration of UIDs and GIDs. Or maybe there's a
better way. Regardless, with this test, such work
can proceed. Or even if not, at least we have tests
and a skeleton for building more.

Note that this uses the check[2] framework, and requires:

    # yum install check check-devel

There's probably a way to tell configure.ac to proceed
even if that's missing; I'll leave that to autoconf experts.

 [1] #36
 [2] http://check.sourceforge.net/doc/check_html/index.html#Top
edsantiago added a commit that referenced this issue Nov 3, 2014
This function looks like a possible place in which to
implement issue #36[1], consistent UID/GID allocation.
With that in mind, this commit implements a small set
of unit tests to act as a starting point for said work.

The work won't be easy: the calling interface needs to
be changed, either by adding an ostree (against which
to compare) or a JSON pointer to config settings with
an enumeration of UIDs and GIDs. Or maybe there's a
better way. Regardless, with this test, such work
can proceed. Or even if not, at least we have tests
and a skeleton for building more.

Note that this uses the check[2] framework, and requires:

    # yum install check check-devel

There's probably a way to tell configure.ac to proceed
even if that's missing; I'll leave that to autoconf experts.

 [1] #36
 [2] http://check.sourceforge.net/doc/check_html/index.html#Top
@cgwalters
Copy link
Member Author

Calling this fixed by #79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants