[Freeipa-devel] [PATCH] Improve performance of get_group_sids()
Sumit Bose
sbose at redhat.com
Tue Jul 10 21:04:20 UTC 2012
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
-------------- next part --------------
From a70dd5049943ae88aba46ef3e95b06a944efcf60 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at 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 <sbose at 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
-------------- next part --------------
From 6d0decc2fb812583c5e1e210cb63cca047e60378 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at 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
More information about the Freeipa-devel
mailing list