Re: [Freeipa-devel] [PATCH 0064] Create ipa-otp-decrement 389DS plugin
On 2014/10/07 10:48, Nathaniel McCallum wrote:
On Tue, 2014-10-07 at 18:54 +0200, thierry bordaz wrote:
On 10/07/2014 06:00 PM, Nathaniel McCallum wrote:
Attached is the latest patch. I believe this includes all of our
discussions up until this point. However, a few bits of additional
information are needed.
First, I have renamed the plugin to ipa-otp-counter. I believe all
replay prevention work can land inside this plugin, so the name is
appropriate.
Second, I uncovered a bug in 389 which prevents me from validating the
non-replication request in bepre. This is the reason for the additional
betxnpre callback. If the upstream 389 bug is fixed, we can merge this
check back into bepre. https://fedorahosted.org/389/ticket/47919
Hi Nathaniel,
Just a rapid question about that dependency on
https://fedorahosted.org/389/ticket/47919.
Using txnpreop_mod you manage to workaround the DS issue.
Do you need a fix for the DS issue in 1.3.2 or can it be fixed
in a later version ?
I would strongly prefer a fix ASAP.
Thanks, Nathaniel,
Do you need the fix just in 389-ds-base-1.3.3.x on F21 and newer? Or any
other versions, e.g., 1.3.2 on F20, 1.3.3.1-x on RHEL-7.1, etc... ?
--noriko
thanks
thierry
Third, I believe we are now handling replication correct. An error is
never returned. When a replication would cause the counter to decrease,
we remove all counter/watermark related mods from the operation. This
will allow the replication to apply without decrementing the value.
There is also a new bepost method which check to see if the replication
was discarded (via CSN) while having a higher counter value. If so, we
apply the higher counter value.
Here is the scenario. Server X receives two quick authentications;
replications A and B are sent to server Y. Before server Y can process
server X's replications, an authentication is performed on server Y;
replication C is sent to server X. The following logic holds true:
* csnA csnB csnC
* valueA = valueC, valueB valueC
When server X receives replication C, ipa-otp-counter will strip out all
counter mod operations; applying the update but not the lower value. The
value of replication B is retained. This is the correct behavior.
When server Y processes replications A and B, ipa-otp-counter will
detect that a higher value has a lower CSN and will manually set the
higher value (in bepost). This initiates replication D, which is sent to
server X. Here is the logic:
* csnA csnB csnC csnD
* valueA = valueC, valueB = valueD, valueD valueC
Server X receives replication D. D has the highest CSN. It has the same
value as replication B (server X's current value). Because the values
are the same, ipa-otp-counter will strip all counter mod operations.
This reduces counter write contention which might become a problem in
N-way replication when N2.
On Fri, 2014-10-03 at 19:52 +0200, thierry bordaz wrote:
Hello Nathaniel,
An additional comment about the patch.
When the new value is detected to be invalid, it is fixed by a
repair operation (trigger_replication).
I did test it and it is fine to update, with an internal
operation, the same entry that is currently updated.
Now if you apply the repair operation into a be_preop or a
betxn_preop, when it returns from preop the txn of the current
operation will overwrite the repaired value.
An option is to register a bepost that checks the value from
the original entry (SLAPI_ENTRY_PRE_OP) and post entry
(SLAPI_ENTRY_POST_OP). Then this postop checks the
orginal/final value and can trigger the repair op.
This time being outside of the main operation txn, the repair
op will be applied.
thanks
thierry
On 09/29/2014 08:30 PM, Nathaniel McCallum wrote:
On Mon, 2014-09-22 at 09:32 -0400, Simo Sorce wrote:
On Sun, 21 Sep 2014 22:33:47 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:
Comments inline.
+
+#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))
please do not redefine slapi functions, it just makes it harder to know
what you used.
+typedef struct {
+bool exists;
+long long value;
+} counter;
please no typedefs of structures, use struct counter { ... }; and
reference it as struct counter in the code.
Btw, FWIW you could use a value of -1 to indicate non-existence of the
counter value, given counters can only be positive, this would avoid
the need for a struct.
+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) {
+
Re: [Freeipa-devel] [PATCH 0032] Update ACIs to permit users to add/delete their own tokens
Hi Simo, Simo Sorce wrote: On Fri, 2014-01-10 at 12:15 -0500, Simo Sorce wrote: This is not what I had in mind, our use cases is something like this: aci: (target=ldap:///dc=bar)(targetattr=*) (version 3.0; acl userattr test; allow (add) userattr = managedby#USERDN;) ldapmodify -D uid=user,dc=bar ... EOF dn: cn=somobj,dc=bar ... managedby: uid=user,dc=bar Sorry this should have been ldapadd. Simo. Yes, it works. aci: (target=ldap:///o=my.com)(targetattr=*) (version 3.0; acl userattr test ; allow (add,write,delete,read,search,compare) userattr = description#USERDN;) $ ldapmodify ... -D 'uid=nuser0,o=my.com' -w Nuser0 -a EOF dn: uid=Nuser6, o=my.com ... description: uid=nuser0,o=my.com EOF $ ldapsearch... -b o=my.com (uid=nuser6) description dn: uid=Nuser6,o=my.com description: uid=nuser0,o=my.com # delete uid=nuser6 # attempt to add the entry by other user fails: $ ldapmodify ... -D 'uid=nuser1,o=my.com' -w Nuser1 -a EOF dn: uid=Nuser6, o=my.com ... description: uid=nuser0,o=my.com EOF ldap_add: Insufficient access ldap_add: additional info: Insufficient 'add' privilege to the 'userPassword' attribute dn: cn=somobj,dc=bar ... managedby: uid=user,dc=bar This should succeed, however if managedby includes anything but uid=user,dc=bar it should fail. Will it allow the user to add multiple values to the same attr as long as one of the is the userDN ? O will it restrict that case ? This is also important, if attrFoo is a multivalued attribute, does this option allow any values to be set as long as one of them is userDN ? Or will it enforce that *only* userDN is add in the add operation ? As long as the type of the attribute is not restricted as DN syntax, it takes any value including DN. I tested with 'description' (e.g., userattr = description#USERDN) and verified it takes userDN as well as any other values. $ ldapmodify ... -D 'cn=directory manager' -w pw dn: uid=nuser4,o=my.com changetype: modify add: description description: uid=nuser4,o=my.com $ ldapmodify ... -D 'uid=nuser4,o=my.com' -w Nuser4 dn: uid=nuser4,o=my.com changetype: modify add: description description: uid=nuser0,o=my.com modifying entry uid=nuser4,o=my.com $ ldapmodify ... -D 'uid=nuser0,o=my.com' -w Nuser0 dn: uid=nuser4,o=my.com changetype: modify add: description description: uid=nuser1,o=my.com modifying entry uid=nuser4,o=my.com $ ldapmodify ... -D 'uid=nuser1,o=my.com' -w Nuser1 dn: uid=nuser4,o=my.com changetype: modify add: description description: value $ ldapsearch ... -D 'cn=directory manager' -w pw -b uid=nuser4,o=my.com description dn: uid=Nuser4,o=my.com description: uid=nuser4,o=my.com description: uid=nuser0,o=my.com description: uid=nuser1,o=my.com description: value If I read this correctly, and I am not 100% sure yet, it seem to me this is exactly the opposite of what IPA needs. Our need is that uid=userX,... can only write its own DN as value or the attributes we are allowing through userattr. You want to allow this $ ldapmodify ... -D 'uid=*userX*,o=my.com' -w userX EOF dn: uid=userY,o=my.com changetype: modify replace: managedby managedby: uid=*userX*,o=my.com EOF But NOT allow this? $ ldapmodify ... -D 'uid=*userX*,o=my.com' -w userX EOF dn: uid=userY,o=my.com changetype: modify replace: managedby managedby: uid=*userZ*,o=my.com EOF I don't think we have the control there... --noriko ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0032] Update ACIs to permit users to add/delete their own tokens
Simo Sorce wrote: On Thu, 2014-01-09 at 16:32 -0500, Nathaniel McCallum wrote: This patch is independent from my patches 0028-0031 and can be merged in any order. This patch has a bug, but I can't figure it out. We need to set nsslapd-access-userattr-strict on cn=config to off. Uhmm what is the effect on ACL evaluation of changing this boolean ? Ticket 47653 - Need a way to allow users to create entries assigned to themselves Bug Description: There are cases where users need to be able to create, edit and delete their own entries. Using an ACI with the userattr keyword does not work with ADD operations(to prevent a security hole). This prevents IPA's OTP plugin from performing some necessary operations. Fix Description: Added a new config attribute nsslapd-access-userattr-strict. The default is on or strict. For the IPA case, it would need to be set to off in order to allow the desired behavior. https://fedorahosted.org/389/ticket/47653 This patch is included in 389-ds-base-1.3.2.10 and newer. I can;t figure out from your commit not from 389ds commit what exactly changes and how it impacts the security of the directory. I ask because I was planning on using userattr to protect some operations in the password plugin but was waiting due to bug: https://fedorahosted.org/389/ticket/47571 which is beeing resolved. Thank you for waiting. We are going to add the fix to the next release (1.3.2.11). Thanks! --noriko I want to make sure your change won't change what this ACIs would allow. Is this option simply allowing the use of add/delete ACIs to be specified in conjunction with userattr, so that a user can add an attr only if it contains its own DN ? Will it allow the user to add multiple values to the same attr as long as one of the is the userDN ? O will it restrict that case ? (I know that ipaTokenOwner is a single-value attribute, but the mechanism you are enabling here is general, and I want to be sure of what the semantics are) Simo. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0032] Update ACIs to permit users to add/delete their own tokens
Simo Sorce wrote:
On Thu, 2014-01-09 at 15:15 -0800, Noriko Hosoi wrote:
Simo Sorce wrote:
On Thu, 2014-01-09 at 16:32 -0500, Nathaniel McCallum wrote:
This patch is independent from my patches 0028-0031 and can be merged in
any order.
This patch has a bug, but I can't figure it out. We need to set
nsslapd-access-userattr-strict on cn=config to off.
Uhmm what is the effect on ACL evaluation of changing this boolean ?
Ticket 47653 - Need a way to allow users to create entries assigned
to themselves
Bug Description: There are cases where users need to be able to
create, edit and delete
their own entries. Using an ACI with the
userattr keyword does not
work with ADD operations(to prevent a security
hole). This prevents IPA's
OTP plugin from performing some necessary operations.
Fix Description: Added a new config attribute
nsslapd-access-userattr-strict. The default
is on or strict. For the IPA case, it would
need to be set to off in
order to allow the desired behavior.
https://fedorahosted.org/389/ticket/47653
This patch is included in 389-ds-base-1.3.2.10 and newer.
Thank you Noriko, but as I mentioned, I already read through the 389ds
patch and it doesn't help me. Below I have a few more questions, I will
add one that is more clear.
Is this option simply allowing the use of add/delete ACIs to be
specified in conjunction with userattr, so that a user can add an attr
only if it contains its own DN ?
So the entry in this case does not exist yet, so my question is whether
the (userattr=attrFoo#userDN) part will actually match that attrFoo is
set to contain the DN of the user that is performing the operation ?
I uesd this ACI:
aci: (target=ldap:///o=my.com)(targetattr=*) (version 3.0; acl userattr
test; allow (add,write,delete,read,search,compare) userattr =
description#USERDN;)
Please note uid=nuser does not exist yet. Does your question mean by
this add? Before coming to the ACL code, bind fails...
$ ldapmodify ... -D 'uid=nuser,o=my.com' -w Nuser -a EOF
dn: uid=Nuser, o=my.com
objectClass: top
[...]
uid: Nuser
description: uid=nuser,o=my.com
userPassword: {CLEAR}Nuser
EOF
ldap_simple_bind: No such object
ldap_simple_bind: matched: o=my.com
Will it allow the user to add multiple values to the same attr as long
as one of the is the userDN ? O will it restrict that case ?
This is also important, if attrFoo is a multivalued attribute, does this
option allow any values to be set as long as one of them is userDN ?
Or will it enforce that *only* userDN is add in the add operation ?
As long as the type of the attribute is not restricted as DN syntax, it
takes any value including DN. I tested with 'description' (e.g.,
userattr = description#USERDN) and verified it takes userDN as well as
any other values.
$ ldapmodify ... -D 'cn=directory manager' -w pw
dn: uid=nuser4,o=my.com
changetype: modify
add: description
description: uid=nuser4,o=my.com
$ ldapmodify ... -D 'uid=nuser4,o=my.com' -w Nuser4
dn: uid=nuser4,o=my.com
changetype: modify
add: description
description: uid=nuser0,o=my.com
modifying entry uid=nuser4,o=my.com
$ ldapmodify ... -D 'uid=nuser0,o=my.com' -w Nuser0
dn: uid=nuser4,o=my.com
changetype: modify
add: description
description: uid=nuser1,o=my.com
modifying entry uid=nuser4,o=my.com
$ ldapmodify ... -D 'uid=nuser1,o=my.com' -w Nuser1
dn: uid=nuser4,o=my.com
changetype: modify
add: description
description: value
$ ldapsearch ... -D 'cn=directory manager' -w pw -b
uid=nuser4,o=my.com description
dn: uid=Nuser4,o=my.com
description: uid=nuser4,o=my.com
description: uid=nuser0,o=my.com
description: uid=nuser1,o=my.com
description: value
--noriko
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1072 enable transaction support
(2012/11/16 16:09), Rich Megginson wrote: On 11/15/2012 09:53 PM, Rob Crittenden wrote: This patch enables transaction support in 389-ds-base and fixes a few transaction issues within IPA. This converts parts of the password and modrnd plugins to support transactions. The password plugin still largely runs as non-transactional because extop plugins aren't supported in transactions yet. I've left the wait_for_attr code in place for now but on reflection we should probably remove it. I'll leave that up to the reviewer, but I can't see the need for it any more. In order for this to work you'll need to apply the last two patches (both 0001) to slapi-nis and spin it up yourself, otherwise you'll have serious deadlock issues. I know this is extra work but this patch is potentially disruptive so I figure the earlier it is out the better. Noriko/Rich/Nalin, can you guys review the slapi-nis pieces? I may have been too aggressive in my cleanup. Noriko/Rich, can you review the 389-ds plugin parts of my 1072 patch? ack Your patch looks good to me, too. Thank you so much! --noriko Once we have an official slapi-nis build with these patches we'll need to set the minimum n-v-r in our spec file. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Please review: take 2: Ticket #1891 - Rewrite IPA plugins to take advantage of the single transaction
ack. Rich Megginson wrote: ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
