[SSSD] Time-Based Policies in FreeIPA
Stanislav Laznicka
slaznick at redhat.com
Tue Jul 14 07:51:06 UTC 2015
On 07/14/2015 09:32 AM, Lukas Slebodnik wrote:
> On (14/07/15 09:11), Stanislav Laznicka wrote:
>> Hi,
>>
>> If you're a member of the FreeIPA-devel mailing list, you may already know
>> me. I am currently trying to re-introduce the time-based policies for FreeIPA
>> HBAC Rules to both FreeIPA and SSSD. For some more information, see the
>> design athttp://www.freeipa.org/page/V4/Time-Based_Account_Policies or read
>> the whole paper at
>> http://www.fit.vutbr.cz/study/DP/DP.php.cs?id=17185&file=t.
>>
>> At the moment, I have a prototype solution which is able to decide whether an
>> HBAC rule should or should not apply based on the time rules supplied with
>> HBAC rule objects from FreeIPA. See the source code attached to this mail, if
>> you will. The biggest problem of this solution is that it is not thread-safe.
>>
>> It seems to be quite a problem to convert time from a certain time zone to a
>> different time zone in C and keep it thread-safe. A very simple and also very
>> ugly solution would be to have a mutex to guard each localtime() call as well
>> as it should wrap the body of the time_to_timezone() function from the second
>> patch. This seems rather unacceptable. The other solution would be to find
>> another way to convert the time. Currently, there seems to be a C++ Boost
>> solution based on a .csv file but it is not accepted well (https://github.com/boostorg/date_time/blob/master/data/date_time_zonespec.csv).
>> I was also thinking on using the glibc tzfile parsers
>> (http://code.woboq.org/userspace/glibc/time/tzfile.c.html#__tzfile_read) but
>> they too seem rather thread-unsafe and trying to rework it in a thread-safe
>> manner might be a painful thing to do.
>>
>> I welcome any suggestions and ideas on the topic as I seem to be quite stuck
>> here.
>>
>> Cheers,
>> Stanislav Laznicka
> I did a very brief review of Makefile changes
> I have a few questions.
>
> >From 428db72ca0a07c7dba1a6959696b3dc2df7d77ec Mon Sep 17 00:00:00 2001
>> From: Stanislav Laznicka<slaznick at redhat.com>
>> Date: Mon, 13 Jul 2015 09:53:10 +0200
>> Subject: [PATCH 1/2] Added caching of time policies for IPA HBAC rules.
>>
>> The time policies which are part of the FreeIPA HBAC Rule object
>> are now being cached along with other HBAC Rule objects' attributes.
>> Also, the cached time policies are transformed into hbac_time_rules
>> structure representation.
>>
>> https://fedorahosted.org/freeipa/ticket/547
>> https://fedorahosted.org/freeipa/ticket/548
>> ---
>> Makefile.am | 4 --
>> src/providers/ipa/hbac_evaluator.c | 7 --
>> src/providers/ipa/ipa_access.c | 3 +
>> src/providers/ipa/ipa_hbac.h | 6 +-
>> src/providers/ipa/ipa_hbac_common.c | 120 +++++++++++++++++++++++++++++++++++
>> src/providers/ipa/ipa_hbac_private.h | 10 +++
>> src/providers/ipa/ipa_hbac_rules.c | 7 +-
>> src/tests/ipa_hbac-tests.c | 6 ++
>> 8 files changed, 149 insertions(+), 14 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index b8cbc6df23ded1edb945a709b6dbe1c44eb54017..8665a0560cc3cd57f148325640c2e709a05f4c2a 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -264,14 +264,10 @@ PYTHON_TESTS =
>>
>> if BUILD_PYTHON2_BINDINGS
>> PYTHON_TESTS += src/config/SSSDConfigTest.py2.sh \
>> - src/tests/pyhbac-test.py2.sh \
>> - src/tests/pysss_murmur-test.py2.sh \
>> $(NULL)
>> endif
>> if BUILD_PYTHON3_BINDINGS
>> PYTHON_TESTS += src/config/SSSDConfigTest.py3.sh \
>> - src/tests/pyhbac-test.py3.sh \
>> - src/tests/pysss_murmur-test.py3.sh \
>> $(NULL)
> Could you explain why did you remove pysss_murmur-test?
> How is it related to hbac?
It was probably just a mistake.
> BTW I would prefer to temporary disable pyhbac-test test
> or fix it rather that removing it.
I made a brief try to fix it but at the time I was writing the code, it
was not my main goal so I decided just to remove it. I will fix it and
add the python tests back once the issues with the solution itself are
resolved.
> >From 856dc04c7bd9b4f46637c7c598dfff0bb8b95105 Mon Sep 17 00:00:00 2001
>> From: Stanislav Laznicka<slaznick at redhat.com>
>> Date: Mon, 13 Jul 2015 10:00:42 +0200
>> Subject: [PATCH 2/2] Added evaluation of time-policies in HBAC objects.
>>
>> The time-policies in FreeIPA HBAC objects are now evaluated in
>> the libipa_hbac module.
>>
>> FIXME: The evaluation is not thread-safe. See the time_to_timezone
>> function in ipa_timerules.c
>>
>> https://fedorahosted.org/freeipa/ticket/547
>> https://fedorahosted.org/freeipa/ticket/548
>> ---
>> Makefile.am | 2 +
>> src/providers/ipa/hbac_evaluator.c | 17 +-
>> src/providers/ipa/ipa_timerules.c | 488 +++++++++++++++++++++++++++++++++++++
>> src/providers/ipa/ipa_timerules.h | 40 +++
>> 4 files changed, 545 insertions(+), 2 deletions(-)
>> create mode 100644 src/providers/ipa/ipa_timerules.c
>> create mode 100644 src/providers/ipa/ipa_timerules.h
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 8665a0560cc3cd57f148325640c2e709a05f4c2a..937cc99757b501f953c91aef40a13e69fb4c4f4e 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -600,6 +600,7 @@ dist_noinst_HEADERS = \
>> src/providers/ldap/sdap_dyndns.h \
>> src/providers/ldap/sdap_async_enum.h \
>> src/providers/ipa/ipa_common.h \
>> + src/providers/ipa/ipa_timerules.h \
> ^^
> '\t' used instead of spaces.
>> src/providers/ipa/ipa_config.h \
>> src/providers/ipa/ipa_access.h \
>> src/providers/ipa/ipa_selinux.h \
>> @@ -877,6 +878,7 @@ lib_LTLIBRARIES = libipa_hbac.la \
>> pkgconfig_DATA += src/providers/ipa/ipa_hbac.pc
>> libipa_hbac_la_DEPENDENCIES = src/providers/ipa/ipa_hbac.exports
>> libipa_hbac_la_SOURCES = \
>> + src/providers/ipa/ipa_timerules.c \
>> src/providers/ipa/hbac_evaluator.c \
>> src/util/sss_utf8.c
>> libipa_hbac_la_LIBADD = \
>> diff --git a/src/providers/ipa/hbac_evaluator.c b/src/providers/ipa/hbac_evaluator.c
>> index 64af36aae2855d0fb5a737989cc3ee8959e1b5b6..5d9bdaf29e6352f7835ee316b00790f8a739fae3 100644
>> --- a/src/providers/ipa/hbac_evaluator.c
>> +++ b/src/providers/ipa/hbac_evaluator.c
>> @@ -27,6 +27,7 @@
>> #include <string.h>
>> #include <errno.h>
>> #include "providers/ipa/ipa_hbac.h"
>> +#include "providers/ipa/ipa_timerules.h"
>> #include "util/sss_utf8.h"
>>
>> #ifndef HAVE_ERRNO_T
>> @@ -106,7 +107,6 @@ enum hbac_eval_result hbac_evaluate(struct hbac_rule **rules,
>> enum hbac_error_code ret;
>> enum hbac_eval_result result = HBAC_EVAL_DENY;
>> enum hbac_eval_result_int intermediate_result;
>> -
> Code is more readable if there is emty line after declarations.
> Plese do not remove it.
>
>> if (info) {
>> *info = malloc(sizeof(struct hbac_info));
>> if (!*info) {
>> @@ -176,7 +176,8 @@ enum hbac_eval_result_int hbac_evaluate_rule(struct hbac_rule *rule,
>> if (!rule->users
>> || !rule->services
>> || !rule->targethosts
>> - || !rule->srchosts) {
>> + || !rule->srchosts
>> + || !rule->timerules) {
>> *error = HBAC_ERROR_UNPARSEABLE_RULE;
>> return HBAC_EVAL_MATCH_ERROR;
>> }
>> @@ -224,6 +225,18 @@ enum hbac_eval_result_int hbac_evaluate_rule(struct hbac_rule *rule,
>> } else if (!matched) {
>> return HBAC_EVAL_UNMATCHED;
>> }
>> +
>> + /* Check time policies */
>> + ret = hbac_evaluate_time_rules(rule->timerules,
>> + hbac_req->request_time,
>> + &matched);
>> + if(ret != EOK) {
>> + *error = HBAC_ERROR_UNPARSEABLE_RULE;
>> + return HBAC_EVAL_MATCH_ERROR;
>> + } else if (!matched) {
>> + return HBAC_EVAL_UNMATCHED;
>> + }
>> +
>> return HBAC_EVAL_MATCHED;
>> }
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
More information about the sssd-devel
mailing list