| Summary: | acct-user.eclass: support systemd-sysusers | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | David Michael <fedora.dm0> |
| Component: | Eclasses | Assignee: | Michał Górny <mgorny> |
| Status: | RESOLVED FIXED | ||
| Severity: | enhancement | CC: | bertrand, leho, sam, systemd |
| Priority: | Normal | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| See Also: | https://bugs.gentoo.org/show_bug.cgi?id=549360 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | 731946 | ||
| Bug Blocks: | |||
| Attachments: |
Support using systemd-sysusers in GLEP 81 packages
0001-acct-.eclass-Create-sysusers.d-files.patch Patch with group support Patch with group support, r1 |
||
|
Description
David Michael
2019-12-12 21:28:55 UTC
Created attachment 599312 [details, diff]
Support using systemd-sysusers in GLEP 81 packages
Are the users/groups created by sysusers any different than those created by user.eclass? I don't mind installing the extra files but I don't think there's a point in having two ways to create them if they're equivalent. I think they would be equivalent except for sysusers always choosing /sbin/nologin instead of the fallback list in user.eclass when a shell isn't specified. In that case, I'm going to limit the change to installing sysusers for now. I'd like to use heredoc syntax for this, so I'll just make a patch and send it to gentoo-dev ml for review. I'll link it here afterwards. Created attachment 599352 [details, diff]
0001-acct-.eclass-Create-sysusers.d-files.patch
Here's my approach. It uses 'printf %q' to be safe with quoting.
However, I've noticed a major hiccup: sysutils.d's 'u' entry creates both user and a matching group. This doesn't match our acct-* semantics, and apparently matching 'g' entry doesn't even correct group id there.
The %q behavior seems to use escaped spaces rather than quotes, but the sysusers.d documentation says the GECOS field is a quoted string. I haven't referenced the source yet to see how it's being parsed, but is that printf method working as expected? Sorry, I hadn't considered the behavior of the implicit primary group since all the acct-* packages I used are defined that way anyway. I can open an upstream systemd bug to suggest optionally specifying the primary GID rather than creating it if that gives enough flexibility. It seems to use the correct GID for me when the group is defined first. I was abusing the fact that acct-group-*.conf files get sorted before acct-user-*.conf files so this isn't a problem when systemd-sysusers is run on the target system. Was your matching "g" line defined before the "u" line, or are you seeing a different issue? (In reply to David Michael from comment #6) > The %q behavior seems to use escaped spaces rather than quotes, but the > sysusers.d documentation says the GECOS field is a quoted string. I haven't > referenced the source yet to see how it's being parsed, but is that printf > method working as expected? Yes. I've removed the user and systemd-sysusers recreated it correctly. > Sorry, I hadn't considered the behavior of the implicit primary group since > all the acct-* packages I used are defined that way anyway. I can open an > upstream systemd bug to suggest optionally specifying the primary GID rather > than creating it if that gives enough flexibility. > > It seems to use the correct GID for me when the group is defined first. I > was abusing the fact that acct-group-*.conf files get sorted before > acct-user-*.conf files so this isn't a problem when systemd-sysusers is run > on the target system. Was your matching "g" line defined before the "u" > line, or are you seeing a different issue? Yes, that is actually fine. What I'm concerned about is the potential case when primary group name != user name. Unless I'm mistaken, systemd-sysusers will create a superfluous group in that case. See acct-user/cmd5checkpw for an example. # systemd-sysusers Creating group cmd5checkpw with gid 905. I've opened an issue in the upstream systemd tracker to see if the implicit primary group behavior can be changed in a future version. https://github.com/systemd/systemd/issues/14340 If such a change is not accepted and this is incompatible with the acct-* packages in general, then we can close this and I'll generate the sysusers files in a post-build step. Thank you. I'd like to see sysusers.d in Gentoo as well; in fact, it was one of the side ideas in refurbishing the account system that were somewhere in my TODO. The next systemd release (v245) will support specifying a primary group: https://github.com/systemd/systemd/commit/649916d3561ac41b41bfc5be6297a78847509abe The required syntax will have to split the primary group from the rest of the groups array, so this change on top of your patch will work: --- eclass/acct-user.eclass +++ eclass/acct-user.eclass @@ -326,12 +326,13 @@ newins - ${CATEGORY}-${ACCT_USER_NAME}.conf < <( printf "u\t%q\t%q\t%q\t%q\t%q\n" \ "${ACCT_USER_NAME}" \ - "${ACCT_USER_ID/#-*/-}" \ + "${ACCT_USER_ID/#-*/-}:${ACCT_USER_GROUPS[0]}" \ "${DESCRIPTION//[:,=]/;}" \ "${ACCT_USER_HOME}" \ "${ACCT_USER_SHELL/#-*/-}" + test ${#ACCT_USER_GROUPS[@]} -lt 2 || printf "m\t${ACCT_USER_NAME}\t%q\n" \ - "${ACCT_USER_GROUPS[@]}" + "${ACCT_USER_GROUPS[@]:1}" ) } The commit applies to the current stable systemd (v244), so it could be backported if it is desirable to implement this sooner than v245 stabilization. Just noting that systemd v245 has been released, so upstream support for this is now in GA. Since systemd 243 was dropped today, 244 and 245 are now the only versions in Gentoo. Upstream support for this feature is included in 245, and the patch can be applied to 244. (I implemented it on 244 in the first place, so I know it applies and works.) Could the commit be included in the "! use vanilla" patches so the eclass changes can be applied, or does this need to wait until 244 is dropped as well? (In reply to David Michael from comment #12) I think we really just need to wait for 245 to be marked stable. I will start that ball rolling in the next couple of weeks. (In reply to Mike Gilbert from comment #13) > (In reply to David Michael from comment #12) > > I think we really just need to wait for 245 to be marked stable. I will > start that ball rolling in the next couple of weeks. Exactly, that's what I was waiting for as well. Created attachment 657352 [details, diff]
Patch with group support
Could you test this patch, please?
I don't think the last printf builtin will work. It will distribute the arguments across the two "%q"s instead of just the last one, e.g.: $ printf 'm\t%q\t%q\n' name 1 2 3 4 m name 1 m 2 3 m 4 '' If it is too unsafe to put $ACCT_USER_NAME directly in the format string, maybe split it into two printf statements. Ignore my "two printf" comment; I don't see what I meant myself now. (I'm not fully awake yet.) An echo loop should work, though, if printf won't cooperate.
local group
for group in "${ACCT_USER_GROUPS[@]:1}"
do echo -e "m\t${ACCT_USER_NAME}\t$group"
done
Created attachment 657394 [details, diff]
Patch with group support, r1
I obviously wasn't thinking when I read that code. I suppose non-escaped name is fine. Does this patch work better?
Maybe I should write some tests for these eclasses.
That patch looks good. I have an automated system building with it that will be running for a little while yet before it's complete, but the patch is now essentially the same as what I've been applying locally for the past 6+ months, so I'm happy with it. Ok, I'll send it to the ml for review. Please let me know if you hit any issues. Also, please ping me if I forget to push it in ~1 week from now. Looks like this is fixed by https://github.com/gentoo/gentoo/commit/0d096cb68e86dd5faa7da8cf68a18e4e29bc9081 . So, as of this writing, there is no way in gentoo to run sysusers? I would also like to be able to run this without rebooting. Debian seems to have a command to do so as prt of its systemd implementation. my comment may have been incorrect, found another bug report, sorry for the noise. |
