Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/providers/krb5/krb5_ccache.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
#include "util/util.h"


/* real id == user id; set id == service id */
/* `switch_to_()` functions expect that
* real id == user id; set id == service id.
* This is prepared (set) in `privileged_krb5_setup()`
* if process has corresponding capabilities.
*/
errno_t switch_to_user(void)
{
int ret;
Expand Down
78 changes: 53 additions & 25 deletions src/providers/krb5/krb5_child.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ struct cli_opts {
};

struct krb5_req {
bool krb5_child_has_setid_caps;

krb5_context ctx;
krb5_principal princ;
krb5_principal princ_orig;
Expand Down Expand Up @@ -1845,6 +1847,10 @@ static errno_t k5c_attach_ccname_msg(struct krb5_req *kr)
char *msg = NULL;
int ret;

if (!kr->krb5_child_has_setid_caps) {
return EOK;
}

if (kr->ccname == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "Error obtaining ccname.\n");
return ERR_INTERNAL;
Expand Down Expand Up @@ -2523,6 +2529,12 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
goto done;
}

if (!kr->krb5_child_has_setid_caps) {
/* no set-id capability => can't populate user ccache */
kerr = 0;
goto done;
}

/* Make sure ccache is created and written as the user */
kerr = switch_to_user();
if (kerr != EOK) {
Expand Down Expand Up @@ -4085,29 +4097,37 @@ static krb5_error_code privileged_krb5_setup(struct krb5_req *kr,
int ret;
char *mem_keytab;

/* Make use of cap_set*id first to bootstap process */
sss_set_cap_effective(CAP_SETGID, true);
if (geteuid() != 0) {
ret = setgroups(0, NULL);
/* Make use of cap_set*id (if available) first to bootstrap process */
kr->krb5_child_has_setid_caps =
((sss_set_cap_effective(CAP_SETGID, true) == EOK) &&
(sss_set_cap_effective(CAP_SETUID, true) == EOK));

if (kr->krb5_child_has_setid_caps) {
if (geteuid() != 0) {
ret = setgroups(0, NULL);
if (ret != 0) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to drop supplementary groups: %d\n", ret);
return ret;
}
} /* Otherwise keep supplementary groups to have access to DB_PATH to store FAST ccache */
ret = setresgid(kr->gid, -1, -1);
if (ret != 0) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to drop supplementary groups: %d\n", ret);
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to set real GID: %d\n", ret);
return ret;
}
} /* Otherwise keep supplementary groups to have access to DB_PATH to store FAST ccache */
ret = setresgid(kr->gid, -1, -1);
if (ret != 0) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to set real GID: %d\n", ret);
return ret;
}
sss_set_cap_effective(CAP_SETUID, true);
ret = setresuid(kr->uid, -1, -1);
if (ret != 0) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to set real UID: %d\n", ret);
return ret;
ret = setresuid(kr->uid, -1, -1);
if (ret != 0) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to set real UID: %d\n", ret);
return ret;
}
} else {
DEBUG(SSSDBG_CONF_SETTINGS, "'krb5_child' doesn't have CAP_SETUID and/or "
"CAP_SETGID. User ccache won't be updated.\n");
}

sss_drop_cap(CAP_SETUID);
sss_drop_cap(CAP_SETGID);

Expand Down Expand Up @@ -4337,7 +4357,7 @@ int main(int argc, const char *argv[])
* is only allowed for authenticated users. Since PKINIT is part of
* the authentication and the user is not authenticated yet, we have
* to use different privileges and can only drop it after the TGT is
* received. IDs the backend (and thus 'krb5_child) is running with are
* received. IDs the backend (and thus 'krb5_child') is running with are
* either root or the 'sssd' user. Root is allowed by default and
* the 'sssd' user is allowed with the help of the sssd-pcsc.rules
* policy-kit rule. So those IDs are a suitable choice and needs to
Expand All @@ -4346,11 +4366,13 @@ int main(int argc, const char *argv[])
* to make sure the empty ccache is created with the expected
* ownership. */
if (!IS_SC_AUTHTOK(kr->pd->authtok) || offline) {
ret = switch_to_user();
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to switch to user IDs: %d\n", ret);
ret = EFAULT;
goto done;
if (kr->krb5_child_has_setid_caps) {
ret = switch_to_user();
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to switch to user IDs: %d\n", ret);
ret = EFAULT;
goto done;
}
}
}

Expand All @@ -4370,7 +4392,9 @@ int main(int argc, const char *argv[])
case SSS_PAM_AUTHENTICATE:
/* If we are offline, we need to create an empty ccache file */
if (offline) {
ret = create_empty_ccache(kr);
if (kr->krb5_child_has_setid_caps) {
ret = create_empty_ccache(kr);
}
Comment on lines +4395 to +4397

Choose a reason for hiding this comment

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

critical

The ret variable is not assigned a value if offline is true and kr->krb5_child_has_setid_caps is false. This leads to k5c_send_data() being called with an uninitialized variable, which is undefined behavior.

When no capabilities are present, no ccache is created, but the authentication should still be considered successful in an offline scenario. You should initialize ret to EOK in this case.

            if (kr->krb5_child_has_setid_caps) {
                ret = create_empty_ccache(kr);
            } else {
                ret = EOK;
            }

Copy link
Member Author

@alexey-tikhonov alexey-tikhonov Dec 23, 2025

Choose a reason for hiding this comment

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

ret is known to be EOK at this point.

} else {
DEBUG(SSSDBG_TRACE_FUNC, "Will perform online auth\n");
ret = tgt_req_child(kr);
Expand All @@ -4391,6 +4415,10 @@ int main(int argc, const char *argv[])
ret = KRB5_KDC_UNREACH;
goto done;
}
if (!kr->krb5_child_has_setid_caps) {
ret = KRB5_CC_NOTFOUND;
goto done;
}
ret = renew_tgt_child(kr);
break;
case SSS_PAM_PREAUTH:
Expand Down