From: Evgeny Sinelnikov <sin@altlinux.org> To: ALT Linux Team development discussions <devel@lists.altlinux.org> Subject: Re: [devel] Новый Kerberos и новая Samba Date: Thu, 23 Nov 2017 15:38:38 +0400 Message-ID: <CAK42-Gp+jbnUyMN0uP+G0ko3UHSaKQOko64HFioZ7HSC3_qTcQ@mail.gmail.com> (raw) In-Reply-To: <CAK42-Go=H4Y5epTkiM0q9JDtbG=tJVRwawRQsMRghut42A7Q6g@mail.gmail.com> [-- 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
prev parent reply other threads:[~2017-11-23 11:38 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-11-22 14:04 Evgeny Sinelnikov 2017-11-22 16:50 ` Alexander Bokovoy 2017-11-22 23:46 ` Evgeny Sinelnikov 2017-11-23 11:38 ` Evgeny Sinelnikov [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAK42-Gp+jbnUyMN0uP+G0ko3UHSaKQOko64HFioZ7HSC3_qTcQ@mail.gmail.com \ --to=sin@altlinux.org \ --cc=devel@lists.altlinux.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
ALT Linux Team development discussions This inbox may be cloned and mirrored by anyone: git clone --mirror http://lore.altlinux.org/devel/0 devel/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 devel devel/ http://lore.altlinux.org/devel \ devel@altlinux.org devel@altlinux.ru devel@lists.altlinux.org devel@lists.altlinux.ru devel@linux.iplabs.ru mandrake-russian@linuxteam.iplabs.ru sisyphus@linuxteam.iplabs.ru public-inbox-index devel Example config snippet for mirrors. Newsgroup available over NNTP: nntp://lore.altlinux.org/org.altlinux.lists.devel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git