Skip to content

krb5: restart krb5_child for Smartcard authentication#8629

Merged
alexey-tikhonov merged 1 commit intoSSSD:masterfrom
sumit-bose:krb5_child_sc_restart
Apr 24, 2026
Merged

krb5: restart krb5_child for Smartcard authentication#8629
alexey-tikhonov merged 1 commit intoSSSD:masterfrom
sumit-bose:krb5_child_sc_restart

Conversation

@sumit-bose
Copy link
Copy Markdown
Contributor

In contrast to other authentication methods for PKINIT some information about the used Smartcard and certificate are already needed for the pre-authentication step to trigger the MIT Kerberos PKINIT module to get back the information if PKINIT is possible or not and if the Smartcard can be used for authentication. If krb5_child is kept running between the pre-authentication and the authentication step the information given during pre-authentication is used if Smartcard authentication was selected.

As long as only a single certificate is available there is no issue. But if there are multiple certificates which all apply to the given mapping and matching rules for the user trying to log in and the user can choose a certificate for authentication the authentication might fail if the certificate use during pre-authentication and the one selected by the user differ. Before the change to keep krb5_child running for all authentication methods this was not an issue since the fresh instance started during the authentication step was using the certificate selected by the user.

With this patch krb5_child is restart during the authentication step is Smartcard authentication was selected.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a change to restart the krb5_child process during Smartcard authentication if a different certificate is selected by the user. The review feedback points out style inconsistencies in the new conditional block, specifically regarding indentation and the placement of logical operators, to maintain consistency with the rest of the codebase.

Comment thread src/providers/krb5/krb5_auth.c
@alexey-tikhonov
Copy link
Copy Markdown
Member

#8267 will need to cherry-pick this?

@sumit-bose
Copy link
Copy Markdown
Contributor Author

#8267 will need to cherry-pick this?

Hi,

yes, this is triggered by "krb5_child: advertise authentication methods".

bye,
Sumit

@spoore1
Copy link
Copy Markdown
Contributor

spoore1 commented Apr 22, 2026

I tested on Fedora 42 containers and I can confirm this fixes the test_smartcard__two_tokens_match_on_both failure in PR#8519:

Version:

[root@client ~]# rpm -q sssd
sssd-2.13.0-99.20260422110653129739.pr8629.168.g4868b17eb.fc42.x86_64

Results:

tests/test_smartcard.py::test_smartcard__two_tokens_match_on_first (ipa) PASSED
tests/test_smartcard.py::test_smartcard__two_tokens_match_on_second (ipa) PASSED
tests/test_smartcard.py::test_smartcard__two_tokens_match_on_both (ipa) PASSED
==== 3 passed, 1 deselected in 200.01s (0:03:20) ====

Copy link
Copy Markdown
Contributor

@justin-stephenson justin-stephenson left a comment

Choose a reason for hiding this comment

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

Ack, thank you.

Copy link
Copy Markdown
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

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

LGTM!

@alexey-tikhonov alexey-tikhonov added no-backport This should go to target branch only. coverity Trigger a coverity scan and removed coverity Trigger a coverity scan labels Apr 24, 2026
@alexey-tikhonov
Copy link
Copy Markdown
Member

Note: Covscan is clean.

In contrast to other authentication methods for PKINIT some information
about the used Smartcard and certificate are already needed for the
pre-authentication step to trigger the MIT Kerberos PKINIT module to get
back the information if PKINIT is possible or not and if the Smartcard
can be used for authentication. If krb5_child is kept running between
the pre-authentication and the authentication step the information given
during pre-authentication is used if Smartcard authentication was
selected.

As long as only a single certificate is available there is no issue. But
if there are multiple certificates which all apply to the given mapping
and matching rules for the user trying to log in and the user can choose
a certificate for authentication the authentication might fail if the
certificate use during pre-authentication and the one selected by the
user differ. Before the change to keep krb5_child running for all
authentication methods this was not an issue since the fresh instance
started during the authentication step was using the certificate
selected by the user.

With this patch krb5_child is restart during the authentication step is
Smartcard authentication was selected.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
@sssd-bot
Copy link
Copy Markdown
Contributor

The pull request was accepted by @alexey-tikhonov with the following PR CI status:


🟢 CodeQL (success)
🟢 osh-diff-scan:fedora-rawhide-x86_64:upstream (success)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-44-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🟢 Build / freebsd (success)
🟢 Build / make-distcheck (success)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / intgcheck (fedora-45) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟢 ci / system (fedora-44) (success)
🟢 ci / system (fedora-45) (success)
➖ Coverity scan / coverity (skipped)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@sssd-bot sssd-bot force-pushed the krb5_child_sc_restart branch from 59f041b to 867c295 Compare April 24, 2026 10:18
@alexey-tikhonov alexey-tikhonov merged commit f3a36be into SSSD:master Apr 24, 2026
10 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted no-backport This should go to target branch only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants