Skip to content

Conversation

@ikerexxe
Copy link
Contributor

@ikerexxe ikerexxe commented Jun 11, 2024

passkey_child owner was incorrectly set to $sssd_user:$sssd_user, when it should be root:$sssd_user. Correcting it.

Fixes: 30daa0c ("spec: update to include passkey")

@ikerexxe ikerexxe added backport-to-stable Waiting for review Trivial A single reviewer is sufficient to review the Pull Request labels Jun 11, 2024
@alexey-tikhonov
Copy link
Member

ACK, thanks.

@alexey-tikhonov
Copy link
Member

Actually... it's unclear if group should be '%{sssd_user}'

Currently the logic is following:

  • non-privileged binaries are root:root and '-rwxr-xr-x' - executable by anyone
  • privileged binaries (+ 'proxy_child') are root:sssd and '-rwxr-x---' - executable only by user and group

The idea is to restrict UIDs able to run privileged binaries.

Since 'passkey_child' isn't privileged, following this logic it should be 'root:root'
Another question, if it actually makes any sense to allow it to be executable by anyone... Maybe for debug purposes? As it happens with 'p11_child'.

Another note: if we stick to root:sssd, then Makefile.am :: install-exec-hook also should be updated:
https://github.com/SSSD/sssd/blob/master/Makefile.am#L5584
to have consistent results for rpm and 'make install'.

@ikerexxe
Copy link
Contributor Author

mmm I don't have a strong position in any sense. @sumit-bose WDYT?

@sumit-bose
Copy link
Contributor

Hi,

I think it would make sense to follow Alexey's "non-privileged binaries" reasoning and use root as group as well.

About "executable by anyone", since users should be able to register their keys with the sssctl or ipa command I guess this is needed.

HTH

bye,
Sumit

@alexey-tikhonov
Copy link
Member

About "executable by anyone", since users should be able to register their keys with the sssctl or ipa command I guess this is needed.

Ah, indeed, thank you.

@ikerexxe , then please make it root:root, as 'p11_child'.

passkey_child owner was incorrectly set to $sssd_user:$sssd_user, when
it should be root:root. Correcting it.

Fixes: 30daa0c ("spec: update to include passkey")

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
@alexey-tikhonov
Copy link
Member

Pushed PR: #7431

  • master
    • bb72b53 - spec: change passkey_child owner
  • sssd-2-9
    • ee8de7e - spec: change passkey_child owner

@alexey-tikhonov alexey-tikhonov removed the Ready to push Ready to push label Jun 18, 2024
@ikerexxe ikerexxe deleted the passkey_owner branch June 19, 2024 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Pushed Trivial A single reviewer is sufficient to review the Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants