[SSSD] [PATCH] Check NSCD configuration file
Jakub Hrozek
jhrozek at redhat.com
Tue May 14 14:44:10 UTC 2013
On Mon, May 13, 2013 at 05:00:42PM +0200, Ondrej Kos wrote:
> Hi,
>
> Attached is patch for the following trac issue
> https://fedorahosted.org/sssd/ticket/1785
>
> Please feel free to correct my wording of the syslog messages.
See some comments inline.
> --- a/configure.ac
> +++ b/configure.ac
> @@ -166,6 +166,10 @@ fi
>
> WITH_LIBNL
>
> +if test x$have_nscd; then
> + WITH_NSCD_CONF
> +fi
> +
Where does $have_nscd come from? git grep have_nscd returns nothing.
> WITH_INITSCRIPT
> if test x$initscript = xsystemd; then
> WITH_SYSTEMD_UNIT_DIR
> diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
> index 26eb4acccc4895e961ecc50d490a3cdd5716d085..1315fc9da8761a62402692ae6f272236955c2d1c 100644
> --- a/src/conf_macros.m4
> +++ b/src/conf_macros.m4
> @@ -352,6 +352,18 @@ AC_DEFUN([WITH_NSCD],
> AC_DEFINE_UNQUOTED(HAVE_NSCD, $NSCD_PATH, [flush nscd cache after local domain operations])
> ])
>
> +AC_DEFUN([WITH_NSCD_CONF],
> + [ AC_ARG_WITH([nscd_conf],
> + [AC_HELP_STRING([--with-nscd-conf=PATH],
> + [Path to nscd.conf file [/etc/nscd.conf]])])
> + NSCD_CONF_PATH="/etc/nscd.conf"
> + if test x"$with_nscd_conf" != x; then
> + NSCD_CONF_PATH=$with_nscd_conf
> + AC_SUBST(NSCD_CONF_PATH)
Why is AC_SUBST used here? I don't see any @NSCD_CONF_PATH at .
> + fi
> + AC_DEFINE_UNQUOTED([NSCD_CONF_PATH], ["$NSCD_CONF_PATH"], [NSCD configuration file])
> + ])
> +
> AC_DEFUN([WITH_SEMANAGE],
> [ AC_ARG_WITH([semanage],
> [AC_HELP_STRING([--with-semanage],
> diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
> index bd22a951d3b48d0ea367e36f628b07e2783d469a..6a69a76fe49a4d6d19561b5d4ae5bed97932b396 100644
> --- a/src/monitor/monitor.c
> +++ b/src/monitor/monitor.c
> @@ -2754,12 +2754,29 @@ int main(int argc, const char *argv[])
> /* Warn if nscd seems to be running */
> ret = check_file(NSCD_SOCKET_PATH, -1, -1, -1, CHECK_SOCK, NULL, false);
> if (ret == EOK) {
> - sss_log(SSS_LOG_NOTICE,
> - "nscd socket was detected. Nscd caching capabilities "
> - "may conflict with SSSD for users and groups. It is "
> - "recommended not to run nscd in parallel with SSSD, unless "
> - "nscd is configured not to cache the passwd, group and "
> - "netgroup nsswitch maps.");
> + ret = sss_nscd_parse_conf();
> +
> + switch (ret) {
> + case ENOENT:
> + sss_log(SSS_LOG_NOTICE,
> + "NSCD socket was detected, but NSCD configuration "
> + "file was not found in the specified path [%s] to "
> + "perform check of its caching settings.",
> + NSCD_CONF_PATH);
I would prefer the original sss_log message here instead of an error
message.
> diff --git a/src/util/util.c b/src/util/util.c
> index ba85e0da2a2e03784c5bb9abbbd038ddaeb3455c..8034ee63c632538af74ff7a3238f69cf914a6895 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -688,3 +688,84 @@ void safezero(void *data, size_t size)
> *p++ = 0;
> }
> }
> +
> +/* NSCD config file parse and check */
> +
> +#define NSCD_LINE_BUFF 120
> +
> +enum nscd_service_type {
> + NSCD_PASSWD = 0,
> + NSCD_GROUP,
> + NSCD_NETGROUP,
> + NSCD_SERVICES
> +};
> +
> +unsigned int nscd_service_flag[4] = {
> + 0x0001, /* passwd */
> + 0x0010, /* group */
> + 0x0100, /* netgroup */
> + 0x1000 /* service */
> +};
> +
> +const char *svc_type_name[] = {
> + "passwd",
> + "group",
> + "netgroup",
> + "services"
> +};
> +
There could be only one structure instead:
struct sss_nscd_db {
const char *svc_type_name;
int nscd_service_flag;
};
The in sss_nscd_check_service you'd have:
struct sss_nscd_db db[] = {
{ passwd, 0x0001 },
...
{ NULL, NULL }
};
And the for loop would iterate until db[i].name != NULL.
This way you don't have to sync three structures. The function could
even be inside the function namespace.
> +unsigned int sss_nscd_check_service(char* svc_name)
> +{
> + int i;
> +
> + unsigned int ret = 0;
> +
> + for (i = NSCD_PASSWD; i <= NSCD_SERVICES; i++) {
> + if (!strcmp(svc_type_name[i], svc_name)) {
> +
> + ret |= nscd_service_flag[i];
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +errno_t sss_nscd_parse_conf(void)
> +{
> + FILE *fp;
> + unsigned int occured = 0;
> + char line[NSCD_LINE_BUFF];
> + char part1[40], part2[40], part3[40];
> +
> + fp = fopen(NSCD_CONF_PATH, "r");
> +
> + if (fp == NULL) {
> + DEBUG(SSSDBG_MINOR_FAILURE, ("Couldn't open NSCD configuration "
> + "file [%s]\n", NSCD_CONF_PATH));
> + return ENOENT;
> + }
> +
> + while (fgets(line, NSCD_LINE_BUFF, fp) != NULL) {
What about getline instead of gets?
> +
> + if (line[0] != '#') {
> +
It's more readable to put the main ("success") flow of the program in
the outmost block. So instead of:
if not_a_comment:
process()
it is more readable:
if comment:
continue
process()
> + sscanf(line, "%s %s %s", part1, part2, part3);
Do you know if the parts can only be separated by spaces? I think also
tabs could do.
Also if any part was larger than 40 chars, you'd get a segfault here.
In glibc, the parsing is done in nscd/nscd_conf.c, mostly in
nscd_parse_file() if you'd like to take a look.
More information about the sssd-devel
mailing list