The attached version of the patch should solve all of these issues. It should also be more performant and use less memory.
Nathaniel On Wed, 2014-09-17 at 15:33 +0200, thierry bordaz wrote: > On 09/15/2014 09:05 PM, Nathaniel McCallum wrote: > > > This plugin ensures that all counter/watermark operations are atomic > > and never decrement. Also, deletion is not permitted. > > > > https://fedorahosted.org/freeipa/ticket/4494 > > > > > > _______________________________________________ > > Freeipa-devel mailing list > > Freeipa-devel@redhat.com > > https://www.redhat.com/mailman/listinfo/freeipa-devel > Hello Nathaniel, > > More thoughts.. I think being "betxnpreoperation" you are safe > with parallel client ops and the check is atomic. I have few > comments: > * Shouldn't be implemented > SLAPI_PLUGIN_CLOSE_FN/SLAPI_PLUGIN_START_FN callback, > even if they are empty. > * In load_counters, in case we have a target entry that > has not 'objectclass' 'ipatokenHOTP|ipatokenTOTP' and > the mod operation: > dn: <entry> > changetype: modify > replace: ipatokenHOTPcounter > ipatokenHOTPcounter: 200 > - > add: objectclass > objectclass: ipatokenHOTP > > > I wonder if the operation will not fail although IMHO > it should succeeds. > Shouldn't let the schema checking reject the operation > if the attribute is not granted by the entry > objectclass > * in load_counters, I am under the impression it may > return ipatokenHOTPcounter or ipatokenTOTPwatermark > (ipatokenHOTPcounter is missing). > But then how the caller knows that the returned value > is a counter or a watermark ? > * in ldapmod_is_attrs you may prefer PL_strcasecmp to > strcasecmp > > About replicated updates, if updates of counters/watermark are > done on several servers. Then a replicated operation may want > to set counters/watermark with a smaller value that the > existing one. In that case, it will return unwilling to > perform. That could break replication. > If the update comes from replication and the value is going > backward, we could make the operation a nop operation (setting > the attribute to its current value). > > > thanks > theirry > >
From 3658531e16ad914b1b8e8eff573180aa5e3947d1 Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum <npmccal...@redhat.com> Date: Wed, 10 Sep 2014 17:31:37 -0400 Subject: [PATCH] Create ipa-otp-decrement 389DS plugin This plugin ensures that all counter/watermark operations are atomic and never decrement. Also, deletion is not permitted. Because this plugin also ensures internal operations behave properly, this also gives ipa-pwd-extop the appropriate behavior for OTP authentication. https://fedorahosted.org/freeipa/ticket/4493 https://fedorahosted.org/freeipa/ticket/4494 --- daemons/configure.ac | 1 + daemons/ipa-slapi-plugins/Makefile.am | 1 + .../ipa-otp-decrement/Makefile.am | 25 ++ .../ipa-otp-decrement/ipa-otp-decrement.sym | 1 + .../ipa-otp-decrement/ipa_otp_decrement.c | 362 +++++++++++++++++++++ .../ipa-otp-decrement/otp-decrement-conf.ldif | 15 + freeipa.spec.in | 2 + ipaserver/install/dsinstance.py | 4 + 8 files changed, 411 insertions(+) create mode 100644 daemons/ipa-slapi-plugins/ipa-otp-decrement/Makefile.am create mode 100644 daemons/ipa-slapi-plugins/ipa-otp-decrement/ipa-otp-decrement.sym create mode 100644 daemons/ipa-slapi-plugins/ipa-otp-decrement/ipa_otp_decrement.c create mode 100644 daemons/ipa-slapi-plugins/ipa-otp-decrement/otp-decrement-conf.ldif diff --git a/daemons/configure.ac b/daemons/configure.ac index b4507a6d972f854331925e72869898576bdfd76f..936cba576818e9e6d2c5399bbfe85de7a27141ab 100644 --- a/daemons/configure.ac +++ b/daemons/configure.ac @@ -314,6 +314,7 @@ AC_CONFIG_FILES([ ipa-slapi-plugins/ipa-dns/Makefile ipa-slapi-plugins/ipa-enrollment/Makefile ipa-slapi-plugins/ipa-lockout/Makefile + ipa-slapi-plugins/ipa-otp-decrement/Makefile ipa-slapi-plugins/ipa-otp-lasttoken/Makefile ipa-slapi-plugins/ipa-pwd-extop/Makefile ipa-slapi-plugins/ipa-extdom-extop/Makefile diff --git a/daemons/ipa-slapi-plugins/Makefile.am b/daemons/ipa-slapi-plugins/Makefile.am index 06e6ee8b86f138cce05f2184ac98c39ffaf9757f..9c0f048c390dac81f58cd0270a002614693a62d8 100644 --- a/daemons/ipa-slapi-plugins/Makefile.am +++ b/daemons/ipa-slapi-plugins/Makefile.am @@ -7,6 +7,7 @@ SUBDIRS = \ ipa-enrollment \ ipa-lockout \ ipa-modrdn \ + ipa-otp-decrement \ ipa-otp-lasttoken \ ipa-pwd-extop \ ipa-extdom-extop \ diff --git a/daemons/ipa-slapi-plugins/ipa-otp-decrement/Makefile.am b/daemons/ipa-slapi-plugins/ipa-otp-decrement/Makefile.am new file mode 100644 index 0000000000000000000000000000000000000000..24aea58d22de57fcf88cbbe27f88c162ad47077c --- /dev/null +++ b/daemons/ipa-slapi-plugins/ipa-otp-decrement/Makefile.am @@ -0,0 +1,25 @@ +MAINTAINERCLEANFILES = *~ Makefile.in +PLUGIN_COMMON_DIR = ../common +AM_CPPFLAGS = \ + -I. \ + -I$(srcdir) \ + -I$(PLUGIN_COMMON_DIR) \ + -I/usr/include/dirsrv \ + -DPREFIX=\""$(prefix)"\" \ + -DBINDIR=\""$(bindir)"\" \ + -DLIBDIR=\""$(libdir)"\" \ + -DLIBEXECDIR=\""$(libexecdir)"\" \ + -DDATADIR=\""$(datadir)"\" \ + $(AM_CFLAGS) \ + $(LDAP_CFLAGS) \ + $(WARN_CFLAGS) + +plugindir = $(libdir)/dirsrv/plugins +plugin_LTLIBRARIES = libipa_otp_decrement.la +libipa_otp_decrement_la_SOURCES = ipa_otp_decrement.c +libipa_otp_decrement_la_LDFLAGS = -avoid-version -export-symbols ipa-otp-decrement.sym +libipa_otp_decrement_la_LIBADD = $(LDAP_LIBS) + +appdir = $(IPA_DATA_DIR) +app_DATA = otp-decrement-conf.ldif +EXTRA_DIST = $(app_DATA) diff --git a/daemons/ipa-slapi-plugins/ipa-otp-decrement/ipa-otp-decrement.sym b/daemons/ipa-slapi-plugins/ipa-otp-decrement/ipa-otp-decrement.sym new file mode 100644 index 0000000000000000000000000000000000000000..27829ead6cc5fcb13b0fbff0b7ac3d638cbf4e2f --- /dev/null +++ b/daemons/ipa-slapi-plugins/ipa-otp-decrement/ipa-otp-decrement.sym @@ -0,0 +1 @@ +ipa_otp_decrement_init diff --git a/daemons/ipa-slapi-plugins/ipa-otp-decrement/ipa_otp_decrement.c b/daemons/ipa-slapi-plugins/ipa-otp-decrement/ipa_otp_decrement.c new file mode 100644 index 0000000000000000000000000000000000000000..f8e8ed106629c79e88d7ebd97f1a974c72874f0a --- /dev/null +++ b/daemons/ipa-slapi-plugins/ipa-otp-decrement/ipa_otp_decrement.c @@ -0,0 +1,362 @@ +/** 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: + * Nathaniel McCallum <npmccal...@redhat.com> + * + * Copyright (C) 2014 Red Hat, Inc. + * All rights reserved. + * END COPYRIGHT BLOCK **/ + +#ifdef HAVE_CONFIG_H +# include <config.h> +#endif + +#include <slapi-plugin.h> +#include <stdbool.h> +#include <stdlib.h> +#include <limits.h> +#include <errno.h> + +#include <plstr.h> + +#define PLUGIN_NAME "ipa-otp-decrement" + +#define ch_malloc(type) \ + (type*) slapi_ch_malloc(sizeof(type)) +#define ch_calloc(count, type) \ + (type*) slapi_ch_calloc(count, sizeof(type)) +#define ch_free(p) \ + slapi_ch_free((void**) &(p)) + +static void *plugin_id; +static const Slapi_PluginDesc preop_desc = { + PLUGIN_NAME, + "FreeIPA", + "FreeIPA/1.0", + "Protect token counters from decrement" +}; + +static int +send_error(Slapi_PBlock *pb, int rc, char *template, ...) +{ + va_list ap; + int res; + + va_start(ap, template); + res = vsnprintf(NULL, 0, template, ap); + va_end(ap); + + if (res > 0) { + char str[res + 1]; + + va_start(ap, template); + res = vsnprintf(str, res + 1, template, ap); + va_end(ap); + + if (res > 0) + slapi_send_ldap_result(pb, rc, NULL, str, 0, NULL); + } + + if (res <= 0) + slapi_send_ldap_result(pb, rc, NULL, NULL, 0, NULL); + + slapi_pblock_set(pb, SLAPI_RESULT_CODE, &rc); + return rc; +} + +/* + * Get's the specified value from the mod. + * + * Returns ENOENT if there is no item. + * Returns ERANGE if the value doesn't fit. + */ +static int +get_value(const LDAPMod *mod, size_t indx, long long *val) +{ + if (mod == NULL) + return ENOENT; + + if (mod->mod_vals.modv_bvals == NULL) + return ENOENT; + + for (size_t i = 0; i <= indx; i++) { + if (mod->mod_vals.modv_bvals[i] == NULL) + return ENOENT; + } + + const struct berval *bv = mod->mod_vals.modv_bvals[indx]; + char buf[bv->bv_len + 1]; + memcpy(buf, bv->bv_val, bv->bv_len); + buf[sizeof(buf)-1] = '\0'; + + *val = strtoll(buf, NULL, 10); + if (*val == LLONG_MIN || *val == LLONG_MAX) + return ERANGE; + + return 0; +} + +static struct berval ** +make_berval_array(long long value) +{ + struct berval **bvs; + + bvs = ch_calloc(2, struct berval*); + bvs[0] = ch_malloc(struct berval); + bvs[0]->bv_val = slapi_ch_smprintf("%lld", value); + bvs[0]->bv_len = strlen(bvs[0]->bv_val); + + return bvs; +} + +static LDAPMod * +make_ldapmod(int op, const char *attr, long long value) +{ + LDAPMod *mod; + + mod = (LDAPMod*) slapi_ch_calloc(1, sizeof(LDAPMod)); + mod->mod_op = op | LDAP_MOD_BVALUES; + mod->mod_type = slapi_ch_strdup(attr); + mod->mod_bvalues = make_berval_array(value); + + return mod; +} + +static void +convert_ldapmod_to_bval(LDAPMod *mod) +{ + if (mod == NULL || (mod->mod_op & LDAP_MOD_BVALUES)) + return; + + mod->mod_op |= LDAP_MOD_BVALUES; + + if (mod->mod_values == NULL) { + mod->mod_bvalues = NULL; + return; + } + + for (size_t i = 0; mod->mod_values[i] != NULL; i++) { + struct berval *bv = ch_malloc(struct berval); + bv->bv_val = mod->mod_values[i]; + bv->bv_len = strlen(bv->bv_val); + mod->mod_bvalues[i] = bv; + } +} + +/* Returns false if target entry is not a token. */ +static void +sanitize_input(Slapi_PBlock *pb, Slapi_Entry *entry, const char *attr, + LDAPMod ***outmods) +{ + size_t count = 0, replaces = 0; + Slapi_Attr *sattr = NULL; + LDAPMod **inmods = NULL; + long long counter = 0; + bool havectr = false; + + /* Get counter. */ + if (slapi_entry_attr_find(entry, attr, &sattr) == 0) { + counter = slapi_entry_attr_get_longlong(entry, attr); + havectr = true; + } + + /* Count items. */ + slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &inmods); + while (inmods != NULL && inmods[count] != NULL) { + LDAPMod *mod = inmods[count++]; + + if (PL_strcasecmp(mod->mod_type, attr) != 0) + continue; + + if ((mod->mod_op & LDAP_MOD_OP) == LDAP_MOD_REPLACE && havectr) + replaces++; + } + + /* Create output array. */ + *outmods = ch_calloc(count + replaces + 1, LDAPMod*); + for (size_t i = 0, j = 0; inmods != NULL && inmods[i] != NULL; i++, j++) { + LDAPMod *mod = inmods[i]; + + convert_ldapmod_to_bval(mod); + + if (PL_strcasecmp(mod->mod_type, attr) == 0) { + switch (mod->mod_op & LDAP_MOD_OP) { + /* Ensure that a DELETE operation specifies a counter value. */ + case LDAP_MOD_DELETE: + if (mod->mod_bvalues != NULL && mod->mod_bvalues[0] == NULL) + ch_free(mod->mod_bvalues); + + if (mod->mod_bvalues == NULL) + mod->mod_bvalues = make_berval_array(counter); + break; + + /* Convert a REPLACE operation to DEL/ADD operations. */ + case LDAP_MOD_REPLACE: + mod->mod_op &= ~LDAP_MOD_OP; + mod->mod_op |= LDAP_MOD_ADD; + if (havectr) + (*outmods)[j++] = make_ldapmod(LDAP_MOD_DELETE, attr, counter); + break; + } + } + + (*outmods)[j] = mod; + } + + slapi_pblock_set(pb, SLAPI_MODIFY_MODS, *outmods); + ch_free(inmods); +} + +static const char * +get_attribute(Slapi_Entry *entry) +{ + static struct { + const char *clss; + const char *attr; + } table[] = { + { "ipatokenHOTP", "ipatokenHOTPcounter" }, + { "ipatokenTOTP", "ipatokenTOTPwatermark" }, + { NULL, NULL } + }; + + char **clsses = slapi_entry_attr_get_charray(entry, "objectClass"); + for (size_t i = 0; clsses != NULL && clsses[i] != NULL; i++) { + for (size_t j = 0; table[j].clss != NULL; j++) { + if (PL_strcasecmp(table[j].clss, clsses[i]) == 0) + return table[j].attr; + } + } + + return NULL; +} + +static int +preop_mod(Slapi_PBlock *pb) +{ + Slapi_Entry *entry = NULL; + const char *attr = NULL; + LDAPMod **mods = NULL; + + /* Get the existing entry. */ + slapi_pblock_get(pb, SLAPI_ENTRY_PRE_OP, &entry); + if (entry == NULL) + return 0; /* No entry!? */ + + /* Get the attribute name. */ + attr = get_attribute(entry); + if (attr == NULL) + return 0; /* Not a token. */ + + /* Sanitize the input. */ + sanitize_input(pb, entry, attr, &mods); + + /* Validate the input. */ + for (size_t i = 0 ; mods[i] != NULL; i++) { + bool haveadd = false; + + if ((mods[i]->mod_op & LDAP_MOD_OP) != LDAP_MOD_DELETE) + continue; + + if (PL_strcasecmp(mods[i]->mod_type, attr) != 0) + continue; + + /* If a delete op is specified, ensure a following add op. + * We don't permit plain deletion of counters/watermarks. */ + for (size_t j = i; mods[j] != NULL; j++) { + long long add, del = 0; + + if ((mods[j]->mod_op & LDAP_MOD_OP) != LDAP_MOD_ADD) + continue; + + if (PL_strcasecmp(mods[j]->mod_type, attr) != 0) + continue; + + if (get_value(mods[j], 0, &add) != 0) + return send_error(pb, LDAP_UNWILLING_TO_PERFORM, + "Invalid %s add value", attr); + + for (size_t k = 0; get_value(mods[i], k, &del) != ENOENT; k++) { + if (del >= add) + return send_error(pb, LDAP_UNWILLING_TO_PERFORM, + "Must increment %s", attr); + } + + haveadd = true; + break; + } + + if (!haveadd) + return send_error(pb, LDAP_UNWILLING_TO_PERFORM, + "Will not delete %s", attr); + } + + return 0; +} + +static int +preop_init(Slapi_PBlock *pb) +{ + int ret = 0; + + ret |= slapi_pblock_set(pb, SLAPI_PLUGIN_BE_TXN_PRE_MODIFY_FN, preop_mod); + + return ret; +} + +static int +start_fn(Slapi_PBlock *pb) +{ + return 0; +} + +static int +close_fn(Slapi_PBlock *pb) +{ + return 0; +} + +int +ipa_otp_decrement_init(Slapi_PBlock *pb) +{ + int ret = 0; + + ret |= slapi_pblock_get(pb, SLAPI_PLUGIN_IDENTITY, &plugin_id); + ret |= slapi_pblock_set(pb, SLAPI_PLUGIN_VERSION, SLAPI_PLUGIN_VERSION_01); + ret |= slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void *) &preop_desc); + ret |= slapi_pblock_set(pb, SLAPI_PLUGIN_START_FN, start_fn); + ret |= slapi_pblock_set(pb, SLAPI_PLUGIN_CLOSE_FN, close_fn); + ret |= slapi_register_plugin("betxnpreoperation", 1, __func__, preop_init, + PLUGIN_NAME, NULL, plugin_id); + + return ret; +} diff --git a/daemons/ipa-slapi-plugins/ipa-otp-decrement/otp-decrement-conf.ldif b/daemons/ipa-slapi-plugins/ipa-otp-decrement/otp-decrement-conf.ldif new file mode 100644 index 0000000000000000000000000000000000000000..4064770f9191fffe1a51167b82d2e09c16ea86b6 --- /dev/null +++ b/daemons/ipa-slapi-plugins/ipa-otp-decrement/otp-decrement-conf.ldif @@ -0,0 +1,15 @@ +dn: cn=IPA OTP Decrement Prevention,cn=plugins,cn=config +changetype: add +objectclass: top +objectclass: nsSlapdPlugin +objectclass: extensibleObject +cn: IPA OTP Decrement Prevention +nsslapd-pluginpath: libipa_otp_decrement +nsslapd-plugininitfunc: ipa_otp_decrement_init +nsslapd-plugintype: preoperation +nsslapd-pluginenabled: on +nsslapd-pluginid: ipa-otp-decrement +nsslapd-pluginversion: 1.0 +nsslapd-pluginvendor: Red Hat, Inc. +nsslapd-plugindescription: IPA OTP Decrement Prevention plugin +nsslapd-plugin-depends-on-type: database diff --git a/freeipa.spec.in b/freeipa.spec.in index 685b345fedb9d157c8deedc66f8712da32c5963b..2b4f42aa028d1112f306a52dfb78da4e1dadce9e 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -348,6 +348,7 @@ rm %{buildroot}/%{plugin_dir}/libipa_sidgen.la rm %{buildroot}/%{plugin_dir}/libipa_sidgen_task.la rm %{buildroot}/%{plugin_dir}/libipa_extdom_extop.la rm %{buildroot}/%{plugin_dir}/libipa_range_check.la +rm %{buildroot}/%{plugin_dir}/libipa_otp_decrement.la rm %{buildroot}/%{plugin_dir}/libipa_otp_lasttoken.la rm %{buildroot}/%{_libdir}/krb5/plugins/kdb/ipadb.la rm %{buildroot}/%{_libdir}/samba/pdb/ipasam.la @@ -697,6 +698,7 @@ fi %attr(755,root,root) %{plugin_dir}/libipa_cldap.so %attr(755,root,root) %{plugin_dir}/libipa_dns.so %attr(755,root,root) %{plugin_dir}/libipa_range_check.so +%attr(755,root,root) %{plugin_dir}/libipa_otp_decrement.so %attr(755,root,root) %{plugin_dir}/libipa_otp_lasttoken.so %dir %{_localstatedir}/lib/ipa %attr(700,root,root) %dir %{_localstatedir}/lib/ipa/backup diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index 0518dd0e0f20255f4e42911af6f1f95fc25f554e..247132f7b76b66368d31d32bfe26763d27637b76 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -276,6 +276,7 @@ class DsInstance(service.Service): self.step("configuring DNS plugin", self.__config_dns_module) self.step("enabling entryUSN plugin", self.__enable_entryusn) self.step("configuring lockout plugin", self.__config_lockout_module) + self.step("configuring OTP decrement prevention plugin", self.__config_otp_decrement_module) self.step("configuring OTP last token plugin", self.__config_otp_lasttoken_module) self.step("creating indices", self.__create_indices) self.step("enabling referential integrity plugin", self.__add_referint_module) @@ -588,6 +589,9 @@ class DsInstance(service.Service): def __config_lockout_module(self): self._ldap_mod("lockout-conf.ldif") + def __config_otp_decrement_module(self): + self._ldap_mod("otp-decrement-conf.ldif") + def __config_otp_lasttoken_module(self): self._ldap_mod("otp-lasttoken-conf.ldif") -- 2.1.0
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
