Hi,

the following two patches are the first step to fix
https://fedorahosted.org/freeipa/ticket/2881. Unit tests with time
measurements are added and the performance of the get_group_sids()
function is improved by an order of magnitude.

The caching of the LDAP results is still missing. I will finish this
after my PTO. But I haven't started to work on this. So if you think it
should be fixed earlier feel free to take the ticket.

bye,
Sumit
From a70dd5049943ae88aba46ef3e95b06a944efcf60 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 6 Jul 2012 12:24:01 +0200
Subject: [PATCH] Add unit tests for ipa-kdb

To improve the performance of some functions used for group filtering we
need a way to see if changes really help or not. This patch adds unit
tests for some of these functions and measures the execution time.
---
 daemons/ipa-kdb/Makefile.am     |   29 +++
 daemons/ipa-kdb/ipa_kdb_mspac.c |   12 +-
 daemons/ipa-kdb/ipa_kdb_tests.c |  548 +++++++++++++++++++++++++++++++++++++++
 util/ipa_pwd.c                  |    2 +
 4 Dateien geändert, 587 Zeilen hinzugefügt(+), 4 Zeilen entfernt(-)
 create mode 100644 daemons/ipa-kdb/ipa_kdb_tests.c

diff --git a/daemons/ipa-kdb/Makefile.am b/daemons/ipa-kdb/Makefile.am
index 
17c090418ec5a0e2a39d948dc385d509c5d05321..34b7d93c4912f988130ae7dd143e054574605071
 100644
--- a/daemons/ipa-kdb/Makefile.am
+++ b/daemons/ipa-kdb/Makefile.am
@@ -50,6 +50,35 @@ ipadb_la_LIBADD =            \
        $(NDRPAC_LIBS)          \
        $(NULL)
 
+if HAVE_CHECK
+TESTS = ipa_kdb_tests
+check_PROGRAMS = ipa_kdb_tests
+endif
+
+ipa_kdb_tests_SOURCES =        \
+       ipa_kdb_tests.c         \
+       ipa_kdb.c               \
+       ipa_kdb_common.c        \
+       ipa_kdb_mkey.c          \
+       ipa_kdb_passwords.c     \
+       ipa_kdb_principals.c    \
+       ipa_kdb_pwdpolicy.c     \
+       ipa_kdb_mspac.c         \
+       ipa_kdb_delegation.c    \
+       ipa_kdb_audit_as.c      \
+       $(KRB5_UTIL_SRCS)       \
+       $(NULL)
+ipa_kdb_tests_CFLAGS = $(CHECK_CFLAGS)
+ipa_kdb_tests_LDADD =          \
+       $(CHECK_LIBS)           \
+       $(KRB5_LIBS)            \
+       $(LDAP_LIBS)            \
+       $(NDRPAC_LIBS)          \
+       -lnss3                  \
+       -lkdb5                  \
+       -lsss_idmap             \
+       $(NULL)
+
 dist_noinst_DATA = ipa_kdb.exports
 
 EXTRA_DIST =                   \
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 
1c7487c3c8f75d02466a2e0746fbef5d36e3d995..ef9aa96943a7e93c21c665e3292a9843fdd8e03d
 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -643,9 +643,9 @@ static char *gen_sid_string(TALLOC_CTX *memctx, struct 
dom_sid *dom_sid,
     return str;
 }
 
-static int get_group_sids(TALLOC_CTX *memctx,
-                          struct PAC_LOGON_INFO_CTR *logon_info,
-                          char ***_group_sids)
+int get_group_sids(TALLOC_CTX *memctx,
+                   struct PAC_LOGON_INFO_CTR *logon_info,
+                   char ***_group_sids)
 {
     int ret;
     size_t c;
@@ -653,6 +653,10 @@ static int get_group_sids(TALLOC_CTX *memctx,
     struct dom_sid *domain_sid = NULL;
     char **group_sids = NULL;
 
+    if (logon_info == NULL || logon_info->info == NULL) {
+        return EINVAL;
+    }
+
     domain_sid = dom_sid_dup(memctx, logon_info->info->info3.base.domain_sid);
     if (domain_sid == NULL) {
         krb5_klog_syslog(LOG_ERR, "dom_sid_dup failed");
@@ -714,7 +718,7 @@ done:
     return ret;
 }
 
-static int add_groups(TALLOC_CTX *memctx,
+int add_groups(TALLOC_CTX *memctx,
                       struct PAC_LOGON_INFO_CTR *logon_info,
                       size_t ipa_group_sids_count,
                       struct dom_sid2 *ipa_group_sids)
diff --git a/daemons/ipa-kdb/ipa_kdb_tests.c b/daemons/ipa-kdb/ipa_kdb_tests.c
new file mode 100644
index 
0000000000000000000000000000000000000000..d3f6c2f38176e2fe3eb081b050c974f351f221ac
--- /dev/null
+++ b/daemons/ipa-kdb/ipa_kdb_tests.c
@@ -0,0 +1,548 @@
+/** BEGIN COPYRIGHT BLOCK
+ * 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/>.
+ *
+ * Additional permission under GPLv3 section 7:
+ *
+ * In the following paragraph, "GPL" means the GNU General Public
+ * License, version 3 or any later version, and "Non-GPL Code" means
+ * code that is governed neither by the GPL nor a license
+ * compatible with the GPL.
+ *
+ * You may link the code of this Program with Non-GPL Code and convey
+ * linked combinations including the two, provided that such Non-GPL
+ * Code only links to the code of this Program through those well
+ * defined interfaces identified in the file named EXCEPTION found in
+ * the source code files (the "Approved Interfaces"). The files of
+ * Non-GPL Code may instantiate templates or use macros or inline
+ * functions from the Approved Interfaces without causing the resulting
+ * work to be covered by the GPL. Only the copyright holders of this
+ * Program may make changes or additions to the list of Approved
+ * Interfaces.
+ *
+ * Authors:
+ * Sumit Bose <sb...@redhat.com>
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ * All rights reserved.
+ * END COPYRIGHT BLOCK **/
+
+#include <check.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <talloc.h>
+#include <sss_idmap.h>
+
+#include "gen_ndr/ndr_krb5pac.h"
+
+#include "ipa_kdb.h"
+
+TALLOC_CTX *global_talloc_context = NULL;
+struct sss_idmap_ctx *global_idmap_ctx = NULL;
+
+#define DOM_SID "S-1-5-21-1234-5678-9012"
+#define OTHER_DOM_SID "S-1-5-21-1111-2222-3333"
+#define PRIMARY_GID 1234
+#define RID_BASE 10000
+
+/* talloc leak checks written by Martin Nagy for sssd */
+#define DLIST_ADD(list, p) \
+do { \
+        if (!(list)) { \
+                (list) = (p); \
+                (p)->next = (p)->prev = NULL; \
+        } else { \
+                (list)->prev = (p); \
+                (p)->next = (list); \
+                (p)->prev = NULL; \
+                (list) = (p); \
+        }\
+} while (0)
+
+#define DLIST_REMOVE(list, p) \
+do { \
+        if ((p) == (list)) { \
+                (list) = (p)->next; \
+                if (list) (list)->prev = NULL; \
+        } else { \
+                if ((p)->prev) (p)->prev->next = (p)->next; \
+                if ((p)->next) (p)->next->prev = (p)->prev; \
+        } \
+        if ((p) != (list)) (p)->next = (p)->prev = NULL; \
+} while (0)
+
+#define check_leaks(ctx, bytes) _check_leaks((ctx), (bytes), __location__)
+void _check_leaks(TALLOC_CTX *ctx,
+                  size_t bytes,
+                  const char *location);
+
+void check_leaks_push(TALLOC_CTX *ctx);
+
+#define check_leaks_pop(ctx) _check_leaks_pop((ctx), __location__)
+void _check_leaks_pop(TALLOC_CTX *ctx, const char *location);
+struct size_snapshot {
+    struct size_snapshot *prev;
+    struct size_snapshot *next;
+
+    TALLOC_CTX *ctx;
+    size_t bytes_allocated;
+};
+
+static struct size_snapshot *snapshot_stack;
+
+void
+_check_leaks(TALLOC_CTX *ctx, size_t bytes, const char *location)
+{
+    size_t bytes_allocated;
+
+    bytes_allocated = talloc_total_size(ctx);
+    if (bytes_allocated != bytes) {
+        fprintf(stderr, "Leak report for %s:\n", location);
+        talloc_report_full(ctx, stderr);
+        fail("%s: memory leaks detected, %d bytes still allocated",
+             location, bytes_allocated - bytes);
+    }
+}
+
+void
+check_leaks_push(TALLOC_CTX *ctx)
+{
+    struct size_snapshot *snapshot;
+
+    snapshot = talloc(NULL, struct size_snapshot);
+    snapshot->ctx = ctx;
+    snapshot->bytes_allocated = talloc_total_size(ctx);
+    DLIST_ADD(snapshot_stack, snapshot);
+}
+
+void
+_check_leaks_pop(TALLOC_CTX *ctx, const char *location)
+{
+    struct size_snapshot *snapshot;
+    TALLOC_CTX *old_ctx;
+    size_t bytes_allocated;
+
+    if (snapshot_stack == NULL) {
+        fail("%s: trying to pop an empty stack");
+    }
+
+    snapshot = snapshot_stack;
+    DLIST_REMOVE(snapshot_stack, snapshot);
+
+    old_ctx = snapshot->ctx;
+    bytes_allocated = snapshot->bytes_allocated;
+
+    fail_if(old_ctx != ctx, "Bad push/pop order");
+
+    talloc_free(snapshot);
+    snapshot = NULL;
+    _check_leaks(old_ctx, bytes_allocated, location);
+}
+
+static void leak_check_setup(void)
+{
+    talloc_enable_null_tracking();
+    global_talloc_context = talloc_new(NULL);
+    fail_unless(global_talloc_context != NULL, "talloc_new failed");
+    check_leaks_push(global_talloc_context);
+}
+
+static void leak_check_teardown(void)
+{
+    check_leaks_pop(global_talloc_context);
+    if (snapshot_stack != NULL) {
+        fail("Exiting with a non-empty stack");
+    }
+    check_leaks(global_talloc_context, 0);
+}
+
+int krb5_klog_syslog(int l, const char *format, ...)
+{
+    va_list ap;
+    char *s = NULL;
+    int ret;
+
+    va_start(ap, format);
+
+    ret = vasprintf(&s, format, ap);
+    va_end(ap);
+    if (ret < 0) {
+        /* ENOMEM */
+        return -1;
+    }
+
+    fprintf(stderr, "%s\n", s);
+    free(s);
+
+    return 0;
+}
+
+int get_group_sids(TALLOC_CTX *memctx,
+                   struct PAC_LOGON_INFO_CTR *logon_info,
+                   char ***_group_sids);
+
+int add_groups(TALLOC_CTX *memctx,
+                      struct PAC_LOGON_INFO_CTR *logon_info,
+                      size_t ipa_group_sids_count,
+                      struct dom_sid2 *ipa_group_sids);
+
+static void *idmap_talloc(size_t size, void *pvt)
+{
+    return talloc_size(pvt, size);
+}
+
+static void idmap_free(void *ptr, void *pvt)
+{
+    talloc_free(ptr);
+}
+
+static int make_dom_sid_array(TALLOC_CTX *mem_ctx, size_t count,
+                              struct dom_sid **_sids)
+{
+    size_t c;
+    char *sid_str;
+    enum idmap_error_code err;
+    struct dom_sid *sids;
+    struct dom_sid *sid;
+
+    sids = talloc_array(mem_ctx, struct dom_sid, count);
+    if (sids == NULL) {
+        return ENOMEM;
+    }
+
+    for (c = 0; c < count; c++) {
+        sid_str = talloc_asprintf(mem_ctx, "%s-%lu", OTHER_DOM_SID,
+                                              (unsigned long) RID_BASE + c);
+        if (sid_str == NULL) {
+            return ENOMEM;
+        }
+        err = sss_idmap_sid_to_smb_sid(global_idmap_ctx, sid_str, &sid);
+        talloc_free(sid_str);
+        if (err != IDMAP_SUCCESS) {
+            return EINVAL;
+        }
+        memcpy(&sids[c], sid, sizeof(struct dom_sid));
+        talloc_free(sid);
+    }
+
+    *_sids = sids;
+    return 0;
+}
+
+static int make_login_info(TALLOC_CTX *mem_ctx, size_t d_group_count,
+                           size_t e_group_count,
+                           struct PAC_LOGON_INFO_CTR  **_logon_info)
+{
+    struct PAC_LOGON_INFO_CTR  *logon_info;
+    enum idmap_error_code err;
+    size_t c;
+    char *sid_str;
+    struct dom_sid *sid;
+
+    logon_info = talloc_zero(mem_ctx, struct PAC_LOGON_INFO_CTR);
+    if (logon_info == NULL) {
+        return ENOMEM;
+    }
+
+    logon_info->info = talloc_zero(logon_info, struct PAC_LOGON_INFO);
+    if (logon_info->info == NULL) {
+        return ENOMEM;
+    }
+
+    err = sss_idmap_sid_to_smb_sid(global_idmap_ctx, DOM_SID, &sid);
+    if (err != IDMAP_SUCCESS) {
+        return EINVAL;
+    }
+    logon_info->info->info3.base.domain_sid = talloc_steal(logon_info->info, 
sid);
+
+    logon_info->info->info3.base.primary_gid = PRIMARY_GID;
+
+    if (d_group_count > 0) {
+        logon_info->info->info3.base.groups.count = d_group_count;
+        logon_info->info->info3.base.groups.rids = talloc_array(logon_info,
+                                                   struct 
samr_RidWithAttribute,
+                                                   d_group_count);
+        if (logon_info->info->info3.base.groups.rids == NULL) {
+            return ENOMEM;
+        }
+
+        for (c = 0; c < d_group_count; c++) {
+            logon_info->info->info3.base.groups.rids[c].rid = RID_BASE + c;
+            logon_info->info->info3.base.groups.rids[c].attributes = 0;
+        }
+    }
+
+    if (e_group_count > 0) {
+        logon_info->info->info3.sidcount = e_group_count;
+        logon_info->info->info3.sids = talloc_array(logon_info,
+                                                    struct netr_SidAttr,
+                                                    e_group_count);
+        if (logon_info->info->info3.sids == NULL) {
+            return ENOMEM;
+        }
+
+        for (c = 0; c < e_group_count; c++) {
+            sid_str = talloc_asprintf(mem_ctx, "%s-%lu", OTHER_DOM_SID,
+                                                  (unsigned long) RID_BASE + 
c);
+            if (sid_str == NULL) {
+                return ENOMEM;
+            }
+            err = sss_idmap_sid_to_smb_sid(global_idmap_ctx, sid_str, &sid);
+            talloc_free(sid_str);
+            if (err != IDMAP_SUCCESS) {
+                return EINVAL;
+            }
+            logon_info->info->info3.sids[c].sid = 
talloc_steal(logon_info->info, sid);
+            logon_info->info->info3.sids[c].attributes = 0;
+        }
+    }
+
+    *_logon_info = logon_info;
+    return 0;
+}
+
+static int check_groups(size_t d_group_count, size_t e_group_count,
+                        char **group_sids)
+{
+    int ret;
+    TALLOC_CTX *tmp_ctx;
+    char *tmp_str = NULL;
+    size_t c;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    tmp_str = talloc_asprintf(tmp_ctx, "%s-%d", DOM_SID, PRIMARY_GID);
+    if (tmp_str == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    if (strcmp(group_sids[0], tmp_str) != 0) {
+        ret = EINVAL;
+        goto done;
+    }
+
+    for (c = 0; c < d_group_count; c++) {
+        tmp_str = talloc_asprintf(tmp_ctx, "%s-%lu", DOM_SID,
+                                                  (unsigned long) RID_BASE + 
c);
+        if (tmp_str == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+
+        if (strcmp(group_sids[1 + c], tmp_str) != 0) {
+            fprintf(stderr, "[%s][%s]\n", group_sids[1 + c], tmp_str);
+            ret = EINVAL;
+            goto done;
+        }
+    }
+
+    for (c = 0; c < e_group_count; c++) {
+        tmp_str = talloc_asprintf(tmp_ctx, "%s-%lu", OTHER_DOM_SID,
+                                                  (unsigned long) RID_BASE + 
c);
+        if (tmp_str == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+
+        if (strcmp(group_sids[1 + d_group_count + c], tmp_str) != 0) {
+            fprintf(stderr, "[%s][%s]\n", group_sids[1 + d_group_count + c],
+                                          tmp_str);
+            ret = EINVAL;
+            goto done;
+        }
+    }
+
+    ret = 0;
+
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
+struct get_groups_test_params {
+    size_t dom_group_count;
+    size_t external_group_count;
+};
+
+START_TEST(test_get_groups)
+{
+    struct PAC_LOGON_INFO_CTR  *logon_info = NULL;
+    char **group_sids = NULL;
+    int ret;
+    size_t c;
+    struct timeval start_time;
+    struct timeval end_time;
+    struct timeval diff_time;
+    struct get_groups_test_params params[] = {
+                {0, 0},
+                {1000, 0},
+                {10000, 0},
+                {100000, 0},
+                {1000000, 0},
+                {0, 1000},
+                {0, 10000},
+                {0, 100000},
+                {0, 1000000},
+                {1000, 1000},
+                {10000, 10000},
+                {100000, 100000},
+                {1000000, 1000000},
+                {(size_t) -1, (size_t) -1}
+    };
+
+    for (c = 0; params[c].dom_group_count != (size_t) - 1; c++) {
+        ret = make_login_info(global_talloc_context, params[c].dom_group_count,
+                              params[c].external_group_count, &logon_info);
+        fail_unless(ret == 0, "make_login_info failed.");
+
+        ret = gettimeofday(&start_time, NULL);
+        fail_unless(ret == 0, "gettimeofday failed.");
+
+        ret = get_group_sids(global_talloc_context, logon_info, &group_sids);
+        fail_unless(ret == 0, "get_group_sids failed [%d][%s].",
+                              ret, strerror(ret));
+
+        ret = gettimeofday(&end_time, NULL);
+        fail_unless(ret == 0, "gettimeofday failed.");
+
+        talloc_free(logon_info);
+        logon_info = NULL;
+
+        fail_unless(check_groups(params[c].dom_group_count,
+                                 params[c].external_group_count,
+                                 group_sids) == 0,
+                    "Unexpected SIDs returned.");
+
+        talloc_free(group_sids);
+        group_sids = NULL;
+
+        timersub(&end_time, &start_time, &diff_time);
+        fprintf(stderr, "get_groups: domain groups [%d] external group [%d] 
duration [%ds %dus]\n",
+                        params[c].dom_group_count,
+                        params[c].external_group_count,
+                        (int) diff_time.tv_sec,
+                        (int) diff_time.tv_usec);
+
+    }
+
+}
+END_TEST
+
+struct add_groups_test_params {
+    size_t dom_group_count;
+};
+
+START_TEST(test_add_groups)
+{
+    struct PAC_LOGON_INFO_CTR  *logon_info = NULL;
+    struct dom_sid *group_sids = NULL;
+    int ret;
+    size_t c;
+    struct timeval start_time;
+    struct timeval end_time;
+    struct timeval diff_time;
+    struct add_groups_test_params params[] = {
+                {0},
+                {1000},
+                {10000},
+                {100000},
+                {1000000},
+                {2000000},
+                {(size_t) -1}
+    };
+
+    for (c = 0; params[c].dom_group_count != (size_t) - 1; c++) {
+        ret = make_login_info(global_talloc_context, 10, 10, &logon_info);
+        fail_unless(ret == 0, "make_login_info failed.");
+
+        ret = make_dom_sid_array(global_talloc_context,
+                                 params[c].dom_group_count, &group_sids);
+        fail_unless(ret == 0, "make_dom_sid_array failed.");
+
+        ret = gettimeofday(&start_time, NULL);
+        fail_unless(ret == 0, "gettimeofday failed.");
+
+        ret = add_groups(global_talloc_context, logon_info,
+                         params[c].dom_group_count, group_sids);
+        fail_unless(ret == 0, "add_groups failed.");
+
+        ret = gettimeofday(&end_time, NULL);
+        fail_unless(ret == 0, "gettimeofday failed.");
+
+        talloc_free(group_sids);
+        group_sids = NULL;
+        talloc_free(logon_info);
+        logon_info = NULL;
+
+        timersub(&end_time, &start_time, &diff_time);
+        fprintf(stderr, "add_groups: domain groups [%d] duration [%ds %dus]\n",
+                        params[c].dom_group_count,
+                        (int) diff_time.tv_sec,
+                        (int) diff_time.tv_usec);
+    }
+
+}
+END_TEST
+
+static void idmap_setup(void)
+{
+    enum idmap_error_code err;
+
+    err = sss_idmap_init(idmap_talloc, global_talloc_context, idmap_free,
+                         &global_idmap_ctx);
+    fail_unless(err == IDMAP_SUCCESS, "sss_idmap_init failed.");
+}
+
+static void idmap_teardown(void)
+{
+    talloc_free(global_idmap_ctx);
+    global_idmap_ctx = NULL;
+}
+
+Suite * ipa_kdb_suite(void)
+{
+    Suite *s = suite_create("IPA kdb");
+
+    TCase *tc_group = tcase_create("Group filtering");
+    tcase_add_checked_fixture(tc_group,
+                              leak_check_setup,
+                              leak_check_teardown);
+    tcase_add_checked_fixture(tc_group,
+                              idmap_setup,
+                              idmap_teardown);
+    tcase_add_test(tc_group, test_get_groups);
+    tcase_add_test(tc_group, test_add_groups);
+    suite_add_tcase(s, tc_group);
+
+    tcase_set_timeout(tc_group, 300);
+    return s;
+}
+
+int main(void)
+{
+    int number_failed;
+
+    Suite *s = ipa_kdb_suite ();
+    SRunner *sr = srunner_create (s);
+    srunner_run_all (sr, CK_VERBOSE);
+    number_failed = srunner_ntests_failed (sr);
+    srunner_free (sr);
+
+    return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c
index 
92fb3b0298418592881d100fb7a9ccfac99fd665..761d1efb8cbcb303d4ec4edd49254b433b048b31
 100644
--- a/util/ipa_pwd.c
+++ b/util/ipa_pwd.c
@@ -20,7 +20,9 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
 #include <stdbool.h>
 #include <stdio.h>
 #include <time.h>
-- 
1.7.10.2

From 6d0decc2fb812583c5e1e210cb63cca047e60378 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 6 Jul 2012 21:40:35 +0200
Subject: [PATCH] Improve performance of get_group_sids()

This patch tries to improve the performance of get_group_sids() by
reducing the number of memory allocations.
---
 daemons/ipa-kdb/ipa_kdb_mspac.c |  110 +++++++++++++--------------------------
 1 Datei geändert, 36 Zeilen hinzugefügt(+), 74 Zeilen entfernt(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 
ef9aa96943a7e93c21c665e3292a9843fdd8e03d..e1144a1d3e1aa559a47a305c1aa14c4462c7899a
 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -169,43 +169,6 @@ static char *dom_sid_string(TALLOC_CTX *memctx, const 
struct dom_sid *dom_sid)
     return buf;
 }
 
-static struct dom_sid *dom_sid_dup(TALLOC_CTX *memctx,
-                                   const struct dom_sid *dom_sid)
-{
-    struct dom_sid *new_sid;
-    size_t c;
-
-    if (dom_sid == NULL) {
-        return NULL;
-    }
-
-    new_sid = talloc(memctx, struct dom_sid);
-    if (new_sid == NULL) {
-        return NULL;
-    }
-
-    new_sid->sid_rev_num = dom_sid->sid_rev_num;
-    for (c = 0; c < SID_ID_AUTHS; c++) {
-        new_sid->id_auth[c] = dom_sid->id_auth[c];
-    }
-    new_sid->num_auths = dom_sid->num_auths;
-    for (c = 0; c < SID_SUB_AUTHS; c++) {
-        new_sid->sub_auths[c] = dom_sid->sub_auths[c];
-    }
-
-    return new_sid;
-}
-
-static int sid_append_rid(struct dom_sid *sid, uint32_t rid)
-{
-    if (sid->num_auths >= SID_SUB_AUTHS) {
-        return EINVAL;
-    }
-
-    sid->sub_auths[sid->num_auths++] = rid;
-    return 0;
-}
-
 /**
 * @brief Takes a user sid and removes the rid.
 *        The sid is changed by this function,
@@ -620,29 +583,6 @@ static bool is_cross_realm_krbtgt(krb5_const_principal 
princ)
     return true;
 }
 
-static char *gen_sid_string(TALLOC_CTX *memctx, struct dom_sid *dom_sid,
-                            uint32_t rid)
-{
-    char *str = NULL;
-    int ret;
-
-    ret = sid_append_rid(dom_sid, rid);
-    if (ret != 0) {
-        krb5_klog_syslog(LOG_ERR, "sid_append_rid failed");
-        return NULL;
-    }
-
-    str = dom_sid_string(memctx, dom_sid);
-    ret = sid_split_rid(dom_sid, NULL);
-    if (ret != 0) {
-        krb5_klog_syslog(LOG_ERR, "sid_split_rid failed");
-        talloc_free(str);
-        return NULL;
-    }
-
-    return str;
-}
-
 int get_group_sids(TALLOC_CTX *memctx,
                    struct PAC_LOGON_INFO_CTR *logon_info,
                    char ***_group_sids)
@@ -650,19 +590,27 @@ int get_group_sids(TALLOC_CTX *memctx,
     int ret;
     size_t c;
     size_t p = 0;
-    struct dom_sid *domain_sid = NULL;
+    char *dom_sid_str = NULL;
     char **group_sids = NULL;
+    char *group_sids_buf = NULL;
+    size_t dom_sid_str_len;
+    size_t sid_len;
+    char *idx;
+
 
     if (logon_info == NULL || logon_info->info == NULL) {
         return EINVAL;
     }
 
-    domain_sid = dom_sid_dup(memctx, logon_info->info->info3.base.domain_sid);
-    if (domain_sid == NULL) {
-        krb5_klog_syslog(LOG_ERR, "dom_sid_dup failed");
+    dom_sid_str = dom_sid_string(memctx,
+                                 logon_info->info->info3.base.domain_sid);
+    if (dom_sid_str == NULL) {
+        krb5_klog_syslog(LOG_ERR, "dom_sid_string failed.");
         ret = ENOMEM;
         goto done;
     }
+    dom_sid_str_len = strlen(dom_sid_str);
+    sid_len = dom_sid_str_len + 12;
 
     group_sids = talloc_array(memctx, char *,
                                      2 +
@@ -674,27 +622,41 @@ int get_group_sids(TALLOC_CTX *memctx,
         goto done;
     }
 
-    group_sids[p] = gen_sid_string(memctx, domain_sid,
-                                  logon_info->info->info3.base.primary_gid);
-    if (group_sids[p] == NULL) {
-        krb5_klog_syslog(LOG_ERR, "gen_sid_string failed");
+    group_sids_buf = talloc_zero_size(group_sids,
+                     sid_len * (1 + 
logon_info->info->info3.base.groups.count));
+    if (group_sids_buf == NULL) {
+        krb5_klog_syslog(LOG_ERR, "talloc_zero failed");
+        ret = ENOMEM;
+        goto done;
+    }
+
+    idx = group_sids_buf+(p*sid_len);
+    memcpy(idx, dom_sid_str, dom_sid_str_len);
+    ret = snprintf(idx+dom_sid_str_len, 12, "-%lu",
+                   (unsigned long) logon_info->info->info3.base.primary_gid);
+    if (ret < 0 || ret >= 12) {
+        krb5_klog_syslog(LOG_ERR, "snprintf failed");
         ret = EINVAL;
         goto done;
     }
+    group_sids[p] = idx;
     p++;
 
     for (c = 0; c < logon_info->info->info3.base.groups.count; c++) {
-        group_sids[p] = gen_sid_string(memctx, domain_sid,
-                               
logon_info->info->info3.base.groups.rids[c].rid);
-        if (group_sids[p] == NULL) {
-        krb5_klog_syslog(LOG_ERR, "gen_sid_string 2 failed");
+        idx = group_sids_buf+(p*sid_len);
+        memcpy(idx, dom_sid_str, dom_sid_str_len);
+        ret = snprintf(idx+dom_sid_str_len, 12, "-%lu",
+               (unsigned long) 
logon_info->info->info3.base.groups.rids[c].rid);
+        if (ret < 0 || ret >= 12) {
+            krb5_klog_syslog(LOG_ERR, "snprintf failed");
             ret = EINVAL;
             goto done;
         }
+        group_sids[p] = idx;
         p++;
     }
     for (c = 0; c < logon_info->info->info3.sidcount; c++) {
-        group_sids[p] = dom_sid_string(memctx,
+        group_sids[p] = dom_sid_string(group_sids,
                                        logon_info->info->info3.sids[c].sid);
         if (group_sids[p] == NULL) {
         krb5_klog_syslog(LOG_ERR, "dom_sid_string failed");
@@ -710,7 +672,7 @@ int get_group_sids(TALLOC_CTX *memctx,
 
     ret = 0;
 done:
-    talloc_free(domain_sid);
+    talloc_free(dom_sid_str);
     if (ret != 0) {
         talloc_free(group_sids);
     }
-- 
1.7.10.2

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to