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

Create system users for FreeIPA services during package installation #697

Closed
wants to merge 1 commit into from
Closed

Create system users for FreeIPA services during package installation #697

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 6, 2017

Previously system users needed by FreeIPA server services was created during
ipa-server-install. This led to problem when DBus policy was configured during
package installation but the user specified in the policy didn't exist yet (and
potentionally similar ones). Now systemd-sysusers service is used to ensure
users freeipa-server package needs exist before any installation or
configuration begins.

https://pagure.io/freeipa/issue/6743

@tjaalton
Copy link
Contributor

tjaalton commented Apr 6, 2017

if I understood the sysusers.d file format correctly, ipa.sysusers.debian.conf would need this line added:

m www-data ipaapi

as you can see from ipaplatform/debian/constants.py. Actually, why not make just one template file ipa.sysusers.conf.in and utilize ipaplatform to substitute values like for most of the conffiles

@@ -0,0 +1 @@
ipa.sysusers.base.conf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The redhat platform is an abstract platform that serves as a base platform for rhel and fedora. I don't think we need a sysuser file for it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created the symbolic links with the same login as platforms in ipaplatform use for inheritance but you're right there's no real platform redhat and we don't need sysusers.conf file for it.

u dirsrv - - -
u pkiuser - - -
u ipaapi - - -
m apache ipaapi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @tjaalton said in #697 (comment)

@@ -0,0 +1,5 @@
u kdcproxy - "IPA KDC Proxy User" -
u dirsrv - - -
u pkiuser - - -
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkiuser has pre-allocated uid 17 and gid 17. Does sysuser.d automatically use pre-allocated uid and gid? Also GECOS is not set to "CS System User".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user is created by pki-server, so I would rather just remove the line.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my answer to previous comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, what Honza said.

@@ -0,0 +1,5 @@
u kdcproxy - "IPA KDC Proxy User" -
u dirsrv - - -
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GECOS "DS System User"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user is created by 389-ds-base, so I would rather just remove the line.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tiran You're right, I omitted the GECOS because create_ds_user didn't set it either.
@HonzaCholasta Doesn't matter if the user is or isn't created by some other package. That's the benefit of sysusers.d, we declare that we need such user and sysusers.d creates it doesn't exist already.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing we achieve by declaring the user ourselves is supporting broken 389-ds-base installs. I don't think we should do that and rather fail early if the user is missing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkupka create_system_user() adds GECOS for DS and PKI user.

https://github.com/freeipa/freeipa/pull/697/files#diff-39163021c252126be7c9fbc0c66b1668L452

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for Honza's suggestion. Let 389-ds-base take care of group and user creation.

u kdcproxy - "IPA KDC Proxy User" -
u dirsrv - - -
u pkiuser - - -
u ipaapi - - -
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness, please give the ipaapi user a sensible GECOS, too.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tiran Makes sense.

@HonzaCholasta
Copy link
Contributor

Note that systemd-sysusers is not available in RHEL and CentOS. It might be better to use the sssd approach: https://github.com/SSSD/sssd/blob/master/contrib/sssd.spec.in#L1228.

@tiran
Copy link
Member

tiran commented Apr 7, 2017

Originally I used a similar approach for the kdcproxy user based on the snippet https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Soft_static_allocation . You changed it in ticket https://pagure.io/freeipa/issue/5314 because the approach violates packaging guidelines.

@HonzaCholasta
Copy link
Contributor

Ah, right, rpmdiff complained about that. Well, that was 2 years ago, and if it works for sssd it must also work for us, so I guess we should ignore rpmdiff.

@adelton
Copy link
Contributor

adelton commented Apr 7, 2017

Previously system users needed by FreeIPA server services was created during
ipa-server-install.

Actually, for any such case I found I filed bugzilla or ticket to get them created during rpm installation.

This led to problem when DBus policy was configured during
package installation but the user specified in the policy didn't exist yet (and
potentionally similar ones). Now systemd-sysusers service is used to ensure
users freeipa-server package needs exist before any installation or
configuration begins.

Please do not use systemd-sysusers, create the group/user entries during rpm installation.

@adelton
Copy link
Contributor

adelton commented Apr 7, 2017

I don't think we should do that and rather fail early if the user is missing.

+1

@martbab
Copy link
Contributor

martbab commented Apr 7, 2017

Right, we do not have systemd available during Docker image build so some fallback mechanism directly in spec would be great. Otherwise we would have to workaround this in containers and I am not a big fan of that.

tiran
tiran previously requested changes Apr 10, 2017
@@ -0,0 +1,5 @@
u kdcproxy - "IPA KDC Proxy User" -
u dirsrv - - -
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for Honza's suggestion. Let 389-ds-base take care of group and user creation.

@@ -0,0 +1,5 @@
u kdcproxy - "IPA KDC Proxy User" -
u dirsrv - - -
u pkiuser - - -
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, what Honza said.

@stlaz stlaz self-assigned this Apr 10, 2017
@stlaz
Copy link
Contributor

stlaz commented Apr 11, 2017

Travis reports wrong usage of the useradd command.

Previously system users needed by FreeIPA server services was created during
ipa-server-install. This led to problem when DBus policy was configured during
package installation but the user specified in the policy didn't exist yet
(and potentionally similar ones). Now the users will be created in package %pre
section so all users freeipa-server package needs exist before any installation
or configuration begins.
Another possibility would be using systemd-sysusers(8) for this purpose but
given that systemd is not available during container build the traditional
approach is superior.
Also dirsrv and pkiuser users are no longer created by FreeIPA instead it
depends on 389ds and dogtag to create those users.

https://pagure.io/freeipa/issue/6743
freeipa.spec.in Outdated
# create users and groups
# create kdcproxy group and user
getent group kdcproxy >/dev/null || groupadd -f -r kdcproxy
useradd -r -g kdcproxy -s /sbin/nologin -d / -c "IPA KDC Proxy User" kdcproxy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, only add the user if it does not exists.

@stlaz stlaz dismissed tiran’s stale review April 11, 2017 14:59

The comments were implemented/replaced with a different solution.

# create users and groups
# create kdcproxy group and user
getent group kdcproxy >/dev/null || groupadd -f -r kdcproxy
getent passwd kdcproxy >/dev/null || useradd -r -g kdcproxy -s /sbin/nologin -d / -c "IPA KDC Proxy User" kdcproxy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please specify uid and gid. Otherwise, when used in containers, one image build can end up with different uid/gid than the other, store data in volume with that uid/gid, and the next image can use different values and not be able to read the data.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not now. When (and more importantly if) we get uid and gid assigned I will update the code. Now I don't have any idea what value I could use.
Also looking into related ticket https://pagure.io/packaging-committee/issue/474 (opened by you by the way) it seems that "we run in container and store data to volume" is not enough to allocate static uid and gid.

getent passwd kdcproxy >/dev/null || useradd -r -g kdcproxy -s /sbin/nologin -d / -c "IPA KDC Proxy User" kdcproxy
# create ipaapi group and user
getent group ipaapi >/dev/null || groupadd -f -r ipaapi
getent passwd ipaapi >/dev/null || useradd -r -g ipaapi -s /sbin/nologin -d / -c "IPA Framework User" ipaapi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dtto.

@stlaz
Copy link
Contributor

stlaz commented Apr 11, 2017

While I don't like to omit @adelton comments, this is a test blocker for us. I propose going with @dkupka's comment on adding the GID/UID later when we get it or if someone could make a PR making this a bit better, that would be nice too.
In the meantime, I have to ACK this.

@stlaz stlaz added the ack Pull Request approved, can be merged label Apr 11, 2017
@pvomacka
Copy link

ipa-4-5:

  • e8a429d Create system users for FreeIPA services during package installation
    master:

  • a726e98 Create system users for FreeIPA services during package installation

@pvomacka pvomacka added pushed Pull Request has already been pushed ack Pull Request approved, can be merged and removed ack Pull Request approved, can be merged labels Apr 11, 2017
@pvomacka pvomacka closed this Apr 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
7 participants