* Re: [devel] Новый Kerberos и новая Samba
2017-11-22 23:46 ` Evgeny Sinelnikov
@ 2017-11-23 11:38 ` Evgeny Sinelnikov
0 siblings, 0 replies; 4+ messages in thread
From: Evgeny Sinelnikov @ 2017-11-23 11:38 UTC (permalink / raw)
To: ALT Linux Team development discussions
[-- Attachment #1: Type: text/plain, Size: 5976 bytes --]
23 ноября 2017 г., 3:46 пользователь Evgeny Sinelnikov
<sin@altlinux.org> написал:
> 22 ноября 2017 г., 20:50 пользователь Alexander Bokovoy
> <ab@altlinux.org> написал:
>> 2017-11-22 16:04 GMT+02:00 Evgeny Sinelnikov <sin@altlinux.org>:
>>> Здравсвуйте,
>>>
>>> В эти выходные в сизиф отправился новый libkrb5. Новая Samba (пока
>>> 4.6.11, готовится 4.7.3) тоже уехала в сизиф и p8.
>>>
>>> В krb5.conf опция includedir для /etc/krb5.conf.d/ теперь включена, по
>>> умолчанию.
>>>
>>> При этом проявилась регрессия, которая имелась и раньше. При включении
>>> includedir проблема проявляется вне зависимости от версии libkrb5. В
>>> p8 на libkrb5-1.14.5 проблема воспроизводится точно также, как и в
>>> сизифе на libkrb5-1.15.2 и для samba-DC-4.6.x выглядит следующим
>>> образом:
>>>
>>> ==> server.domain.alt: + samba-tool domain provision
>>> --realm=DOMAIN.ALT --domain DOMAIN '--adminpass=Pa$$word'
>>> --dns-backend=SAMBA_INTERNAL --server-role=dc --use-rfc2307
>>> --host-ip=192.168.56.2
>>> ==> server.domain.alt: Looking up IPv6 addresses
>>> ==> server.domain.alt: Setting up share.ldb
>>> ==> server.domain.alt: Setting up secrets.ldb
>>> ==> server.domain.alt: Setting up the registry
>>> ==> server.domain.alt: Setting up the privileges database
>>> ==> server.domain.alt: Setting up idmap db
>>> ==> server.domain.alt: Setting up SAM db
>>> ==> server.domain.alt: Setting up sam.ldb partitions and settings
>>> ==> server.domain.alt: Setting up sam.ldb rootDSE
>>> ==> server.domain.alt: Pre-loading the Samba 4 and AD schema
>>> ==> server.domain.alt: Adding DomainDN: DC=domain,DC=alt
>>> ==> server.domain.alt: Adding configuration container
>>> ==> server.domain.alt: Setting up sam.ldb schema
>>> ==> server.domain.alt: Setting up sam.ldb configuration data
>>> ==> server.domain.alt: Setting up display specifiers
>>> ==> server.domain.alt: Modifying display specifiers
>>> ==> server.domain.alt: Adding users container
>>> ==> server.domain.alt: Modifying users container
>>> ==> server.domain.alt: Adding computers container
>>> ==> server.domain.alt: Modifying computers container
>>> ==> server.domain.alt: Setting up sam.ldb data
>>> ==> server.domain.alt: Setting up well known security principals
>>> ==> server.domain.alt: Setting up sam.ldb users and groups
>>> ==> server.domain.alt: ERROR(ldb): uncaught exception - operations
>>> error at ../source4/dsdb/samdb/ldb_modules/password_hash.c:2820
>>> ==> server.domain.alt: File
>>> "/usr/lib64/python2.7/site-packages/samba/netcmd/__init__.py", line
>>> 176, in _run
>>> ==> server.domain.alt: return self.run(*args, **kwargs)
>>> ==> server.domain.alt: File
>>> "/usr/lib64/python2.7/site-packages/samba/netcmd/domain.py", line 471,
>>> in run
>>> ==> server.domain.alt: nosync=ldap_backend_nosync,
>>> ldap_dryrun_mode=ldap_dryrun_mode)
>>> ==> server.domain.alt: File
>>> "/usr/lib64/python2.7/site-packages/samba/provision/__init__.py", line
>>> 2175, in provision
>>> ==> server.domain.alt: skip_sysvolacl=skip_sysvolacl)
>>> ==> server.domain.alt: File
>>> "/usr/lib64/python2.7/site-packages/samba/provision/__init__.py", line
>>> 1787, in provision_fill
>>> ==> server.domain.alt: next_rid=next_rid, dc_rid=dc_rid)
>>> ==> server.domain.alt: File
>>> "/usr/lib64/python2.7/site-packages/samba/provision/__init__.py", line
>>> 1447, in fill_samdb
>>> ==> server.domain.alt: "KRBTGTPASS_B64":
>>> b64encode(krbtgtpass.encode('utf-16-le'))
>>> ==> server.domain.alt: File
>>> "/usr/lib64/python2.7/site-packages/samba/provision/common.py", line
>>> 55, in setup_add_ldif
>>> ==> server.domain.alt: ldb.add_ldif(data, controls)
>>> ==> server.domain.alt: File
>>> "/usr/lib64/python2.7/site-packages/samba/__init__.py", line 225, in
>>> add_ldif
>>> ==> server.domain.alt: self.add(msg, controls)
>>>
>>>
>>> Мы уже с этим сталкивались:
>>> - Не работает создание домена SambaDC
>>> https://bugzilla.altlinux.org/show_bug.cgi?id=33409
>>>
>>> И не мы одни, но при нашем участии:
>>> - [Samba] samba 4.6.0 dc provisioning fails with exception
>>> https://lists.samba.org/archive/samba/2017-March/207031.html
>>> - Uncaught exception at ldb_modules/password_hash.c:2241 during new
>>> domain provision
>>> https://bugzilla.samba.org/show_bug.cgi?id=11573
>>>
>>> В последней баге (Samba #11573) Andrew Bartlett, в итоге, ответил:
>>>> We need to backport:
>>>> https://github.com/heimdal/heimdal/commit/fe43be85587f834266623adb0ecf2793d212a7ca
>> Проще говоря, heimdal до какой-то версии не умел include/includedir.
>>
>>> В целом, "это не бага, а фича". Так сложилось. Причём для samba-4.7,
>>> которая собрана в новой федоре это уже и не актуально. Новая Samba
>>> поддерживает MIT Kerberos, а не только Нeimdal.
>> Думаю, что можно добавить этот патч в Самбу 4.6.3 в сборке. И
>> избавиться от него при переезде на MIT.
>>
>
> Да, я именно так и думал поступить. Спасибо, что укрепили меня в
> логичности этого варианта.
>
Сделал бекпорт - не успел пока проверить:
#195197 TESTED #1 [test-only] sisyphus samba-DC.git=dc/4.6.11-alt2%ubt
--
Sin (Sinelnikov Evgeny)
[-- Attachment #2: 0001-heimdal-add-include-includedir-directives-for-krb5.c.patch --]
[-- Type: text/x-patch, Size: 8246 bytes --]
From 9baaed98f22598648d5fb24cf32dca8eaafea1cd Mon Sep 17 00:00:00 2001
From: Evgeny Sinelnikov <sin@altlinux.org>
Date: Thu, 23 Nov 2017 06:32:36 +0400
Subject: [PATCH 1/2] heimdal: add include/includedir directives for krb5.conf
---
source4/heimdal/lib/krb5/config_file.c | 98 ++++++++++++++++++++++++++++++++--
source4/heimdal/lib/krb5/context.c | 3 +-
source4/heimdal/lib/krb5/krb5_locl.h | 1 +
3 files changed, 98 insertions(+), 4 deletions(-)
diff --git a/source4/heimdal/lib/krb5/config_file.c b/source4/heimdal/lib/krb5/config_file.c
index 4ac25ae..dfd50cd 100644
--- a/source4/heimdal/lib/krb5/config_file.c
+++ b/source4/heimdal/lib/krb5/config_file.c
@@ -41,6 +41,7 @@
/* Gaah! I want a portable funopen */
struct fileptr {
+ krb5_context context;
const char *s;
FILE *f;
};
@@ -363,18 +364,34 @@ krb5_config_parse_debug (struct fileptr *f,
++p;
if (*p == '#' || *p == ';')
continue;
- if (*p == '[') {
+ if (*p == '[') {
ret = parse_section(p, &s, res, err_message);
if (ret)
return ret;
b = NULL;
} else if (*p == '}') {
*err_message = "unmatched }";
- return EINVAL; /* XXX */
+ return KRB5_CONFIG_BADFORMAT;
+ } else if (strncmp(p, "include", sizeof("include") - 1) == 0 &&
+ isspace(p[sizeof("include") - 1])) {
+ p += sizeof("include");
+ while (isspace(*p))
+ p++;
+ ret = krb5_config_parse_file_multi(f->context, p, res);
+ if (ret)
+ return ret;
+ } else if (strncmp(p, "includedir", sizeof("includedir") - 1) == 0 &&
+ isspace(p[sizeof("includedir") - 1])) {
+ p += sizeof("includedir");
+ while (isspace(*p))
+ p++;
+ ret = krb5_config_parse_dir_multi(f->context, p, res);
+ if (ret)
+ return ret;
} else if(*p != '\0') {
if (s == NULL) {
*err_message = "binding before section";
- return EINVAL;
+ return KRB5_CONFIG_BADFORMAT;
}
ret = parse_binding(f, lineno, p, &b, &s->u.list, err_message);
if (ret)
@@ -397,6 +414,64 @@ is_plist_file(const char *fname)
}
/**
+ * Parse configuration files in the given directory and add the result
+ * into res. Only files whose names consist only of alphanumeric
+ * characters, hyphen, and underscore, will be parsed, though files
+ * ending in ".conf" will also be parsed.
+ *
+ * This interface can be used to parse several configuration directories
+ * into one resulting krb5_config_section by calling it repeatably.
+ *
+ * @param context a Kerberos 5 context.
+ * @param dname a directory name to a Kerberos configuration file
+ * @param res the returned result, must be free with krb5_free_config_files().
+ * @return Return an error code or 0, see krb5_get_error_message().
+ *
+ * @ingroup krb5_support
+ */
+
+KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
+krb5_config_parse_dir_multi(krb5_context context,
+ const char *dname,
+ krb5_config_section **res)
+{
+ struct dirent *entry;
+ krb5_error_code ret;
+ DIR *d;
+
+ if ((d = opendir(dname)) == NULL)
+ return errno;
+
+ while ((entry = readdir(d)) != NULL) {
+ char *p = entry->d_name;
+ char *path;
+ int is_valid = 1;
+
+ while (*p) {
+ if (!isalpha(*p) && *p != '_' && *p != '-' &&
+ strcmp(p, ".conf") != 0) {
+ is_valid = 0;
+ break;
+ }
+ p++;
+ }
+ if (!is_valid)
+ continue;
+
+ if (asprintf(&path, "%s/%s", dname, entry->d_name) == -1 ||
+ path == NULL)
+ return krb5_enomem(context);
+ ret = krb5_config_parse_file_multi(context, path, res);
+ free(path);
+ if (ret == ENOMEM)
+ return krb5_enomem(context);;
+ /* Ignore malformed config files */
+ }
+ (void) closedir(d);
+ return 0;
+}
+
+/**
* Parse a configuration file and add the result into res. This
* interface can be used to parse several configuration files into one
* resulting krb5_config_section by calling it repeatably.
@@ -420,6 +495,13 @@ krb5_config_parse_file_multi (krb5_context context,
krb5_error_code ret;
struct fileptr f;
+ if (context->config_include_depth > 5) {
+ krb5_warnx(context, "Maximum config file include depth reached; "
+ "not including %s", fname);
+ return 0;
+ }
+ context->config_include_depth++;
+
/**
* If the fname starts with "~/" parse configuration file in the
* current users home directory. The behavior can be disabled and
@@ -430,6 +512,7 @@ krb5_config_parse_file_multi (krb5_context context,
const char *home = NULL;
if (!_krb5_homedir_access(context)) {
+ context->config_include_depth--;
krb5_set_error_message(context, EPERM,
"Access to home directory not allowed");
return EPERM;
@@ -446,6 +529,7 @@ krb5_config_parse_file_multi (krb5_context context,
if (home) {
asprintf(&newfname, "%s%s", home, &fname[1]);
if (newfname == NULL) {
+ context->config_include_depth--;
krb5_set_error_message(context, ENOMEM,
N_("malloc: out of memory", ""));
return ENOMEM;
@@ -456,6 +540,7 @@ krb5_config_parse_file_multi (krb5_context context,
if (asprintf(&newfname, "%%{USERCONFIG}%s", &fname[1]) < 0 ||
newfname == NULL)
{
+ context->config_include_depth--;
krb5_set_error_message(context, ENOMEM,
N_("malloc: out of memory", ""));
return ENOMEM;
@@ -467,6 +552,7 @@ krb5_config_parse_file_multi (krb5_context context,
if (is_plist_file(fname)) {
#ifdef __APPLE__
ret = parse_plist_config(context, fname, res);
+ context->config_include_depth--;
if (ret) {
krb5_set_error_message(context, ret,
"Failed to parse plist %s", fname);
@@ -485,6 +571,7 @@ krb5_config_parse_file_multi (krb5_context context,
ret = _krb5_expand_path_tokens(context, fname, &exp_fname);
if (ret) {
+ context->config_include_depth--;
if (newfname)
free(newfname);
return ret;
@@ -495,9 +582,11 @@ krb5_config_parse_file_multi (krb5_context context,
fname = newfname = exp_fname;
#endif
+ f.context = context;
f.f = fopen(fname, "r");
f.s = NULL;
if(f.f == NULL) {
+ context->config_include_depth--;
ret = errno;
krb5_set_error_message (context, ret, "open %s: %s",
fname, strerror(ret));
@@ -507,6 +596,7 @@ krb5_config_parse_file_multi (krb5_context context,
}
ret = krb5_config_parse_debug (&f, res, &lineno, &str);
+ context->config_include_depth--;
fclose(f.f);
if (ret) {
krb5_set_error_message (context, ret, "%s:%u: %s",
@@ -1310,6 +1400,8 @@ krb5_config_parse_string_multi(krb5_context context,
unsigned lineno = 0;
krb5_error_code ret;
struct fileptr f;
+
+ f.context = context;
f.f = NULL;
f.s = string;
diff --git a/source4/heimdal/lib/krb5/context.c b/source4/heimdal/lib/krb5/context.c
index 23e3879..770f012 100644
--- a/source4/heimdal/lib/krb5/context.c
+++ b/source4/heimdal/lib/krb5/context.c
@@ -646,7 +646,8 @@ krb5_set_config_files(krb5_context context, char **filenames)
krb5_config_binding *tmp = NULL;
while(filenames != NULL && *filenames != NULL && **filenames != '\0') {
ret = krb5_config_parse_file_multi(context, *filenames, &tmp);
- if(ret != 0 && ret != ENOENT && ret != EACCES && ret != EPERM) {
+ if (ret != 0 && ret != ENOENT && ret != EACCES && ret != EPERM
+ && ret != KRB5_CONFIG_BADFORMAT) {
krb5_config_file_free(context, tmp);
return ret;
}
diff --git a/source4/heimdal/lib/krb5/krb5_locl.h b/source4/heimdal/lib/krb5/krb5_locl.h
index 49c614d..f9c40e3 100644
--- a/source4/heimdal/lib/krb5/krb5_locl.h
+++ b/source4/heimdal/lib/krb5/krb5_locl.h
@@ -262,6 +262,7 @@ typedef struct krb5_context_data {
int32_t kdc_sec_offset;
int32_t kdc_usec_offset;
krb5_config_section *cf;
+ size_t config_include_depth;
struct et_list *et_list;
struct krb5_log_facility *warn_dest;
struct krb5_log_facility *debug_dest;
--
2.10.2
[-- Attachment #3: 0002-heimdal-fix-CR-comments-on-include-includedir.patch --]
[-- Type: text/x-patch, Size: 6573 bytes --]
From cf32f7d532952b2bbd661b49bb5f0853d1462294 Mon Sep 17 00:00:00 2001
From: Evgeny Sinelnikov <sin@altlinux.org>
Date: Thu, 23 Nov 2017 06:45:58 +0400
Subject: [PATCH 2/2] heimdal: fix CR comments on include/includedir
---
source4/heimdal/lib/krb5/config_file.c | 95 +++++++++++++++++++++++++++++-----
source4/heimdal/lib/krb5/krb5_locl.h | 7 +++
2 files changed, 88 insertions(+), 14 deletions(-)
diff --git a/source4/heimdal/lib/krb5/config_file.c b/source4/heimdal/lib/krb5/config_file.c
index dfd50cd..4bac296 100644
--- a/source4/heimdal/lib/krb5/config_file.c
+++ b/source4/heimdal/lib/krb5/config_file.c
@@ -337,6 +337,41 @@ parse_plist_config(krb5_context context, const char *path, krb5_config_section *
#endif
+static int
+is_absolute_path(const char *path)
+{
+ /*
+ * An absolute path is one that refers to an explicit object
+ * without ambiguity.
+ */
+#ifdef WIN32
+ size_t len = strlen(path);
+
+ /* UNC path is by definition absolute */
+ if (len > 2
+ && ISPATHSEP(path[0])
+ && ISPATHSEP(path[1]))
+ return 1;
+
+ /* A drive letter path might be absolute */
+ if (len > 3
+ && isalpha(path[0])
+ && path[1] == ':'
+ && ISPATHSEP(path[2]))
+ return 1;
+
+ /*
+ * if no drive letter but first char is a path
+ * separator then the drive letter must be obtained
+ * from the including file.
+ */
+#else
+ /* UNIX is easy, first char '/' is absolute */
+ if (ISPATHSEP(path[0]))
+ return 1;
+#endif
+ return 0;
+}
/*
* Parse the config file `fname', generating the structures into `res'
@@ -377,6 +412,12 @@ krb5_config_parse_debug (struct fileptr *f,
p += sizeof("include");
while (isspace(*p))
p++;
+ if (!is_absolute_path(p)) {
+ krb5_set_error_message(f->context, EINVAL,
+ "Configuration include path must be "
+ "absolute");
+ return EINVAL;
+ }
ret = krb5_config_parse_file_multi(f->context, p, res);
if (ret)
return ret;
@@ -385,6 +426,12 @@ krb5_config_parse_debug (struct fileptr *f,
p += sizeof("includedir");
while (isspace(*p))
p++;
+ if (!is_absolute_path(p)) {
+ krb5_set_error_message(f->context, EINVAL,
+ "Configuration includedir path must be "
+ "absolute");
+ return EINVAL;
+ }
ret = krb5_config_parse_dir_multi(f->context, p, res);
if (ret)
return ret;
@@ -448,7 +495,14 @@ krb5_config_parse_dir_multi(krb5_context context,
int is_valid = 1;
while (*p) {
- if (!isalpha(*p) && *p != '_' && *p != '-' &&
+ /*
+ * Here be dragons. The call to krb5_config_parse_file_multi()
+ * below expands path tokens. Because of the limitations here
+ * on file naming, we can't have path tokens in the file name,
+ * so we're safe. Anyone changing this if condition here should
+ * be aware.
+ */
+ if (!isalnum(*p) && *p != '_' && *p != '-' &&
strcmp(p, ".conf") != 0) {
is_valid = 0;
break;
@@ -459,13 +513,17 @@ krb5_config_parse_dir_multi(krb5_context context,
continue;
if (asprintf(&path, "%s/%s", dname, entry->d_name) == -1 ||
- path == NULL)
+ path == NULL) {
+ (void) closedir(d);
return krb5_enomem(context);
+ }
ret = krb5_config_parse_file_multi(context, path, res);
free(path);
- if (ret == ENOMEM)
+ if (ret == ENOMEM) {
+ (void) closedir(d);
return krb5_enomem(context);;
- /* Ignore malformed config files */
+ }
+ /* Ignore malformed config files so we don't lock out admins, etc... */
}
(void) closedir(d);
return 0;
@@ -494,6 +552,7 @@ krb5_config_parse_file_multi (krb5_context context,
unsigned lineno = 0;
krb5_error_code ret;
struct fileptr f;
+ struct stat st;
if (context->config_include_depth > 5) {
krb5_warnx(context, "Maximum config file include depth reached; "
@@ -550,14 +609,13 @@ krb5_config_parse_file_multi (krb5_context context,
}
if (is_plist_file(fname)) {
+ context->config_include_depth--;
#ifdef __APPLE__
ret = parse_plist_config(context, fname, res);
- context->config_include_depth--;
if (ret) {
krb5_set_error_message(context, ret,
"Failed to parse plist %s", fname);
- if (newfname)
- free(newfname);
+ free(newfname);
return ret;
}
#else
@@ -585,24 +643,33 @@ krb5_config_parse_file_multi (krb5_context context,
f.context = context;
f.f = fopen(fname, "r");
f.s = NULL;
- if(f.f == NULL) {
+ if (f.f == NULL || fstat(fileno(f.f), &st) == -1) {
+ if (f.f != NULL)
+ (void) fclose(f.f);
context->config_include_depth--;
ret = errno;
- krb5_set_error_message (context, ret, "open %s: %s",
- fname, strerror(ret));
- if (newfname)
- free(newfname);
+ krb5_set_error_message(context, ret, "open or stat %s: %s",
+ fname, strerror(ret));
+ free(newfname);
return ret;
}
+ if (!S_ISREG(st.st_mode)) {
+ (void) fclose(f.f);
+ context->config_include_depth--;
+ free(newfname);
+ krb5_set_error_message(context, EISDIR, "not a regular file %s: %s",
+ fname, strerror(EISDIR));
+ return EISDIR;
+ }
+
ret = krb5_config_parse_debug (&f, res, &lineno, &str);
context->config_include_depth--;
fclose(f.f);
if (ret) {
krb5_set_error_message (context, ret, "%s:%u: %s",
fname, lineno, str);
- if (newfname)
- free(newfname);
+ free(newfname);
return ret;
}
}
diff --git a/source4/heimdal/lib/krb5/krb5_locl.h b/source4/heimdal/lib/krb5/krb5_locl.h
index f9c40e3..ab0cf87 100644
--- a/source4/heimdal/lib/krb5/krb5_locl.h
+++ b/source4/heimdal/lib/krb5/krb5_locl.h
@@ -358,4 +358,11 @@ enum krb5_pk_type {
#endif /* PKINIT */
+#define ISTILDE(x) (x == '~')
+#ifdef _WIN32
+# define ISPATHSEP(x) (x == '/' || x =='\\')
+#else
+# define ISPATHSEP(x) (x == '/')
+#endif
+
#endif /* __KRB5_LOCL_H__ */
--
2.10.2
^ permalink raw reply [flat|nested] 4+ messages in thread