On Mon, Nov 17, 2014 at 06:35:57PM +0100, Sumit Bose wrote:
> On Fri, Nov 14, 2014 at 01:52:20PM +0100, Jakub Hrozek wrote:
> > On Thu, Nov 13, 2014 at 07:30:41PM +0100, Jakub Hrozek wrote:
> > > On Wed, Nov 12, 2014 at 05:08:09PM +0100, Lukas Slebodnik wrote:
> > >
>
> ...
>
> > > Thank you, see the attached patches.
> >
> > I forgot to remove the extra find_uid.c from Makefile.am
>
> I only have minor comments, see below. I understand that most part of
> the patches are refactorings to make sure the related code is run with
> the right permissions. (See next mail for some additional suggestions).
Currently with a bit more complex/secure setup with e.g. FAST and ticket
validation most of the krb5_child still runs as root.
I still haven't found a way to proper serialize keytab or ccache data
so that it can be send from one process to another, but both keytab and
ccache have a memory type. The three attached patches read the keytab
and for FAST the FAST ccache into a MEMORY type, drop the privileges and
run all other operations as unprivileged users.
For the ldap_child this is straight forward because only the keytab has
to be read. With a bit of refactoring it might be even possible to drop
the privileges earlier.
FAST makes the situation for the krb5_child a bit more complicated
because if there is no valid FAST ccache a whole kinit with the keytab
has to be run and it would be nice if the result can be read by other
krb5_child processes as well as long as the ticket is valid. What would
be possible is to read the keytab as root, switch to the SSSD user to
check the FAST ccache and eventually do a kinit to renew it, switch
back to root and finally drop all privileges and become the user. I'm
not sure about the switching to the SSSD user because we not really drop
the privileges because we can become root again. As an alternative we
can become the user immediately after reading the keytab and create the
FAST ccache only in memory. But this means that we always have to get a
fresh FAST ccache.
Please note that the patches are only a POC, tests are missing and there
might be some memory issues. Nevertheless it would be nice to know if
you think that this idea is worth to follow further.
bye,
Sumit
>
> The ccache handling always was delicate, e.g. directory creation with
> DIR type and determine if FILE with _XXXXXX should be recreated or
> reused. I run couple to tests without an issue but I think only
> regression testing might give clarity here. So ACK from my side.
>
> bye,
> Sumit
>
From 56dc2b83ba5aabaa7282e91cb75238e394edd57e Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 17 Nov 2014 17:39:38 +0100
Subject: [PATCH 1/3] krb5: add copy_ccache_into_memory()
---
src/providers/krb5/krb5_ccache.c | 105 +++++++++++++++++++++++++++++++++++++++
src/providers/krb5/krb5_ccache.h | 3 ++
2 files changed, 108 insertions(+)
diff --git a/src/providers/krb5/krb5_ccache.c b/src/providers/krb5/krb5_ccache.c
index 7aa36b7..55e7021 100644
--- a/src/providers/krb5/krb5_ccache.c
+++ b/src/providers/krb5/krb5_ccache.c
@@ -667,3 +667,108 @@ errno_t safe_remove_old_ccache_file(const char
*old_ccache,
return sss_krb5_cc_destroy(old_ccache, uid, gid);
}
+
+krb5_error_code copy_ccache_into_memory(TALLOC_CTX *mem_ctx, krb5_context kctx,
+ const char *ccache_file,
+ char **_mem_name)
+{
+ krb5_error_code kerr;
+ krb5_ccache ccache;
+ krb5_ccache mem_ccache;
+ char *ccache_name;
+ krb5_principal princ;
+ char *mem_name = NULL;
+ char *sep;
+
+ kerr = krb5_cc_resolve(kctx, ccache_file, &ccache);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "error resolving ccache [%s].\n",
+ ccache_file);
+ return kerr;
+ }
+
+ kerr = krb5_cc_get_full_name(kctx, ccache, &ccache_name);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to read name for ccache [%s].\n",
+ ccache_file);
+ goto done;
+ }
+
+ sep = strchr(ccache_name, ':');
+ if (sep == NULL || sep[1] == '\0') {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Ccache name [%s] does not have delimiter[:] .\n", ccache_name);
+ kerr = KRB5KRB_ERR_GENERIC;
+ goto done;
+ }
+
+ if (strncmp(ccache_name, "MEMORY:", sizeof("MEMORY:") -1) == 0) {
+ DEBUG(SSSDBG_TRACE_FUNC, "Ccache [%s] is already memory ccache.\n",
+ ccache_name);
+ *_mem_name = talloc_strdup(mem_ctx, ccache_name);
+ if(*_mem_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+ kerr = KRB5KRB_ERR_GENERIC;
+ goto done;
+ }
+ kerr = 0;
+ goto done;
+ }
+ if (strncmp(ccache_name, "FILE:", sizeof("FILE:") -1) == 0) {
+ mem_name = talloc_asprintf(mem_ctx, "MEMORY:%s", sep + 1);
+ if (mem_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_asprintf failed.\n");
+ kerr = KRB5KRB_ERR_GENERIC;
+ goto done;
+ }
+ } else {
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unexpected ccache type for ccache [%s], "
\
+ "currently only FILE is supported.\n",
+ ccache_name);
+ kerr = KRB5KRB_ERR_GENERIC;
+ goto done;
+ }
+
+ kerr = krb5_cc_resolve(kctx, mem_name, &mem_ccache);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "error resolving ccache [%s].\n", mem_name);
+ goto done;
+ }
+
+ kerr = krb5_cc_get_principal(kctx, ccache, &princ);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "error reading principal from ccache [%s].\n", ccache_name);
+ goto done;
+ }
+
+ kerr = krb5_cc_initialize(kctx, mem_ccache, princ);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to initialize ccache [%s].\n", mem_name);
+ goto done;
+ }
+
+ kerr = krb5_cc_copy_creds(kctx, ccache, mem_ccache);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to copy ccache [%s] to [%s].\n", ccache_name, mem_name);
+ goto done;
+ }
+
+ *_mem_name = mem_name;
+ kerr = 0;
+
+done:
+ if (kerr != 0) {
+ talloc_free(mem_name);
+ }
+
+ krb5_free_principal(kctx, princ);
+
+ if (krb5_cc_close(kctx, ccache) != 0) {
+ DEBUG(SSSDBG_OP_FAILURE, "krb5_cc_close failed.\n");
+ }
+
+ return kerr;
+}
diff --git a/src/providers/krb5/krb5_ccache.h b/src/providers/krb5/krb5_ccache.h
index e47df36..2e574c4 100644
--- a/src/providers/krb5/krb5_ccache.h
+++ b/src/providers/krb5/krb5_ccache.h
@@ -53,4 +53,7 @@ errno_t safe_remove_old_ccache_file(const char *old_ccache,
const char *new_ccache,
uid_t uid, gid_t gid);
+krb5_error_code copy_ccache_into_memory(TALLOC_CTX *mem_ctx, krb5_context kctx,
+ const char *ccache_file,
+ char **_mem_name);
#endif /* __KRB5_CCACHE_H__ */
--
1.8.3.1
From 06f6c63937e9ae893372f1013375d2f3e7fc247f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 17 Nov 2014 17:40:26 +0100
Subject: [PATCH 2/3] krb5: add copy_keytab_into_memory()
---
src/providers/krb5/krb5_common.h | 4 ++
src/providers/krb5/krb5_keytab.c | 149 +++++++++++++++++++++++++++++++++++++++
2 files changed, 153 insertions(+)
create mode 100644 src/providers/krb5/krb5_keytab.c
diff --git a/src/providers/krb5/krb5_common.h b/src/providers/krb5/krb5_common.h
index a5cee64..5936bd4 100644
--- a/src/providers/krb5/krb5_common.h
+++ b/src/providers/krb5/krb5_common.h
@@ -189,4 +189,8 @@ int sssm_krb5_auth_init(struct be_ctx *bectx,
struct bet_ops **ops,
void **pvt_auth_data);
+/* from krb5_keytab.c */
+krb5_error_code copy_keytab_into_memory(TALLOC_CTX *mem_ctx, krb5_context kctx,
+ const char *keytab_file,
+ char **_mem_name);
#endif /* __KRB5_COMMON_H__ */
diff --git a/src/providers/krb5/krb5_keytab.c b/src/providers/krb5/krb5_keytab.c
new file mode 100644
index 0000000..79a9db1
--- /dev/null
+++ b/src/providers/krb5/krb5_keytab.c
@@ -0,0 +1,149 @@
+/*
+ SSSD
+
+ Kerberos 5 Backend Module -- keytab related utilities
+
+ Authors:
+ Sumit Bose <sb...@redhat.com>
+
+ Copyright (C) 2014 Red Hat
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "util/util.h"
+#include "util/sss_krb5.h"
+
+krb5_error_code copy_keytab_into_memory(TALLOC_CTX *mem_ctx, krb5_context kctx,
+ const char *keytab_file,
+ char **_mem_name)
+{
+ krb5_error_code kerr;
+ krb5_error_code kt_err;
+ krb5_keytab keytab = NULL;
+ krb5_keytab mem_keytab = NULL;
+ krb5_kt_cursor cursor;
+ krb5_keytab_entry entry;
+ char keytab_name[MAX_KEYTAB_NAME_LEN];
+ char *sep;
+ char *mem_name = NULL;
+
+ kerr = krb5_kt_resolve(kctx, keytab_file, &keytab);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "error resolving keytab [%s].\n",
+ keytab_file);
+ return kerr;
+ }
+
+ kerr = krb5_kt_have_content(kctx, keytab);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "keytab [%s] has not entries.\n",
+ keytab_file);
+ goto done;
+ }
+
+ kerr = krb5_kt_get_name(kctx, keytab, keytab_name, sizeof(keytab_name));
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to read name for keytab [%s].\n",
+ keytab_file);
+ goto done;
+ }
+
+ sep = strchr(keytab_name, ':');
+ if (sep == NULL || sep[1] == '\0') {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Keytab name [%s] does not have delimiter[:] .\n", keytab_name);
+ kerr = KRB5KRB_ERR_GENERIC;
+ goto done;
+ }
+
+ if (strncmp(keytab_name, "MEMORY:", sizeof("MEMORY:") -1) == 0) {
+ DEBUG(SSSDBG_TRACE_FUNC, "Keytab [%s] is already memory keytab.\n",
+ keytab_name);
+ *_mem_name = talloc_strdup(mem_ctx, keytab_name);
+ if(*_mem_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+ kerr = KRB5KRB_ERR_GENERIC;
+ goto done;
+ }
+ kerr = 0;
+ goto done;
+ }
+
+ mem_name = talloc_asprintf(mem_ctx, "MEMORY:%s", sep + 1);
+ if (mem_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_asprintf failed.\n");
+ kerr = KRB5KRB_ERR_GENERIC;
+ goto done;
+ }
+
+ kerr = krb5_kt_resolve(kctx, mem_name, &mem_keytab);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "error resolving keytab [%s].\n",
+ mem_name);
+ goto done;
+ }
+
+ memset(&cursor, 0, sizeof(cursor));
+ kerr = krb5_kt_start_seq_get(kctx, keytab, &cursor);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "error reading keytab [%s].\n",
keytab_file);
+ goto done;
+ }
+
+ memset(&entry, 0, sizeof(entry));
+ while ((kt_err = krb5_kt_next_entry(kctx, keytab, &entry, &cursor)) == 0) {
+ kerr = krb5_kt_add_entry(kctx, mem_keytab, &entry);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_OP_FAILURE, "krb5_kt_add_entry failed.\n");
+ goto done;
+ }
+
+ kerr = sss_krb5_free_keytab_entry_contents(kctx, &entry);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "Failed to free keytab entry.\n");
+ }
+ memset(&entry, 0, sizeof(entry));
+ }
+
+ kerr = krb5_kt_end_seq_get(kctx, keytab, &cursor);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "krb5_kt_end_seq_get failed.\n");
+ goto done;
+ }
+
+ /* check if we got any errors from krb5_kt_next_entry */
+ if (kt_err != 0 && kt_err != KRB5_KT_END) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "error reading keytab [%s].\n",
keytab_file);
+ kerr = KRB5KRB_ERR_GENERIC;
+ goto done;
+ }
+
+ *_mem_name = mem_name;
+ kerr = 0;
+done:
+
+ if (kerr != 0) {
+ talloc_free(mem_name);
+ }
+
+// if (mem_keytab != NULL && krb5_kt_close(kctx, mem_keytab) != 0) {
+// DEBUG(SSSDBG_MINOR_FAILURE, "krb5_kt_close failed");
+// }
+ if (keytab != NULL && krb5_kt_close(kctx, keytab) != 0) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "krb5_kt_close failed");
+ }
+
+ return kerr;
+}
--
1.8.3.1
From 6a986d51bb16e7c01c50ea4a58e761c1f67a9d4f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 17 Nov 2014 17:42:07 +0100
Subject: [PATCH 3/3] krb5/ldap: use MEMORY ccache and keytab in *_child
processes
---
Makefile.am | 2 ++
src/providers/krb5/krb5_child.c | 33 +++++++++++++++++++++++++++++++++
src/providers/ldap/ldap_child.c | 37 ++++++++++++++++++++++++++++++-------
3 files changed, 65 insertions(+), 7 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index c264e90..5647c9d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2500,6 +2500,7 @@ libsss_ad_la_LDFLAGS = \
krb5_child_SOURCES = \
src/providers/krb5/krb5_child.c \
src/providers/krb5/krb5_ccache.c \
+ src/providers/krb5/krb5_keytab.c \
src/providers/dp_pam_data_util.c \
src/util/user_info_msg.c \
src/util/sss_krb5.c \
@@ -2532,6 +2533,7 @@ krb5_child_LDADD = \
ldap_child_SOURCES = \
src/providers/ldap/ldap_child.c \
+ src/providers/krb5/krb5_keytab.c \
src/util/sss_krb5.c \
src/util/atomic_io.c \
src/util/authtok.c \
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index b983068..ac6cab6 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1838,6 +1838,7 @@ static int k5c_setup_fast(struct krb5_req *kr, bool
demand)
char *fast_principal;
krb5_error_code kerr;
char *tmp_str;
+ char *new_ccname;
tmp_str = getenv(SSSD_KRB5_FAST_PRINCIPAL);
if (tmp_str) {
@@ -1880,6 +1881,15 @@ static int k5c_setup_fast(struct krb5_req *kr, bool
demand)
return kerr;
}
+ kerr = copy_ccache_into_memory(kr, kr->ctx, kr->fast_ccname, &new_ccname);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "copy_ccache_into_memory failed.\n");
+ return kerr;
+ }
+
+ talloc_free(kr->fast_ccname);
+ kr->fast_ccname = new_ccname;
+
kerr = sss_krb5_get_init_creds_opt_set_fast_ccache_name(kr->ctx,
kr->options,
kr->fast_ccname);
@@ -2062,6 +2072,7 @@ static int k5c_setup(struct krb5_req *kr, uint32_t
offline)
krb5_error_code kerr;
int parse_flags;
enum k5c_fast_opt fast_val;
+ char *mem_keytab;
kerr = check_use_fast(&fast_val);
if (kerr != EOK) {
@@ -2182,6 +2193,28 @@ static int k5c_setup(struct krb5_req *kr, uint32_t
offline)
}
}
+ if (!(offline || (fast_val == K5C_FAST_NEVER && kr->validate == false))) {
+ if (kr->keytab != NULL) {
+ kerr = copy_keytab_into_memory(kr, kr->ctx, kr->keytab,
+ &mem_keytab);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_OP_FAILURE, "copy_keytab_into_memory failed.\n");
+ return kerr;
+ }
+
+ talloc_free(kr->keytab);
+ kr->keytab = mem_keytab;
+ }
+
+ kerr = become_user(kr->uid, kr->gid);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed.\n");
+ return kerr;
+ }
+ }
+ DEBUG(SSSDBG_TRACE_INTERNAL,
+ "Running as [%"SPRIuid"][%"SPRIgid"].\n", geteuid(), getegid());
+
/* TODO: set options, e.g.
* krb5_get_init_creds_opt_set_forwardable
* krb5_get_init_creds_opt_set_proxiable
diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index a922b18..e7febdf 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -33,6 +33,7 @@
#include "util/sss_krb5.h"
#include "util/child_common.h"
#include "providers/dp_backend.h"
+#include "providers/krb5/krb5_common.h"
static krb5_context krb5_error_ctx;
#define LDAP_CHILD_DEBUG(level, error) KRB5_DEBUG(level, krb5_error_ctx, error)
@@ -248,7 +249,7 @@ static int lc_verify_keytab_ex(const char *principal,
static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
const char *realm_str,
const char *princ_str,
- const char *keytab_name,
+ const char *inp_keytab_name,
const krb5_deltat lifetime,
uid_t uid,
gid_t gid,
@@ -277,6 +278,8 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX
*memctx,
char *ccname_file_dummy;
char *ccname_file;
mode_t old_umask;
+ char *keytab_name;
+ char default_keytab_name[MAX_KEYTAB_NAME_LEN];
krberr = krb5_init_context(&context);
if (krberr) {
@@ -291,6 +294,32 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX
*memctx,
goto done;
}
+ if (inp_keytab_name != NULL) {
+ krberr = copy_keytab_into_memory(tmp_ctx, context, inp_keytab_name,
+ &keytab_name);
+ } else {
+ krberr = krb5_kt_default_name(context, default_keytab_name,
+ sizeof(default_keytab_name));
+ if (krberr != 0) {
+ DEBUG(SSSDBG_OP_FAILURE, "krb5_kt_default_name failed.\n");
+ goto done;
+ }
+
+ krberr = copy_keytab_into_memory(tmp_ctx, context, default_keytab_name,
+ &keytab_name);
+ }
+ if (krberr != 0) {
+ DEBUG(SSSDBG_OP_FAILURE, "copy_keytab_into_memory failed.\n");
+ goto done;
+ }
+
+ krberr = become_user(uid, gid);
+ if (krberr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed.\n");
+ goto done;
+ }
+
+
krberr = set_child_debugging(context);
if (krberr != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE, "Cannot set krb5_child debugging\n");
@@ -440,12 +469,6 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX
*memctx,
}
DEBUG(SSSDBG_TRACE_INTERNAL, "credentials initialized\n");
- krberr = become_user(uid, gid);
- if (krberr != 0) {
- DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed.\n");
- goto done;
- }
-
ccname_dummy = talloc_asprintf(tmp_ctx, "FILE:%s", ccname_file_dummy);
ccname = talloc_asprintf(tmp_ctx, "FILE:%s", ccname_file);
if (ccname_dummy == NULL || ccname == NULL) {
--
1.8.3.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
