krb5: check authtok in answer_pkinit()#8470
Conversation
Currently the loop over the different Kerberos pre-authentication types might fail if PKINIT is detected but there are no related credentials. With this patch the check is move inside of answer_pkinit() and the expected error code ERR_CHECK_NEXT_AUTH_TYPE is returned. This patch is for the sssd-2-9 branch since the issue is already fixed as part of 4cb99a2 in the master branch.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly refactors the pre-authentication logic for PKINIT by moving the authtok type check into the answer_pkinit function. This change ensures that if PKINIT is attempted without the required smart card credentials, SSSD can gracefully fall back to other authentication methods. The implementation is sound, though I've noted a minor code redundancy that results from this change, which could be cleaned up for better maintainability.
| type = sss_authtok_get_type(kr->pd->authtok); | ||
| if (type != SSS_AUTHTOK_TYPE_SC_PIN && type != SSS_AUTHTOK_TYPE_SC_KEYPAD) { | ||
| DEBUG(SSSDBG_MINOR_FAILURE, "Unexpected authentication token type [%s]\n", | ||
| sss_authtok_type_to_str(type)); | ||
| kerr = ERR_CHECK_NEXT_AUTH_TYPE; | ||
| goto done; | ||
| } |
There was a problem hiding this comment.
While this check is correctly moved to the beginning of the function, it introduces redundancy with a later check. The if condition at line 750 will now always evaluate to true, making the else block at line 789 unreachable. As a follow-up, you should consider removing the redundant if-else structure (lines 750-795) and placing the code from the if block directly under if (kr->pd->cmd == SSS_PAM_AUTHENTICATE).
alexey-tikhonov
added
no-backport
2 participants
Currently the loop over the different Kerberos pre-authentication types might fail if PKINIT is detected but there are no related credentials.
With this patch the check is move inside of answer_pkinit() and the expected error code ERR_CHECK_NEXT_AUTH_TYPE is returned.
This patch is for the sssd-2-9 branch since the issue is already fixed as part of 4cb99a2 in the master branch.