URL: https://github.com/SSSD/sssd/pull/5762
Author: pbrezina
Title: #5762: krb5: add support for oauth2 challenge (wip)
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5762/head:pr5762
git checkout pr5762
From 3b78d96d82a48e6746d848b199f4320c3204eec6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 20 Jul 2021 13:51:19 +0200
Subject: [PATCH 01/11] authtok: add SSS_AUTHTOK_TYPE_OAUTH2
Add a new authentication token type: SSS_AUTHTOK_TYPE_OAUTH2.
It will be used later to enable OAuth2 authentication via Kerberos.
---
src/sss_client/sss_cli.h | 5 ++++
src/tests/cmocka/test_authtok.c | 44 +++++++++++++++++++++++++++++++
src/util/authtok.c | 46 +++++++++++++++++++++++++++++++++
src/util/authtok.h | 31 ++++++++++++++++++++++
4 files changed, 126 insertions(+)
diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
index 1347ce71d7..4cce858484 100644
--- a/src/sss_client/sss_cli.h
+++ b/src/sss_client/sss_cli.h
@@ -351,6 +351,11 @@ enum sss_authtok_type {
SSS_AUTHTOK_TYPE_2FA_SINGLE = 0x0006, /**< Authentication token has two
* factors in a single string, it may
* or may no contain a trailing \\0 */
+ SSS_AUTHTOK_TYPE_OAUTH2 = 0x0007, /**< Authentication token is a
+ * oauth2 token for presented
+ * challenge that is acquired from
+ * Kerberos. It may or may no
+ * contain a trailing \\0 */
};
/**
diff --git a/src/tests/cmocka/test_authtok.c b/src/tests/cmocka/test_authtok.c
index a31014eb6b..711dc489bb 100644
--- a/src/tests/cmocka/test_authtok.c
+++ b/src/tests/cmocka/test_authtok.c
@@ -709,6 +709,48 @@ static void test_sss_authtok_2fa_single(void **state)
sss_authtok_set_empty(ts->authtoken);
}
+/* Test when type has value SSS_AUTHTOK_TYPE_OAUTH2 */
+static void test_sss_authtok_oauth2(void **state)
+{
+ size_t len;
+ errno_t ret;
+ char *data;
+ size_t ret_len;
+ const char *pwd;
+ struct test_state *ts;
+ enum sss_authtok_type type;
+
+ ts = talloc_get_type_abort(*state, struct test_state);
+ data = talloc_strdup(ts, "one-time-password");
+ assert_non_null(data);
+
+ len = strlen(data) + 1;
+ type = SSS_AUTHTOK_TYPE_OAUTH2;
+ ret = sss_authtok_set(ts->authtoken, type, (const uint8_t *)data, len);
+
+ assert_int_equal(ret, EOK);
+ assert_int_equal(type, sss_authtok_get_type(ts->authtoken));
+ assert_int_equal(len, sss_authtok_get_size(ts->authtoken));
+ assert_string_equal(data, sss_authtok_get_data(ts->authtoken));
+
+ ret = sss_authtok_get_oauth2(ts->authtoken, &pwd, &ret_len);
+
+ assert_int_equal(ret, EOK);
+ assert_string_equal(data, pwd);
+ assert_int_equal(len - 1, ret_len);
+
+ ret = sss_authtok_set_oauth2(ts->authtoken, data, len);
+ assert_int_equal(ret, EOK);
+
+ ret = sss_authtok_get_oauth2(ts->authtoken, &pwd, &ret_len);
+ assert_int_equal(ret, EOK);
+ assert_string_equal(data, pwd);
+ assert_int_equal(len - 1, ret_len);
+
+ talloc_free(data);
+ sss_authtok_set_empty(ts->authtoken);
+}
+
int main(int argc, const char *argv[])
{
@@ -747,6 +789,8 @@ int main(int argc, const char *argv[])
setup, teardown),
cmocka_unit_test_setup_teardown(test_sss_authtok_2fa_single,
setup, teardown),
+ cmocka_unit_test_setup_teardown(test_sss_authtok_oauth2,
+ setup, teardown),
};
/* Set debug level to invalid value so we can decide if -d 0 was used. */
diff --git a/src/util/authtok.c b/src/util/authtok.c
index 7254ed1da1..4a8fc5ea32 100644
--- a/src/util/authtok.c
+++ b/src/util/authtok.c
@@ -42,6 +42,8 @@ const char *sss_authtok_type_to_str(enum sss_authtok_type type)
return "Smart Card PIN will be entered at the card reader";
case SSS_AUTHTOK_TYPE_2FA_SINGLE:
return "Two factors in a single string";
+ case SSS_AUTHTOK_TYPE_OAUTH2:
+ return "OAuth2";
}
DEBUG(SSSDBG_MINOR_FAILURE, "Unknown authtok type %d\n", type);
@@ -65,6 +67,7 @@ size_t sss_authtok_get_size(struct sss_auth_token *tok)
case SSS_AUTHTOK_TYPE_SC_PIN:
case SSS_AUTHTOK_TYPE_SC_KEYPAD:
case SSS_AUTHTOK_TYPE_2FA_SINGLE:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
return tok->length;
case SSS_AUTHTOK_TYPE_EMPTY:
return 0;
@@ -101,6 +104,7 @@ errno_t sss_authtok_get_password(struct sss_auth_token *tok,
case SSS_AUTHTOK_TYPE_SC_PIN:
case SSS_AUTHTOK_TYPE_SC_KEYPAD:
case SSS_AUTHTOK_TYPE_2FA_SINGLE:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
return EACCES;
}
@@ -127,6 +131,7 @@ errno_t sss_authtok_get_ccfile(struct sss_auth_token *tok,
case SSS_AUTHTOK_TYPE_SC_PIN:
case SSS_AUTHTOK_TYPE_SC_KEYPAD:
case SSS_AUTHTOK_TYPE_2FA_SINGLE:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
return EACCES;
}
@@ -153,6 +158,34 @@ errno_t sss_authtok_get_2fa_single(struct sss_auth_token *tok,
case SSS_AUTHTOK_TYPE_SC_PIN:
case SSS_AUTHTOK_TYPE_SC_KEYPAD:
case SSS_AUTHTOK_TYPE_CCFILE:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
+ return EACCES;
+ }
+
+ return EINVAL;
+}
+
+errno_t sss_authtok_get_oauth2(struct sss_auth_token *tok,
+ const char **str, size_t *len)
+{
+ if (!tok) {
+ return EINVAL;
+ }
+ switch (tok->type) {
+ case SSS_AUTHTOK_TYPE_EMPTY:
+ return ENOENT;
+ case SSS_AUTHTOK_TYPE_OAUTH2:
+ *str = (const char *)tok->data;
+ if (len) {
+ *len = tok->length - 1;
+ }
+ return EOK;
+ case SSS_AUTHTOK_TYPE_PASSWORD:
+ case SSS_AUTHTOK_TYPE_2FA:
+ case SSS_AUTHTOK_TYPE_SC_PIN:
+ case SSS_AUTHTOK_TYPE_SC_KEYPAD:
+ case SSS_AUTHTOK_TYPE_CCFILE:
+ case SSS_AUTHTOK_TYPE_2FA_SINGLE:
return EACCES;
}
@@ -204,6 +237,7 @@ void sss_authtok_set_empty(struct sss_auth_token *tok)
case SSS_AUTHTOK_TYPE_2FA:
case SSS_AUTHTOK_TYPE_SC_PIN:
case SSS_AUTHTOK_TYPE_2FA_SINGLE:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
sss_erase_mem_securely(tok->data, tok->length);
break;
case SSS_AUTHTOK_TYPE_CCFILE:
@@ -243,6 +277,15 @@ errno_t sss_authtok_set_2fa_single(struct sss_auth_token *tok,
"2fa_single", str, len);
}
+errno_t sss_authtok_set_oauth2(struct sss_auth_token *tok,
+ const char *str, size_t len)
+{
+ sss_authtok_set_empty(tok);
+
+ return sss_authtok_set_string(tok, SSS_AUTHTOK_TYPE_OAUTH2,
+ "oauth2", str, len);
+}
+
static errno_t sss_authtok_set_2fa_from_blob(struct sss_auth_token *tok,
const uint8_t *data, size_t len);
@@ -263,6 +306,8 @@ errno_t sss_authtok_set(struct sss_auth_token *tok,
return sss_authtok_set_sc_from_blob(tok, data, len);
case SSS_AUTHTOK_TYPE_2FA_SINGLE:
return sss_authtok_set_2fa_single(tok, (const char *) data, len);
+ case SSS_AUTHTOK_TYPE_OAUTH2:
+ return sss_authtok_set_oauth2(tok, (const char *) data, len);
case SSS_AUTHTOK_TYPE_EMPTY:
sss_authtok_set_empty(tok);
return EOK;
@@ -645,6 +690,7 @@ errno_t sss_authtok_get_sc_pin(struct sss_auth_token *tok, const char **_pin,
case SSS_AUTHTOK_TYPE_2FA:
case SSS_AUTHTOK_TYPE_SC_KEYPAD:
case SSS_AUTHTOK_TYPE_2FA_SINGLE:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
return EACCES;
}
diff --git a/src/util/authtok.h b/src/util/authtok.h
index 6fd3e9ef03..6b96c7af74 100644
--- a/src/util/authtok.h
+++ b/src/util/authtok.h
@@ -402,4 +402,35 @@ errno_t sss_authtok_get_2fa_single(struct sss_auth_token *tok,
errno_t sss_authtok_set_2fa_single(struct sss_auth_token *tok,
const char *str, size_t len);
+/**
+ * @brief Returns a const string if the auth token is of type
+ SSS_AUTHTOK_TYPE_OAUTH2, otherwise it returns an error
+ *
+ * @param tok A pointer to an sss_auth_token
+ * @param pwd A pointer to a const char *, that will point to a null
+ * terminated string
+ * @param len The length of the credential string
+ *
+ * @return EOK on success
+ * ENOENT if the token is empty
+ * EACCESS if the token is not a password token
+ */
+errno_t sss_authtok_get_oauth2(struct sss_auth_token *tok,
+ const char **str, size_t *len);
+
+/**
+ * @brief Set one-time password into an auth token, replacing any previous data.
+ *
+ * @param tok A pointer to an sss_auth_token structure to change, also
+ * used as a memory context to allocate the internal data.
+ * @param password A string that holds the one-time password.
+ * @param len The length of the string or, if 0 is passed,
+ * then strlen(password) will be used internally.
+ *
+ * @return EOK on success
+ * ENOMEM on error
+ */
+errno_t sss_authtok_set_oauth2(struct sss_auth_token *tok,
+ const char *str, size_t len);
+
#endif /* __AUTHTOK_H__ */
From 5d621cbc2fa4e904f8fded6f78a336264dc463c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Mon, 16 Aug 2021 13:24:39 +0200
Subject: [PATCH 02/11] pam: add new SSS_CHILD_KEEP_ALIVE pam item
This pam item indicates that the child process performing authenticate
is kept alive and should be used to further continue with the
authentication instead of creating a new child process.
This patch only adds the pam item and forwards it back and forth pam_sss
and the backend. It will be used in following commits.
---
src/responder/pam/pamsrv_cmd.c | 5 +++++
src/sss_client/pam_message.c | 4 ++++
src/sss_client/pam_message.h | 1 +
src/sss_client/pam_sss.c | 4 ++++
src/sss_client/sss_cli.h | 5 +++++
src/sss_iface/sss_iface_types.c | 10 ++++++++++
src/util/sss_pam_data.c | 1 +
src/util/sss_pam_data.h | 1 +
8 files changed, 31 insertions(+)
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index 20c332b1a4..cdc3ae83a2 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -347,6 +347,11 @@ static int pam_parse_in_data_v2(struct pam_data *pd,
body, blen, &c);
if (ret != EOK) return ret;
break;
+ case SSS_PAM_ITEM_CHILD_PID:
+ ret = extract_uint32_t(&pd->child_pid, size,
+ body, blen, &c);
+ if (ret != EOK) return ret;
+ break;
case SSS_PAM_ITEM_AUTHTOK:
ret = extract_authtok_v2(pd->authtok,
size, body, blen, &c);
diff --git a/src/sss_client/pam_message.c b/src/sss_client/pam_message.c
index 036ae2ad17..9bf61fa6bd 100644
--- a/src/sss_client/pam_message.c
+++ b/src/sss_client/pam_message.c
@@ -124,6 +124,7 @@ int pack_message_v3(struct pam_items *pi, size_t *size, uint8_t **buffer)
len += pi->pam_newauthtok != NULL ?
3*sizeof(uint32_t) + pi->pam_newauthtok_size : 0;
len += 3*sizeof(uint32_t); /* cli_pid */
+ len += 3*sizeof(uint32_t); /* child_pid */
len += *pi->requested_domains != '\0' ?
2*sizeof(uint32_t) + pi->requested_domains_size : 0;
len += 3*sizeof(uint32_t); /* flags */
@@ -158,6 +159,9 @@ int pack_message_v3(struct pam_items *pi, size_t *size, uint8_t **buffer)
rp += add_uint32_t_item(SSS_PAM_ITEM_CLI_PID, (uint32_t) pi->cli_pid,
&buf[rp]);
+ rp += add_uint32_t_item(SSS_PAM_ITEM_CHILD_PID, (uint32_t) pi->child_pid,
+ &buf[rp]);
+
rp += add_authtok_item(SSS_PAM_ITEM_AUTHTOK, pi->pam_authtok_type,
pi->pam_authtok, pi->pam_authtok_size, &buf[rp]);
diff --git a/src/sss_client/pam_message.h b/src/sss_client/pam_message.h
index e37e2f082b..4ec8270c58 100644
--- a/src/sss_client/pam_message.h
+++ b/src/sss_client/pam_message.h
@@ -51,6 +51,7 @@ struct pam_items {
enum sss_authtok_type pam_newauthtok_type;
size_t pam_newauthtok_size;
pid_t cli_pid;
+ pid_t child_pid;
uint32_t flags;
const char *login_name;
char *domain_name;
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index 106e301038..65d0c90044 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -1201,6 +1201,9 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf,
}
}
break;
+ case SSS_CHILD_KEEP_ALIVE:
+ memcpy(&pi->child_pid, &buf[p], len);
+ break;
default:
D(("Unknown response type [%d]", type));
}
@@ -1335,6 +1338,7 @@ static void print_pam_items(struct pam_items *pi)
D(("Authtok: %s", CHECK_AND_RETURN_PI_STRING(pi->pam_authtok)));
D(("Newauthtok: %s", CHECK_AND_RETURN_PI_STRING(pi->pam_newauthtok)));
D(("Cli_PID: %d", pi->cli_pid));
+ D(("Child_PID: %d", pi->child_pid));
D(("Requested domains: %s", pi->requested_domains));
D(("Flags: %d", pi->flags));
}
diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
index 4cce858484..df903449fd 100644
--- a/src/sss_client/sss_cli.h
+++ b/src/sss_client/sss_cli.h
@@ -378,6 +378,7 @@ enum pam_item_type {
SSS_PAM_ITEM_NEWAUTHTOK,
SSS_PAM_ITEM_CLI_LOCALE,
SSS_PAM_ITEM_CLI_PID,
+ SSS_PAM_ITEM_CHILD_PID,
SSS_PAM_ITEM_REQUESTED_DOMAINS,
SSS_PAM_ITEM_FLAGS,
};
@@ -497,6 +498,10 @@ enum response_type {
SSS_PAM_PROMPT_CONFIG, /**< Contains data which controls which credentials
* are expected and how the user is prompted for
* them. */
+ SSS_CHILD_KEEP_ALIVE, /**< Indicates that the child process is kept alived
+ * and further communication must be done with the
+ * same child. The message is the pid of the child
+ * process. */
};
/**
diff --git a/src/sss_iface/sss_iface_types.c b/src/sss_iface/sss_iface_types.c
index 2588e68e36..978f4ac4e1 100644
--- a/src/sss_iface/sss_iface_types.c
+++ b/src/sss_iface/sss_iface_types.c
@@ -114,6 +114,11 @@ errno_t sbus_iterator_read_pam_data(TALLOC_CTX *mem_ctx,
goto done;
}
+ ret = sbus_iterator_read_u(iterator, &pd->child_pid);
+ if (ret != EOK) {
+ goto done;
+ }
+
ret = sbus_iterator_read_u(iterator, &pd->client_id_num);
if (ret != EOK) {
goto done;
@@ -249,6 +254,11 @@ errno_t sbus_iterator_write_pam_data(DBusMessageIter *iterator,
goto done;
}
+ ret = sbus_iterator_write_u(iterator, pd->child_pid);
+ if (ret != EOK) {
+ goto done;
+ }
+
ret = sbus_iterator_write_u(iterator, pd->client_id_num);
if (ret != EOK) {
goto done;
diff --git a/src/util/sss_pam_data.c b/src/util/sss_pam_data.c
index ebea11ea59..2986d95fc4 100644
--- a/src/util/sss_pam_data.c
+++ b/src/util/sss_pam_data.c
@@ -182,6 +182,7 @@ void pam_print_data(int l, struct pam_data *pd)
sss_authtok_type_to_str(sss_authtok_get_type(pd->newauthtok)));
DEBUG(l, "[CID #%u] priv: %d\n", pd->client_id_num, pd->priv);
DEBUG(l, "[CID #%u] cli_pid: %d\n", pd->client_id_num, pd->cli_pid);
+ DEBUG(l, "[CID #%u] child_pid: %d\n", pd->client_id_num, pd->child_pid);
DEBUG(l, "[CID #%u] logon name: %s\n", pd->client_id_num, PAM_SAFE_ITEM(pd->logon_name));
DEBUG(l, "[CID #%u] flags: %d\n", pd->client_id_num, pd->cli_flags);
}
diff --git a/src/util/sss_pam_data.h b/src/util/sss_pam_data.h
index 5f97391838..d80ab31195 100644
--- a/src/util/sss_pam_data.h
+++ b/src/util/sss_pam_data.h
@@ -57,6 +57,7 @@ struct pam_data {
struct sss_auth_token *authtok;
struct sss_auth_token *newauthtok;
uint32_t cli_pid;
+ uint32_t child_pid;
char *logon_name;
uint32_t cli_flags;
From 2ea8b060992cf7894298d2fdb8b2113b22ac6518 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 24 Aug 2021 12:21:33 +0200
Subject: [PATCH 03/11] pam: add new SSS_PAM_OAUTH2_INFO pam item
This item will hold OAuth2 authentication url and pin. It will be used
in one of the next patch to allow OAuth2 authentication via Kerberos.
---
src/sss_client/pam_message.h | 2 ++
src/sss_client/pam_sss.c | 29 +++++++++++++++++++++++++++++
src/sss_client/sss_cli.h | 4 ++++
3 files changed, 35 insertions(+)
diff --git a/src/sss_client/pam_message.h b/src/sss_client/pam_message.h
index 4ec8270c58..a4f79385e2 100644
--- a/src/sss_client/pam_message.h
+++ b/src/sss_client/pam_message.h
@@ -60,6 +60,8 @@ struct pam_items {
char *otp_vendor;
char *otp_token_id;
char *otp_challenge;
+ char *oauth2_url;
+ char *oauth2_pin;
char *first_factor;
bool password_prompting;
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index 65d0c90044..7af4f7cfe3 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -1203,6 +1203,35 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf,
break;
case SSS_CHILD_KEEP_ALIVE:
memcpy(&pi->child_pid, &buf[p], len);
+ break;
+ case SSS_PAM_OAUTH2_INFO:
+ if (buf[p + (len - 1)] != '\0') {
+ D(("oauth2 info does not end with \\0."));
+ break;
+ }
+
+ free(pi->oauth2_url);
+ pi->oauth2_url = strdup((char *) &buf[p]);
+ if (pi->oauth2_url == NULL) {
+ D(("strdup failed"));
+ break;
+ }
+
+ offset = strlen(pi->oauth2_url) + 1;
+ if (offset >= len) {
+ D(("OAuth2 message size mismatch"));
+ free(pi->oauth2_url);
+ pi->oauth2_url = NULL;
+ break;
+ }
+
+ free(pi->oauth2_pin);
+ pi->oauth2_pin = strdup((char *) &buf[p + offset]);
+ if (pi->oauth2_pin == NULL) {
+ D(("strdup failed"));
+ break;
+ }
+
break;
default:
D(("Unknown response type [%d]", type));
diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
index df903449fd..b68a4639fd 100644
--- a/src/sss_client/sss_cli.h
+++ b/src/sss_client/sss_cli.h
@@ -502,6 +502,10 @@ enum response_type {
* and further communication must be done with the
* same child. The message is the pid of the child
* process. */
+ SSS_PAM_OAUTH2_INFO, /**< A message which contains the oauth2 url and pin
+ * for the user.
+ * @param Two zero terminated strings (url + pin)
+ */
};
/**
From 185476b1350f00f1f840187ebf1756c3bdff9dcf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Wed, 18 Aug 2021 12:34:14 +0200
Subject: [PATCH 04/11] krb5: support to exchange multiple messages with the
same child
Previously, we expected the krb5_child to exit immediately after
receiving a response. However, now we require to exchange multiple
messages in order to maintain internal Kerberos state.
This patch adds a hash table that stores file descriptors for each child
(pid:child_io_fds). The file descriptors are closed when the child exits
but are kept open until then.
If pam_data->child_pid is not 0 we lookup the file descriptors in the
hash table and use them to continue the authentication process instead
of creating a new child.
If the pid is zero, we create a new child and store new file descriptors
in the table.
---
src/providers/krb5/krb5_child_handler.c | 246 +++++++++++++++++-------
src/providers/krb5/krb5_common.h | 1 +
src/util/child_common.h | 2 +
3 files changed, 183 insertions(+), 66 deletions(-)
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 778e38fc8e..eed1f7fa2a 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -29,6 +29,7 @@
#include "providers/krb5/krb5_common.h"
#include "providers/krb5/krb5_auth.h"
#include "src/providers/krb5/krb5_utils.h"
+#include "util/sss_ptr_hash.h"
#ifndef KRB5_CHILD_DIR
#ifndef SSSD_LIBEXEC_PATH
@@ -247,6 +248,17 @@ static errno_t create_send_buffer(struct krb5child_req *kr,
return EOK;
}
+static void krb5_child_terminate(pid_t pid)
+{
+ int ret;
+
+ ret = kill(pid, SIGKILL);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, "kill failed [%d]: %s\n",
+ ret, sss_strerror(ret));
+ }
+}
static void krb5_child_timeout(struct tevent_context *ev,
struct tevent_timer *te,
@@ -255,7 +267,6 @@ static void krb5_child_timeout(struct tevent_context *ev,
struct tevent_req *req = talloc_get_type(pvt, struct tevent_req);
struct handle_child_state *state = tevent_req_data(req,
struct handle_child_state);
- int ret;
if (state->timeout_handler == NULL) {
return;
@@ -266,11 +277,7 @@ static void krb5_child_timeout(struct tevent_context *ev,
"is slow you may consider increasing value of krb5_auth_timeout.\n",
state->child_pid);
- ret = kill(state->child_pid, SIGKILL);
- if (ret == -1) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "kill failed [%d][%s].\n", errno, strerror(errno));
- }
+ krb5_child_terminate(state->child_pid);
tevent_req_error(req, ETIMEDOUT);
}
@@ -433,20 +440,47 @@ errno_t set_extra_args(TALLOC_CTX *mem_ctx, struct krb5_ctx *krb5_ctx,
return ret;
}
-static errno_t fork_child(struct tevent_req *req)
+static void child_exited(int child_status,
+ struct tevent_signal *sige,
+ void *pvt)
{
+ struct child_io_fds *io = talloc_get_type(pvt, struct child_io_fds);
+
+ /* Do not free it if we still need to read some data. Just mark that the
+ * child has exited so we know we need to free it later. */
+ if (io->in_use) {
+ io->child_exited = true;
+ return;
+ }
+
+ /* The child has finished and we don't need to use the file descriptors
+ * any more. This will close them and remove them from io hash table. */
+ talloc_free(io);
+}
+
+static errno_t fork_child(struct tevent_context *ev,
+ struct krb5child_req *kr,
+ pid_t *_child_pid,
+ struct child_io_fds **_io)
+{
+ TALLOC_CTX *tmp_ctx;
int pipefd_to_child[2] = PIPE_INIT;
int pipefd_from_child[2] = PIPE_INIT;
+ const char **krb5_child_extra_args;
+ struct child_io_fds *io;
+ char *io_key;
pid_t pid;
errno_t ret;
- const char **krb5_child_extra_args;
- struct handle_child_state *state = tevent_req_data(req,
- struct handle_child_state);
- ret = set_extra_args(state, state->kr->krb5_ctx, state->kr->dom, &krb5_child_extra_args);
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return ENOMEM;
+ }
+
+ ret = set_extra_args(tmp_ctx, kr->krb5_ctx, kr->dom, &krb5_child_extra_args);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "set_extra_args failed.\n");
- goto fail;
+ goto done;
}
ret = pipe(pipefd_from_child);
@@ -454,20 +488,21 @@ static errno_t fork_child(struct tevent_req *req)
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
"pipe (from) failed [%d][%s].\n", errno, strerror(errno));
- goto fail;
+ goto done;
}
+
ret = pipe(pipefd_to_child);
if (ret == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
"pipe (to) failed [%d][%s].\n", errno, strerror(errno));
- goto fail;
+ goto done;
}
pid = fork();
if (pid == 0) { /* child */
- exec_child_ex(state,
+ exec_child_ex(tmp_ctx,
pipefd_to_child, pipefd_from_child,
KRB5_CHILD, KRB5_CHILD_LOG_FILE,
krb5_child_extra_args, false,
@@ -475,41 +510,71 @@ static errno_t fork_child(struct tevent_req *req)
/* We should never get here */
DEBUG(SSSDBG_CRIT_FAILURE, "BUG: Could not exec KRB5 child\n");
- } else if (pid > 0) { /* parent */
- state->child_pid = pid;
- state->io->read_from_child_fd = pipefd_from_child[0];
- PIPE_FD_CLOSE(pipefd_from_child[1]);
- state->io->write_to_child_fd = pipefd_to_child[1];
- PIPE_FD_CLOSE(pipefd_to_child[0]);
- sss_fd_nonblocking(state->io->read_from_child_fd);
- sss_fd_nonblocking(state->io->write_to_child_fd);
-
- ret = child_handler_setup(state->ev, pid, NULL, NULL, NULL);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "Could not set up child signal handler\n");
- goto fail;
- }
+ ret = ERR_INTERNAL;
+ goto done;
+ } else if (pid < 0) { /* error */
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, "fork failed [%d]: %s\n", ret, strerror(ret));
+ goto done;
+ }
- ret = activate_child_timeout_handler(req, state->ev,
- dp_opt_get_int(state->kr->krb5_ctx->opts, KRB5_AUTH_TIMEOUT));
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "activate_child_timeout_handler failed.\n");
- }
+ /* parent */
- } else { /* error */
- ret = errno;
- DEBUG(SSSDBG_CRIT_FAILURE,
- "fork failed [%d][%s].\n", errno, strerror(ret));
- goto fail;
+ io = talloc_zero(tmp_ctx, struct child_io_fds);
+ if (io == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "talloc failed.\n");
+ ret = ENOMEM;
+ goto done;
}
+ talloc_set_destructor((void*)io, child_io_destructor);
- return EOK;
+ /* Set file descriptors. */
+ io->read_from_child_fd = pipefd_from_child[0];
+ io->write_to_child_fd = pipefd_to_child[1];
+ PIPE_FD_CLOSE(pipefd_from_child[1]);
+ PIPE_FD_CLOSE(pipefd_to_child[0]);
+ sss_fd_nonblocking(io->read_from_child_fd);
+ sss_fd_nonblocking(io->write_to_child_fd);
-fail:
- PIPE_CLOSE(pipefd_from_child);
- PIPE_CLOSE(pipefd_to_child);
+ /* Add io to pid:io hash table. */
+ io_key = talloc_asprintf(tmp_ctx, "%d", pid);
+ if (io_key == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = sss_ptr_hash_add(kr->krb5_ctx->io_table, io_key, io,
+ struct child_io_fds);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "Unable to add child io to hash table "
+ "[%d]: %s\n", ret, sss_strerror(ret));
+ goto done;
+ }
+
+ /* Setup the child handler. It will free io and remove it from the hash
+ * table when it exits. */
+ ret = child_handler_setup(ev, pid, child_exited, io, NULL);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Could not set up child signal handler "
+ "[%d]: %s\n", ret, sss_strerror(ret));
+ goto done;
+ }
+
+ /* Steal the io pair so it can outlive this request if needed. */
+ talloc_steal(kr->krb5_ctx->io_table, io);
+
+ *_child_pid = pid;
+ *_io = io;
+
+ ret = EOK;
+
+done:
+ if (ret != EOK) {
+ PIPE_CLOSE(pipefd_from_child);
+ PIPE_CLOSE(pipefd_to_child);
+ }
+
+ talloc_free(tmp_ctx);
return ret;
}
@@ -522,6 +587,7 @@ struct tevent_req *handle_child_send(TALLOC_CTX *mem_ctx,
{
struct tevent_req *req, *subreq;
struct handle_child_state *state;
+ char *io_key;
int ret;
struct io_buffer *buf = NULL;
@@ -530,6 +596,15 @@ struct tevent_req *handle_child_send(TALLOC_CTX *mem_ctx,
return NULL;
}
+ if (kr->krb5_ctx->io_table == NULL) {
+ /* Create IO/pipe table if it does not exist. */
+ kr->krb5_ctx->io_table = sss_ptr_hash_create(kr->krb5_ctx, NULL, NULL);
+ if (kr->krb5_ctx->io_table == NULL) {
+ ret = ENOMEM;
+ goto fail;
+ }
+ }
+
state->ev = ev;
state->kr = kr;
state->buf = NULL;
@@ -537,28 +612,48 @@ struct tevent_req *handle_child_send(TALLOC_CTX *mem_ctx,
state->child_pid = -1;
state->timeout_handler = NULL;
- state->io = talloc(state, struct child_io_fds);
- if (state->io == NULL) {
- DEBUG(SSSDBG_CRIT_FAILURE, "talloc failed.\n");
- ret = ENOMEM;
- goto fail;
- }
- state->io->write_to_child_fd = -1;
- state->io->read_from_child_fd = -1;
- talloc_set_destructor((void *) state->io, child_io_destructor);
-
ret = create_send_buffer(kr, &buf);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "create_send_buffer failed.\n");
goto fail;
}
- ret = fork_child(req);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, "fork_child failed.\n");
- goto fail;
+ if (kr->pd->child_pid == 0) {
+ /* Create new child. */
+ ret = fork_child(ev, kr, &state->child_pid, &state->io);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "fork_child failed.\n");
+ goto fail;
+ }
+
+ /* Setup timeout. If failed, terminate the child process. */
+ ret = activate_child_timeout_handler(req, ev,
+ dp_opt_get_int(kr->krb5_ctx->opts, KRB5_AUTH_TIMEOUT));
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to setup child timeout "
+ "[%d]: %s\n", ret, sss_strerror(ret));
+ krb5_child_terminate(state->child_pid);
+ goto fail;
+ }
+ } else {
+ /* Continue talking to an existing child. */
+ io_key = talloc_asprintf(state, "%d", kr->pd->child_pid);
+ if (io_key == NULL) {
+ ret = ENOMEM;
+ goto fail;
+ }
+
+ state->io = sss_ptr_hash_lookup(kr->krb5_ctx->io_table, io_key,
+ struct child_io_fds);
+ if (state->io == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "Unable to locate pipe for child pid=%s\n",
+ io_key);
+ ret = ERR_INTERNAL;
+ goto fail;
+ }
}
+ state->io->in_use = true;
subreq = write_pipe_send(state, ev, buf->data, buf->size,
state->io->write_to_child_fd);
if (!subreq) {
@@ -586,18 +681,27 @@ static void handle_child_step(struct tevent_req *subreq)
ret = write_pipe_recv(subreq);
talloc_zfree(subreq);
if (ret != EOK) {
- tevent_req_error(req, ret);
- return;
+ goto done;
}
PIPE_FD_CLOSE(state->io->write_to_child_fd);
subreq = read_pipe_send(state, state->ev, state->io->read_from_child_fd);
if (!subreq) {
- tevent_req_error(req, ENOMEM);
- return;
+ ret = ENOMEM;
+ goto done;
}
tevent_req_set_callback(subreq, handle_child_done, req);
+
+done:
+ if (ret != EOK) {
+ state->io->in_use = false;
+ if (state->io->child_exited) {
+ talloc_free(state->io);
+ }
+
+ tevent_req_error(req, ret);
+ }
}
static void handle_child_done(struct tevent_req *subreq)
@@ -611,16 +715,26 @@ static void handle_child_done(struct tevent_req *subreq)
talloc_zfree(state->timeout_handler);
ret = read_pipe_recv(subreq, state, &state->buf, &state->len);
+ state->io->in_use = false;
talloc_zfree(subreq);
if (ret != EOK) {
- tevent_req_error(req, ret);
- return;
+ goto done;
}
PIPE_FD_CLOSE(state->io->read_from_child_fd);
+done:
+ state->io->in_use = false;
+ if (state->io->child_exited) {
+ talloc_free(state->io);
+ }
+
+ if (ret != EOK) {
+ tevent_req_error(req, ret);
+ return;
+ }
+
tevent_req_done(req);
- return;
}
int handle_child_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
diff --git a/src/providers/krb5/krb5_common.h b/src/providers/krb5/krb5_common.h
index 151f446d10..3522e14c6c 100644
--- a/src/providers/krb5/krb5_common.h
+++ b/src/providers/krb5/krb5_common.h
@@ -135,6 +135,7 @@ struct krb5_ctx {
bool sss_creds_password;
hash_table_t *wait_queue_hash;
+ hash_table_t *io_table;
enum krb5_config_type config_type;
diff --git a/src/util/child_common.h b/src/util/child_common.h
index 92d66a5009..e68bf44f0a 100644
--- a/src/util/child_common.h
+++ b/src/util/child_common.h
@@ -51,6 +51,8 @@ struct io_buffer {
struct child_io_fds {
int read_from_child_fd;
int write_to_child_fd;
+ bool child_exited;
+ bool in_use;
};
/* COMMON SIGCHLD HANDLING */
From d236fe9392380c64260c9c95c0bec10738bb0711 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Wed, 25 Aug 2021 14:05:49 +0200
Subject: [PATCH 05/11] krb5: terminate child if it fails to setup
---
src/providers/krb5/krb5_child_handler.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index eed1f7fa2a..51edf38a69 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -252,6 +252,10 @@ static void krb5_child_terminate(pid_t pid)
{
int ret;
+ if (pid == 0) {
+ return;
+ }
+
ret = kill(pid, SIGKILL);
if (ret == -1) {
ret = errno;
@@ -469,7 +473,7 @@ static errno_t fork_child(struct tevent_context *ev,
const char **krb5_child_extra_args;
struct child_io_fds *io;
char *io_key;
- pid_t pid;
+ pid_t pid = 0;
errno_t ret;
tmp_ctx = talloc_new(NULL);
@@ -572,6 +576,7 @@ static errno_t fork_child(struct tevent_context *ev,
if (ret != EOK) {
PIPE_CLOSE(pipefd_from_child);
PIPE_CLOSE(pipefd_to_child);
+ krb5_child_terminate(pid);
}
talloc_free(tmp_ctx);
From ca3f9b955cfa8b82d16ce0e79f64ff765a919cf7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Mon, 23 Aug 2021 12:55:28 +0200
Subject: [PATCH 06/11] krb5: exchange messages with krb5_child with exact
length
This is needed so we don't rely on received EOF when reading from
a pipe so we can exchange multiple messages. Now the protocol
contains a uin32_t header that contains length of the rest of
the message.
---
src/providers/krb5/krb5_child.c | 4 +-
src/providers/krb5/krb5_child_handler.c | 15 +-
src/util/atomic_io.c | 34 +++++
src/util/atomic_io.h | 13 ++
src/util/child_common.c | 175 ++++++++++++++++++------
src/util/child_common.h | 29 +++-
6 files changed, 211 insertions(+), 59 deletions(-)
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 4e55d9a374..a0a5b576dc 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1202,7 +1202,7 @@ static errno_t k5c_send_data(struct krb5_req *kr, int fd, errno_t error)
}
errno = 0;
- written = sss_atomic_write_s(fd, buf, len);
+ written = sss_atomic_write_safe_s(fd, buf, len);
if (written == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
@@ -2816,7 +2816,7 @@ static errno_t k5c_recv_data(struct krb5_req *kr, int fd, uint32_t *offline)
errno_t ret;
errno = 0;
- len = sss_atomic_read_s(fd, buf, IN_BUF_SIZE);
+ len = sss_atomic_read_safe_s(fd, buf, IN_BUF_SIZE, NULL);
if (len == -1) {
ret = errno;
ret = (ret == 0) ? EINVAL: ret;
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 51edf38a69..7b4802c964 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -659,8 +659,8 @@ struct tevent_req *handle_child_send(TALLOC_CTX *mem_ctx,
}
state->io->in_use = true;
- subreq = write_pipe_send(state, ev, buf->data, buf->size,
- state->io->write_to_child_fd);
+ subreq = write_pipe_safe_send(state, ev, buf->data, buf->size,
+ state->io->write_to_child_fd);
if (!subreq) {
ret = ENOMEM;
goto fail;
@@ -683,15 +683,14 @@ static void handle_child_step(struct tevent_req *subreq)
struct handle_child_state);
int ret;
- ret = write_pipe_recv(subreq);
+ ret = write_pipe_safe_recv(subreq);
talloc_zfree(subreq);
if (ret != EOK) {
goto done;
}
- PIPE_FD_CLOSE(state->io->write_to_child_fd);
-
- subreq = read_pipe_send(state, state->ev, state->io->read_from_child_fd);
+ subreq = read_pipe_safe_send(state, state->ev,
+ state->io->read_from_child_fd);
if (!subreq) {
ret = ENOMEM;
goto done;
@@ -719,15 +718,13 @@ static void handle_child_done(struct tevent_req *subreq)
talloc_zfree(state->timeout_handler);
- ret = read_pipe_recv(subreq, state, &state->buf, &state->len);
+ ret = read_pipe_safe_recv(subreq, state, &state->buf, &state->len);
state->io->in_use = false;
talloc_zfree(subreq);
if (ret != EOK) {
goto done;
}
- PIPE_FD_CLOSE(state->io->read_from_child_fd);
-
done:
state->io->in_use = false;
if (state->io->child_exited) {
diff --git a/src/util/atomic_io.c b/src/util/atomic_io.c
index 1543af9a02..a3ba402f1d 100644
--- a/src/util/atomic_io.c
+++ b/src/util/atomic_io.c
@@ -18,6 +18,8 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <stdint.h>
+
#include "util/atomic_io.h"
/* based on code from libssh <http://www.libssh.org> */
@@ -58,3 +60,35 @@ ssize_t sss_atomic_io_s(int fd, void *buf, size_t n, bool do_read)
return pos;
}
+
+ssize_t sss_atomic_write_safe_s(int fd, void *buf, size_t len)
+{
+ uint32_t ulen = len;
+ ssize_t ret;
+
+ ret = sss_atomic_write_s(fd, &ulen, sizeof(uint32_t));
+ if (ret != sizeof(uint32_t)) {
+ errno = errno == 0 ? EIO : errno;
+ return -1;
+ }
+
+ return sss_atomic_write_s(fd, buf, len);
+}
+
+ssize_t sss_atomic_read_safe_s(int fd, void *buf, size_t buf_len, size_t *_len)
+{
+ uint32_t ulen;
+ ssize_t ret;
+
+ ret = sss_atomic_read_s(fd, &ulen, sizeof(uint32_t));
+ if (ret != sizeof(uint32_t)) {
+ errno = errno == 0 ? EIO : errno;
+ return -1;
+ }
+
+ if (_len != NULL) {
+ *_len = ulen;
+ }
+
+ return sss_atomic_read_s(fd, buf, ulen);
+}
diff --git a/src/util/atomic_io.h b/src/util/atomic_io.h
index ffae31d6c2..fee09984f0 100644
--- a/src/util/atomic_io.h
+++ b/src/util/atomic_io.h
@@ -37,4 +37,17 @@ ssize_t sss_atomic_io_s(int fd, void *buf, size_t n, bool do_read);
#define sss_atomic_read_s(fd, buf, n) sss_atomic_io_s(fd, buf, n, true)
#define sss_atomic_write_s(fd, buf, n) sss_atomic_io_s(fd, buf, n, false)
+/**
+ * Write length of the buffer then the buffer itself.
+ *
+ * (uint32_t)length + buffer
+ */
+ssize_t sss_atomic_write_safe_s(int fd, void *buf, size_t len);
+
+/**
+ * First, read uint32_t as a message length, then read the rest of the message
+ * expecting given length. The exact length is returned in _len parameter.
+ */
+ssize_t sss_atomic_read_safe_s(int fd, void *buf, size_t max_len, size_t *_len);
+
#endif /* __SSSD_ATOMIC_IO_H__ */
diff --git a/src/util/child_common.c b/src/util/child_common.c
index 1ae0fddd50..9367fb2818 100644
--- a/src/util/child_common.c
+++ b/src/util/child_common.c
@@ -329,35 +329,41 @@ void child_handler_destroy(struct sss_child_ctx_old *ctx)
/* Async communication with the child process via a pipe */
-struct write_pipe_state {
+struct _write_pipe_state {
int fd;
uint8_t *buf;
size_t len;
+ bool safe;
ssize_t written;
};
-static void write_pipe_handler(struct tevent_context *ev,
- struct tevent_fd *fde,
- uint16_t flags, void *pvt);
-
-struct tevent_req *write_pipe_send(TALLOC_CTX *mem_ctx,
- struct tevent_context *ev,
- uint8_t *buf, size_t len, int fd)
+static void _write_pipe_handler(struct tevent_context *ev,
+ struct tevent_fd *fde,
+ uint16_t flags,
+ void *pvt);
+
+static struct tevent_req *_write_pipe_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ uint8_t *buf,
+ size_t len,
+ bool safe,
+ int fd)
{
struct tevent_req *req;
- struct write_pipe_state *state;
+ struct _write_pipe_state *state;
struct tevent_fd *fde;
- req = tevent_req_create(mem_ctx, &state, struct write_pipe_state);
+ req = tevent_req_create(mem_ctx, &state, struct _write_pipe_state);
if (req == NULL) return NULL;
state->fd = fd;
state->buf = buf;
state->len = len;
+ state->safe = safe;
state->written = 0;
fde = tevent_add_fd(ev, state, fd, TEVENT_FD_WRITE,
- write_pipe_handler, req);
+ _write_pipe_handler, req);
if (fde == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "tevent_add_fd failed.\n");
goto fail;
@@ -370,29 +376,35 @@ struct tevent_req *write_pipe_send(TALLOC_CTX *mem_ctx,
return NULL;
}
-static void write_pipe_handler(struct tevent_context *ev,
- struct tevent_fd *fde,
- uint16_t flags, void *pvt)
+static void _write_pipe_handler(struct tevent_context *ev,
+ struct tevent_fd *fde,
+ uint16_t flags,
+ void *pvt)
{
struct tevent_req *req = talloc_get_type(pvt, struct tevent_req);
- struct write_pipe_state *state = tevent_req_data(req,
- struct write_pipe_state);
+ struct _write_pipe_state *state;
errno_t ret;
+ state = tevent_req_data(req, struct _write_pipe_state);
+
if (flags & TEVENT_FD_READ) {
DEBUG(SSSDBG_CRIT_FAILURE,
- "write_pipe_done called with TEVENT_FD_READ,"
- " this should not happen.\n");
+ "_write_pipe_done called with TEVENT_FD_READ,"
+ " this should not happen.\n");
tevent_req_error(req, EINVAL);
return;
}
errno = 0;
- state->written = sss_atomic_write_s(state->fd, state->buf, state->len);
+ if (state->safe) {
+ state->written = sss_atomic_write_safe_s(state->fd, state->buf, state->len);
+ } else {
+ state->written = sss_atomic_write_s(state->fd, state->buf, state->len);
+ }
if (state->written == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
- "write failed [%d][%s].\n", ret, strerror(ret));
+ "write failed [%d][%s].\n", ret, strerror(ret));
tevent_req_error(req, ret);
return;
}
@@ -409,39 +421,72 @@ static void write_pipe_handler(struct tevent_context *ev,
return;
}
-int write_pipe_recv(struct tevent_req *req)
+static int _write_pipe_recv(struct tevent_req *req)
{
TEVENT_REQ_RETURN_ON_ERROR(req);
return EOK;
}
-struct read_pipe_state {
+struct tevent_req *write_pipe_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ uint8_t *buf,
+ size_t len,
+ int fd)
+{
+ return _write_pipe_send(mem_ctx, ev, buf, len, false, fd);
+}
+
+int write_pipe_recv(struct tevent_req *req)
+{
+ return _write_pipe_recv(req);
+}
+
+struct tevent_req *write_pipe_safe_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ uint8_t *buf,
+ size_t len,
+ int fd)
+{
+ return _write_pipe_send(mem_ctx, ev, buf, len, true, fd);
+}
+
+int write_pipe_safe_recv(struct tevent_req *req)
+{
+ return _write_pipe_recv(req);
+}
+
+struct _read_pipe_state {
int fd;
uint8_t *buf;
size_t len;
+ bool safe;
};
-static void read_pipe_handler(struct tevent_context *ev,
- struct tevent_fd *fde,
- uint16_t flags, void *pvt);
+static void _read_pipe_handler(struct tevent_context *ev,
+ struct tevent_fd *fde,
+ uint16_t flags,
+ void *pvt);
-struct tevent_req *read_pipe_send(TALLOC_CTX *mem_ctx,
- struct tevent_context *ev, int fd)
+static struct tevent_req *_read_pipe_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ bool safe,
+ int fd)
{
struct tevent_req *req;
- struct read_pipe_state *state;
+ struct _read_pipe_state *state;
struct tevent_fd *fde;
- req = tevent_req_create(mem_ctx, &state, struct read_pipe_state);
+ req = tevent_req_create(mem_ctx, &state, struct _read_pipe_state);
if (req == NULL) return NULL;
state->fd = fd;
state->buf = NULL;
state->len = 0;
+ state->safe = safe;
fde = tevent_add_fd(ev, state, fd, TEVENT_FD_READ,
- read_pipe_handler, req);
+ _read_pipe_handler, req);
if (fde == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "tevent_add_fd failed.\n");
goto fail;
@@ -454,27 +499,32 @@ struct tevent_req *read_pipe_send(TALLOC_CTX *mem_ctx,
return NULL;
}
-static void read_pipe_handler(struct tevent_context *ev,
- struct tevent_fd *fde,
- uint16_t flags, void *pvt)
+static void _read_pipe_handler(struct tevent_context *ev,
+ struct tevent_fd *fde,
+ uint16_t flags,
+ void *pvt)
{
struct tevent_req *req = talloc_get_type(pvt, struct tevent_req);
- struct read_pipe_state *state = tevent_req_data(req,
- struct read_pipe_state);
+ struct _read_pipe_state *state;
ssize_t size;
errno_t err;
uint8_t buf[CHILD_MSG_CHUNK];
+ size_t len = 0;
+
+ state = tevent_req_data(req, struct _read_pipe_state);
if (flags & TEVENT_FD_WRITE) {
- DEBUG(SSSDBG_CRIT_FAILURE, "read_pipe_done called with TEVENT_FD_WRITE,"
- " this should not happen.\n");
+ DEBUG(SSSDBG_CRIT_FAILURE, "_read_pipe_done called with "
+ "TEVENT_FD_WRITE, this should not happen.\n");
tevent_req_error(req, EINVAL);
return;
}
- size = sss_atomic_read_s(state->fd,
- buf,
- CHILD_MSG_CHUNK);
+ if (state->safe) {
+ size = sss_atomic_read_safe_s(state->fd, buf, CHILD_MSG_CHUNK, &len);
+ } else {
+ size = sss_atomic_read_s(state->fd, buf, CHILD_MSG_CHUNK);
+ }
if (size == -1) {
err = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
@@ -492,6 +542,11 @@ static void read_pipe_handler(struct tevent_context *ev,
safealign_memcpy(&state->buf[state->len], buf,
size, &state->len);
+
+ if (state->len == len) {
+ DEBUG(SSSDBG_TRACE_FUNC, "All data received\n");
+ tevent_req_done(req);
+ }
return;
} else if (size == 0) {
@@ -507,11 +562,13 @@ static void read_pipe_handler(struct tevent_context *ev,
}
}
-int read_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
- uint8_t **buf, ssize_t *len)
+static errno_t _read_pipe_recv(struct tevent_req *req,
+ TALLOC_CTX *mem_ctx,
+ uint8_t **buf,
+ ssize_t *len)
{
- struct read_pipe_state *state;
- state = tevent_req_data(req, struct read_pipe_state);
+ struct _read_pipe_state *state;
+ state = tevent_req_data(req, struct _read_pipe_state);
TEVENT_REQ_RETURN_ON_ERROR(req);
@@ -521,6 +578,36 @@ int read_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
return EOK;
}
+struct tevent_req *read_pipe_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ int fd)
+{
+ return _read_pipe_send(mem_ctx, ev, false, fd);
+}
+
+errno_t read_pipe_recv(struct tevent_req *req,
+ TALLOC_CTX *mem_ctx,
+ uint8_t **_buf,
+ ssize_t *_len)
+{
+ return _read_pipe_recv(req, mem_ctx, _buf, _len);
+}
+
+struct tevent_req *read_pipe_safe_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ int fd)
+{
+ return _read_pipe_send(mem_ctx, ev, true, fd);
+}
+
+errno_t read_pipe_safe_recv(struct tevent_req *req,
+ TALLOC_CTX *mem_ctx,
+ uint8_t **_buf,
+ ssize_t *_len)
+{
+ return _read_pipe_recv(req, mem_ctx, _buf, _len);
+}
+
static void child_invoke_callback(struct tevent_context *ev,
struct tevent_immediate *imm,
void *pvt);
diff --git a/src/util/child_common.h b/src/util/child_common.h
index e68bf44f0a..c6f3f2f712 100644
--- a/src/util/child_common.h
+++ b/src/util/child_common.h
@@ -94,13 +94,34 @@ void child_handler_destroy(struct sss_child_ctx_old *ctx);
/* Async communication with the child process via a pipe */
struct tevent_req *write_pipe_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
- uint8_t *buf, size_t len, int fd);
+ uint8_t *buf,
+ size_t len,
+ int fd);
int write_pipe_recv(struct tevent_req *req);
struct tevent_req *read_pipe_send(TALLOC_CTX *mem_ctx,
- struct tevent_context *ev, int fd);
-int read_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
- uint8_t **buf, ssize_t *len);
+ struct tevent_context *ev,
+ int fd);
+errno_t read_pipe_recv(struct tevent_req *req,
+ TALLOC_CTX *mem_ctx,
+ uint8_t **_buf,
+ ssize_t *_len);
+
+/* Include buffer length in a message header, read does not wait for EOF. */
+struct tevent_req *write_pipe_safe_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ uint8_t *buf,
+ size_t len,
+ int fd);
+int write_pipe_safe_recv(struct tevent_req *req);
+
+struct tevent_req *read_pipe_safe_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ int fd);
+errno_t read_pipe_safe_recv(struct tevent_req *req,
+ TALLOC_CTX *mem_ctx,
+ uint8_t **_buf,
+ ssize_t *_len);
/* The pipes to communicate with the child must be nonblocking */
void fd_nonblocking(int fd);
From 88ab25eb61eee5176a6f34b8dd50ae9a1242886a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Thu, 12 Aug 2021 13:13:54 +0200
Subject: [PATCH 07/11] krb5: add support for oauth2 url+pin challenge
---
src/providers/krb5/krb5_auth.c | 3 +-
src/providers/krb5/krb5_child.c | 299 ++++++++++++++++++++++--
src/providers/krb5/krb5_child_handler.c | 5 +
3 files changed, 293 insertions(+), 14 deletions(-)
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 699c2467b8..2a076ae023 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -495,7 +495,8 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
&& authtok_type != SSS_AUTHTOK_TYPE_2FA
&& authtok_type != SSS_AUTHTOK_TYPE_2FA_SINGLE
&& authtok_type != SSS_AUTHTOK_TYPE_SC_PIN
- && authtok_type != SSS_AUTHTOK_TYPE_SC_KEYPAD) {
+ && authtok_type != SSS_AUTHTOK_TYPE_SC_KEYPAD
+ && authtok_type != SSS_AUTHTOK_TYPE_OAUTH2) {
/* handle empty password gracefully */
if (authtok_type == SSS_AUTHTOK_TYPE_EMPTY) {
DEBUG(SSSDBG_CRIT_FAILURE,
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index a0a5b576dc..52ae1dbc11 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -116,6 +116,17 @@ struct krb5_req {
static krb5_context krb5_error_ctx;
#define KRB5_CHILD_DEBUG(level, error) KRB5_DEBUG(level, krb5_error_ctx, error)
+static errno_t k5c_attach_otp_info_msg(struct krb5_req *kr);
+static errno_t k5c_attach_oauth2_info_msg(struct krb5_req *kr, const char *url, const char *pin);
+static errno_t k5c_attach_keep_alive_msg(struct krb5_req *kr);
+static errno_t k5c_recv_data(struct krb5_req *kr, int fd, uint32_t *offline);
+static errno_t k5c_send_data(struct krb5_req *kr, int fd, errno_t error);
+
+static errno_t otp_parse_oauth2_challenge(TALLOC_CTX *mem_ctx,
+ const char *challenge,
+ char **_url,
+ char **_pin);
+
static errno_t k5c_become_user(uid_t uid, gid_t gid, bool is_posix)
{
if (is_posix == false) {
@@ -391,6 +402,7 @@ static krb5_error_code tokeninfo_matches_2fa(TALLOC_CTX *mem_ctx,
*out_pin = pin;
return 0;
}
+
static krb5_error_code tokeninfo_matches_pwd(TALLOC_CTX *mem_ctx,
const krb5_responder_otp_tokeninfo *ti,
const char *pwd, size_t len,
@@ -487,6 +499,63 @@ static krb5_error_code tokeninfo_matches_pwd(TALLOC_CTX *mem_ctx,
return 0;
}
+static krb5_error_code
+tokeninfo_matches_oauth2(TALLOC_CTX *mem_ctx,
+ const krb5_responder_otp_tokeninfo *ti,
+ const char *pwd,
+ size_t len,
+ char **out_token,
+ char **out_pin)
+{
+ TALLOC_CTX *tmp_ctx;
+ char *oauth2_url;
+ char *oauth2_pin;
+ char *token;
+ errno_t ret;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return ENOMEM;
+ }
+
+ if (ti->flags & KRB5_RESPONDER_OTP_FLAGS_NEXTOTP) {
+ ret = ENOTSUP;
+ goto done;
+ }
+
+ ret = otp_parse_oauth2_challenge(tmp_ctx, ti->challenge, &oauth2_url,
+ &oauth2_pin);
+ if (ret == ENOENT) {
+ ret = ENOTSUP;
+ goto done;
+ } else if (ret != EOK) {
+ goto done;
+ }
+
+ token = talloc_strndup(tmp_ctx, pwd, len);
+ if (token == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ talloc_set_destructor(token, token_pin_destructor);
+
+ if (strlen(oauth2_pin) != len || strncmp(oauth2_pin, token, len) != 0) {
+ DEBUG(SSSDBG_OP_FAILURE, "OAuth2 PINs do not match!\n");
+ ret = EINVAL;
+ goto done;
+ }
+
+ *out_token = talloc_steal(mem_ctx, token);
+ *out_pin = NULL;
+
+ ret = EOK;
+
+done:
+ talloc_free(tmp_ctx);
+
+ return ret;
+}
+
static krb5_error_code tokeninfo_matches(TALLOC_CTX *mem_ctx,
const krb5_responder_otp_tokeninfo *ti,
struct sss_auth_token *auth_tok,
@@ -527,6 +596,16 @@ static krb5_error_code tokeninfo_matches(TALLOC_CTX *mem_ctx,
return tokeninfo_matches_2fa(mem_ctx, ti, pwd, len, fa2, fa2_len,
out_token, out_pin);
break;
+ case SSS_AUTHTOK_TYPE_OAUTH2:
+ ret = sss_authtok_get_oauth2(auth_tok, &pwd, &len);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "sss_authtok_get_oauth2 failed.\n");
+ return ret;
+ }
+
+ return tokeninfo_matches_oauth2(mem_ctx, ti, pwd, len,
+ out_token, out_pin);
+ break;
default:
DEBUG(SSSDBG_CRIT_FAILURE,
"Unsupported authtok type %d\n", sss_authtok_get_type(auth_tok));
@@ -535,6 +614,137 @@ static krb5_error_code tokeninfo_matches(TALLOC_CTX *mem_ctx,
return EINVAL;
}
+static errno_t otp_parse_oauth2_challenge(TALLOC_CTX *mem_ctx,
+ const char *challenge,
+ char **_url,
+ char **_pin)
+{
+ TALLOC_CTX *tmp_ctx;
+ char *url;
+ char *pin;
+ errno_t ret;
+
+ if (challenge == NULL) {
+ return ENOENT;
+ }
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return ENOMEM;
+ }
+
+ url = talloc_strdup(mem_ctx, "https://visit.me/oauth2";);
+ if (url == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ pin = talloc_strdup(mem_ctx, "381924");
+ if (pin == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ *_url = talloc_steal(mem_ctx, url);
+ *_pin = talloc_steal(mem_ctx, pin);
+
+ ret = EOK;
+
+done:
+ talloc_free(tmp_ctx);
+
+ return ret;
+}
+
+static errno_t otp_challenge(struct krb5_req *kr,
+ krb5_responder_otp_tokeninfo *ti)
+{
+ TALLOC_CTX *tmp_ctx;
+ char *oauth2_url;
+ char *oauth2_pin;
+ char *keytab;
+ uint32_t offline;
+ errno_t ret;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return ENOMEM;
+ }
+
+ if (ti->vendor != NULL) {
+ kr->otp_vendor = talloc_strdup(kr, ti->vendor);
+ if (kr->otp_vendor == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ }
+
+ if (ti->token_id != NULL) {
+ kr->otp_token_id = talloc_strdup(kr, ti->token_id);
+ if (kr->otp_token_id == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ }
+
+ if (ti->challenge != NULL) {
+ kr->otp_challenge = talloc_strdup(kr, ti->challenge);
+ if (kr->otp_challenge == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ }
+
+ ret = otp_parse_oauth2_challenge(tmp_ctx, kr->otp_challenge,
+ &oauth2_url, &oauth2_pin);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ /* Challenge was presented. We need to continue the authentication
+ * with this exact child process in order to maintain internal Kerberos
+ * state so we are able to respond to this particular challenge. */
+
+ ret = k5c_attach_otp_info_msg(kr);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "k5c_attach_otp_info_msg failed.\n");
+ return ret;
+ }
+
+ ret = k5c_attach_oauth2_info_msg(kr, oauth2_url, oauth2_pin);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "k5c_attach_oauth2_info_msg failed.\n");
+ return ret;
+ }
+
+ ret = k5c_attach_keep_alive_msg(kr);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "k5c_attach_keep_alive_msg failed.\n");
+ return ret;
+ }
+
+ keytab = kr->keytab;
+
+ /* Send reply and wait for next step. */
+ ret = k5c_send_data(kr, STDOUT_FILENO, ret);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to send reply\n");
+ }
+
+ ret = k5c_recv_data(kr, STDIN_FILENO, &offline);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ kr->keytab = keytab;
+
+ ret = EOK;
+
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
static krb5_error_code answer_otp(krb5_context ctx,
struct krb5_req *kr,
krb5_responder_context rctx)
@@ -570,19 +780,17 @@ static krb5_error_code answer_otp(krb5_context ctx,
i, chl->tokeninfo[i]->flags);
}
- if (chl->tokeninfo[0]->vendor != NULL) {
- kr->otp_vendor = talloc_strdup(kr, chl->tokeninfo[0]->vendor);
- }
- if (chl->tokeninfo[0]->token_id != NULL) {
- kr->otp_token_id = talloc_strdup(kr, chl->tokeninfo[0]->token_id);
- }
- if (chl->tokeninfo[0]->challenge != NULL) {
- kr->otp_challenge = talloc_strdup(kr, chl->tokeninfo[0]->challenge);
+ ret = otp_challenge(kr, chl->tokeninfo[0]);
+ if (ret == ENOENT) {
+ DEBUG(SSSDBG_TRACE_INTERNAL, "Exit answer_otp during pre-auth.\n");
+ return EAGAIN;
+ } else if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "Unable to check otp challenge [%d]: %s\n",
+ ret, sss_strerror(ret));
+ return ret;
}
- /* Allocation errors are ignored on purpose */
- DEBUG(SSSDBG_TRACE_INTERNAL, "Exit answer_otp during pre-auth.\n");
- return EAGAIN;
+ /* Challenge was presented and we got the authentication token. */
}
/* Find the first supported tokeninfo which matches our authtoken. */
@@ -1163,6 +1371,72 @@ static errno_t k5c_attach_otp_info_msg(struct krb5_req *kr)
return ret;
}
+static errno_t k5c_attach_oauth2_info_msg(struct krb5_req *kr,
+ const char *url,
+ const char *pin)
+{
+ uint8_t *msg;
+ size_t msg_len;
+ size_t url_len = 0;
+ size_t pin_len = 0;
+ size_t idx = 0;
+ errno_t ret;
+
+ if (url == NULL || pin == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Empty oauth2 url or pin\n");
+ return EINVAL;
+ }
+
+ msg_len = 0;
+
+ url_len = strlen(url) + 1;
+ msg_len += url_len;
+
+ pin_len = strlen(pin) + 1;
+ msg_len += pin_len;
+
+ msg = talloc_zero_size(NULL, msg_len);
+ if (msg == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_size failed.\n");
+ return ENOMEM;
+ }
+
+ memcpy(msg, url, url_len);
+ idx += url_len;
+
+ memcpy(msg + idx, pin, pin_len);
+ idx += pin_len;
+
+ ret = pam_add_response(kr->pd, SSS_PAM_OAUTH2_INFO, msg_len, msg);
+ talloc_zfree(msg);
+
+ return ret;
+}
+
+static errno_t k5c_attach_keep_alive_msg(struct krb5_req *kr)
+{
+ uint8_t *msg;
+ pid_t pid;
+ int ret;
+
+ pid = getpid();
+
+ msg = talloc_memdup(kr, &pid, sizeof(pid_t));
+ if (msg == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_size failed.\n");
+ return ENOMEM;
+ }
+
+ /* Indicate that the krb5 child must be kept alive to continue
+ * authentication with correct internal state of Kerberos API.
+ *
+ * Further communication must be done against the same child process */
+ ret = pam_add_response(kr->pd, SSS_CHILD_KEEP_ALIVE, sizeof(pid_t), msg);
+ talloc_zfree(msg);
+
+ return ret;
+}
+
static errno_t k5c_attach_ccname_msg(struct krb5_req *kr)
{
char *msg = NULL;
@@ -2399,6 +2673,7 @@ static errno_t unpack_authtok(struct sss_auth_token *tok,
case SSS_AUTHTOK_TYPE_2FA:
case SSS_AUTHTOK_TYPE_SC_PIN:
case SSS_AUTHTOK_TYPE_SC_KEYPAD:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
ret = sss_authtok_set(tok, auth_token_type, (buf + *p),
auth_token_length);
break;
@@ -3412,8 +3687,6 @@ int main(int argc, const char *argv[])
goto done;
}
- close(STDIN_FILENO);
-
kerr = privileged_krb5_setup(kr, offline);
if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE, "privileged_krb5_setup failed.\n");
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 7b4802c964..5ea5f3d16e 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -87,6 +87,7 @@ static errno_t pack_authtok(struct io_buffer *buf, size_t *rp,
case SSS_AUTHTOK_TYPE_2FA:
case SSS_AUTHTOK_TYPE_SC_PIN:
case SSS_AUTHTOK_TYPE_SC_KEYPAD:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
data = (char *) sss_authtok_get_data(tok);
auth_token_length = sss_authtok_get_size(tok);
break;
@@ -774,6 +775,10 @@ static const char *krb5_child_response_type_to_str(int32_t type)
return "TGT lifetime info";
case SSS_KRB5_INFO_UPN:
return "UPN info";
+ case SSS_CHILD_KEEP_ALIVE:
+ return "Keep alive";
+ case SSS_PAM_OAUTH2_INFO:
+ return "OAuth2 info";
}
DEBUG(SSSDBG_MINOR_FAILURE, "Unexpected response type %d\n", type);
From f238586b34215deb71bf6d144d04a9799ea75c02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Thu, 26 Aug 2021 13:52:31 +0200
Subject: [PATCH 08/11] krb5: fix memory hierarchy in krb5_child
unpack_buffer()
Fields that belong to krb5_req were attached to pam_data which caused
unexpected troubles when kr->pd gets freed and swapped with new one.
---
src/providers/krb5/krb5_child.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 52ae1dbc11..2be746f499 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -2745,7 +2745,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
kr->use_enterprise_princ = (use_enterprise_princ == 0) ? false : true;
SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
if (len > size - p) return EINVAL;
- kr->upn = talloc_strndup(pd, (char *)(buf + p), len);
+ kr->upn = talloc_strndup(kr, (char *)(buf + p), len);
if (kr->upn == NULL) return ENOMEM;
p += len;
@@ -2764,7 +2764,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM || pd->cmd == SSS_PAM_CHAUTHTOK) {
SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
if (len > size - p) return EINVAL;
- kr->ccname = talloc_strndup(pd, (char *)(buf + p), len);
+ kr->ccname = talloc_strndup(kr, (char *)(buf + p), len);
if (kr->ccname == NULL) return ENOMEM;
p += len;
@@ -2772,7 +2772,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
if (len > size - p) return EINVAL;
if (len > 0) {
- kr->old_ccname = talloc_strndup(pd, (char *)(buf + p), len);
+ kr->old_ccname = talloc_strndup(kr, (char *)(buf + p), len);
if (kr->old_ccname == NULL) return ENOMEM;
p += len;
} else {
@@ -2783,7 +2783,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
if (len > size - p) return EINVAL;
if (len > 0) {
- kr->keytab = talloc_strndup(pd, (char *)(buf + p), len);
+ kr->keytab = talloc_strndup(kr, (char *)(buf + p), len);
p += len;
} else {
kr->keytab = NULL;
From 1af724d1fb3bdcd2bcc2a1bd199e9ce07df7d0e8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Thu, 26 Aug 2021 13:49:20 +0200
Subject: [PATCH 09/11] krb5: be little paranoid when receiving new
krb5_request in the child
I believe this check is not really necessary, but it is better to be
safe.
---
src/providers/krb5/krb5_child.c | 43 ++++++++++++++++++++++++++++-----
1 file changed, 37 insertions(+), 6 deletions(-)
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 2be746f499..eb283c8a1e 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -614,6 +614,29 @@ static krb5_error_code tokeninfo_matches(TALLOC_CTX *mem_ctx,
return EINVAL;
}
+static errno_t krb5_req_update(struct krb5_req *dest, struct krb5_req *src)
+{
+ /* Check request validity. This should never happen, but it is better to
+ * be little paranoid. */
+ if (strcmp(dest->ccname, src->ccname) != 0) {
+ return EINVAL;
+ }
+
+ if (strcmp(dest->upn, src->upn) != 0) {
+ return EINVAL;
+ }
+
+ if (dest->uid != src->uid || dest->gid != src->gid) {
+ return EINVAL;
+ }
+
+ /* Update PAM data. */
+ talloc_free(dest->pd);
+ dest->pd = talloc_steal(dest, src->pd);
+
+ return EOK;
+}
+
static errno_t otp_parse_oauth2_challenge(TALLOC_CTX *mem_ctx,
const char *challenge,
char **_url,
@@ -660,9 +683,9 @@ static errno_t otp_challenge(struct krb5_req *kr,
krb5_responder_otp_tokeninfo *ti)
{
TALLOC_CTX *tmp_ctx;
+ struct krb5_req *tmpkr;
char *oauth2_url;
char *oauth2_pin;
- char *keytab;
uint32_t offline;
errno_t ret;
@@ -723,7 +746,12 @@ static errno_t otp_challenge(struct krb5_req *kr,
return ret;
}
- keytab = kr->keytab;
+ tmpkr = talloc_zero(tmp_ctx, struct krb5_req);
+ if (tmpkr == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
/* Send reply and wait for next step. */
ret = k5c_send_data(kr, STDOUT_FILENO, ret);
@@ -731,14 +759,17 @@ static errno_t otp_challenge(struct krb5_req *kr,
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to send reply\n");
}
- ret = k5c_recv_data(kr, STDIN_FILENO, &offline);
+ ret = k5c_recv_data(tmpkr, STDIN_FILENO, &offline);
if (ret != EOK) {
goto done;
}
- kr->keytab = keytab;
-
- ret = EOK;
+ ret = krb5_req_update(kr, tmpkr);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to update krb request [%d]: %s\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
done:
talloc_free(tmp_ctx);
From b7b38c7d1bcd1187257c0ec4ae656173d5d6a259 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Wed, 25 Aug 2021 14:04:47 +0200
Subject: [PATCH 10/11] krb5: add keep alive timeout for krb5_child
This timeout will kill the child after a longer time in order to
allow interactive communication with the user.
---
src/providers/krb5/krb5_child_handler.c | 30 ++++++++++++++++++++++++-
src/util/child_common.h | 1 +
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 5ea5f3d16e..ecba00b222 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -463,6 +463,19 @@ static void child_exited(int child_status,
talloc_free(io);
}
+static void child_keep_alive_timeout(struct tevent_context *ev,
+ struct tevent_timer *te,
+ struct timeval tv,
+ void *pvt)
+{
+ struct child_io_fds *io = talloc_get_type(pvt, struct child_io_fds);
+
+ DEBUG(SSSDBG_IMPORTANT_INFO, "Keep alive timeout for child [%d] reached.\n",
+ io->pid);
+
+ krb5_child_terminate(io->pid);
+}
+
static errno_t fork_child(struct tevent_context *ev,
struct krb5child_req *kr,
pid_t *_child_pid,
@@ -473,6 +486,8 @@ static errno_t fork_child(struct tevent_context *ev,
int pipefd_from_child[2] = PIPE_INIT;
const char **krb5_child_extra_args;
struct child_io_fds *io;
+ struct tevent_timer *te;
+ struct timeval tv;
char *io_key;
pid_t pid = 0;
errno_t ret;
@@ -533,6 +548,8 @@ static errno_t fork_child(struct tevent_context *ev,
}
talloc_set_destructor((void*)io, child_io_destructor);
+ io->pid = pid;
+
/* Set file descriptors. */
io->read_from_child_fd = pipefd_from_child[0];
io->write_to_child_fd = pipefd_to_child[1];
@@ -556,6 +573,17 @@ static errno_t fork_child(struct tevent_context *ev,
goto done;
}
+ /* Setup child's keep alive timeout for open file descriptors. This timeout
+ * is quite big to allow additional user interactions when the child is kept
+ * alive for further communication. */
+ tv = tevent_timeval_current_ofs(300, 0);
+ te = tevent_add_timer(ev, io, tv, child_keep_alive_timeout, io);
+ if (te == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to setup child timeout\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
/* Setup the child handler. It will free io and remove it from the hash
* table when it exits. */
ret = child_handler_setup(ev, pid, child_exited, io, NULL);
@@ -654,7 +682,7 @@ struct tevent_req *handle_child_send(TALLOC_CTX *mem_ctx,
if (state->io == NULL) {
DEBUG(SSSDBG_OP_FAILURE, "Unable to locate pipe for child pid=%s\n",
io_key);
- ret = ERR_INTERNAL;
+ ret = ENOENT;
goto fail;
}
}
diff --git a/src/util/child_common.h b/src/util/child_common.h
index c6f3f2f712..d9d48e5fc8 100644
--- a/src/util/child_common.h
+++ b/src/util/child_common.h
@@ -51,6 +51,7 @@ struct io_buffer {
struct child_io_fds {
int read_from_child_fd;
int write_to_child_fd;
+ pid_t pid;
bool child_exited;
bool in_use;
};
From 552a37ddfb7cbc7456f84e908ac6d9556f16047a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 24 Aug 2021 16:00:38 +0200
Subject: [PATCH 11/11] pam: add oauth2 url+pin prompt
Add a new prompt for oauth2 authentication. The user is prompted with
url and pin. He/she must visit the url, provide the pin and
authenticate. Then just press enter to continue the authentication.
The RADIUS server will then ask the identify provider if the
authentication was successful and then reply back.
The answer is set to the same pin, just to provide some verification
mechanism and because Kerberos requires non-empty token.
---
src/responder/pam/pamsrv_cmd.c | 1 +
src/sss_client/pam_sss.c | 36 +++++++++++++++++++++++++++++++++-
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index cdc3ae83a2..63f672a42c 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -202,6 +202,7 @@ static int extract_authtok_v2(struct sss_auth_token *tok,
case SSS_AUTHTOK_TYPE_2FA_SINGLE:
case SSS_AUTHTOK_TYPE_SC_PIN:
case SSS_AUTHTOK_TYPE_SC_KEYPAD:
+ case SSS_AUTHTOK_TYPE_OAUTH2:
ret = sss_authtok_set(tok, auth_token_type,
auth_token_data, auth_token_length);
break;
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index 7af4f7cfe3..8d9f6fd4a3 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -1693,6 +1693,36 @@ static int prompt_2fa_single(pam_handle_t *pamh, struct pam_items *pi,
return PAM_SUCCESS;
}
+static int prompt_oauth2(pam_handle_t *pamh, struct pam_items *pi,
+ const char *prompt)
+{
+ char *answer = NULL;
+ char *msg;
+ int ret;
+
+ ret = asprintf(&msg, prompt, pi->oauth2_pin, pi->oauth2_url);
+ if (ret == -1) {
+ return PAM_SYSTEM_ERR;
+ }
+
+ ret = do_pam_conversation(pamh, PAM_PROMPT_ECHO_OFF, msg, NULL, &answer);
+ free(msg);
+ if (ret != PAM_SUCCESS) {
+ D(("do_pam_conversation failed."));
+ return ret;
+ }
+
+ /* We don't care about answer here. We just need to notify that the
+ * authentication has finished. */
+ free(answer);
+
+ pi->pam_authtok = strdup(pi->oauth2_pin);
+ pi->pam_authtok_type = SSS_AUTHTOK_TYPE_OAUTH2;
+ pi->pam_authtok_size=strlen(pi->oauth2_pin);
+
+ return PAM_SUCCESS;
+}
+
#define SC_PROMPT_FMT "PIN for %s: "
#ifndef discard_const
@@ -2231,7 +2261,11 @@ static int get_authtok_for_authentication(pam_handle_t *pamh,
}
pi->pam_authtok_size = strlen(pi->pam_authtok);
} else {
- if (pi->pc != NULL) {
+ if (pi->oauth2_url != NULL) {
+ /* Prompt config is not supported for OAuth2. */
+ ret = prompt_oauth2(pamh, pi, _("Authenticate with PIN "
+ "%1$s at %2$s and press ENTER."));
+ } else if (pi->pc != NULL) {
ret = prompt_by_config(pamh, pi);
} else {
if (flags & PAM_CLI_FLAGS_USE_2FA
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
