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

SPEC: enabled 'sysusers' for f-41+ #7267

Closed
wants to merge 3 commits into from

Conversation

alexey-tikhonov
Copy link
Member

No description provided.

@alexey-tikhonov alexey-tikhonov added no-backport This should go to target branch only. non-privileged labels Apr 2, 2024
@alexey-tikhonov
Copy link
Member Author

Hmm... @pbrezina, @sumit-bose,
as a follow up of #7193 (comment) discussion.

%sysusers_create_compat contrib/sssd.sysusers actually works somehow "as is", but not properly:

  Running scriptlet: sssd-common-9.pr7267-04361.fc41.x86_64                                                                         9/9 
  Installing       : sssd-common-9.pr7267-04361.fc41.x86_64                                                                         9/9 
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root
warning: user sssd does not exist - using root
warning: group sssd does not exist - using root

  Running scriptlet: sssd-common-9.pr7267-04361.fc41.x86_64                                                                         9/9 
Created symlink /etc/systemd/system/multi-user.target.wants/sssd.service → /usr/lib/systemd/system/sssd.service.
/usr/bin/chown: invalid user: ‘sssd:sssd’
/usr/bin/chown: invalid user: ‘sssd:sssd’
/usr/bin/chown: invalid user: ‘sssd:sssd’
/usr/bin/chown: invalid user: ‘sssd:sssd’
warning: %post(sssd-common-9.pr7267-04361.fc41.x86_64) scriptlet failed, exit status 1

Error in POSTIN scriptlet in rpm package sssd-common
Creating group 'sssd' with GID 976.
Creating user 'sssd' (User for sssd) with UID 976 and GID 976.

@alexey-tikhonov
Copy link
Member Author

Issue is only seen if user didn't exist before, so at a fresh install, not upgrade.

@alexey-tikhonov
Copy link
Member Author

alexey-tikhonov commented Apr 2, 2024

I don't find any 'pre' script in INFO/SCRIPTS of 'sssd-common' at all...

What I find is:

$ grep -rn user *
PROVIDES:21:user(sssd) = dSBzc3NkIC0gIlVzZXIgZm9yIHNzc2QiIC8gL3NiaW4vbm9sb2dpbgAA

$ grep -rn group *
PROVIDES:2:group(sssd) 

@alexey-tikhonov
Copy link
Member Author

alexey-tikhonov commented Apr 2, 2024

Well,

- %sysusers_create_compat contrib/sssd.sysusers
+ %sysusers_create_compat %{buildroot}%{_sysusersdir}/sssd.conf

doesn't help.

@pbrezina, do you have any ideas (besides reverting 736430a)?

@pbrezina
Copy link
Member

pbrezina commented Apr 2, 2024

I believe we need to install sssd.sysuser to the proper location on the system.

@alexey-tikhonov
Copy link
Member Author

I believe we need to install sssd.sysuser to the proper location on the system.

Isn't it this:

install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.conf
and this
%{_sysusersdir}/sssd.conf
?

Copy link
Member

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

You are right, I forgot we already did this. Then we also need to use that installed file with %sysusers_create_compat, you are still pointing to buildroot which does not exist.

@@ -1047,7 +1047,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con
%if %{use_sssd_user}
%pre common
%if %{use_sysusers}
%sysusers_create_compat contrib/sssd.sysusers
%sysusers_create_compat %{buildroot}%{_sysusersdir}/sssd.conf
Copy link
Member

Choose a reason for hiding this comment

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

s/%{buildroot}//

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi,

but if you remove %{buildroot} you will end up with /usr/lib/sysusers.d/sssd.conf, i.e. referencing a file from the filesystem of the build host.

The %sysuser_create_compat macro is executed at the time the scriptlets for the rpm packages are created and it calls /usr/lib/rpm/sysusers.generate-pre.sh with the sysusers config file as argument and the output

# generated from sssd.sysusers
getent group 'sssd' >/dev/null || groupadd -r 'sssd' || :
getent passwd 'sssd' >/dev/null || \
    useradd -r -g 'sssd' -d '/' -s '/sbin/nologin' -c 'User for sssd' 'sssd' || :

is written into the scriptlet.

Since we install the sysusers config file as well in /usr/lib/sysusers.d/ the %systemd_post macro will create users and groups if then not exists as well. But since we need the user and group so that the files installed by rpm will have proper ownership doing this in %post is too late. And we cannot let systemd do it at install time in %pre because the sysusers config file is not installed at this time.

To cut it short, I still see no other way than having the sysusers config file as additional source file.

bye,
Sumit

Copy link
Member

Choose a reason for hiding this comment

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

Hmm... you are right that %pre is executed before the file is actually installed.

But what difference does it make if we read it from out tarball or from an additional source file? The macro is evaluated before our tarball is extracted?

Copy link
Member Author

Choose a reason for hiding this comment

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

At this point I wonder if sysusers_create_compat is really better than "manual" user/group creation...

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... you are right that %pre is executed before the file is actually installed.

But what difference does it make if we read it from out tarball or from an additional source file? The macro is evaluated before our tarball is extracted?

Hi,

no, it is evaluated after the tar ball is extracted, after SSSD is build and after content of the BUILD and BUILDROOT directories is removed (I put an ls in the pre section to check this), so there is no extracted or built data available anymore.

HTH

bye,
Sumit

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like 736430a needs to be reverted fully, including sssd.sysusers creation by autoconf?

I don't understand why but otherwise build fails with:

error: Bad file: /builddir/build/SOURCES/sssd.sysusers: No such file or directory
    Bad file: /builddir/build/SOURCES/sssd.sysusers: No such file or directory

alexey-tikhonov added a commit to alexey-tikhonov/sssd that referenced this pull request Apr 3, 2024
This partially reverts 736430a

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

See SSSD#7267 (comment) for
details.
alexey-tikhonov added a commit to alexey-tikhonov/sssd that referenced this pull request Apr 3, 2024
This partially reverts 736430a

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

See SSSD#7267 (comment) for
details.
alexey-tikhonov added a commit to alexey-tikhonov/sssd that referenced this pull request Apr 4, 2024
This partially reverts 736430a

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

See SSSD#7267 (comment) for
details.
alexey-tikhonov added a commit to alexey-tikhonov/sssd that referenced this pull request Apr 4, 2024
This partially reverts 736430a

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

See SSSD#7267 (comment) for
details.
@alexey-tikhonov
Copy link
Member Author

alexey-tikhonov commented Apr 4, 2024

Recent versions seems to work fine both during package upgrade and fresh install (when user doesn't exist) besides weird

useradd: failed to reset the lastlog entry of UID 972: No such file or directory

@alexey-tikhonov
Copy link
Member Author

alexey-tikhonov commented Apr 5, 2024

For 'covscan' job, I've fixed Build source rpm - pr step to use build-sssd-srpm action from pr checkout folder, but it still doesn't provide 'sourcefile':

Run next-actions/build-srpm@master
  with:
    tarball: pr/sssd-pr7267.tar.gz
    specfile: pr/sssd.spec

@pbrezina , what do I miss?

alexey-tikhonov added a commit to alexey-tikhonov/sssd that referenced this pull request Apr 5, 2024
This partially reverts 736430a

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

See SSSD#7267 (comment) for
details.
@pbrezina
Copy link
Member

pbrezina commented Apr 5, 2024

For 'covscan' job, I've fixed Build source rpm - pr step to use build-sssd-srpm action from pr checkout folder, but it still doesn't provide 'sourcefile':

Run next-actions/build-srpm@master
  with:
    tarball: pr/sssd-pr7267.tar.gz
    specfile: pr/sssd.spec

@pbrezina , what do I miss?

covscan is pullrequest_target, the workflow (yml) file is read from the master branch, not from this pull request. So changes in this pull request are not and can not be reflected.

pullrequest_target has access to credentials where pull_request does not. The copr_build is pull_request_target as well but it includes build-srpm from checked out repository (which might be potential security problem, I'm not sure if the action has access to the credentials as well... maybe we should probably check).

@alexey-tikhonov
Copy link
Member Author

Ok, I removed 3rd commit.
So 'covscan' will be red until this PR is merged.

alexey-tikhonov added a commit to alexey-tikhonov/sssd that referenced this pull request Apr 5, 2024
This partially reverts 736430a

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

See SSSD#7267 (comment) for
details.
@alexey-tikhonov
Copy link
Member Author

Rebased.

@pbrezina
Copy link
Member

pbrezina commented Apr 5, 2024

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

So what is the available content/directories/paths when the macro is being evaluated? If I got it right, you have rpmbuild directories, but content under BUILD was deleted? Would it be possible to copy the file to SOURCES in %prep or %install? Or to some location that is not deleted?

  • rpmbuild/BUILD
  • rpmbuild/RPMS
  • rpmbuild/SOURCES
  • rpmbuild/SPECS
  • rpmbuild/SRPMS

I'm not against adding the source file, I am mostly curious at this point.

This partially reverts 736430a

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

See SSSD#7267 (comment) for
details.
Set '/run/sssd/' as 'sssd' user home dir.
This is required to accomodate for needs of some Samba libraries that
create cache while fetching GPO files.
Copy link
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

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

Hi,

thanks, the changes work for my. I tested AD GPO access control with the /run/sssd home directory and it is working fine. libsmbclient.so is creating /run/sssd/.cache/samba/gencache.tdb and the two new directories are created with 700 permissions, so even if libsmbclient.so would write something sensitive it would be only readable for the SSSD user (and root). ACK

bye,
Sumit

Copy link
Member

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

Ack. Can you please open pull request for Fedora rawhide as well?

@pbrezina
Copy link
Member

Pushed PR: #7267

  • master
    • 5b9a2f8 - SPEC: define a home dir for 'sssd' user
    • 5045e43 - SPEC: enabled 'sysusers' for f-41+
    • 01d09bb - SPEC: use sysusers as additional source

pbrezina pushed a commit that referenced this pull request Apr 11, 2024
This partially reverts 736430a

The reason is that 'sysusers_create_compat' macro is evaluated after
the tar ball is extracted, after SSSD is built and after content of
the BUILD and BUILDROOT directories is removed, so otherwise there is
no extracted or built data available anymore.

See #7267 (comment) for
details.

Reviewed-by: Pavel Březina <[email protected]>
Reviewed-by: Sumit Bose <[email protected]>
@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Apr 11, 2024
@pbrezina pbrezina closed this Apr 11, 2024
@alexey-tikhonov
Copy link
Member Author

Ack. Can you please open pull request for Fedora rawhide as well?

It's sssd-2.9 based, so only once we release sssd-2.10 and rebase rawhide...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport This should go to target branch only. non-privileged Pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants