Hi,
attached are patches that in my opinion make libipa_hbac easier to use
by external projects like pam_hbac.
[PATCH 1/4] libipa_hbac: Do not use C99
- even though we use C99 in the deamon, pam_hbac needs to run on very
exotic platforms. I don't think the code readability is decreased too
much
[PATCH 2/4] libipa_hbac: Add more debug messages
- just adds debug messages
[PATCH 3/4] libipa_hbac: Fix typo in constant name
- fixes a typo that breaks compilation on non-GNU systems
[PATCH 4/4] libipa_hbac: Move the library to src/lib/ipa_hbac
- This might be a bit controversial, but in my opinion a library
shouldn't be present in the provider subdirectory. Please also note
I added "I$(top_srcdir)/src/util", this is just so that I can copy the
libipa_hbac code without any changes to pam_hbac (even changes to
header file locations..)
>From 3ae3c1fba3e14d3299b121c03c166489fba55741 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 22 Feb 2016 09:44:19 +0100
Subject: [PATCH 1/4] libipa_hbac: Do not use C99
libipa_hbac can be used by external consumers like pam_hbac who run on
old platforms that do not support C99. Refrain from using C99 features
in that codebase.
---
src/providers/ipa/hbac_evaluator.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/providers/ipa/hbac_evaluator.c
b/src/providers/ipa/hbac_evaluator.c
index
f4b76d8e6ca3b63b092f65cdb4aac9e98e8c717e..36376abb4eb834681cf62030c2bc1ba11e1369ce
100644
--- a/src/providers/ipa/hbac_evaluator.c
+++ b/src/providers/ipa/hbac_evaluator.c
@@ -146,6 +146,8 @@ enum hbac_eval_result hbac_evaluate(struct hbac_rule
**rules,
struct hbac_eval_req *hbac_req,
struct hbac_info **info)
{
+ uint32_t i;
+
enum hbac_error_code ret;
enum hbac_eval_result result = HBAC_EVAL_DENY;
enum hbac_eval_result_int intermediate_result;
@@ -163,7 +165,7 @@ enum hbac_eval_result hbac_evaluate(struct hbac_rule
**rules,
(*info)->rule_name = NULL;
}
- for (uint32_t i = 0; rules[i]; i++) {
+ for (i = 0; rules[i]; i++) {
hbac_rule_debug_print(rules[i]);
intermediate_result = hbac_evaluate_rule(rules[i], hbac_req, &ret);
if (intermediate_result == HBAC_EVAL_UNMATCHED) {
@@ -381,6 +383,8 @@ const char *hbac_error_string(enum hbac_error_code code)
static void hbac_request_element_debug_print(struct hbac_request_element *el,
const char *label)
{
+ int i;
+
if (el) {
if (el->name) {
HBAC_DEBUG(HBAC_DBG_TRACE, "\t\t%s [%s]\n", label, el->name);
@@ -389,7 +393,7 @@ static void hbac_request_element_debug_print(struct
hbac_request_element *el,
if (el->groups) {
if (el->groups[0]) {
HBAC_DEBUG(HBAC_DBG_TRACE, "\t\t%s_group:\n", label);
- for (int i = 0; el->groups[i]; i++) {
+ for (i = 0; el->groups[i]; i++) {
HBAC_DEBUG(HBAC_DBG_TRACE, "\t\t\t[%s]\n", el->groups[i]);
}
} else {
@@ -434,6 +438,8 @@ static void hbac_req_debug_print(struct hbac_eval_req *req)
static void hbac_rule_element_debug_print(struct hbac_rule_element *el,
const char *label)
{
+ int i;
+
if (el) {
HBAC_DEBUG(HBAC_DBG_TRACE, "\t\tcategory [%#x] [%s]\n", el->category,
(el->category == HBAC_CATEGORY_ALL) ? "ALL" : "NONE");
@@ -441,7 +447,7 @@ static void hbac_rule_element_debug_print(struct
hbac_rule_element *el,
if (el->names) {
if (el->names[0]) {
HBAC_DEBUG(HBAC_DBG_TRACE, "\t\t%s_names:\n", label);
- for (int i = 0; el->names[i]; i++) {
+ for (i = 0; el->names[i]; i++) {
HBAC_DEBUG(HBAC_DBG_TRACE, "\t\t\t[%s]\n", el->names[i]);
}
} else {
@@ -452,7 +458,7 @@ static void hbac_rule_element_debug_print(struct
hbac_rule_element *el,
if (el->groups) {
if (el->groups[0]) {
HBAC_DEBUG(HBAC_DBG_TRACE, "\t\t%s_groups:\n", label);
- for (int i = 0; el->groups[i]; i++) {
+ for (i = 0; el->groups[i]; i++) {
HBAC_DEBUG(HBAC_DBG_TRACE, "\t\t\t[%s]\n", el->groups[i]);
}
} else {
--
2.4.3
>From 58bf614791539f407495a4b09cb4cccc0bf1f51f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Thu, 10 Mar 2016 07:38:06 +0100
Subject: [PATCH 2/4] libipa_hbac: Add more debug messages
Adding more debug messages proved to be useful during pam_hbac
development.
---
src/providers/ipa/hbac_evaluator.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/providers/ipa/hbac_evaluator.c
b/src/providers/ipa/hbac_evaluator.c
index
36376abb4eb834681cf62030c2bc1ba11e1369ce..271b170faba36fc531c312c24061e7250d5ebe93
100644
--- a/src/providers/ipa/hbac_evaluator.c
+++ b/src/providers/ipa/hbac_evaluator.c
@@ -189,8 +189,8 @@ enum hbac_eval_result hbac_evaluate(struct hbac_rule
**rules,
} else {
/* An error occurred processing this rule */
HBAC_DEBUG(HBAC_DBG_ERROR,
- "Error occurred during evaluating of rule [%s].\n",
- rules[i]->name);
+ "Error %d occurred during evaluating of rule [%s].\n",
+ ret, rules[i]->name);
result = HBAC_EVAL_ERROR;
if (info) {
(*info)->code = ret;
@@ -223,13 +223,19 @@ enum hbac_eval_result_int hbac_evaluate_rule(struct
hbac_rule *rule,
errno_t ret;
bool matched;
- if (!rule->enabled) return HBAC_EVAL_UNMATCHED;
+ if (!rule->enabled) {
+ HBAC_DEBUG(HBAC_DBG_INFO, "Rule [%s] is not enabled\n", rule->name);
+ return HBAC_EVAL_UNMATCHED;
+ }
/* Make sure we have all elements */
if (!rule->users
|| !rule->services
|| !rule->targethosts
|| !rule->srchosts) {
+ HBAC_DEBUG(HBAC_DBG_INFO,
+ "Rule [%s] cannot be parsed, some elements are empty\n",
+ rule->name);
*error = HBAC_ERROR_UNPARSEABLE_RULE;
return HBAC_EVAL_MATCH_ERROR;
}
@@ -239,6 +245,8 @@ enum hbac_eval_result_int hbac_evaluate_rule(struct
hbac_rule *rule,
hbac_req->user,
&matched);
if (ret != EOK) {
+ HBAC_DEBUG(HBAC_DBG_ERROR,
+ "Cannot parse user elements of rule [%s]\n", rule->name);
*error = HBAC_ERROR_UNPARSEABLE_RULE;
return HBAC_EVAL_MATCH_ERROR;
} else if (!matched) {
@@ -250,6 +258,8 @@ enum hbac_eval_result_int hbac_evaluate_rule(struct
hbac_rule *rule,
hbac_req->service,
&matched);
if (ret != EOK) {
+ HBAC_DEBUG(HBAC_DBG_ERROR,
+ "Cannot parse service elements of rule [%s]\n", rule->name);
*error = HBAC_ERROR_UNPARSEABLE_RULE;
return HBAC_EVAL_MATCH_ERROR;
} else if (!matched) {
@@ -261,6 +271,9 @@ enum hbac_eval_result_int hbac_evaluate_rule(struct
hbac_rule *rule,
hbac_req->targethost,
&matched);
if (ret != EOK) {
+ HBAC_DEBUG(HBAC_DBG_ERROR,
+ "Cannot parse targethost elements of rule [%s]\n",
+ rule->name);
*error = HBAC_ERROR_UNPARSEABLE_RULE;
return HBAC_EVAL_MATCH_ERROR;
} else if (!matched) {
@@ -272,6 +285,9 @@ enum hbac_eval_result_int hbac_evaluate_rule(struct
hbac_rule *rule,
hbac_req->srchost,
&matched);
if (ret != EOK) {
+ HBAC_DEBUG(HBAC_DBG_ERROR,
+ "Cannot parse srchost elements of rule [%s]\n",
+ rule->name);
*error = HBAC_ERROR_UNPARSEABLE_RULE;
return HBAC_EVAL_MATCH_ERROR;
} else if (!matched) {
--
2.4.3
>From 36496d9b8b7850baab9d8c67f35e5d699379c0a6 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Thu, 10 Mar 2016 07:45:28 +0100
Subject: [PATCH 3/4] libipa_hbac: Fix typo in constant name
On platforms without the format attribute, libhbac could not be
compiled.
---
src/providers/ipa/ipa_hbac.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/providers/ipa/ipa_hbac.h b/src/providers/ipa/ipa_hbac.h
index
ee5f1919b0e216bd6dc09e26b1488fc4bf2d28ef..22dd8ffc401d2e6a2b6e4f83e99447b50192102b
100644
--- a/src/providers/ipa/ipa_hbac.h
+++ b/src/providers/ipa/ipa_hbac.h
@@ -53,7 +53,7 @@ enum hbac_debug_level {
#ifdef HAVE_FUNCTION_ATTRIBUTE_FORMAT
#define HBAC_ATTRIBUTE_PRINTF(a1, a2) __attribute__((format(printf, a1, a2)))
#else
-#define HABC_ATTRIBUTE_PRINTF(a1, a2)
+#define HBAC_ATTRIBUTE_PRINTF(a1, a2)
#endif
/**
--
2.4.3
>From 606e66fe7c18a9e78cd1709f6a95e3a53be24991 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Thu, 10 Mar 2016 08:19:58 +0100
Subject: [PATCH 4/4] libipa_hbac: Move the library to src/lib/ipa_hbac
Moving the library to the lib directory will force maintainers to think
twice about changes, because it would be obvious this is a library.
Also don't use includes from sssd source tree paths, but add the util
path to Makefile's CFLAGS so that other projects can copy the
hbac_evaluator.c file verbatim.
---
Makefile.am | 18 +++++++++++-------
configure.ac | 2 +-
src/{providers/ipa => lib/ipa_hbac}/hbac_evaluator.c | 4 ++--
src/{providers/ipa => lib/ipa_hbac}/ipa_hbac.doxy.in | 0
src/{providers/ipa => lib/ipa_hbac}/ipa_hbac.exports | 0
src/{providers/ipa => lib/ipa_hbac}/ipa_hbac.h | 0
src/{providers/ipa => lib/ipa_hbac}/ipa_hbac.pc.in | 0
src/providers/ipa/ipa_access.c | 1 -
src/providers/ipa/ipa_hbac_common.c | 1 -
src/providers/ipa/ipa_hbac_private.h | 2 +-
src/python/pyhbac.c | 2 +-
src/tests/ipa_hbac-tests.c | 2 +-
12 files changed, 17 insertions(+), 15 deletions(-)
rename src/{providers/ipa => lib/ipa_hbac}/hbac_evaluator.c (99%)
rename src/{providers/ipa => lib/ipa_hbac}/ipa_hbac.doxy.in (100%)
rename src/{providers/ipa => lib/ipa_hbac}/ipa_hbac.exports (100%)
rename src/{providers/ipa => lib/ipa_hbac}/ipa_hbac.h (100%)
rename src/{providers/ipa => lib/ipa_hbac}/ipa_hbac.pc.in (100%)
diff --git a/Makefile.am b/Makefile.am
index
d6eb0fc732a3b566dba91952bd6598a31f8d8d3e..8b554cf6a93d141b82aad85cf67d38b3bad2b016
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -941,18 +941,22 @@ lib_LTLIBRARIES = libipa_hbac.la \
libsss_nss_idmap.la \
$(NULL)
-pkgconfig_DATA += src/providers/ipa/ipa_hbac.pc
-libipa_hbac_la_DEPENDENCIES = src/providers/ipa/ipa_hbac.exports
+pkgconfig_DATA += src/lib/ipa_hbac/ipa_hbac.pc
+libipa_hbac_la_DEPENDENCIES = src/lib/ipa_hbac/ipa_hbac.exports
libipa_hbac_la_SOURCES = \
- src/providers/ipa/hbac_evaluator.c \
+ src/lib/ipa_hbac/hbac_evaluator.c \
src/util/sss_utf8.c
+libipa_hbac_la_CFLAGS = \
+ $(AM_CFLAGS) \
+ -I$(top_srcdir)/src/util \
+ $(NULL)
libipa_hbac_la_LIBADD = \
$(UNICODE_LIBS)
libipa_hbac_la_LDFLAGS = \
- -Wl,--version-script,$(srcdir)/src/providers/ipa/ipa_hbac.exports \
+ -Wl,--version-script,$(srcdir)/src/lib/ipa_hbac/ipa_hbac.exports \
-version-info 1:0:1
-dist_noinst_DATA += src/providers/ipa/ipa_hbac.exports
+dist_noinst_DATA += src/lib/ipa_hbac/ipa_hbac.exports
pkgconfig_DATA += src/lib/idmap/sss_idmap.pc
libsss_idmap_la_DEPENDENCIES = src/lib/idmap/sss_idmap.exports
@@ -981,7 +985,7 @@ libsss_nss_idmap_la_LDFLAGS = \
dist_noinst_DATA += src/sss_client/idmap/sss_nss_idmap.exports
include_HEADERS = \
- src/providers/ipa/ipa_hbac.h \
+ src/lib/ipa_hbac/ipa_hbac.h \
src/lib/idmap/sss_idmap.h \
src/sss_client/idmap/sss_nss_idmap.h \
$(NULL)
@@ -3556,7 +3560,7 @@ endif
if HAVE_DOXYGEN
docs:
$(DOXYGEN) src/doxy.config
- $(DOXYGEN) src/providers/ipa/ipa_hbac.doxy
+ $(DOXYGEN) src/lib/ipa_hbac/ipa_hbac.doxy
$(DOXYGEN) src/lib/idmap/sss_idmap.doxy
$(DOXYGEN) src/sss_client/idmap/sss_nss_idmap.doxy
if BUILD_IFP
diff --git a/configure.ac b/configure.ac
index
9b674ba02b2348fe2e1d61ecfc3f5084098b0f23..2c36049ca04f57fe94f359c5dc19a506dd2b9388
100644
--- a/configure.ac
+++ b/configure.ac
@@ -443,7 +443,7 @@ AC_CONFIG_FILES([Makefile contrib/sssd.spec
src/examples/rwtab src/doxy.config
src/sysv/sssd src/sysv/gentoo/sssd src/sysv/SUSE/sssd
po/Makefile.in src/man/Makefile src/tests/cwrap/Makefile
src/tests/intg/Makefile
- src/providers/ipa/ipa_hbac.pc src/providers/ipa/ipa_hbac.doxy
+ src/lib/ipa_hbac/ipa_hbac.pc src/lib/ipa_hbac/ipa_hbac.doxy
src/lib/idmap/sss_idmap.pc src/lib/idmap/sss_idmap.doxy
src/sss_client/idmap/sss_nss_idmap.pc
src/sss_client/idmap/sss_nss_idmap.doxy
diff --git a/src/providers/ipa/hbac_evaluator.c
b/src/lib/ipa_hbac/hbac_evaluator.c
similarity index 99%
rename from src/providers/ipa/hbac_evaluator.c
rename to src/lib/ipa_hbac/hbac_evaluator.c
index
271b170faba36fc531c312c24061e7250d5ebe93..a71c453e3f07e9da407bb35e5e324cb27ba85e27
100644
--- a/src/providers/ipa/hbac_evaluator.c
+++ b/src/lib/ipa_hbac/hbac_evaluator.c
@@ -28,8 +28,8 @@
#include <stdlib.h>
#include <string.h>
#include <errno.h>
-#include "providers/ipa/ipa_hbac.h"
-#include "util/sss_utf8.h"
+#include "ipa_hbac.h"
+#include "sss_utf8.h"
#ifndef HAVE_ERRNO_T
#define HAVE_ERRNO_T
diff --git a/src/providers/ipa/ipa_hbac.doxy.in
b/src/lib/ipa_hbac/ipa_hbac.doxy.in
similarity index 100%
rename from src/providers/ipa/ipa_hbac.doxy.in
rename to src/lib/ipa_hbac/ipa_hbac.doxy.in
diff --git a/src/providers/ipa/ipa_hbac.exports
b/src/lib/ipa_hbac/ipa_hbac.exports
similarity index 100%
rename from src/providers/ipa/ipa_hbac.exports
rename to src/lib/ipa_hbac/ipa_hbac.exports
diff --git a/src/providers/ipa/ipa_hbac.h b/src/lib/ipa_hbac/ipa_hbac.h
similarity index 100%
rename from src/providers/ipa/ipa_hbac.h
rename to src/lib/ipa_hbac/ipa_hbac.h
diff --git a/src/providers/ipa/ipa_hbac.pc.in b/src/lib/ipa_hbac/ipa_hbac.pc.in
similarity index 100%
rename from src/providers/ipa/ipa_hbac.pc.in
rename to src/lib/ipa_hbac/ipa_hbac.pc.in
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index
ad12f21fc36e52a2e8d7affcfc7128f018c50e4c..3ec5a8df8b2cdc027860ab808b26433142c13e24
100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -30,7 +30,6 @@
#include "providers/ldap/sdap_access.h"
#include "providers/ipa/ipa_common.h"
#include "providers/ipa/ipa_access.h"
-#include "providers/ipa/ipa_hbac.h"
#include "providers/ipa/ipa_hosts.h"
#include "providers/ipa/ipa_hbac_private.h"
#include "providers/ipa/ipa_hbac_rules.h"
diff --git a/src/providers/ipa/ipa_hbac_common.c
b/src/providers/ipa/ipa_hbac_common.c
index
72a620ef0971a8bc657bd7bda3f61b4abdd614ee..82c531f15338a2ff609bd58ad449783d7a3a6af1
100644
--- a/src/providers/ipa/ipa_hbac_common.c
+++ b/src/providers/ipa/ipa_hbac_common.c
@@ -21,7 +21,6 @@
*/
#include "providers/ipa/ipa_hbac_private.h"
-#include "providers/ipa/ipa_hbac.h"
#include "providers/ipa/ipa_common.h"
static errno_t
diff --git a/src/providers/ipa/ipa_hbac_private.h
b/src/providers/ipa/ipa_hbac_private.h
index
c831cd5c6dd2ed1ff2bc0d649a25ae1212548dda..8fc5dc6d03cc2373e32641a399157c900ec18107
100644
--- a/src/providers/ipa/ipa_hbac_private.h
+++ b/src/providers/ipa/ipa_hbac_private.h
@@ -24,7 +24,7 @@
#define IPA_HBAC_PRIVATE_H_
#include "providers/ipa/ipa_access.h"
-#include "providers/ipa/ipa_hbac.h"
+#include "lib/ipa_hbac/ipa_hbac.h"
#define IPA_HBAC_RULE "ipaHBACRule"
diff --git a/src/python/pyhbac.c b/src/python/pyhbac.c
index
820ef11b57f1226725fd7acf97598a42e6bf0bc0..eb424c6ddb8f7b44437079f0f9c7d63a583b7198
100644
--- a/src/python/pyhbac.c
+++ b/src/python/pyhbac.c
@@ -23,7 +23,7 @@
#include "util/util.h"
#include "util/sss_python.h"
-#include "providers/ipa/ipa_hbac.h"
+#include "lib/ipa_hbac/ipa_hbac.h"
#define PYTHON_MODULE_NAME "pyhbac"
diff --git a/src/tests/ipa_hbac-tests.c b/src/tests/ipa_hbac-tests.c
index
f2192a6fbc5188a7a7f6b204e03ca5961bb53f75..c8ef7fe44d41770cfeca706e6d3396a8d0e7d6cf
100644
--- a/src/tests/ipa_hbac-tests.c
+++ b/src/tests/ipa_hbac-tests.c
@@ -27,7 +27,7 @@
#include <talloc.h>
#include "tests/common_check.h"
-#include "providers/ipa/ipa_hbac.h"
+#include "lib/ipa_hbac/ipa_hbac.h"
#define HBAC_TEST_USER "testuser"
#define HBAC_TEST_INVALID_USER "nosuchuser"
--
2.4.3
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
